On Fri, Oct 02, 2020 at 09:19:44AM -0400, James Coleman wrote:

...

I've been able to confirm that the problem goes away if we stop adding
the gather merge paths in generate_useful_gather_paths().

I'm not sure yet what conclusion that leads us to. It seems to be that
the biggest clue remains that all of this works correctly unless one
of the selected columns (which happens to be a pathkey at this point
because it's a DISTINCT query) contains a volatile expression.


Yeah. It seems to me this is a bug in get_useful_pathkeys_for_relation,
which is calling find_em_expr_for_rel and is happy with anything it
returns. But this was copied from postgres_fdw, which however does a bit
more here:

    if (pathkey_ec->ec_has_volatile ||
        !(em_expr = find_em_expr_for_rel(pathkey_ec, rel)) ||
        !is_foreign_expr(root, rel, em_expr))

So not only does it explicitly check volatility of the pathkey, it also
calls is_foreign_expr which checks the expression for mutable functions.

The attached patch seems to fix this, but it only adds the check for
mutable functions. Maybe it should check ec_has_volatile too ...


regards

--
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/optimizer/path/allpaths.c 
b/src/backend/optimizer/path/allpaths.c
index b399592ff8..33df9b0783 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2771,6 +2771,7 @@ get_useful_pathkeys_for_relation(PlannerInfo *root, 
RelOptInfo *rel)
                {
                        PathKey    *pathkey = (PathKey *) lfirst(lc);
                        EquivalenceClass *pathkey_ec = pathkey->pk_eclass;
+                       Expr       *em_expr;
 
                        /*
                         * We can only build an Incremental Sort for pathkeys 
which
@@ -2783,7 +2784,8 @@ get_useful_pathkeys_for_relation(PlannerInfo *root, 
RelOptInfo *rel)
                         * enable not just an incremental sort on the entirety 
of
                         * query_pathkeys but also incremental sort below a 
JOIN.
                         */
-                       if (!find_em_expr_for_rel(pathkey_ec, rel))
+                       if (!(em_expr = find_em_expr_for_rel(pathkey_ec, rel)) 
||
+                               contain_mutable_functions((Node *) em_expr))
                                break;
 
                        npathkeys++;

Reply via email to