Thread: fix schema ownership for database owner on first connection

fix schema ownership for database owner on first connection

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

Attachment

Re: fix schema ownership for database owner on first connection

From
Tom Lane
Date:
Fabien COELHO <coelho@cri.ensmp.fr> writes:
> (2) This boolean is tested in postinit.c:ReverifyMyDatabase,
>      and InitializeDatabase is called if necessary.

And what happens if multiple backends try to connect at the same time?

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

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.

            regards, tom lane

Re: fix schema ownership for database owner on first

From
Fabien COELHO
Date:
Dear Tom,

>> (2) This boolean is tested in postinit.c:ReverifyMyDatabase,
>>      and InitializeDatabase is called if necessary.
>
> And what happens if multiple backends try to connect at the same time?

I took care of that one!

There is a lock on the update of pg_database when switching off datisinit.
The backend which gets the lock is to update the schema ownership, and
others will wait for the lock to be released, and skip the stuff.

I don't think I forgot something, but I may be wrong.

Also, as I noted I used SPI internally to do that simply with sql. I don't
know if this is an issue.


>> (4) Some validation is added.
>
> 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.

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

Re: fix schema ownership for database owner on first connection

From
Tom Lane
Date:
Fabien COELHO <coelho@cri.ensmp.fr> 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

Re: fix schema ownership for database owner on first

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

Hummm... an interesting view, indeed. It fits neither my experience of
programming nor my experience of computer security;-)

It taught me that anything which is not tested does not work or will not
work later on because someone will break some assumption.

On the other hand, I understand that heavy test on "make check" are not
necessary. So maybe some "make big_check" or the like?

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

Re: fix schema ownership for database owner on first connection

From
Christopher Kings-Lynne
Date:
>>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


Re: fix schema ownership for database owner on first connection

From
Bruce Momjian
Date:
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
  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

Re: fix schema ownership for database owner on first

From
Fabien COELHO
Date:
Dear Bruce,

On Wed, 9 Jun 2004, Bruce Momjian wrote:

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

Done.

! Date: Wed, 9 Jun 2004 14:31:59 +0200 (CEST)
! From: Fabien COELHO <coelho@cri.ensmp.fr>
! To: PostgreSQL Patches <pgsql-patches@postgresql.org>
! Subject: [PATCHES] fix schema ownership on first connection v2

Have a nice day,

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