fix schema ownership on first connection preliminary patch

Started by Fabien COELHOover 21 years ago9 messages

coelho@cri.ensmp.fr

over 21 years ago

1 attachment(s)

Dear hackers,

Please find attached a preliminary patch to fix schema ownership on first
connection. It is for comments and advices as I still have doubts about
various how-and-where issues, thus it is not submitted to the patch list.

(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. The acl item update part is not yet
appropriate: I'd like some answers before going on.

(4) Some validation is added. It passes for me.

My questions:

- 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 your answers and comments.

Have a nice day.

--
Fabien Coelho - coelho@cri.ensmp.fr

Attachments:

database_owner.patchtext/plain; charset=US-ASCII; name=database_owner.patchDownload

*** ./doc/src/sgml/catalogs.sgml.orig	Mon Apr  5 12:06:40 2004
--- ./doc/src/sgml/catalogs.sgml	Fri May  7 08:46:38 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	Tue Apr 20 09:35:43 2004
--- ./src/backend/commands/dbcommands.c	Fri May  7 21:52:38 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/init/postinit.c.orig	Fri Dec 12 19:45:09 2003
--- ./src/backend/utils/init/postinit.c	Sat May  8 11:05:36 2004
***************
*** 49,54 ****
--- 49,108 ----
  
  /*** 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! otherwise, some concurrent connection did it. */
+ 		
+ 		if (SPI_exec("UPDATE " SystemCatalogName "." NamespaceRelationName
+ 					 " SET nspowner=datdba,"
+ 					 "     nspacl=NULL "
+ 					 // nspacl   = aclitems_switch_grantor(nspacl, 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)));
+ 	}
+ 
+ 	if (SPI_finish() != SPI_OK_FINISH)
+ 		ereport(ERROR, (errmsg("SPI_finish failed")));
+ 
+ 	SetUserId(saved_user);
+ }
  
  /* --------------------------------
   *		ReverifyMyDatabase
***************
*** 129,134 ****
--- 183,194 ----
  		 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	Sat May  8 09:09:11 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 May  3 10:45:38 2004
--- ./src/include/catalog/catversion.h	Sat May  8 10:43:47 2004
***************
*** 53,58 ****
   */
  
  /*							yyyymmddN */
! #define CATALOG_VERSION_NO	200405020
  
  #endif
--- 53,58 ----
   */
  
  /*							yyyymmddN */
! #define CATALOG_VERSION_NO	200405080
  
  #endif
*** ./src/include/catalog/pg_attribute.h.orig	Mon Apr  5 12:06:43 2004
--- ./src/include/catalog/pg_attribute.h	Fri May  7 21:46:24 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	Fri May  7 21:39:32 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	Fri May  7 08:57:38 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/test/regress/expected/create_database.out.orig	Sat May  8 11:07:33 2004
--- ./src/test/regress/expected/create_database.out	Sat May  8 11:08:57 2004
***************
*** 0 ****
--- 1,66 ----
+ CREATE USER calvin;
+ CREATE DATABASE calvin WITH OWNER calvin;
+ CREATE USER hobbes;
+ CREATE DATABASE hobbes WITH OWNER hobbes;
+ 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)
+ 
+ SELECT * 
+ FROM pg_namespace
+ WHERE nspname NOT LIKE ALL(ARRAY['pg_%', 'information_schema'])
+ ORDER BY nspname;
+  nspname | nspowner | nspacl 
+ ---------+----------+--------
+  public  |      100 | 
+ (1 row)
+ 
+ \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)
+ 
+ SELECT * 
+ FROM pg_namespace
+ WHERE nspname NOT LIKE ALL(ARRAY['pg_%', 'information_schema'])
+ ORDER BY nspname;
+  nspname | nspowner | nspacl 
+ ---------+----------+--------
+  public  |      101 | 
+ (1 row)
+ 
*** ./src/test/regress/serial_schedule.orig	Sun Jan 11 05:58:17 2004
--- ./src/test/regress/serial_schedule	Sat May  8 11:02:52 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	Fri May  7 21:58:06 2004
--- ./src/test/regress/sql/create_database.sql	Sat May  8 10:01:47 2004
***************
*** 0 ****
--- 1,36 ----
+ CREATE USER calvin;
+ CREATE DATABASE calvin WITH OWNER calvin;
+ 
+ CREATE USER hobbes;
+ CREATE DATABASE hobbes WITH OWNER hobbes;
+ 
+ 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;
+ 
+ SELECT * 
+ FROM pg_namespace
+ WHERE nspname NOT LIKE ALL(ARRAY['pg_%', 'information_schema'])
+ ORDER BY nspname;
+ 
+ \c hobbes calvin
+ SELECT CURRENT_USER;
+ 
+ SELECT datname, datdba, datisinit 
+ FROM pg_database
+ WHERE datname = ANY(ARRAY['calvin', 'hobbes'])
+ ORDER BY datname;
+ 
+ SELECT * 
+ FROM pg_namespace
+ WHERE nspname NOT LIKE ALL(ARRAY['pg_%', 'information_schema'])
+ ORDER BY nspname;

