Re: Rethinking opclass member checks and dependency strength

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
Cc: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Rethinking opclass member checks and dependency strength
Date: 2020-01-05 17:33:10
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> writes:
> The latest version of this patch (from 2019/09/14) no longer applies,
> although maybe it's some issue with patch format (applying it using
> patch works fine, git am fails with "Patch format detection failed.").

Hm, seems to be just a trivial conflict against the copyright-date-update
patch. Updated version attached.

> On Sat, Sep 14, 2019 at 07:01:33PM -0400, Tom Lane wrote:
>> This change results in a possibly surprising change in the expected output
>> for the test: an optional support function that had been
>> created as part of CREATE OPERATOR CLASS will now be dumped as part of
>> ALTER OPERATOR FAMILY. Maybe that's too surprising? Another approach
>> that we could use is to give up the premise that soft dependencies are
>> always on the opfamily. If we kept the dependencies pointing to the
>> same objects as before (opclass or opfamily) and only twiddled the
>> dependency strength, then pg_dump's output would not change. Now,
>> this would possibly result in dropping a still-useful family member
>> if it were incorrectly tied to an opclass that's dropped --- but that
>> would have happened before, too. I'm not quite sure if we really want
>> to editorialize on the user's decisions about which grouping to tie
>> family members to.

> I agree it's a bit weird to add a dependency on an opfamily and not the
> opclass. Not just because of the pg_dump weirdness, but doesn't it mean
> that after a DROP OPERATOR CLASS we might still reject a DROP OPERATOR
> because of the remaining opfamily dependency? (Haven't tried, so maybe
> that works fine).

I poked at the idea of retaining the user's decisions as to whether
a member object is a member of an individual opclass or an opfamily,
but soon realized that there's a big problem with that: we don't have
So there's no way to express the concept of "add this at the opclass
level", if you forgot to add it during initial opclass creation.

I suppose that some case could be made for adding such syntax, but
it seems like a significant amount of work, and in the end it seems
like it's better to trust the system to get these assignments right.
Letting the user do it doesn't add much except the opportunity
to shoot oneself in the foot.

> One minor comment from me is that maybe "amcheckmembers" is a bit
> misleading. In my mind "check" implies a plain passive check, not
> something that may actually tweak the dependency type.

Hmm. I'm not wedded to that name, but do you have a better proposal?
The end goal (not realized in this patch, of course) is that these
callbacks would perform fairly thorough checking of opclass members,
missing only the ability to check that all required members are present.
So I don't want to name them something like "amfixdependencies", even
if that's all they're doing right now.

I see your point that "check" suggests a read-only operation, but
I'm not sure about a better verb. I thought of "amvalidatemembers",
but that's not really much better than "check" is it?

regards, tom lane

Attachment Content-Type Size
am-check-members-callback-3.patch text/x-diff 45.1 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message legrand legrand 2020-01-05 18:01:59 Re: Planning counters in pg_stat_statements (using pgss_store)
Previous Message Dagfinn Ilmari Mannsåker 2020-01-05 16:48:18 Re: pgsql: Add basic TAP tests for psql's tab-completion logic.