Thread: Re: [COMMITTERS] pgsql/src/include/parser (parse_node.h parse_oper.h)

Re: [COMMITTERS] pgsql/src/include/parser (parse_node.h parse_oper.h)

From
Thomas Lockhart
Date:
> Modified Files:
>         parse_node.h parse_oper.h
> Remove bogus code in oper_exact --- if it didn't find an exact
> match then it tried for a self-commutative operator with the reversed input
> data types.  This is pretty silly; there could never be such an operator,
> except maybe in binary-compatible-type scenarios, and we have oper_inexact
> for that.  Besides which, the oprsanity regress test would complain about
> such an operator.  Remove nonfunctional code and simplify routine calling
> convention accordingly.

Ooh! That codes sounds familiar. What I was trying for was to cover
the case that, for example, (int4 < float4) was not implemented, but
that (float4 >= int4) was. If this is already handled elsewhere, or if
this goal is nonsensical, then cutting the defective code is the right
thing. But if the code just needed repairing, we should put it back in
and get it right next time...
                    - Thomas

-- 
Thomas Lockhart                lockhart@alumni.caltech.edu
South Pasadena, California


Thomas Lockhart <lockhart@alumni.caltech.edu> writes:
>> Remove bogus code in oper_exact --- if it didn't find an exact
>> match then it tried for a self-commutative operator with the reversed input
>> data types.  This is pretty silly;

> Ooh! That codes sounds familiar. What I was trying for was to cover
> the case that, for example, (int4 < float4) was not implemented, but
> that (float4 >= int4) was. If this is already handled elsewhere, or if
> this goal is nonsensical, then cutting the defective code is the right
> thing. But if the code just needed repairing, we should put it back in
> and get it right next time...

Well, what it was actually looking for was not a commuted operator but
the *same* operator name with the reversed data types; and then
demanding that this operator link to itself as its own commutator.
I don't believe such a case can ever arise in practice --- it certainly
does not now, since the opr_sanity regress test would complain if it
did.

I don't see any really good way for operator lookup to substitute
commutative operators, since it has only an operator name and not (yet)
any pg_operator entry to check the commutator link of.  Surely you don't
want to hardwire in knowledge that, say, '<' and '>=' are likely to be
names of commutators.

In any case, failing to provide a full set of commutable comparison
operators will hobble the optimizer, so an implementor of a new data
type would be pretty foolish not to provide both operators.  So I don't
think it's worth providing code in operator lookup to handle this
scenario.
        regards, tom lane