Amit-san,
(2019/01/09 9:30), Amit Langote wrote:
> (sorry about the repeated email, but my previous attempt failed due to
> trying to send to the -hackers and -performance lists at the same time, so
> trying again after removing -performance)
Thanks! (Actually, I also failed to send my post to those lists...)
> On 2019/01/08 20:07, Etsuro Fujita wrote:
>> (2018/12/07 20:14), Ashutosh Bapat wrote:
>>> On Fri, Dec 7, 2018 at 11:13 AM Ashutosh Bapat
>>> <ashutosh.bapat.oss@gmail.com<mailto:ashutosh.bapat.oss@gmail.com>> wrote:
>>
>>> Robert, Ashutosh, any comments on this? I'm unfamiliar with the
>>> partitionwise join code.
>>
>>> As the comment says it has to do with the equivalence classes being
>>> used during merge append. EC's are used to create pathkeys used for
>>> sorting. Creating a sort node which has column on the nullable side
>>> of an OUTER join will fail if it doesn't find corresponding
>>> equivalence class. You may not notice this if both the partitions
>>> being joined are pruned for some reason. Amit's idea to make
>>> partition-wise join code do this may work, but will add a similar
>>> overhead esp. in N-way partition-wise join once those equivalence
>>> classes are added.
>>
>>> I looked at the patch. The problem there is that for a given relation,
>>> we will add child ec member multiple times, as many times as the number
>>> of joins it participates in. We need to avoid that to keep ec_member
>>> list length in check.
>>
>> Amit-san, are you still working on this, perhaps as part of the
>> speeding-up-planning-with-partitions patch [1]?
>
> I had tried to continue working on it after PGConf.ASIA last month, but
> got distracted by something else.
>
> So, while the patch at [1] can take care of this issue as I also mentioned
> upthread, I was trying to come up with a solution that can be back-patched
> to PG 11. The patch I posted above is one such solution and as Ashutosh
> points out it's perhaps not the best, because it can result in potentially
> creating many copies of the same child EC member if we do it in joinrel.c,
> as the patch proposes. I will try to respond to the concerns he raised in
> the next week if possible.
Thanks for working on this!
I like your patch in general. I think one way to address Ashutosh's
concerns would be to use the consider_partitionwise_join flag:
originally, that was introduced for partitioned relations to show that
they can be partitionwise-joined, but I think that flag could also be
used for non-partitioned relations to show that they have been set up
properly for partitionwise-joins, and I think by checking that flag we
could avoid creating those copies for child dummy rels in
try_partitionwise_join. Please find attached an updated version of the
patch. I modified your version so that building tlists for child dummy
rels are also postponed until after they actually participate in
partitionwise-joins, to avoid that possibly-useless work as well. I
haven't done any performance tests yet though.
Best regards,
Etsuro Fujita
