Re: Server crashed with TRAP: FailedAssertion("!(parallel_workers > 0)" when partitionwise_aggregate true.

On Tue, Jun 19, 2018 at 2:13 PM, Jeevan Chalke <
jeevan(dot)chalke(at)enterprisedb(dot)com> wrote:

>
>
> On Mon, Jun 18, 2018 at 9:27 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
>> On 2018-06-18 17:10:12 +0530, Jeevan Chalke wrote:
>> > On Mon, Jun 18, 2018 at 5:02 PM, Rajkumar Raghuwanshi <
>> > rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com> wrote:
>> >
>> > > Hi,
>> > >
>> > > Below test case crashed, when set enable_partitionwise_aggregate to
>> true.
>> > >
>> >
>> > I will have a look over this.
>>
>
> In the reported testcase, parallel_workers is set to 0 for all partition
> (child) relations. Which means partial parallel paths are not possible for
> child rels. However, the parent can have partial parallel paths. Thus, when
> we have a full partitionwise aggregate possible (as the group by clause
> matches with the partition key), we end-up in a situation where we do
> create a partially_grouped_rel for the parent but there won't be any
> partially_grouped_live_children.
>
> The current code was calling add_paths_to_append_rel() without making sure
> of any live children present or not (sorry, it was my fault). This causes
> an Assertion failure in add_paths_to_append_rel() as this function assumes
> that it will have non-NIL live_childrels list.
>
> I have fixed partitionwise aggregate code which is calling
> add_paths_to_append_rel() by checking the live children list correctly. And
> for robustness, I have also added an Assert() in add_paths_to_append_rel().
>
> I have verified the callers of add_paths_to_append_rel() and except one,
> all are calling it by making sure that they have a non-NIL live children
> list. The one which is calling add_paths_to_append_rel() directly is
> set_append_rel_pathlist(). And I think, at that place, we will never have
> an empty live children list, I may be wrong though. And if that's the case
> then newly added Assert() in add_paths_to_append_rel() will fail anyway to
> catch any programming error in that code path.
>
> Attached patch fixing the crash and also added a simple test-coverage for
> that.
>
> Let me know if I missed any.
>

Rajkumar offlist reported another issue related to data-loss. If few of the
partitions has parallel_workers = 0, not all, then PWA plan ended up with a
plan having children which has parallel_workers != 0. So the partitions
with parallel_workers = 0; were not scanned.

Fixed this in attached version of the patch.

>
> Thanks
>
>
>> >
>> > Thanks for reporting.
>>
>> I've added an v11 open-items entry.
>>
>> - Andres
>>
>
> --
> Jeevan Chalke
> Technical Architect, Product Development
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company
>
>

--
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company