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

> 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’re backwards.

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

P