Re: postgres_fdw fails to see that array type belongs to extension

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: David Geier <geidav(dot)pg(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: postgres_fdw fails to see that array type belongs to extension
Date: 2024-01-15 19:01:54
Message-ID: 1584532.1705345314@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

David Geier <geidav(dot)pg(at)gmail(dot)com> writes:
> On 12/27/23 18:38, Tom Lane wrote:
>> Hmm. It seems odd that if an extension defines a type, the type is
>> listed as a member of the extension but the array type is not.
>> That makes it look like the array type is an externally-created
>> thing that happens to depend on the extension, when it's actually
>> part of the extension. I'm surprised we've not run across other
>> misbehaviors traceable to that.

> Agreed.
> The attached patch just adds a 2nd dependency between the array type and
> the extension, using recordDependencyOnCurrentExtension().

I don't like this patch too much: it fixes the problem in a place far
away from GenerateTypeDependencies' existing treatment of extension
dependencies, and relatedly to that, fails to update the comments
it has falsified. I think we should do it more like the attached.
This approach means that relation rowtypes will also be explicitly
listed as extension members, which seems like it is fixing the same
sort of bug as the array case.

I also noted that you'd not run check-world, because there's a test
case that changes output. This is good though, because we don't need
to add any new test to prove it does what we want.

There's one big remaining to-do item, I think: experimentation with
pg_upgrade proves that a binary upgrade fails to fix the extension
membership of arrays/rowtypes. That's because pg_dump needs to
manually reconstruct the membership dependencies, and it thinks that
it doesn't need to do anything for implicit arrays. Normally the
point of that is that we want to exactly reconstruct the extension's
contents, but I think in this case we'd really like to add the
additional pg_depend entries even if they weren't there before.
Otherwise people wouldn't get the new behavior until they do a
full dump/reload.

I can see two ways we could do that:

* add logic to pg_dump

* modify ALTER EXTENSION ADD TYPE so that it automatically recurses
from a base type to its array type (and I guess we'd need to add
something for relation rowtypes and multiranges, too).

I think I like the latter approach because it's like how we
handle ownership: pg_dump doesn't emit any commands to explicitly
change the ownership of dependent types, either. (But see [1].)
We could presumably steal some logic from ALTER TYPE OWNER.
I've not tried to code that here, though.

regards, tom lane

[1] https://www.postgresql.org/message-id/1580383.1705343264%40sss.pgh.pa.us

Attachment Content-Type Size
v2-0001-Fix-dependency-of-array-of-type-owned-by-extension.patch text/x-diff 4.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2024-01-15 19:07:13 Re: [PATCH] LockAcquireExtended improvement
Previous Message Robert Haas 2024-01-15 18:34:46 Re: pg_ctl start may return 0 even if the postmaster has been already started on Windows