> On Mar 5, 2019, at 3:26 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Paul Ramsey <pramsey@cleverelephant.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’rebackwards.
I feel OK about it, if for no other reason than it passes all the tests :)
P