Bruce Momjian

pgman@candle.pha.pa.us

over 21 years ago

In reply to: Fabien COELHO (#1)

1 attachment(s)

fix schema ownership on first connection preliminary patch v2

I have added the v2 version of this patch to the patch queue (attached).
I agree with Tom that there is no need for regression tests for this
feature and have removed that part of the patch.

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------

Fabien COELHO wrote:

Dear hackers,

Please find attached a preliminary patch to fix schema ownership on first
connection. It is for comments and advices as I still have doubts about
various how-and-where issues, thus it is not submitted to the patch list.

(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. The acl item update part is not yet
appropriate: I'd like some answers before going on.

(4) Some validation is added. It passes for me.

My questions:

- 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 your answers and comments.

Have a nice day.

--
Fabien Coelho - coelho@cri.ensmp.fr

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Attachments:

/bjm/difftext/plainDownload

Dear hackers,

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


This revised patch includes fixes after Tom comments on names used in
the validation. However, the validation is still there. It is easy to
edit it out if required, but I still think that such a test is a good thing.


(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) The datisnit 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.


    I think that the race condition if two backends connect for
    the first time to the same database is correctly taken care of,
    as only one backend will update the datisinit flag and then will fix the
    schema ownership.


(4) Some validation is added.


Some questions:

- 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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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	Wed Jun  9 11:52:02 2004
***************
*** 50,55 ****
--- 50,110 ----
  
  /*** InitPostgres support ***/
  
+ #include "executor/spi.h"
+ 
+ /* Do housekeeping initializations if required, on first connection.
+  * This function is expected to be called from within a transaction block.
+  */
+ 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 ****
--- 185,196 ----
  		 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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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

Bruce Momjian

pgman@candle.pha.pa.us

over 21 years ago

In reply to: Bruce Momjian (#2)

Re: fix schema ownership on first connection preliminary patch

Tom is reviewing this.

---------------------------------------------------------------------------

Bruce Momjian wrote:

I have added the v2 version of this patch to the patch queue (attached).
I agree with Tom that there is no need for regression tests for this
feature and have removed that part of the patch.

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------

Fabien COELHO wrote:

Dear hackers,

Please find attached a preliminary patch to fix schema ownership on first
connection. It is for comments and advices as I still have doubts about
various how-and-where issues, thus it is not submitted to the patch list.

(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. The acl item update part is not yet
appropriate: I'd like some answers before going on.

(4) Some validation is added. It passes for me.

My questions:

- 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 your answers and comments.

Have a nice day.

--
Fabien Coelho - coelho@cri.ensmp.fr

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings
-- 
Bruce Momjian                        |  http://candle.pha.pa.us
pgman@candle.pha.pa.us               |  (610) 359-1001
+  If your life is a hard drive,     |  13 Roberts Road
+  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Dear hackers,

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

This revised patch includes fixes after Tom comments on names used in
the validation. However, the validation is still there. It is easy to
edit it out if required, but I still think that such a test is a good thing.

(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) The datisnit 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.

I think that the race condition if two backends connect for
the first time to the same database is correctly taken care of,
as only one backend will update the datisinit flag and then will fix the
schema ownership.

(4) Some validation is added.

Some questions:

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

- 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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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	Wed Jun  9 11:52:02 2004
***************
*** 50,55 ****
--- 50,110 ----

/*** InitPostgres support ***/

+ #include "executor/spi.h"
+ 
+ /* Do housekeeping initializations if required, on first connection.
+  * This function is expected to be called from within a transaction block.
+  */
+ 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 ****
--- 185,196 ----
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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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	Wed Jun  9 10:26:39 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

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Tom Lane

tgl@sss.pgh.pa.us

over 21 years ago

In reply to: Bruce Momjian (#2)

Re: fix schema ownership on first connection preliminary patch v2

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Fabien COELHO wrote:

Please find attached a preliminary patch to fix schema ownership on first
connection. It is for comments and advices as I still have doubts about
various how-and-where issues, thus it is not submitted to the patch list.

I have added the v2 version of this patch to the patch queue (attached).

I do apologize for not having looked at this patch sooner, but it's been
at the bottom of the priority queue :-(

In general I do not like the approach of using SPI for this, because
it has to execute before the system is really quite fully up. Just
looking at InitPostgres, I note that the namespace search path isn't
set yet, for example, and I'm not real sure what that implies for
execution of these queries. I'm also uncomfortable with the fact that
RelationCacheInitializePhase3 isn't done yet --- the SPI execution could
pollute the relcache with stuff we don't really want written into
the relcache cache file. (Much of this could be dodged by having
ReverifyMyDatabase just pass back the datinit flag, and then taking
action on it (if any) at the bottom of InitPostgres instead of where
the patch puts it. But I'm still against using SPI for it.)

Also, since we already have AlterSchemaOwner coded at the C level,
the original rationale of not wanting to add code has been rendered
moot.

I do not like the hardwired assumption that userID 1 exists and is
a superuser. The code is also broken to assume that ID 1 is the owner
of the public schema in the source database (though this part is at
least easy to fix, and would go away anyway if using AlterSchemaOwner).

Lastly, I'm unconvinced that the (lack of) locking is safe. Two
backends trying to connect at the same time would make conflicting
attempts to UPDATE pg_database, and if the default transaction isolation
mode is SERIALIZABLE then one of them is going to get an actual failure,
not just decide it has nothing to do. A possible alternative is to take
out ExclusiveLock on pg_namespace before re-examining pg_database.

However, enough about implementation deficiencies. Did we really have
consensus that the system should do this in the first place? I was
only about halfway sold on the notion of changing public's ownership.
I especially dislike the detail that it will alter the ownership of
*all* non-builtin schemas, not only "public". If someone has added
special-purpose schemas to template1, it seems unlikely that this is
the behavior they'd want.

I'm also wondering about what side-effects this will have on pg_dump
behavior. In particular, will pg_dump try to "ALTER OWNER public",
and if so will that be appropriate? We haven't previously needed to
assume that we are restoring into a database with the same datowner as
we dumped from...

I think we should leave the behavior alone, at least for this release
cycle.

regards, tom lane

Christopher Kings-Lynne

chriskl@familyhealth.com.au

over 21 years ago

In reply to: Tom Lane (#4)

Re: fix schema ownership on first connection preliminary

I'm also wondering about what side-effects this will have on pg_dump
behavior. In particular, will pg_dump try to "ALTER OWNER public",
and if so will that be appropriate? We haven't previously needed to
assume that we are restoring into a database with the same datowner as
we dumped from...

In my batch of pg_dump patches that have gone in, pg_dump will now do
exactly that. The reason was so that if someone altered the owner of
their public schema, it should be dumped like that.

Chris

Bruce Momjian

pgman@candle.pha.pa.us

over 21 years ago

In reply to: Tom Lane (#4)

Re: fix schema ownership on first connection preliminary patch

Is there a TODO here?

---------------------------------------------------------------------------

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Fabien COELHO wrote:

Please find attached a preliminary patch to fix schema ownership on first
connection. It is for comments and advices as I still have doubts about
various how-and-where issues, thus it is not submitted to the patch list.

I have added the v2 version of this patch to the patch queue (attached).

I do apologize for not having looked at this patch sooner, but it's been
at the bottom of the priority queue :-(

In general I do not like the approach of using SPI for this, because
it has to execute before the system is really quite fully up. Just
looking at InitPostgres, I note that the namespace search path isn't
set yet, for example, and I'm not real sure what that implies for
execution of these queries. I'm also uncomfortable with the fact that
RelationCacheInitializePhase3 isn't done yet --- the SPI execution could
pollute the relcache with stuff we don't really want written into
the relcache cache file. (Much of this could be dodged by having
ReverifyMyDatabase just pass back the datinit flag, and then taking
action on it (if any) at the bottom of InitPostgres instead of where
the patch puts it. But I'm still against using SPI for it.)

Also, since we already have AlterSchemaOwner coded at the C level,
the original rationale of not wanting to add code has been rendered
moot.

I do not like the hardwired assumption that userID 1 exists and is
a superuser. The code is also broken to assume that ID 1 is the owner
of the public schema in the source database (though this part is at
least easy to fix, and would go away anyway if using AlterSchemaOwner).

Lastly, I'm unconvinced that the (lack of) locking is safe. Two
backends trying to connect at the same time would make conflicting
attempts to UPDATE pg_database, and if the default transaction isolation
mode is SERIALIZABLE then one of them is going to get an actual failure,
not just decide it has nothing to do. A possible alternative is to take
out ExclusiveLock on pg_namespace before re-examining pg_database.

However, enough about implementation deficiencies. Did we really have
consensus that the system should do this in the first place? I was
only about halfway sold on the notion of changing public's ownership.
I especially dislike the detail that it will alter the ownership of
*all* non-builtin schemas, not only "public". If someone has added
special-purpose schemas to template1, it seems unlikely that this is
the behavior they'd want.

I'm also wondering about what side-effects this will have on pg_dump
behavior. In particular, will pg_dump try to "ALTER OWNER public",
and if so will that be appropriate? We haven't previously needed to
assume that we are restoring into a database with the same datowner as
we dumped from...

I think we should leave the behavior alone, at least for this release
cycle.

regards, tom lane

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Fabien COELHO

coelho@cri.ensmp.fr

over 21 years ago

In reply to: Tom Lane (#4)

Re: fix schema ownership on first connection preliminary

I have added the v2 version of this patch to the patch queue (attached).

I do apologize for not having looked at this patch sooner, but it's been
at the bottom of the priority queue :-(

No need to apoligize.

In general I do not like the approach of using SPI for this. [...]

Ok.

Well, my actual motivation for using SPI is to write 3 lines of SQL were
the intent is obvious instead of 60 lines of obscure C. I'm just lazy.

Also, since we already have AlterSchemaOwner coded at the C level,
the original rationale of not wanting to add code has been rendered
moot.

Well, I did not noticed this good function.

I do not like the hardwired assumption that userID 1 exists and is
a superuser.

With UNIX, there is a pretty hard assumption that root is number 0, and I
assumed - wrongly - that the same line of reasonning applied to pg.

The code is also broken to assume that ID 1 is the owner of the public
schema in the source database (though this part is at least easy to fix,
and would go away anyway if using AlterSchemaOwner).

My implicit assumption was that if it is not 1, it means that someone
decide to give some template1 schemas to someone, and I don't have to
change that.

However, enough about implementation deficiencies.

Anyway, it is good to know.

Did we really have consensus that the system should do this in the first
place?

I'm convinced that the current status is not appropriate, but that does
not mean that what the patch suggests a good fix.

The current status is that when you do "CREATE DATABASE foo WITH OWNER
calvin", user calvin which "owns" the database is prevented from
manipulating the "public" schema...

The underlying question is "what is a database owner?". My feeling is that
it is like a unix-directory owner, that is someone who can do whatever it
likes at home, but cannot change the system (/bin, /lib).

I was only about halfway sold on the notion of changing public's
ownership. I especially dislike the detail that it will alter the
ownership of *all* non-builtin schemas, not only "public". If someone
has added special-purpose schemas to template1, it seems unlikely that
this is the behavior they'd want.

Mmm... my assumption here was that either it is a "system" schema, and it
is special, or it is not, and then it is not special. But it is only an
assumption, which makes sense with the "unix-like" owner approach.

I think we should leave the behavior alone, at least for this release
cycle.

Ok. Thanks for your comments.

--
Fabien Coelho - coelho@cri.ensmp.fr

Fabien COELHO

coelho@cri.ensmp.fr

over 21 years ago

In reply to: Bruce Momjian (#6)

Re: fix schema ownership on first connection preliminary

Dear Bruce,

Is there a TODO here?

I think yes. I would suggest an open entry like:

. decide what a "database owner" is expected to be able do (esp. wrt acls)
on non-system schema, e.g. "public", and then implement it.
- should the database owner own the "public" schema by default?
- what about other non-system schemas?
- what about other objects?

ISTM that it reflects that although the "what" is unclear, there is an
issue somewhere;-)

--
Fabien Coelho - coelho@cri.ensmp.fr

Bruce Momjian

pgman@candle.pha.pa.us

over 21 years ago

In reply to: Fabien COELHO (#8)

Re: fix schema ownership on first connection preliminary

Added to TODO:

* Set proper permissions on non-system schemas during db creation

Currently all schemas are owned by the super-user because they are
copied from the template1 database.

---------------------------------------------------------------------------

Fabien COELHO wrote:

Dear Bruce,

Is there a TODO here?

I think yes. I would suggest an open entry like:

. decide what a "database owner" is expected to be able do (esp. wrt acls)
on non-system schema, e.g. "public", and then implement it.
- should the database owner own the "public" schema by default?
- what about other non-system schemas?
- what about other objects?

ISTM that it reflects that although the "what" is unclear, there is an
issue somewhere;-)

--
Fabien Coelho - coelho@cri.ensmp.fr

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073