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

Reply via email to