Re: v12.0: ERROR: could not find pathkey item to sort

Amit Langote <amitlangote09(at)gmail(dot)com> writes:
>> This would
>> probably require refactoring things so that there are separate
>> entry points to add child equivalences for base rels and join rels.
>> But that seems cleaner anyway than what you've got here.

> Separate entry points sounds better, but only in HEAD?

I had actually had in mind that we might have two wrappers around a
common search-and-replace routine, but after studying the code I see that
there's just enough differences to make it probably not worth the trouble
to do it like that. I did spend a bit of time removing cosmetic
differences between the two versions, though, mostly by creating
common local variables.

I think the way you did the matching_ecs computation is actually wrong:
we need to find ECs that reference any rel of the join, not only those
that reference both inputs. If nothing else, the way you have it here
makes the results dependent on which pair of input rels gets considered
first, and that's certainly bad for multiway joins.

Also, I thought we should try to put more conditions on whether we
invoke add_child_join_rel_equivalences at all. In the attached I did

if ((enable_partitionwise_join || enable_partitionwise_aggregate) &&
(joinrel->has_eclass_joins ||
has_useful_pathkeys(root, parent_joinrel)))

but I wonder if we could do more, like checking to see if the parent
joinrel is partitioned. Alternatively, maybe it's unnecessary because
we won't try to build child joinrels unless these conditions are true?

I did not like the test case either. Creating a whole new (and rather
large) test table just for this one case is unreasonably expensive ---
it about doubles the runtime of the equivclass test for me. There's
already a perfectly good test table in partition_join.sql, which seems
like a more natural home for this anyhow. After a bit of finagling
I was able to adapt the test query to fail on that table.

Patch v4 attached. I've not looked at what we need to do to make this
work in v12.

regards, tom lane