Dear patchers,

Please find attached a patch to fix schema ownership on first connection, so that non system schemas reflect the database owner.

(1) It adds a new "datisinit" attribute to pg_database, which tells
    whether the database initialization was performed or not.
    The documentation is updated accordingly.

(2) This boolean is tested in postinit.c:ReverifyMyDatabase,
    and InitializeDatabase is called if necessary.

(3) The routine updates pg_database datisinit, as well as pg_namespace
    ownership and acl stuff.

(4) Some validation is added. This part validates for me
    (although rules and errors regression tests are broken in current
     cvs head, independtly of this patch).

Some questions/comments :

- is the place for the first connection housekeeping updates appropriate?
  it seems so from ReverifyMyDatabase comments, but these are only comments.

- it is quite convenient to use SPI_* stuff for this very rare updates,
  but I'm not that confident about the issue... On the other hand, the
  all-C solution would result in a much longer and less obvious code:-(

- it is unclear to me whether it should be allowed to skip this under
  some condition, when the database is accessed in "debug" mode from
  a standalone postgres for instance?

- also I'm wondering how to react (what to do and how to do it) on
  errors within or under these initialization. For instance, how to
  be sure that the CurrentUser is reset as expected?

Thanks in advance for any comments.

Have a nice day.

--
Fabien.
*** ./doc/src/sgml/catalogs.sgml.orig   Mon Jun  7 09:08:11 2004
--- ./doc/src/sgml/catalogs.sgml        Tue Jun  8 10:14:26 2004
***************
*** 1536,1541 ****
--- 1536,1552 ----
       </row>
  
       <row>
+       <entry><structfield>datisinit</structfield></entry>
+       <entry><type>bool</type></entry>
+       <entry></entry>
+       <entry>
+        False when a database is just created but housekeeping initialization
+        tasks are not performed yet. On the first connection, the initialization
+        is performed and the boolean is turned to true.
+       </entry>
+      </row>
+ 
+      <row>
        <entry><structfield>datlastsysoid</structfield></entry>
        <entry><type>oid</type></entry>
        <entry></entry>
*** ./src/backend/commands/dbcommands.c.orig    Wed May 26 17:28:40 2004
--- ./src/backend/commands/dbcommands.c Tue Jun  8 10:14:26 2004
***************
*** 424,429 ****
--- 424,430 ----
        new_record[Anum_pg_database_encoding - 1] = Int32GetDatum(encoding);
        new_record[Anum_pg_database_datistemplate - 1] = BoolGetDatum(false);
        new_record[Anum_pg_database_datallowconn - 1] = BoolGetDatum(true);
+       new_record[Anum_pg_database_datisinit - 1] = BoolGetDatum(false);
        new_record[Anum_pg_database_datlastsysoid - 1] = 
ObjectIdGetDatum(src_lastsysoid);
        new_record[Anum_pg_database_datvacuumxid - 1] = 
TransactionIdGetDatum(src_vacuumxid);
        new_record[Anum_pg_database_datfrozenxid - 1] = 
TransactionIdGetDatum(src_frozenxid);
*** ./src/backend/utils/adt/acl.c.orig  Thu Jun  3 15:05:57 2004
--- ./src/backend/utils/adt/acl.c       Tue Jun  8 10:16:16 2004
***************
*** 2203,2205 ****
--- 2203,2225 ----
                         errmsg("unrecognized privilege type: \"%s\"", priv_type)));
        return ACL_NO_RIGHTS;           /* keep compiler quiet */
  }
+ 
+ /* acl acl_switch_grantor(acl, oldgrantor, newgrantor);
+  * switch grantor id in aclitem array. 
+  * used internally for fixing owner rights in new databases.
+  * must be STRICT.
+  */
+ Datum acl_switch_grantor(PG_FUNCTION_ARGS)
+ {
+       Acl * acls = PG_GETARG_ACL_P_COPY(0);
+       int i, 
+               old_grantor = PG_GETARG_INT32(1), 
+               new_grantor = PG_GETARG_INT32(2);
+       AclItem * item;
+ 
+       for (i=0, item=ACL_DAT(acls); i<ACL_NUM(acls); i++, item++)
+               if (item->ai_grantor == old_grantor)
+                       item->ai_grantor = new_grantor;
+ 
+       PG_RETURN_ACL_P(acls);
+ }
*** ./src/backend/utils/init/postinit.c.orig    Tue Jun  1 10:21:23 2004
--- ./src/backend/utils/init/postinit.c Tue Jun  8 10:23:09 2004
***************
*** 50,55 ****
--- 50,109 ----
  
  /*** InitPostgres support ***/
  
