On Wed, Apr 24, 2019 at 4:56 PM amul sul <sula...@gmail.com> wrote: > Attached version is rebase atop of the latest master head(fdc7efcc30), also > incorporates the Ashutosh's suggestion, thanks. > Thanks for rebase patch, patches applied cleanly on PG head. I did some crash testing with extra test case [0006 patch] [1] and found no more issue.
Thanks & Regards, Rajkumar Raghuwanshi QMG, EnterpriseDB. [1] https://www.postgresql.org/message-id/CAFjFpReKuV_4LRRfdy80BqX8oZfwbo%2BHWLQNv3CsJ5iGPSyTfA%40mail.gmail.com <https://www.postgresql.org/message-id/CA%2Bq6zcU3X4%3DBfqnWXAUPBFtKK7vy0HO7-%2BmAW6KB2Zy_EPtC_Q%40mail.gmail.com> > > Regards, > Amul > > On Mon, Mar 11, 2019 at 10:14 PM Ashutosh Bapat < > ashutosh.bapat....@gmail.com> wrote: > >> >> >> On Mon, Mar 11, 2019 at 10:40 AM amul sul <sula...@gmail.com> wrote: >> >>> >>> All the places from where this handle_missing_partition() get called >>> have the following code to decide the value for >>> missing_side_outer/_inner which >>> I yet to understand. Do you think this has some flaw? >>> >>> /* >>> * For a FULL join, inner relation acts as both OUTER and INNER >>> * relation. For LEFT and ANTI join the inner relation acts as >>> * INNER relation. For INNER and SEMI join OUTER and INNER >>> * differentiation is immaterial. >>> */ >>> missing_side_inner = (jointype == JOIN_FULL || >>> jointype == JOIN_LEFT || >>> jointype == JOIN_ANTI); >>> missing_side_outer = (jointype == JOIN_FULL); >>> >> >> I was wrong, sorry. The comment says it all. >> >> >>> >>> >>> >>>> argument value which fails to set merged_index. >>>>> >>>>> In the attached patch, I tried to fix this case by setting >>>>> merged_index >>>>> explicitly which fixes the reported crash. >>>>> >>>> >>>> I expect handle_missing_partition() to set the merged_index always. In >>>> your patches, I don't see that function in your patches is setting it >>>> explicitly. If we are setting merged_index explicitly somewhere else, other >>>> places may miss that explicit assignment. So it's better to move it inside >>>> this function. >>>> >>>> >>> >>> Ok, that can be fixed. >>> >>> Similarly, I think merge_null_partitions should set null_index instead >>> of >>> asserting when null partitions missing from both the side, make sense? >>> >> >> I think not. null_index, once set shouldn't change and hence does not >> change with each pair of partitions being matched. So, it makes sense to >> make sure that null_index remains invalid if none of the tables have null >> partition. >> >> -- >> Best Wishes, >> Ashutosh Bapat >> >
