Re: [PATCH] Extend ALTER OPERATOR to support adding commutator, negator, hashes, and merges

From: Tommy Pavlicek <tommypav122(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, jian(dot)universality(at)gmail(dot)com
Subject: Re: [PATCH] Extend ALTER OPERATOR to support adding commutator, negator, hashes, and merges
Date: 2023-10-11 15:11:00
Message-ID: CAEhP-W_1bMTLan2LtzS+xr=b-Wep56gw+OoirABQ8b2aOw+5yg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Oct 10, 2023 at 9:32 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Tommy Pavlicek <tommypav122(at)gmail(dot)com> writes:
> > I did notice one further potential bug. When creating an operator and
> > adding a commutator, PostgreSQL only links the commutator back to the
> > operator if the commutator has no commutator of its own, but the
> > create operation succeeds regardless of whether this linkage happens.
>
> > In other words, if A and B are a pair of commutators and one creates
> > another operator, C, with A as its commutator, then C will link to A,
> > but A will still link to B (and B to A). It's not clear to me if this
> > in itself is a problem, but unless I've misunderstood something
> > operator C must be the same as B so it implies an error by the user
> > and there could also be issues if A was deleted since C would continue
> > to refer to the deleted A.
>
> Yeah, it'd make sense to tighten that up. Per the discussion so far,
> we should not allow an operator's commutator/negator links to change
> once set, so overwriting the existing link with a different value
> would be wrong. But allowing creation of the new operator to proceed
> with a different outcome than expected isn't good either. I think
> we should start throwing an error for that.
>
> regards, tom lane

Thanks.

I've added another patch (0002-require_unused_neg_com-v1.patch) that
prevents using a commutator or negator that's already part of a pair.
The only other changes from my email yesterday are that in the ALTER
command I moved the post alter hook to after OperatorUpd and the
addition of tests to verify that we can't use an existing commutator
or negator with the ALTER command.

I believe this can all be looked at again.

Cheers,
Tommy

Attachment Content-Type Size
0002-require_unused_neg_com-v1.patch application/octet-stream 6.2 KB
0003-refactor-v3.patch application/octet-stream 5.7 KB
0001-bug-fixes-v3.patch application/octet-stream 5.5 KB
0004-alter_op-v4.patch application/octet-stream 40.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2023-10-11 15:14:24 Re: Add null termination to string received in parallel apply worker
Previous Message Matthias van de Meent 2023-10-11 14:09:21 Re: Lowering the default wal_blocksize to 4K