+ #include "executor/spi.h"
+ 
+ /* Do housekeeping initializations if required, on first connection.
+  */
+ static void InitializeDatabase(const char * dbname)
+ {
+       /* su */
+       AclId saved_user = GetUserId();
+       SetUserId(1);
+                       
+       /* Querying in C is nice, but SQL is nicer.
+        * This is only done once in a lifetime of the database,
+        * so paying for the parser/optimiser cost is not that bad?
+        * What if that fails?
+        */
+       SetQuerySnapshot();
+ 
+       if (SPI_connect() != SPI_OK_CONNECT)
+               ereport(ERROR, (errmsg("SPI_connect failed")));
+ 
+       if (SPI_exec("UPDATE " SystemCatalogName "." DatabaseRelationName
+                                " SET datisinit=TRUE"
+                                " WHERE datname=CURRENT_DATABASE()"
+                                "   AND datisinit=FALSE" , 0) != SPI_OK_UPDATE)
+               ereport(ERROR, (errmsg("database initialization %s update failed",
+                                                          DatabaseRelationName)));
+ 
+       if (SPI_processed==1)
+       { 
+               /* ok, we have it! */
+               
+               if (SPI_exec("UPDATE " SystemCatalogName "." NamespaceRelationName
+                                        " SET nspowner=datdba,"
+                                        "     nspacl  = acl_switch_grantor(nspacl, 1, 
datdba)"
+                                        " FROM " SystemCatalogName "." 
DatabaseRelationName " "
+                                        " WHERE nspname NOT LIKE"
+                                        "        
ALL(ARRAY['pg_%','information_schema'])"
+                                        "   AND datname=CURRENT_DATABASE()"
+                                        "   AND nspowner!=datdba;", 0) != 
SPI_OK_UPDATE)
+                       ereport(ERROR, (errmsg("database initialization %s update 
failed",
+                                                                  
NamespaceRelationName)));
+ 
+               if (SPI_processed>0)
+                       ereport(LOG, /* don't bother the user about these details... */
+                                       (errmsg("database initialization schema owner 
updates: %d",
+                                                       SPI_processed)));
+       }
+       /* some other concurrent connection did it, let us proceed. */
+ 
+       if (SPI_finish() != SPI_OK_FINISH)
+               ereport(ERROR, (errmsg("SPI_finish failed")));
+ 
+       SetUserId(saved_user);
+ }
  
  /* --------------------------------
   *            ReverifyMyDatabase
***************
*** 130,135 ****
--- 184,195 ----
                 errmsg("database \"%s\" is not currently accepting connections",
                                name)));
  
+       /* Do we need the housekeeping initialization of the database?
+        * could be skipped on standalone "panic" mode?
+        */
+       if (!dbform->datisinit)
+               InitializeDatabase(name);
+ 
        /*
         * OK, we're golden.  Only other to-do item is to save the encoding
         * info out of the pg_database tuple.
*** ./src/include/catalog/catname.h.orig        Sat Nov 29 23:40:58 2003
--- ./src/include/catalog/catname.h     Tue Jun  8 10:14:26 2004
***************
*** 14,19 ****
--- 14,20 ----
  #ifndef CATNAME_H
  #define CATNAME_H
  
+ #define  SystemCatalogName "pg_catalog"
  
  #define  AggregateRelationName "pg_aggregate"
  #define  AccessMethodRelationName "pg_am"
*** ./src/include/catalog/catversion.h.orig     Mon Jun  7 09:08:19 2004
--- ./src/include/catalog/catversion.h  Tue Jun  8 13:04:16 2004
***************
*** 53,58 ****
   */
  
  /*                                                    yyyymmddN */
