Re: Oversight in reparameterize_path_by_child leading to executor crash

Hi!

Thank you for your work on the subject.

During review your patch I didn't understand why are you checking that
the variable is path and not new_path of type T_SamplePath (I
highlighted it)?

Path *
reparameterize_path_by_child(PlannerInfo *root, Path *path,
 RelOptInfo *child_rel)

...
switch (nodeTag(path))
    {
        case T_Path:
            new_path = path;
ADJUST_CHILD_ATTRS(new_path->parent->baserestrictinfo);
            if (*path*->pathtype == T_SampleScan)
            {

Is it a typo and should be new_path?

Besides, it may not be important, but reviewing your code all the time
stumbled on the statement of the comments while reading it (I
highlighted it too). This:

* path_is_reparameterizable_by_child
 *         Given a path parameterized by the parent of the given child
relation,
 *         see if it can be translated to be parameterized by the child
relation.
 *
 * This must return true if *and only if *reparameterize_path_by_child()
 * would succeed on this path.

Maybe is it better to rewrite it simplier:

 * This must return true *only if *reparameterize_path_by_child()
 * would succeed on this path.

And can we add assert in reparameterize_pathlist_by_child function that
pathlist is not a NIL, because according to the comment it needs to be
added there:

Returns NIL to indicate failure, so pathlist had better not be NIL.

--
Regards,
Alena Rybakina