Thread: Re: [COMMITTERS] pgsql/src/include/parser (parse_node.h parse_oper.h)
> 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
Re: [HACKERS] Re: [COMMITTERS] pgsql/src/include/parser (parse_node.h parse_oper.h)
From
Tom Lane
Date:
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