! #define CATALOG_VERSION_NO    200406061
  
  #endif
--- 53,58 ----
   */
  
  /*                                                    yyyymmddN */
! #define CATALOG_VERSION_NO    200406081
  
  #endif
*** ./src/include/catalog/pg_attribute.h.orig   Mon Apr  5 12:06:43 2004
--- ./src/include/catalog/pg_attribute.h        Tue Jun  8 10:14:26 2004
***************
*** 287,299 ****
  DATA(insert ( 1262 encoding                   23 -1 4   3 0 -1 -1 t p i t f f t 0));
  DATA(insert ( 1262 datistemplate      16 -1 1   4 0 -1 -1 t p c t f f t 0));
  DATA(insert ( 1262 datallowconn               16 -1 1   5 0 -1 -1 t p c t f f t 0));
! DATA(insert ( 1262 datlastsysoid      26 -1 4   6 0 -1 -1 t p i t f f t 0));
! DATA(insert ( 1262 datvacuumxid               28 -1 4   7 0 -1 -1 t p i t f f t 0));
! DATA(insert ( 1262 datfrozenxid               28 -1 4   8 0 -1 -1 t p i t f f t 0));
  /* do not mark datpath as toastable; GetRawDatabaseInfo won't cope */
! DATA(insert ( 1262 datpath                    25 -1 -1  9 0 -1 -1 f p i t f f t 0));
! DATA(insert ( 1262 datconfig    1009 -1 -1 10 1 -1 -1 f x i f f f t 0));
! DATA(insert ( 1262 datacl               1034 -1 -1 11 1 -1 -1 f x i f f f t 0));
  DATA(insert ( 1262 ctid                               27 0  6  -1 0 -1 -1 f p s t f 
f t 0));
  DATA(insert ( 1262 oid                                26 0  4  -2 0 -1 -1 t p i t f 
f t 0));
  DATA(insert ( 1262 xmin                               28 0  4  -3 0 -1 -1 t p i t f 
f t 0));
--- 287,300 ----
  DATA(insert ( 1262 encoding                   23 -1 4   3 0 -1 -1 t p i t f f t 0));
  DATA(insert ( 1262 datistemplate      16 -1 1   4 0 -1 -1 t p c t f f t 0));
  DATA(insert ( 1262 datallowconn               16 -1 1   5 0 -1 -1 t p c t f f t 0));
! DATA(insert ( 1262 datisinit          16 -1 1   6 0 -1 -1 t p c t f f t 0));
! DATA(insert ( 1262 datlastsysoid      26 -1 4   7 0 -1 -1 t p i t f f t 0));
! DATA(insert ( 1262 datvacuumxid               28 -1 4   8 0 -1 -1 t p i t f f t 0));
! DATA(insert ( 1262 datfrozenxid               28 -1 4   9 0 -1 -1 t p i t f f t 0));
  /* do not mark datpath as toastable; GetRawDatabaseInfo won't cope */
! DATA(insert ( 1262 datpath                    25 -1 -1 10 0 -1 -1 f p i t f f t 0));
! DATA(insert ( 1262 datconfig    1009 -1 -1 11 1 -1 -1 f x i f f f t 0));
! DATA(insert ( 1262 datacl               1034 -1 -1 12 1 -1 -1 f x i f f f t 0));
  DATA(insert ( 1262 ctid                               27 0  6  -1 0 -1 -1 f p s t f 
f t 0));
  DATA(insert ( 1262 oid                                26 0  4  -2 0 -1 -1 t p i t f 
f t 0));
  DATA(insert ( 1262 xmin                               28 0  4  -3 0 -1 -1 t p i t f 
f t 0));
*** ./src/include/catalog/pg_class.h.orig       Mon Apr  5 12:06:43 2004
--- ./src/include/catalog/pg_class.h    Tue Jun  8 10:14:26 2004
***************
*** 146,152 ****
  DESCR("");
  DATA(insert OID = 1261 (  pg_group            PGNSP 87 PGUID 0 1261 0 0 0 0 f t r 3  
0 0 0 0 0 f f f f _null_ ));
  DESCR("");
