Re: fix schema ownership on first connection preliminary patch

Is there a TODO here?

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

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073