>> Just to note that I've started looking at this, and I've detected a rather >> significant omission: there's no check that the join operator has anything >> to do with the subquery's grouping operator.
> hmm, good point. If I understand this correctly we can just ensure that the > same operator is used for both the grouping and the join condition.
Well, that's sort of the zero-order solution, but it doesn't work if the join operators are cross-type.
I poked around to see if we didn't have some code already for that, and soon found that not only do we have such code (equality_ops_are_compatible) but actually almost this entire patch duplicates logic that already exists in optimizer/util/pathnode.c, to wit create_unique_path's subroutines query_is_distinct_for et al. So I'm thinking what this needs to turn into is an exercise in refactoring to allow that logic to be used for both purposes.
Well, it seems that might just reduce the patch size a little!
I currently have this half hacked up to use query_is_distinct_for, but I see there's no code that allows Const's to exist in the join condition. I had allowed for this in groupinglist_is_unique_on_restrictinfo() and I tested it worked in a regression test (which now fails). I think to fix this, all it would take would be to modify query_is_distinct_for to take a list of Node's rather than a list of column numbers then just add some logic that skips if it's a Const and checks it as it does now if it's a Var Would you see a change of this kind a valid refactor for this patch?
I notice that create_unique_path is not paying attention to the question
of whether the subselect's tlist contains SRFs or volatile functions. It's possible that that's a pre-existing bug.
*shrug*, perhaps the logic for that is best moved into query_is_distinct_for then? It might save a bug in the future too that way.