Message-ID: <5B5958D2.7060805@lab.ntt.co.jp>
Date: Thu, 26 Jul 2018 14:14:58 +0900
From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64;
 rv:11.0) Gecko/20120327 Thunderbird/11.0.1
MIME-Version: 1.0
Subject: Re: print_path is missing GatherMerge and CustomScan support
References: 
 <CAD21AoBQMLoc=ohH-oocuAPsELrmk8_EsRJjOyR8FQLZkbE0wA@mail.gmail.com>
 <20180718062202.GC8565@paquier.xyz>
 <CAFjFpReLXu9qa2a-bt9-yDiT6UZn1+nh_1dEVkJuCXu6y838Aw@mail.gmail.com>
 <20180719000722.GA3411@paquier.xyz>
 <CAFjFpRf3yGODbCyrR4zfcbWbQOv=DqtfvwZ=qUh2Wfa3ODe-TA@mail.gmail.com>
In-Reply-To: 
 <CAFjFpRf3yGODbCyrR4zfcbWbQOv=DqtfvwZ=qUh2Wfa3ODe-TA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Cc: Michael Paquier <michael@paquier.xyz>,
        Masahiko Sawada <sawada.mshk@gmail.com>,
        PostgreSQL-development <pgsql-hackers@postgresql.org>,
        Robert Haas <robertmhaas@gmail.com>
Precedence: bulk

(2018/07/19 14:11), Ashutosh Bapat wrote:
> On Thu, Jul 19, 2018 at 5:37 AM, Michael Paquier<michael@paquier.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