Re: print_path is missing GatherMerge and CustomScan support

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: print_path is missing GatherMerge and CustomScan support
Date: 2018-07-26 05:14:58
Message-ID: 5B5958D2.7060805@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(2018/07/19 14:11), Ashutosh Bapat wrote:
> On Thu, Jul 19, 2018 at 5:37 AM, Michael Paquier<michael(at)paquier(dot)xyz> wrote:
>> On Wed, Jul 18, 2018 at 12:15:25PM +0530, Ashutosh Bapat wrote:
>>> Yes that's right. Thanks for taking care of it.
>>
>> Okay, I have pushed a fix for this one as that's wrong and
>> back-patched to v11. The coverage of reparameterize_path_by_child is
>> actually quite poor if you look at the reports:
>> https://coverage.postgresql.org/src/backend/optimizer/util/pathnode.c.gcov.html
>>
>> Could it be possible to stress that more? This way mistakes like this
>> one could have been avoided from the start.
>
> I had extensive testcases in my original patch-set to exercise that
> code but 1. that testset was too extensive; even today
> partition_join.sql is a separate testcase and it's quite large. 2.
> that function returns NULL rather than throwing an error, if it can
> not produce a parameterized path. So, unless we check whether each of
> those paths get created no test is useful and that can only be done
> through an EXPLAIN OUTPUT which means that the testcase becomes
> fragile. I fine if we want to add more tests just to cover the code if
> those are not as fragile and do not blow up partition_join too much.

It would not be possible to cover these cases:

case T_GatherPath:
{
GatherPath *gpath;

FLAT_COPY_PATH(gpath, path, GatherPath);
REPARAMETERIZE_CHILD_PATH(gpath->subpath);
new_path = (Path *) gpath;
}
break;

case T_GatherMergePath:
{
GatherMergePath *gmpath;

FLAT_COPY_PATH(gmpath, path, GatherMergePath);
REPARAMETERIZE_CHILD_PATH(gmpath->subpath);
new_path = (Path *) gmpath;
}
break;

because we currently don't consider gathering partial child-scan or
child-join paths. I think we might consider that in future, though.

Best regards,
Etsuro Fujita

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-07-26 05:24:16 Re: BUG #15182: Canceling authentication due to timeout aka Denial of Service Attack
Previous Message Michael Paquier 2018-07-26 04:49:07 Re: BUG #15182: Canceling authentication due to timeout aka Denial of Service Attack