Re: Allowing extensions to supply operator-/function-specific info

> On Mar 5, 2019, at 3:26 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Paul Ramsey <pramsey(at)cleverelephant(dot)ca> writes:
>> Thanks for the patch, I’ve applied and smoothed and taken your advice on schema-qualified lookups as well.
>
> Hm, I think your addition of this bit is wrong:
>
> + /*
> + * Arguments were swapped to put the index value on the
> + * left, so we need the commutated operator for
> + * the OpExpr
> + */
> + if (swapped)
> + {
> + oproid = get_commutator(oproid);
> + if (!OidIsValid(oproid))
> PG_RETURN_POINTER((Node *)NULL);
> + }
>
> We already did the operator lookup with the argument types in the desired
> order, so this is introducing an extra swap. The only reason it appears
> to work, I suspect, is that all your index operators are self-commutators.

I was getting regression failures until I re-swapped the operator…

SELEcT * FROM foobar WHERE ST_Within(ConstA, VarB)

Place the indexed operator in the Left, now:

Left == VarB
Right == ConstA
Strategy == Within
get_opfamily_member(opfamilyoid, Left, Right, Within)

Unless we change the strategy number when we assign the left/right we’re looking up an operator for “B within A”, so we’re backwards.

I feel OK about it, if for no other reason than it passes all the tests :)

P