! DATA(insert OID = 1262 (  pg_database PGNSP 88 PGUID 0 1262 0 0 0 0 f t r 11  0 0 0 
0 0 t f f f _null_ ));
  DESCR("");
  DATA(insert OID = 376  (  pg_xactlock PGNSP  0 PGUID 0        0 0 0 0 0 f t s 1  0 0 
0 0 0 f f f f _null_ ));
  DESCR("");
--- 146,152 ----
  DESCR("");
  DATA(insert OID = 1261 (  pg_group            PGNSP 87 PGUID 0 1261 0 0 0 0 f t r 3  
0 0 0 0 0 f f f f _null_ ));
  DESCR("");
! DATA(insert OID = 1262 (  pg_database PGNSP 88 PGUID 0 1262 0 0 0 0 f t r 12  0 0 0 
0 0 t f f f _null_ ));
  DESCR("");
  DATA(insert OID = 376  (  pg_xactlock PGNSP  0 PGUID 0        0 0 0 0 0 f t s 1  0 0 
0 0 0 f f f f _null_ ));
  DESCR("");
*** ./src/include/catalog/pg_database.h.orig    Tue Feb 10 02:55:26 2004
--- ./src/include/catalog/pg_database.h Tue Jun  8 10:14:26 2004
***************
*** 38,43 ****
--- 38,44 ----
        int4            encoding;               /* character encoding */
        bool            datistemplate;  /* allowed as CREATE DATABASE template? */
        bool            datallowconn;   /* new connections allowed? */
+       bool            datisinit;              /* was it already initialized? */
        Oid                     datlastsysoid;  /* highest OID to consider a system 
OID */
        TransactionId datvacuumxid; /* all XIDs before this are vacuumed */
        TransactionId datfrozenxid; /* all XIDs before this are frozen */
***************
*** 57,76 ****
   *            compiler constants for pg_database
   * ----------------
   */
! #define Natts_pg_database                             11
  #define Anum_pg_database_datname              1
  #define Anum_pg_database_datdba                       2
  #define Anum_pg_database_encoding             3
  #define Anum_pg_database_datistemplate        4
  #define Anum_pg_database_datallowconn 5
! #define Anum_pg_database_datlastsysoid        6
! #define Anum_pg_database_datvacuumxid 7
! #define Anum_pg_database_datfrozenxid 8
! #define Anum_pg_database_datpath              9
! #define Anum_pg_database_datconfig            10
! #define Anum_pg_database_datacl                       11
  
! DATA(insert OID = 1 (  template1 PGUID ENCODING t t 0 0 0 "" _null_ _null_ ));
  DESCR("Default template database");
  #define TemplateDbOid                 1
  
--- 58,78 ----
   *            compiler constants for pg_database
   * ----------------
   */
! #define Natts_pg_database                             12
  #define Anum_pg_database_datname              1
  #define Anum_pg_database_datdba                       2
  #define Anum_pg_database_encoding             3
  #define Anum_pg_database_datistemplate        4
  #define Anum_pg_database_datallowconn 5
! #define Anum_pg_database_datisinit            6
! #define Anum_pg_database_datlastsysoid        7
! #define Anum_pg_database_datvacuumxid 8
! #define Anum_pg_database_datfrozenxid 9
! #define Anum_pg_database_datpath              10
! #define Anum_pg_database_datconfig            11
! #define Anum_pg_database_datacl                       12
  
! DATA(insert OID = 1 (  template1 PGUID ENCODING t t t 0 0 0 "" _null_ _null_ ));
  DESCR("Default template database");
  #define TemplateDbOid                 1
  
*** ./src/include/catalog/pg_proc.h.orig        Mon Jun  7 09:08:20 2004
--- ./src/include/catalog/pg_proc.h     Tue Jun  8 11:17:11 2004
***************
*** 3588,3593 ****
--- 3588,3596 ----
  DATA(insert OID = 2243 ( bit_or                                                  
PGNSP PGUID 12 t f f f i 1 1560 "1560" _null_ aggregate_dummy - _null_));
  DESCR("bitwise-or bit aggregate");
  
