Re: fix schema ownership on first connection preliminary patch - Mailing list pgsql-patches

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