Hi,
On 11/30/20 11:29 PM, Tom Lane wrote:
> "Drouvot, Bertrand" <bdrouvot@amazon.com> writes:
>> here is a scenario that produces an orphaned function (means it does not
>> belong to any namespace):
>> [ drop schema before committing function creation ]
> Hm. Historically we've not been too fussed about preventing such race
> conditions, and I wonder just how far is sane to take it. Should we
> acquire lock on the function's argument/result data types? Its language?
>
> Given the precedent of RangeVarGetAndCheckCreationNamespace, I'm
> willing to accept this patch's goals as stated.
Thanks!
Forgot to mention an observed side effect: pg_upgrade failing during the
"Creating dump of database schemas" step, with things like:
pg_dump: last built-in OID is 16383
pg_dump: reading extensions
pg_dump: identifying extension members
pg_dump: reading schemas
pg_dump: reading user-defined tables
pg_dump: reading user-defined functions
pg_dump: error: schema with OID 16385 does not exist
> But it feels like
> we need some clearer shared understanding of which things we are
> willing to bother with preventing races for, and which we aren't.
>
>> Please find attached a patch that adds a LockDatabaseObject() call in
>> QualifiedNameGetCreationNamespace() to prevent such orphaned situations.
> I don't think that actually succeeds in preventing the race, it'll
> just delay your process till the deletion is committed.
oooh, i was thinking the other way around: it was protecting the other
situation (when the drop schema is waiting for the create function to
finish).
The other scenario you are referring to needs to be addressed as well,
thanks!
> But you
> already have the namespaceId. Note the complex retry logic in
> RangeVarGetAndCheckCreationNamespace; without something on the same
> order, you're not closing the hole in any meaningful way.
understood with the scenario you were referring to previously (which was
not the one I focused on).
> Likely
> what this patch should do is refactor that function so that its guts
> can be used for other object-creation scenarios.
Will have a look.
> (The fact that
> this is so far from trivial is one reason I'm not in a hurry to
> extend this sort of protection to other dependencies.)
>
> regards, tom lane
Thanks!
Bertrand