+ DATA(insert OID = 2245 ( acl_switch_grantor            PGNSP PGUID 12 f f t f i 3 
1034 "1034 23 23" _null_ acl_switch_grantor - _null_));
+ DESCR("internal function to update grantors in acls");
+ 
  /*
   * Symbolic values for provolatile column: these indicate whether the result
   * of a function is dependent *only* on the values of its explicit arguments,
*** ./src/include/utils/acl.h.orig      Thu Jun  3 15:05:58 2004
--- ./src/include/utils/acl.h   Tue Jun  8 10:14:26 2004
***************
*** 236,241 ****
--- 236,242 ----
  extern Datum makeaclitem(PG_FUNCTION_ARGS);
  extern Datum aclitem_eq(PG_FUNCTION_ARGS);
  extern Datum hash_aclitem(PG_FUNCTION_ARGS);
+ extern Datum acl_switch_grantor(PG_FUNCTION_ARGS);
  
  /*
   * prototypes for functions in aclchk.c
*** ./src/test/regress/expected/create_database.out.orig        Tue Jun  8 10:14:26 
2004
--- ./src/test/regress/expected/create_database.out     Tue Jun  8 14:52:37 2004
***************
*** 0 ****
--- 1,84 ----
+ -- partial tests.
+ CREATE USER calvin CREATEUSER CREATEDB;
+ CREATE DATABASE calvin WITH OWNER calvin;
+ CREATE USER hobbes;
+ CREATE DATABASE hobbes WITH OWNER hobbes;
+ CREATE USER suzy;
+ SELECT datname, datdba, datisinit 
+ FROM pg_database
+ WHERE datname = ANY(ARRAY['calvin', 'hobbes'])
+ ORDER BY datname;
+  datname | datdba | datisinit 
+ ---------+--------+-----------
+  calvin  |    100 | f
+  hobbes  |    101 | f
+ (2 rows)
+ 
+ \c calvin calvin
+ SELECT CURRENT_USER;
+  current_user 
+ --------------
+  calvin
+ (1 row)
+ 
+ SELECT datname, datdba, datisinit 
+ FROM pg_database
+ WHERE datname = ANY(ARRAY['calvin', 'hobbes'])
+ ORDER BY datname;
+  datname | datdba | datisinit 
+ ---------+--------+-----------
+  calvin  |    100 | t
+  hobbes  |    101 | f
+ (2 rows)
+ 
+ -- It depends on the super-user name:-(
+ -- I do not have any accessor to test anything based on userids:-(
+ SELECT nspname, usename --, nspacl
+ FROM pg_catalog.pg_namespace JOIN pg_catalog.pg_user ON nspowner=usesysid
+ WHERE nspname NOT LIKE ALL(ARRAY['pg_%', 'information_schema'])
+ ORDER BY nspname;
+  nspname | usename 
+ ---------+---------
+  public  | calvin
+ (1 row)
+ 
+ REVOKE ALL PRIVILEGES ON SCHEMA public FROM PUBLIC;
+ GRANT ALL PRIVILEGES ON SCHEMA public TO suzy;
+ \c hobbes calvin
+ SELECT CURRENT_USER;
+  current_user 
+ --------------
+  calvin
+ (1 row)
+ 
+ SELECT datname, datdba, datisinit 
+ FROM pg_database
+ WHERE datname = ANY(ARRAY['calvin', 'hobbes'])
+ ORDER BY datname;
+  datname | datdba | datisinit 
+ ---------+--------+-----------
+  calvin  |    100 | t
+  hobbes  |    101 | t
+ (2 rows)
+ 
+ -- see comments above.
+ SELECT nspname, usename --, nspacl
+ FROM pg_catalog.pg_namespace JOIN pg_catalog.pg_user ON nspowner=usesysid
+ WHERE nspname NOT LIKE ALL(ARRAY['pg_%', 'information_schema'])
+ ORDER BY nspname;
+  nspname | usename 
+ ---------+---------
+  public  | hobbes
+ (1 row)
+ 
+ --
+ -- A part here was designed to test quite artificially the grantor switch.
+ -- the test cannot work simply... so I just put these comments here.
+ -- 
+ -- For one thing, it is not often needed:
+ -- . Indeed, template1 as "default" null acl, so there is nothing to fix.
+ -- . when copying from another template, you must own it, so the rights
+ --   on schemas already belong to you!
+ -- Moreover, only the super-user granted rights are switched, but... 
+ -- the super-user name is not fixed, so testing it is not easy:-(
+ -- It would only be useful for rights set by THE super-user in template1.
*** ./src/test/regress/parallel_schedule.orig   Mon Jun  7 09:08:23 2004
--- ./src/test/regress/parallel_schedule        Tue Jun  8 11:25:38 2004
***************
*** 38,44 ****
  # ----------
  # The third group of parallel test
  # ----------
! test: constraints triggers create_misc create_aggregate create_operator inherit 
vacuum
  
  # Depends on the above
  test: create_index create_view
--- 38,44 ----
  # ----------
  # The third group of parallel test
  # ----------
! test: constraints triggers create_misc create_aggregate create_operator inherit 
vacuum create_database
  
  # Depends on the above
  test: create_index create_view
*** ./src/test/regress/serial_schedule.orig     Mon Jun  7 09:08:23 2004
--- ./src/test/regress/serial_schedule  Tue Jun  8 10:14:26 2004
***************
*** 49,54 ****
--- 49,55 ----
  test: create_aggregate
  test: create_operator
  test: create_index
+ test: create_database
  test: inherit
  test: vacuum
  test: create_view
*** ./src/test/regress/sql/create_database.sql.orig     Tue Jun  8 10:14:26 2004
--- ./src/test/regress/sql/create_database.sql  Tue Jun  8 14:50:48 2004
***************
*** 0 ****
--- 1,59 ----
+ -- partial tests.
+ 
+ CREATE USER calvin CREATEUSER CREATEDB;
+ CREATE DATABASE calvin WITH OWNER calvin;
+ 
+ CREATE USER hobbes;
+ CREATE DATABASE hobbes WITH OWNER hobbes;
+ 
+ CREATE USER suzy;
+ 
+ SELECT datname, datdba, datisinit 
+ FROM pg_database
+ WHERE datname = ANY(ARRAY['calvin', 'hobbes'])
+ ORDER BY datname;
+ 
+ \c calvin calvin
+ SELECT CURRENT_USER;
+ 
+ SELECT datname, datdba, datisinit 
+ FROM pg_database
+ WHERE datname = ANY(ARRAY['calvin', 'hobbes'])
+ ORDER BY datname;
+ 
+ -- It depends on the super-user name:-(
+ -- I do not have any accessor to test anything based on userids:-(
+ SELECT nspname, usename --, nspacl
+ FROM pg_catalog.pg_namespace JOIN pg_catalog.pg_user ON nspowner=usesysid
+ WHERE nspname NOT LIKE ALL(ARRAY['pg_%', 'information_schema'])
+ ORDER BY nspname;
+ 
+ REVOKE ALL PRIVILEGES ON SCHEMA public FROM PUBLIC;
+ GRANT ALL PRIVILEGES ON SCHEMA public TO suzy;
+ 
+ \c hobbes calvin
+ SELECT CURRENT_USER;
+ 
+ SELECT datname, datdba, datisinit 
+ FROM pg_database
+ WHERE datname = ANY(ARRAY['calvin', 'hobbes'])
+ ORDER BY datname;
+ 
+ -- see comments above.
+ SELECT nspname, usename --, nspacl
+ FROM pg_catalog.pg_namespace JOIN pg_catalog.pg_user ON nspowner=usesysid
+ WHERE nspname NOT LIKE ALL(ARRAY['pg_%', 'information_schema'])
+ ORDER BY nspname;
+ 
+ --
+ -- A part here was designed to test quite artificially the grantor switch.
+ -- the test cannot work simply... so I just put these comments here.
+ -- 
+ -- For one thing, it is not often needed:
+ -- . Indeed, template1 as "default" null acl, so there is nothing to fix.
+ -- . when copying from another template, you must own it, so the rights
+ --   on schemas already belong to you!
+ -- Moreover, only the super-user granted rights are switched, but... 
+ -- the super-user name is not fixed, so testing it is not easy:-(
+ -- It would only be useful for rights set by THE super-user in template1.
+ 
---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

               http://archives.postgresql.org

Reply via email to