While fooling with my longstanding outer-join variables changes (I am making progress on that, honest), I happened to notice that equivclass.c is leaving some money on the table by generating redundant RestrictInfo clauses. It already attempts to not generate the same clause twice, which can save a nontrivial amount of work because we cache selectivity estimates and so on per-RestrictInfo. I realized though that it will build and return a clause like "a.x = b.y" even if it already has "b.y = a.x". This is just wasteful. It's always been the case that equivclass.c will produce clauses that are ordered according to its own whims. Consumers that need the operands in a specific order, such as index scans or hash joins, are required to commute the clause to be the way they want it while building the finished plan. Therefore, it shouldn't matter which order of the operands we return, and giving back the commutator clause if available could potentially save as much as half of the selectivity-estimation work we do with these clauses subsequently.
Hence, PFA a patch that adjusts create_join_clause() to notice commuted as well as exact matches among the EquivalenceClass's existing clauses. This results in a number of changes visible in regression test cases, but they're all clearly inconsequential.
I think there is no problem with this idea, given the operands of EC-derived clauses are commutative, and it seems no one would actually rely on the order of the operands. I can see hashjoin/mergejoin would commute hash/merge joinclauses if needed with get_switched_clauses().
The only thing that I think might be controversial here is that I dropped the check for matching operator OID. To preserve that, we'd have needed to use get_commutator() in the reverse-match cases, which it seemed to me would be a completely unjustified expenditure of cycles. The operators we select for freshly-generated clauses will certainly always match those of previously-generated clauses. Maybe there's a chance that they'd not match those of ec_sources clauses (that is, the user-written clauses we started from), but if they don't and that actually makes any difference then surely we are talking about a buggy opclass definition.
The operator is chosen according to the two given EC members's data type. Since we are dealing with the same pair of EC members, I think the operator is always the same one. So it also seems no problem to drop the check for operator. I wonder if we can even add an assertion if we've found a RestrictInfo from ec_derives that the operator matches.
