While reviewing the SP-Gist patch I noticed an error that I've seen
before, notably in the rangetypes patch: cross-type operators should
not be included in an opclass merely because their commutators are.
To be useful in an index opclass, an operator has to take the indexed
column's datatype as its *left* input, because index scan conditions
are always "indexed_column OP comparison_value". So, for example,
if you've got a GiST opclass on points, it might be useful to include
"point <@ box" in that opclass, but there is no reason to include
"box @> point" in it. (If the operators are marked as commutators,
the planner can figure out what to do with "box @> indexed_point_col",
but this is not dependent on any opclass entries.)
Both of the aforementioned patches contained not only useless pg_amop
entries, but utterly broken "support" code that would have crashed
if it ever could have been reached, because it made the wrong
assumptions about which input would be on which side of the index
clause.
So it finally occurred to me to add an opr_sanity test case that
accounts for this, and lo and behold, it found three similarly bogus
entries that were already in the code:
***************
*** 1077,1082 ****
--- 1092,1115 ---- ---------+----------- (0 rows)
+ -- Check that each operator listed in pg_amop has an associated opclass,
+ -- that is one whose opcintype matches oprleft (possibly by coercion).
+ -- Otherwise the operator is useless because it cannot be matched to an index.
+ -- (In principle it could be useful to list such operators in multiple-datatype
+ -- btree opfamilies, but in practice you'd expect there to be an opclass for
+ -- every datatype the family knows about.)
+ SELECT p1.amopfamily, p1.amopstrategy, p1.amopopr
+ FROM pg_amop AS p1
+ WHERE NOT EXISTS(SELECT 1 FROM pg_opclass AS p2
+ WHERE p2.opcfamily = p1.amopfamily
+ AND binary_coercible(p2.opcintype, p1.amoplefttype));
+ amopfamily | amopstrategy | amopopr
+ ------------+--------------+---------
+ 1029 | 27 | 433
+ 1029 | 47 | 757
+ 1029 | 67 | 759
+ (3 rows)
+ -- Operators that are primary members of opclasses must be immutable (else -- it suggests that the index ordering
isn'tfixed). Operators that are -- cross-type members need only be stable, since they are just shorthands
(Those are in the gist point_ops opclass, if you were wondering.)
I'm planning to leave it like this for the moment in the spgist patch,
and then go back and clean up the useless point_ops entries and their
underlying dead code in a separate commit. Just FYI.
regards, tom lane
