Re: [PATCHES] fix schema ownership for database owner on first connection

2004-06-09 Thread Bruce Momjian

Would you adjust based on Tom's comments and resubmit?  Thanks.

---

Fabien COELHO wrote:
 
 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.

Content-Description: 

[ Attachment, skipping... ]

 
 ---(end of broadcast)---
 TIP 6: Have you searched our list archives?
 
http://archives.postgresql.org

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

---(end of broadcast)---
TIP 8: explain analyze is your friend


[PATCHES] fix schema ownership for database owner on first connection

2004-06-08 Thread Fabien COELHO
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.sgmlTue Jun  8 10:14:26 2004
***
*** 1536,1541 
--- 1536,1552 
   /row
  
   row
+   entrystructfielddatisinit/structfield/entry
+   entrytypebool/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
entrystructfielddatlastsysoid/structfield/entry
entrytypeoid/type/entry
entry/entry
*** ./src/backend/commands/dbcommands.c.origWed 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); iACL_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.origTue 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 

Re: [PATCHES] fix schema ownership for database owner on first connection

2004-06-08 Thread Tom Lane
Fabien COELHO [EMAIL PROTECTED] writes:
 I do not think it's a good idea for the regression tests to do anything 
 to any databases other than regression. Especially not databases with 
 names that might match people's real databases.

 Oh, you mean calvin and hobbes might use postgresql? ;-)

 Ok, so I guess I can use regressionuser[123], regression[123] as names in 
 the validation. Writing tests cases is not fun, so I tried to put some fun 
 by using these characters.

I don't really think it's necessary for the regression tests to test
this functionality.

regards, tom lane

---(end of broadcast)---
TIP 8: explain analyze is your friend


Re: [PATCHES] fix schema ownership for database owner on first connection

2004-06-08 Thread Christopher Kings-Lynne
Ok, so I guess I can use regressionuser[123], regression[123] as names in 
the validation. Writing tests cases is not fun, so I tried to put some fun 
by using these characters.
I don't really think it's necessary for the regression tests to test
this functionality.
Once this machinery is in...can we use it to safely remove users?
Chris
---(end of broadcast)---
TIP 6: Have you searched our list archives?
  http://archives.postgresql.org