cascading column drop to index predicates
Hey Tom,
With regards to our previous conversation about dropping columns now
properly dropping indexes that contain predicates that reference that
column, I now find it a bit disconcerting that such indexes are
automatically removed when the column is dropped, instead of requiring a
CASCADE.
The thing is, if you drop a column that is used in a normal index, yes
the index is now useless - drop it.
However, since you can have (and I have) indexes like this:
CREATE INDEX asdf ON table (a, b, c) WHERE d IS NOT NULL;
If I drop column d, there is no way I want that index to just disappear!
This has already caught me out...
Can we change it to requiring a CASCADE? Is that a good idea?
Chris
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
The thing is, if you drop a column that is used in a normal index, yes
the index is now useless - drop it.
However, since you can have (and I have) indexes like this:
CREATE INDEX asdf ON table (a, b, c) WHERE d IS NOT NULL;
If I drop column d, there is no way I want that index to just disappear!
Uh, why not? I don't quite see the argument why d stands in a different
relationship to this index than a,b,c do. The index is equally
meaningless without any of them.
Can we change it to requiring a CASCADE?
It'd likely be a simple code change, but first let's have the argument
why it's a good idea.
regards, tom lane
Tom Lane wrote:
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
The thing is, if you drop a column that is used in a normal index, yes
the index is now useless - drop it.
However, since you can have (and I have) indexes like this:
CREATE INDEX asdf ON table (a, b, c) WHERE d IS NOT NULL;
If I drop column d, there is no way I want that index to just disappear!Uh, why not? I don't quite see the argument why d stands in a different
relationship to this index than a,b,c do. The index is equally
meaningless without any of them.Can we change it to requiring a CASCADE?
It'd likely be a simple code change, but first let's have the argument
why it's a good idea.
In that sample mentioned the index might be used mostly with a,b
columns. Dropping the index silently might damage the application
because it relies on an (a,b) index to be present. IMHO only Indexes
that span that single column should be dropped without CASCADE.
Regards,
Andreas
Andreas Pflug <pgadmin@pse-consulting.de> writes:
In that sample mentioned the index might be used mostly with a,b
columns. Dropping the index silently might damage the application
because it relies on an (a,b) index to be present. IMHO only Indexes
that span that single column should be dropped without CASCADE.
That argument makes no sense to me at all. If you drop the *column*
a or b, and do not thereby break your application, how is the
disappearance of the index on it going to break anything? The index
is meaningless without something to index.
I think the question at hand is whether the same logic applies to
partial indexes: if the index's condition is no longer meaningful, is
the index meaningful? I think we can handle both cases the same way.
But clearly an index condition isn't quite the same thing as an index
column, so maybe someone can make a good argument for treating them
differently.
regards, tom lane
On Mon, 2003-12-22 at 10:55, Tom Lane wrote:
Andreas Pflug <pgadmin@pse-consulting.de> writes:
In that sample mentioned the index might be used mostly with a,b
columns. Dropping the index silently might damage the application
because it relies on an (a,b) index to be present. IMHO only Indexes
that span that single column should be dropped without CASCADE.That argument makes no sense to me at all. If you drop the *column*
a or b, and do not thereby break your application, how is the
disappearance of the index on it going to break anything? The index
is meaningless without something to index.
I think Andreas is trying to argue that if you drop column b from index
(a, b) that the index should be converted into index(a) -- assuming of
course there isn't already an index(a).
Rod Taylor <pg@rbt.ca> writes:
I think Andreas is trying to argue that if you drop column b from index
(a, b) that the index should be converted into index(a) -- assuming of
course there isn't already an index(a).
That seems to be well outside the charter of DROP CASCADE. I think we
either drop or don't drop; we don't go building new indexes, which is
what this would take. There are also definitional problems --- for
instance, if the index is UNIQUE, does it transmogrify into a UNIQUE
constraint on A alone (which would most likely fail)?
regards, tom lane
Tom Lane wrote:
Rod Taylor <pg@rbt.ca> writes:
I think Andreas is trying to argue that if you drop column b from index
(a, b) that the index should be converted into index(a) -- assuming of
course there isn't already an index(a).That seems to be well outside the charter of DROP CASCADE. I think we
either drop or don't drop; we don't go building new indexes, which is
what this would take. There are also definitional problems --- for
instance, if the index is UNIQUE, does it transmogrify into a UNIQUE
constraint on A alone (which would most likely fail)?
Agreed, auto creation wouldn't be necessary/expected. If you drop,
objects disappear, you don't expect them to morph. But I'd like to be
inhibited to drop the column if it requires a somewhat recreated index
on (a). So IMHO a DROP INDEX [RESTRICT] should drop only dependent
objects if this won't affect others.
Regards,
Andreas
This is a preliminary patch - don't commit it.
What this patch adds are the CATALOG and NOCATALOG clauses to the CREATE
USER and ALTER USER commands.
These clauses affect the usecatupd column. This makes it easy to create
superusers who cannot manually modify columns (a very nasty power...)
These days, Postgres's built-in command set can do everything you need to
to (except disable triggers and delete all the users...), so I don't see
why people who have the power to create users should also have the power
to munge your entire db server.
There are a few problems that need thinking about, and I would like
comments on how to address them:
1. Should we only allow users who currently hold the catalog perm to grant
it to others? I think yes, since otherwise a regular superuser can create
themselves another account with the catalog priv.
2. Restoring a dump (or dumpall more specifically perhaps) now requires
that the restoring user is more than just a superuser, they must also hold
the catalog priv. This is why:
DELETE FROM pg_shadow WHERE usesysid <> (SELECT datdba FROM pg_database
WHERE datname = 'template0');
And also this:
-X disable-triggers
3. Upgrading from previous postgres will not give their old superusers
back their catalog privilege, unless they dump with 7.5's pg_dump.
Comments?
Chris
Attachments:
user.txttext/plain; name=user.txtDownload
? GNUmakefile
? config.log
? config.status
? configure.lineno
? src/Makefile.global
? src/backend/postgres
? src/backend/access/common/.deps
? src/backend/access/gist/.deps
? src/backend/access/hash/.deps
? src/backend/access/heap/.deps
? src/backend/access/index/.deps
? src/backend/access/nbtree/.deps
? src/backend/access/rtree/.deps
? src/backend/access/transam/.deps
? src/backend/bootstrap/.deps
? src/backend/catalog/.deps
? src/backend/catalog/pg_location.c
? src/backend/catalog/postgres.bki
? src/backend/catalog/postgres.description
? src/backend/commands/.deps
? src/backend/executor/.deps
? src/backend/lib/.deps
? src/backend/libpq/.deps
? src/backend/main/.deps
? src/backend/nodes/.deps
? src/backend/optimizer/geqo/.deps
? src/backend/optimizer/path/.deps
? src/backend/optimizer/plan/.deps
? src/backend/optimizer/prep/.deps
? src/backend/optimizer/util/.deps
? src/backend/parser/.deps
? src/backend/port/.deps
? src/backend/postmaster/.deps
? src/backend/regex/.deps
? src/backend/rewrite/.deps
? src/backend/storage/buffer/.deps
? src/backend/storage/file/.deps
? src/backend/storage/freespace/.deps
? src/backend/storage/ipc/.deps
? src/backend/storage/large_object/.deps
? src/backend/storage/lmgr/.deps
? src/backend/storage/page/.deps
? src/backend/storage/smgr/.deps
? src/backend/tcop/.deps
? src/backend/utils/.deps
? src/backend/utils/adt/.deps
? src/backend/utils/cache/.deps
? src/backend/utils/error/.deps
? src/backend/utils/fmgr/.deps
? src/backend/utils/hash/.deps
? src/backend/utils/init/.deps
? src/backend/utils/mb/.deps
? src/backend/utils/mb/conversion_procs/conversion_create.sql
? src/backend/utils/mb/conversion_procs/ascii_and_mic/.deps
? src/backend/utils/mb/conversion_procs/ascii_and_mic/libascii_and_mic.so.0
? src/backend/utils/mb/conversion_procs/cyrillic_and_mic/.deps
? src/backend/utils/mb/conversion_procs/cyrillic_and_mic/libcyrillic_and_mic.so.0
? src/backend/utils/mb/conversion_procs/euc_cn_and_mic/.deps
? src/backend/utils/mb/conversion_procs/euc_cn_and_mic/libeuc_cn_and_mic.so.0
? src/backend/utils/mb/conversion_procs/euc_jp_and_sjis/.deps
? src/backend/utils/mb/conversion_procs/euc_jp_and_sjis/libeuc_jp_and_sjis.so.0
? src/backend/utils/mb/conversion_procs/euc_kr_and_mic/.deps
? src/backend/utils/mb/conversion_procs/euc_kr_and_mic/libeuc_kr_and_mic.so.0
? src/backend/utils/mb/conversion_procs/euc_tw_and_big5/.deps
? src/backend/utils/mb/conversion_procs/euc_tw_and_big5/libeuc_tw_and_big5.so.0
? src/backend/utils/mb/conversion_procs/latin2_and_win1250/.deps
? src/backend/utils/mb/conversion_procs/latin2_and_win1250/liblatin2_and_win1250.so.0
? src/backend/utils/mb/conversion_procs/latin_and_mic/.deps
? src/backend/utils/mb/conversion_procs/latin_and_mic/liblatin_and_mic.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_ascii/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_ascii/libutf8_and_ascii.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_big5/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_big5/libutf8_and_big5.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_cyrillic/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_cyrillic/libutf8_and_cyrillic.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_euc_cn/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_euc_cn/libutf8_and_euc_cn.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_euc_jp/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_euc_jp/libutf8_and_euc_jp.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_euc_kr/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_euc_kr/libutf8_and_euc_kr.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_euc_tw/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_euc_tw/libutf8_and_euc_tw.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_gb18030/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_gb18030/libutf8_and_gb18030.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_gbk/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_gbk/libutf8_and_gbk.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_iso8859/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_iso8859/libutf8_and_iso8859.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/libutf8_and_iso8859_1.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_johab/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_johab/libutf8_and_johab.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_sjis/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_sjis/libutf8_and_sjis.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_tcvn/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_tcvn/libutf8_and_tcvn.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_uhc/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_uhc/libutf8_and_uhc.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_win1250/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_win1250/libutf8_and_win1250.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_win1256/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_win1256/libutf8_and_win1256.so.0
? src/backend/utils/mb/conversion_procs/utf8_and_win874/.deps
? src/backend/utils/mb/conversion_procs/utf8_and_win874/libutf8_and_win874.so.0
? src/backend/utils/misc/.deps
? src/backend/utils/mmgr/.deps
? src/backend/utils/sort/.deps
? src/backend/utils/time/.deps
? src/bin/initdb/.deps
? src/bin/initdb/initdb
? src/bin/initlocation/initlocation
? src/bin/ipcclean/ipcclean
? src/bin/pg_config/pg_config
? src/bin/pg_controldata/.deps
? src/bin/pg_controldata/pg_controldata
? src/bin/pg_ctl/pg_ctl
? src/bin/pg_dump/.deps
? src/bin/pg_dump/pg_dump
? src/bin/pg_dump/pg_dumpall
? src/bin/pg_dump/pg_restore
? src/bin/pg_encoding/.deps
? src/bin/pg_encoding/pg_encoding
? src/bin/pg_id/.deps
? src/bin/pg_id/pg_id
? src/bin/pg_resetxlog/.deps
? src/bin/pg_resetxlog/pg_resetxlog
? src/bin/psql/.deps
? src/bin/psql/psql
? src/bin/scripts/.deps
? src/bin/scripts/clusterdb
? src/bin/scripts/createdb
? src/bin/scripts/createlang
? src/bin/scripts/createuser
? src/bin/scripts/dropdb
? src/bin/scripts/droplang
? src/bin/scripts/dropuser
? src/bin/scripts/vacuumdb
? src/include/pg_config.h
? src/include/stamp-h
? src/include/catalog/pg_location.h
? src/interfaces/ecpg/compatlib/.deps
? src/interfaces/ecpg/compatlib/libecpg_compat.so.1
? src/interfaces/ecpg/ecpglib/.deps
? src/interfaces/ecpg/ecpglib/libecpg.so.3
? src/interfaces/ecpg/ecpglib/libecpg.so.4
? src/interfaces/ecpg/lib/libecpg.so.3
? src/interfaces/ecpg/pgtypeslib/.deps
? src/interfaces/ecpg/pgtypeslib/libpgtypes.so.1
? src/interfaces/ecpg/preproc/.deps
? src/interfaces/ecpg/preproc/ecpg
? src/interfaces/libpq/.deps
? src/interfaces/libpq/libpq.so.3
? src/pl/plpgsql/src/.deps
? src/pl/plpgsql/src/libplpgsql.so.1
? src/port/.deps
? src/test/regress/postgres.core
Index: doc/src/sgml/ref/alter_user.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql-server/doc/src/sgml/ref/alter_user.sgml,v
retrieving revision 1.32
diff -c -r1.32 alter_user.sgml
*** doc/src/sgml/ref/alter_user.sgml 29 Nov 2003 19:51:38 -0000 1.32
--- doc/src/sgml/ref/alter_user.sgml 24 Dec 2003 12:35:16 -0000
***************
*** 27,32 ****
--- 27,33 ----
[ ENCRYPTED | UNENCRYPTED ] PASSWORD '<replaceable class="PARAMETER">password</replaceable>'
| CREATEDB | NOCREATEDB
| CREATEUSER | NOCREATEUSER
+ | CATALOG | NOCATALOG
| VALID UNTIL '<replaceable class="PARAMETER">abstime</replaceable>'
ALTER USER <replaceable class="PARAMETER">name</replaceable> RENAME TO <replaceable>newname</replaceable>
***************
*** 132,137 ****
--- 133,156 ----
</varlistentry>
<varlistentry>
+ <term><literal>CATALOG</literal></term>
+ <term><literal>NOCATALOG</literal></term>
+ <listitem>
+ <para>
+ This clause determines whether or not a user will be permitted
+ to manually edit the system catalogs, using the regular SQL DML
+ statements.
+ </para>
+
+ <para>
+ Since any user granted this permission can easily make themselves
+ a superuser, the owner of any object or cause complete database
+ destruction, this is a very dangerous privilege to allow.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><replaceable class="PARAMETER">abstime</replaceable></term>
<listitem>
<para>
***************
*** 233,242 ****
</para>
<para>
! Give a user the ability to create other users and new databases:
<programlisting>
! ALTER USER miriam CREATEUSER CREATEDB;
</programlisting>
</para>
</refsect1>
--- 252,262 ----
</para>
<para>
! Give a user the ability to create other users and new databases, but
! not the ability to manually edit the system catalogs:
<programlisting>
! ALTER USER miriam CREATEUSER CREATEDB NOCATALOG;
</programlisting>
</para>
</refsect1>
Index: doc/src/sgml/ref/create_user.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql-server/doc/src/sgml/ref/create_user.sgml,v
retrieving revision 1.32
diff -c -r1.32 create_user.sgml
*** doc/src/sgml/ref/create_user.sgml 14 Dec 2003 00:15:03 -0000 1.32
--- doc/src/sgml/ref/create_user.sgml 24 Dec 2003 12:35:17 -0000
***************
*** 28,33 ****
--- 28,34 ----
| [ ENCRYPTED | UNENCRYPTED ] PASSWORD '<replaceable class="PARAMETER">password</replaceable>'
| CREATEDB | NOCREATEDB
| CREATEUSER | NOCREATEUSER
+ | CATALOG | NOCATALOG
| IN GROUP <replaceable class="PARAMETER">groupname</replaceable> [, ...]
| VALID UNTIL '<replaceable class="PARAMETER">abstime</replaceable>'
</synopsis>
***************
*** 144,149 ****
--- 145,169 ----
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>CATALOG</literal></term>
+ <term><literal>NOCATALOG</literal></term>
+ <listitem>
+ <para>
+ This clause determines whether or not a user will be permitted
+ to manually edit the system catalogs, using the regular SQL DML
+ statements. If this clause is omitted, <literal>NOCATALOG</literal>
+ will be assumed.
+ </para>
+
+ <para>
+ Since any user granted this permission can easily make themselves
+ a superuser, the owner of any object or cause complete database
+ destruction, this is a very dangerous privilege to allow.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><replaceable class="parameter">groupname</replaceable></term>
<listitem>
Index: src/backend/commands/user.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/commands/user.c,v
retrieving revision 1.131
diff -c -r1.131 user.c
*** src/backend/commands/user.c 29 Nov 2003 19:51:47 -0000 1.131
--- src/backend/commands/user.c 24 Dec 2003 12:35:25 -0000
***************
*** 500,505 ****
--- 500,506 ----
int sysid = 0; /* PgSQL system id (valid if havesysid) */
bool createdb = false; /* Can the user create databases? */
bool createuser = false; /* Can this user create users? */
+ bool catalog = false; /* Can this user manually edit the catalogs? */
List *groupElts = NIL; /* The groups the user is a member of */
char *validUntil = NULL; /* The time the login is valid
* until */
***************
*** 507,512 ****
--- 508,514 ----
DefElem *dsysid = NULL;
DefElem *dcreatedb = NULL;
DefElem *dcreateuser = NULL;
+ DefElem *dcatalog = NULL;
DefElem *dgroupElts = NULL;
DefElem *dvalidUntil = NULL;
***************
*** 553,558 ****
--- 555,568 ----
errmsg("conflicting or redundant options")));
dcreateuser = defel;
}
+ else if (strcmp(defel->defname, "catalog") == 0)
+ {
+ if (dcatalog)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("conflicting or redundant options")));
+ dcatalog = defel;
+ }
else if (strcmp(defel->defname, "groupElts") == 0)
{
if (dgroupElts)
***************
*** 578,583 ****
--- 588,595 ----
createdb = intVal(dcreatedb->arg) != 0;
if (dcreateuser)
createuser = intVal(dcreateuser->arg) != 0;
+ if (dcatalog)
+ catalog = intVal(dcatalog->arg) != 0;
if (dsysid)
{
sysid = intVal(dsysid->arg);
***************
*** 603,608 ****
--- 615,623 ----
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to create users")));
+ /* XXX: Need to check here that the superuser doing the granting
+ has the catalog priv if they're granting the catalog priv... */
+
if (strcmp(stmt->user, "public") == 0)
ereport(ERROR,
(errcode(ERRCODE_RESERVED_NAME),
***************
*** 667,674 ****
new_record[Anum_pg_shadow_usecreatedb - 1] = BoolGetDatum(createdb);
AssertState(BoolIsValid(createuser));
new_record[Anum_pg_shadow_usesuper - 1] = BoolGetDatum(createuser);
! /* superuser gets catupd right by default */
! new_record[Anum_pg_shadow_usecatupd - 1] = BoolGetDatum(createuser);
if (password)
{
--- 682,689 ----
new_record[Anum_pg_shadow_usecreatedb - 1] = BoolGetDatum(createdb);
AssertState(BoolIsValid(createuser));
new_record[Anum_pg_shadow_usesuper - 1] = BoolGetDatum(createuser);
! AssertState(BoolIsValid(catalog));
! new_record[Anum_pg_shadow_usecatupd - 1] = BoolGetDatum(catalog);
if (password)
{
***************
*** 753,763 ****
--- 768,780 ----
char encrypted_password[MD5_PASSWD_LEN + 1];
int createdb = -1; /* Can the user create databases? */
int createuser = -1; /* Can this user create users? */
+ int catalog = -1; /* Can this user manually edit the catalogs? */
char *validUntil = NULL; /* The time the login is valid
* until */
DefElem *dpassword = NULL;
DefElem *dcreatedb = NULL;
DefElem *dcreateuser = NULL;
+ DefElem *dcatalog = NULL;
DefElem *dvalidUntil = NULL;
/* Extract options from the statement node tree */
***************
*** 795,800 ****
--- 812,825 ----
errmsg("conflicting or redundant options")));
dcreateuser = defel;
}
+ else if (strcmp(defel->defname, "catalog") == 0)
+ {
+ if (dcatalog)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("conflicting or redundant options")));
+ dcatalog = defel;
+ }
else if (strcmp(defel->defname, "validUntil") == 0)
{
if (dvalidUntil)
***************
*** 812,817 ****
--- 837,844 ----
createdb = intVal(dcreatedb->arg);
if (dcreateuser)
createuser = intVal(dcreateuser->arg);
+ if (dcatalog)
+ catalog = intVal(dcatalog->arg);
if (dvalidUntil)
validUntil = strVal(dvalidUntil->arg);
if (dpassword)
***************
*** 824,835 ****
--- 851,866 ----
if (!superuser() &&
!(createdb < 0 &&
createuser < 0 &&
+ catalog < 0 &&
!validUntil &&
password &&
strcmp(GetUserNameFromId(GetUserId()), stmt->user) == 0))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied")));
+
+ /* XXX: Need to check here that the superuser doing the granting
+ has the catalog priv if they're granting the catalog priv... */
/*
* Scan the pg_shadow relation to be certain the user exists. Note we
***************
*** 865,884 ****
new_record_repl[Anum_pg_shadow_usecreatedb - 1] = 'r';
}
! /*
! * createuser (superuser) and catupd
! *
! * XXX It's rather unclear how to handle catupd. It's probably best to
! * keep it equal to the superuser status, otherwise you could end up
! * with a situation where no existing superuser can alter the
! * catalogs, including pg_shadow!
! */
if (createuser >= 0)
{
new_record[Anum_pg_shadow_usesuper - 1] = BoolGetDatum(createuser > 0);
new_record_repl[Anum_pg_shadow_usesuper - 1] = 'r';
!
! new_record[Anum_pg_shadow_usecatupd - 1] = BoolGetDatum(createuser > 0);
new_record_repl[Anum_pg_shadow_usecatupd - 1] = 'r';
}
--- 896,912 ----
new_record_repl[Anum_pg_shadow_usecreatedb - 1] = 'r';
}
! /* createuser (superuser) */
if (createuser >= 0)
{
new_record[Anum_pg_shadow_usesuper - 1] = BoolGetDatum(createuser > 0);
new_record_repl[Anum_pg_shadow_usesuper - 1] = 'r';
! }
!
! /* catupd */
! if (catalog >= 0)
! {
! new_record[Anum_pg_shadow_usecatupd - 1] = BoolGetDatum(catalog > 0);
new_record_repl[Anum_pg_shadow_usecatupd - 1] = 'r';
}
Index: src/backend/parser/gram.y
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/parser/gram.y,v
retrieving revision 2.441
diff -c -r2.441 gram.y
*** src/backend/parser/gram.y 1 Dec 2003 22:07:58 -0000 2.441
--- src/backend/parser/gram.y 24 Dec 2003 12:37:00 -0000
***************
*** 334,340 ****
BACKWARD BEFORE BEGIN_P BETWEEN BIGINT BINARY BIT
BOOLEAN_P BOTH BY
! CACHE CALLED CASCADE CASE CAST CHAIN CHAR_P
CHARACTER CHARACTERISTICS CHECK CHECKPOINT CLASS CLOSE
CLUSTER COALESCE COLLATE COLUMN COMMENT COMMIT
COMMITTED CONSTRAINT CONSTRAINTS CONVERSION_P CONVERT COPY CREATE CREATEDB
--- 334,340 ----
BACKWARD BEFORE BEGIN_P BETWEEN BIGINT BINARY BIT
BOOLEAN_P BOTH BY
! CACHE CALLED CASCADE CASE CAST CATALOG_P CHAIN CHAR_P
CHARACTER CHARACTERISTICS CHECK CHECKPOINT CLASS CLOSE
CLUSTER COALESCE COLLATE COLUMN COMMENT COMMIT
COMMITTED CONSTRAINT CONSTRAINTS CONVERSION_P CONVERT COPY CREATE CREATEDB
***************
*** 370,376 ****
MATCH MAXVALUE MINUTE_P MINVALUE MODE MONTH_P MOVE
! NAMES NATIONAL NATURAL NCHAR NEW NEXT NO NOCREATEDB
NOCREATEUSER NONE NOT NOTHING NOTIFY NOTNULL NULL_P
NULLIF NUMERIC
--- 370,376 ----
MATCH MAXVALUE MINUTE_P MINVALUE MODE MONTH_P MOVE
! NAMES NATIONAL NATURAL NCHAR NEW NEXT NO NOCATALOG_P NOCREATEDB
NOCREATEUSER NONE NOT NOTHING NOTIFY NOTNULL NULL_P
NULLIF NUMERIC
***************
*** 672,677 ****
--- 672,685 ----
{
$$ = makeDefElem("createuser", (Node *)makeInteger(FALSE));
}
+ | CATALOG_P
+ {
+ $$ = makeDefElem("catalog", (Node *)makeInteger(TRUE));
+ }
+ | NOCATALOG_P
+ {
+ $$ = makeDefElem("catalog", (Node *)makeInteger(FALSE));
+ }
| IN_P GROUP_P user_list
{
$$ = makeDefElem("groupElts", (Node *)$3);
***************
*** 7348,7353 ****
--- 7356,7362 ----
| CACHE
| CALLED
| CASCADE
+ | CATALOG_P
| CHAIN
| CHARACTERISTICS
| CHECKPOINT
***************
*** 7431,7436 ****
--- 7440,7446 ----
| NATIONAL
| NEXT
| NO
+ | NOCATALOG_P
| NOCREATEDB
| NOCREATEUSER
| NOTHING
Index: src/backend/parser/keywords.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/parser/keywords.c,v
retrieving revision 1.144
diff -c -r1.144 keywords.c
*** src/backend/parser/keywords.c 29 Nov 2003 19:51:51 -0000 1.144
--- src/backend/parser/keywords.c 24 Dec 2003 12:37:00 -0000
***************
*** 65,70 ****
--- 65,71 ----
{"cascade", CASCADE},
{"case", CASE},
{"cast", CAST},
+ {"catalog", CATALOG_P},
{"chain", CHAIN},
{"char", CHAR_P},
{"character", CHARACTER},
***************
*** 205,210 ****
--- 206,212 ----
{"new", NEW},
{"next", NEXT},
{"no", NO},
+ {"nocatalog", NOCATALOG_P},
{"nocreatedb", NOCREATEDB},
{"nocreateuser", NOCREATEUSER},
{"none", NONE},
Index: src/bin/pg_dump/pg_dumpall.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/pg_dump/pg_dumpall.c,v
retrieving revision 1.29
diff -c -r1.29 pg_dumpall.c
*** src/bin/pg_dump/pg_dumpall.c 29 Nov 2003 19:52:05 -0000 1.29
--- src/bin/pg_dump/pg_dumpall.c 24 Dec 2003 12:37:03 -0000
***************
*** 277,289 ****
if (server_version >= 70100)
res = executeQuery(conn,
"SELECT usename, usesysid, passwd, usecreatedb, "
! "usesuper, valuntil "
"FROM pg_shadow "
"WHERE usesysid <> (SELECT datdba FROM pg_database WHERE datname = 'template0')");
else
res = executeQuery(conn,
"SELECT usename, usesysid, passwd, usecreatedb, "
! "usesuper, valuntil "
"FROM pg_shadow "
"WHERE usesysid <> (SELECT datdba FROM pg_database WHERE datname = 'template1')");
--- 277,289 ----
if (server_version >= 70100)
res = executeQuery(conn,
"SELECT usename, usesysid, passwd, usecreatedb, "
! "usesuper, valuntil, usecatupd "
"FROM pg_shadow "
"WHERE usesysid <> (SELECT datdba FROM pg_database WHERE datname = 'template0')");
else
res = executeQuery(conn,
"SELECT usename, usesysid, passwd, usecreatedb, "
! "usesuper, valuntil, usecatupd "
"FROM pg_shadow "
"WHERE usesysid <> (SELECT datdba FROM pg_database WHERE datname = 'template1')");
***************
*** 312,317 ****
--- 312,322 ----
appendPQExpBuffer(buf, " CREATEUSER");
else
appendPQExpBuffer(buf, " NOCREATEUSER");
+
+ if (strcmp(PQgetvalue(res, i, 6), "t") == 0)
+ appendPQExpBuffer(buf, " CATALOG");
+ else
+ appendPQExpBuffer(buf, " NOCATALOG");
if (!PQgetisnull(res, i, 5))
appendPQExpBuffer(buf, " VALID UNTIL '%s'",
Index: src/bin/scripts/createuser.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/scripts/createuser.c,v
retrieving revision 1.7
diff -c -r1.7 createuser.c
*** src/bin/scripts/createuser.c 29 Nov 2003 19:52:07 -0000 1.7
--- src/bin/scripts/createuser.c 24 Dec 2003 12:37:03 -0000
***************
*** 32,37 ****
--- 32,39 ----
{"no-createdb", no_argument, NULL, 'D'},
{"adduser", no_argument, NULL, 'a'},
{"no-adduser", no_argument, NULL, 'A'},
+ {"catalog", no_argument, NULL, 'c'},
+ {"no-catalog", no_argument, NULL, 'C'},
{"sysid", required_argument, NULL, 'i'},
{"pwprompt", no_argument, NULL, 'P'},
{"encrypted", no_argument, NULL, 'E'},
***************
*** 52,57 ****
--- 54,60 ----
bool quiet = false;
int createdb = 0;
int adduser = 0;
+ int catalog = 0;
char *sysid = NULL;
bool pwprompt = false;
int encrypted = 0; /* 0 uses server default */
***************
*** 100,105 ****
--- 103,114 ----
case 'D':
createdb = -1;
break;
+ case 'c':
+ catalog = +1;
+ break;
+ case 'C':
+ catalog = -1;
+ break;
case 'i':
sysid = optarg;
break;
***************
*** 184,189 ****
--- 193,209 ----
adduser = -1;
}
+ if (catalog == 0)
+ {
+ char *reply;
+
+ reply = simple_prompt("Shall the new user be allowed to manually edit the system catalogs? (y/n) ", 1, true);
+ if (check_yesno_response(reply) == 1)
+ catalog = +1;
+ else
+ catalog = -1;
+ }
+
initPQExpBuffer(&sql);
printfPQExpBuffer(&sql, "CREATE USER %s", fmtId(newuser));
***************
*** 206,211 ****
--- 226,235 ----
appendPQExpBuffer(&sql, " CREATEUSER");
if (adduser == -1)
appendPQExpBuffer(&sql, " NOCREATEUSER");
+ if (catalog == +1)
+ appendPQExpBuffer(&sql, " CATALOG");
+ if (catalog == -1)
+ appendPQExpBuffer(&sql, " NOCATALOG");
appendPQExpBuffer(&sql, ";\n");
conn = connectDatabase("template1", host, port, username, password, progname);
***************
*** 240,245 ****
--- 264,271 ----
printf(_(" -A, --no-adduser user cannot add new users\n"));
printf(_(" -d, --createdb user can create new databases\n"));
printf(_(" -D, --no-createdb user cannot create databases\n"));
+ printf(_(" -c, --catalog user can manually edit the system catalogs\n"));
+ printf(_(" -C, --no-catalog user cannot manually edit the system catalogs\n"));
printf(_(" -P, --pwprompt assign a password to new user\n"));
printf(_(" -E, --encrypted encrypt stored password\n"));
printf(_(" -N, --unencrypted do no encrypt stored password\n"));
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
1. Should we only allow users who currently hold the catalog perm to grant
it to others? I think yes, since otherwise a regular superuser can create
themselves another account with the catalog priv.
That brings up the whole business of just how super is a superuser,
and does it even make sense to try to design a "not quite superuser"
protection state. I'm not convinced that the usecatupd flag is so well
thought out that we should expose it for general use without some
consideration of alternative designs.
As an example, it might make more sense to create a separate flag bit
that simply grants the ability to add and delete users (non-superusers,
presumably), with none of the other attributes of a superuser. If I
recall your original concern properly, this would be a safer facility
for what you wanted to accomplish.
3. Upgrading from previous postgres will not give their old superusers
back their catalog privilege, unless they dump with 7.5's pg_dump.
Only if you make it default to NOCATALOG, which is highly debatable in
my mind, since it is non-backwards-compatible.
regards, tom lane
Tom Lane wrote:
As an example, it might make more sense to create a separate flag bit
that simply grants the ability to add and delete users
(non-superusers, presumably), with none of the other attributes of a
superuser. If I recall your original concern properly, this would be
a safer facility for what you wanted to accomplish.
I agree, this would be a more useful way to slice it up. Or maybe
someone wants to implement the SQL equivalent of "sudo".