Re: ORDER BY pushdowns seem broken in postgres_fdw - Mailing list pgsql-hackers

Ronan Dunklau <ronan.dunklau@aiven.io> writes:
> [ v8-0001-Fix-orderby-handling-in-postgres_fdw.patch ]

I looked through this patch.  It's going in the right direction,
but I have a couple of nitpicks:

1. There are still some more places that aren't checking shippability
of the relevant opfamily.

2. The existing usage of find_em_expr_for_rel is fundamentally broken,
because that function will seize on the first EC member that is from the
given rel, whether it's shippable or not.  There might be another one
later that is shippable, so this is just the wrong API.  It's not like
this function gives us any useful isolation from the details of ECs,
because postgres_fdw is already looking into those elsewhere, notably
in find_em_expr_for_input_target --- which has the same order-sensitivity
bug.

I think that instead of doubling down on a wrong API, we should just
take that out and move the logic into postgres_fdw.c.  This also has
the advantage of producing a patch that's much safer to backpatch,
because it doesn't rely on the core backend getting updated before
postgres_fdw.so is.

So hacking on those two points, and doing some additional cleanup,
led me to the attached v9.  (In this patch, the removal of code
from equivclass.c is only meant to be applied to HEAD; we have to
leave the function in place in the back branches for API stability.)

If no objections, I think this is committable.

            regards, tom lane

diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index bf12eac028..8f4d8a5022 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -40,6 +40,7 @@
 #include "catalog/pg_collation.h"
 #include "catalog/pg_namespace.h"
 #include "catalog/pg_operator.h"
+#include "catalog/pg_opfamily.h"
 #include "catalog/pg_proc.h"
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
@@ -183,6 +184,8 @@ static void deparseRangeTblRef(StringInfo buf, PlannerInfo *root,
                                Index ignore_rel, List **ignore_conds, List **params_list);
 static void deparseAggref(Aggref *node, deparse_expr_cxt *context);
 static void appendGroupByClause(List *tlist, deparse_expr_cxt *context);
+static void appendOrderBySuffix(Oid sortop, Oid sortcoltype, bool nulls_first,
+                                deparse_expr_cxt *context);
 static void appendAggOrderBy(List *orderList, List *targetList,
                              deparse_expr_cxt *context);
 static void appendFunctionName(Oid funcid, deparse_expr_cxt *context);
@@ -1038,6 +1041,33 @@ is_foreign_param(PlannerInfo *root,
     return false;
 }

+/*
+ * Returns true if it's safe to push down the sort expression described by
+ * 'pathkey' to the foreign server.
+ */
+bool
+is_foreign_pathkey(PlannerInfo *root,
+                   RelOptInfo *baserel,
+                   PathKey *pathkey)
+{
+    EquivalenceClass *pathkey_ec = pathkey->pk_eclass;
+    PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) baserel->fdw_private;
+
+    /*
+     * is_foreign_expr would detect volatile expressions as well, but checking
+     * ec_has_volatile here saves some cycles.
+     */
+    if (pathkey_ec->ec_has_volatile)
+        return false;
+
+    /* can't push down the sort if the pathkey's opfamily is not shippable */
+    if (!is_shippable(pathkey->pk_opfamily, OperatorFamilyRelationId, fpinfo))
+        return false;
+
+    /* can push if a suitable EC member exists */
+    return (find_em_for_rel(root, pathkey_ec, baserel) != NULL);
+}
+
 /*
  * Convert type OID + typmod info into a type name we can ship to the remote
  * server.  Someplace else had better have verified that this type name is
@@ -3445,44 +3475,59 @@ appendAggOrderBy(List *orderList, List *targetList, deparse_expr_cxt *context)
     {
         SortGroupClause *srt = (SortGroupClause *) lfirst(lc);
         Node       *sortexpr;
-        Oid            sortcoltype;
-        TypeCacheEntry *typentry;

         if (!first)
             appendStringInfoString(buf, ", ");
         first = false;

+        /* Deparse the sort expression proper. */
         sortexpr = deparseSortGroupClause(srt->tleSortGroupRef, targetList,
                                           false, context);
-        sortcoltype = exprType(sortexpr);
-        /* See whether operator is default < or > for datatype */
-        typentry = lookup_type_cache(sortcoltype,
-                                     TYPECACHE_LT_OPR | TYPECACHE_GT_OPR);
-        if (srt->sortop == typentry->lt_opr)
-            appendStringInfoString(buf, " ASC");
-        else if (srt->sortop == typentry->gt_opr)
-            appendStringInfoString(buf, " DESC");
-        else
-        {
-            HeapTuple    opertup;
-            Form_pg_operator operform;
-
-            appendStringInfoString(buf, " USING ");
-
-            /* Append operator name. */
-            opertup = SearchSysCache1(OPEROID, ObjectIdGetDatum(srt->sortop));
-            if (!HeapTupleIsValid(opertup))
-                elog(ERROR, "cache lookup failed for operator %u", srt->sortop);
-            operform = (Form_pg_operator) GETSTRUCT(opertup);
-            deparseOperatorName(buf, operform);
-            ReleaseSysCache(opertup);
-        }
+        /* Add decoration as needed. */
+        appendOrderBySuffix(srt->sortop, exprType(sortexpr), srt->nulls_first,
+                            context);
+    }
+}

-        if (srt->nulls_first)
-            appendStringInfoString(buf, " NULLS FIRST");
-        else
-            appendStringInfoString(buf, " NULLS LAST");
+/*
+ * Append the ASC, DESC, USING <OPERATOR> and NULLS FIRST / NULLS LAST parts
+ * of an ORDER BY clause.
+ */
+static void
+appendOrderBySuffix(Oid sortop, Oid sortcoltype, bool nulls_first,
+                    deparse_expr_cxt *context)
+{
+    StringInfo    buf = context->buf;
+    TypeCacheEntry *typentry;
+
+    /* See whether operator is default < or > for sort expr's datatype. */
+    typentry = lookup_type_cache(sortcoltype,
+                                 TYPECACHE_LT_OPR | TYPECACHE_GT_OPR);
+
+    if (sortop == typentry->lt_opr)
+        appendStringInfoString(buf, " ASC");
+    else if (sortop == typentry->gt_opr)
+        appendStringInfoString(buf, " DESC");
+    else
+    {
+        HeapTuple    opertup;
+        Form_pg_operator operform;
+
+        appendStringInfoString(buf, " USING ");
+
+        /* Append operator name. */
+        opertup = SearchSysCache1(OPEROID, ObjectIdGetDatum(sortop));
+        if (!HeapTupleIsValid(opertup))
+            elog(ERROR, "cache lookup failed for operator %u", sortop);
+        operform = (Form_pg_operator) GETSTRUCT(opertup);
+        deparseOperatorName(buf, operform);
+        ReleaseSysCache(opertup);
     }
+
+    if (nulls_first)
+        appendStringInfoString(buf, " NULLS FIRST");
+    else
+        appendStringInfoString(buf, " NULLS LAST");
 }

 /*
@@ -3565,9 +3610,13 @@ appendGroupByClause(List *tlist, deparse_expr_cxt *context)
 }

 /*
- * Deparse ORDER BY clause according to the given pathkeys for given base
- * relation. From given pathkeys expressions belonging entirely to the given
- * base relation are obtained and deparsed.
+ * Deparse ORDER BY clause defined by the given pathkeys.
+ *
+ * The clause should use Vars from context->scanrel if !has_final_sort,
+ * or from context->foreignrel's targetlist if has_final_sort.
+ *
+ * We find a suitable pathkey expression (some earlier step
+ * should have verified that there is one) and deparse it.
  */
 static void
 appendOrderByClause(List *pathkeys, bool has_final_sort,
@@ -3575,8 +3624,7 @@ appendOrderByClause(List *pathkeys, bool has_final_sort,
 {
     ListCell   *lcell;
     int            nestlevel;
-    char       *delim = " ";
-    RelOptInfo *baserel = context->scanrel;
+    const char *delim = " ";
     StringInfo    buf = context->buf;

     /* Make sure any constants in the exprs are printed portably */
@@ -3586,7 +3634,9 @@ appendOrderByClause(List *pathkeys, bool has_final_sort,
     foreach(lcell, pathkeys)
     {
         PathKey    *pathkey = lfirst(lcell);
+        EquivalenceMember *em;
         Expr       *em_expr;
+        Oid            oprid;

         if (has_final_sort)
         {
@@ -3594,26 +3644,48 @@ appendOrderByClause(List *pathkeys, bool has_final_sort,
              * By construction, context->foreignrel is the input relation to
              * the final sort.
              */
-            em_expr = find_em_expr_for_input_target(context->root,
-                                                    pathkey->pk_eclass,
-                                                    context->foreignrel->reltarget);
+            em = find_em_for_rel_target(context->root,
+                                        pathkey->pk_eclass,
+                                        context->foreignrel);
         }
         else
-            em_expr = find_em_expr_for_rel(pathkey->pk_eclass, baserel);
+            em = find_em_for_rel(context->root,
+                                 pathkey->pk_eclass,
+                                 context->scanrel);
+
+        /*
+         * We don't expect any error here; it would mean that shippability
+         * wasn't verified earlier.  For the same reason, we don't recheck
+         * shippability of the sort operator.
+         */
+        if (em == NULL)
+            elog(ERROR, "could not find pathkey item to sort");
+
+        em_expr = em->em_expr;

-        Assert(em_expr != NULL);
+        /*
+         * Lookup the operator corresponding to the strategy in the opclass.
+         * The datatype used by the opfamily is not necessarily the same as
+         * the expression type (for array types for example).
+         */
+        oprid = get_opfamily_member(pathkey->pk_opfamily,
+                                    em->em_datatype,
+                                    em->em_datatype,
+                                    pathkey->pk_strategy);
+        if (!OidIsValid(oprid))
+            elog(ERROR, "missing operator %d(%u,%u) in opfamily %u",
+                 pathkey->pk_strategy, em->em_datatype, em->em_datatype,
+                 pathkey->pk_opfamily);

         appendStringInfoString(buf, delim);
         deparseExpr(em_expr, context);
-        if (pathkey->pk_strategy == BTLessStrategyNumber)
-            appendStringInfoString(buf, " ASC");
-        else
-            appendStringInfoString(buf, " DESC");

-        if (pathkey->pk_nulls_first)
-            appendStringInfoString(buf, " NULLS FIRST");
-        else
-            appendStringInfoString(buf, " NULLS LAST");
+        /*
+         * Here we need to use the expression's actual type to discover
+         * whether the desired operator will be the default or not.
+         */
+        appendOrderBySuffix(oprid, exprType((Node *) em_expr),
+                            pathkey->pk_nulls_first, context);

         delim = ", ";
     }
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index f210f91188..2725b2b211 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -3265,6 +3265,19 @@ select array_agg(c1 order by c1 using operator(public.<^)) from ft2 where c2 = 6
          Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" WHERE (("C 1" < 100)) AND ((c2 = 6))
 (6 rows)

+-- This should not be pushed either.
+explain (verbose, costs off)
+select * from ft2 order by c1 using operator(public.<^);
+                                  QUERY PLAN
+-------------------------------------------------------------------------------
+ Sort
+   Output: c1, c2, c3, c4, c5, c6, c7, c8
+   Sort Key: ft2.c1 USING <^
+   ->  Foreign Scan on public.ft2
+         Output: c1, c2, c3, c4, c5, c6, c7, c8
+         Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1"
+(6 rows)
+
 -- Update local stats on ft2
 ANALYZE ft2;
 -- Add into extension
@@ -3292,6 +3305,16 @@ select array_agg(c1 order by c1 using operator(public.<^)) from ft2 where c2 = 6
  {6,16,26,36,46,56,66,76,86,96}
 (1 row)

+-- This should be pushed too.
+explain (verbose, costs off)
+select * from ft2 order by c1 using operator(public.<^);
+                                                         QUERY PLAN
      

+-----------------------------------------------------------------------------------------------------------------------------
+ Foreign Scan on public.ft2
+   Output: c1, c2, c3, c4, c5, c6, c7, c8
+   Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" ORDER BY "C 1" USING OPERATOR(public.<^)
NULLSLAST 
+(3 rows)
+
 -- Remove from extension
 alter extension postgres_fdw drop operator class my_op_class using btree;
 alter extension postgres_fdw drop function my_op_cmp(a int, b int);
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 56654844e8..c51dd68722 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -18,6 +18,7 @@
 #include "access/sysattr.h"
 #include "access/table.h"
 #include "catalog/pg_class.h"
+#include "catalog/pg_opfamily.h"
 #include "commands/defrem.h"
 #include "commands/explain.h"
 #include "commands/vacuum.h"
@@ -918,8 +919,6 @@ get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel)
         foreach(lc, root->query_pathkeys)
         {
             PathKey    *pathkey = (PathKey *) lfirst(lc);
-            EquivalenceClass *pathkey_ec = pathkey->pk_eclass;
-            Expr       *em_expr;

             /*
              * The planner and executor don't have any clever strategy for
@@ -927,13 +926,8 @@ get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel)
              * getting it to be sorted by all of those pathkeys. We'll just
              * end up resorting the entire data set.  So, unless we can push
              * down all of the query pathkeys, forget it.
-             *
-             * is_foreign_expr would detect volatile expressions as well, but
-             * checking ec_has_volatile here saves some cycles.
              */
-            if (pathkey_ec->ec_has_volatile ||
-                !(em_expr = find_em_expr_for_rel(pathkey_ec, rel)) ||
-                !is_foreign_expr(root, rel, em_expr))
+            if (!is_foreign_pathkey(root, rel, pathkey))
             {
                 query_pathkeys_ok = false;
                 break;
@@ -980,16 +974,19 @@ get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel)
     foreach(lc, useful_eclass_list)
     {
         EquivalenceClass *cur_ec = lfirst(lc);
-        Expr       *em_expr;
         PathKey    *pathkey;

         /* If redundant with what we did above, skip it. */
         if (cur_ec == query_ec)
             continue;

+        /* Can't push down the sort if the EC's opfamily is not shippable. */
+        if (!is_shippable(linitial_oid(cur_ec->ec_opfamilies),
+                          OperatorFamilyRelationId, fpinfo))
+            continue;
+
         /* If no pushable expression for this rel, skip it. */
-        em_expr = find_em_expr_for_rel(cur_ec, rel);
-        if (em_expr == NULL || !is_foreign_expr(root, rel, em_expr))
+        if (find_em_for_rel(root, cur_ec, rel) == NULL)
             continue;

         /* Looks like we can generate a pathkey, so let's do it. */
@@ -6543,7 +6540,6 @@ add_foreign_ordered_paths(PlannerInfo *root, RelOptInfo *input_rel,
     {
         PathKey    *pathkey = (PathKey *) lfirst(lc);
         EquivalenceClass *pathkey_ec = pathkey->pk_eclass;
-        Expr       *sort_expr;

         /*
          * is_foreign_expr would detect volatile expressions as well, but
@@ -6552,13 +6548,20 @@ add_foreign_ordered_paths(PlannerInfo *root, RelOptInfo *input_rel,
         if (pathkey_ec->ec_has_volatile)
             return;

-        /* Get the sort expression for the pathkey_ec */
-        sort_expr = find_em_expr_for_input_target(root,
-                                                  pathkey_ec,
-                                                  input_rel->reltarget);
+        /*
+         * Can't push down the sort if pathkey's opfamily is not shippable.
+         */
+        if (!is_shippable(pathkey->pk_opfamily, OperatorFamilyRelationId,
+                          fpinfo))
+            return;

-        /* If it's unsafe to remote, we cannot push down the final sort */
-        if (!is_foreign_expr(root, input_rel, sort_expr))
+        /*
+         * The EC must contain a shippable EM that is computed in input_rel's
+         * reltarget, else we can't push down the sort.
+         */
+        if (find_em_for_rel_target(root,
+                                   pathkey_ec,
+                                   input_rel) == NULL)
             return;
     }

@@ -7384,14 +7387,55 @@ conversion_error_callback(void *arg)
 }

 /*
- * Find an equivalence class member expression to be computed as a sort column
- * in the given target.
+ * Given an EquivalenceClass and a foreign relation, find an EC member
+ * that can be used to sort the relation remotely according to a pathkey
+ * using this EC.
+ *
+ * If there is more than one suitable candidate, return an arbitrary
+ * one of them.  If there is none, return NULL.
+ *
+ * This checks that the EC member expression uses only Vars from the given
+ * rel and is shippable.  Caller must separately verify that the pathkey's
+ * ordering operator is shippable.
+ */
+EquivalenceMember *
+find_em_for_rel(PlannerInfo *root, EquivalenceClass *ec, RelOptInfo *rel)
+{
+    ListCell   *lc;
+
+    foreach(lc, ec->ec_members)
+    {
+        EquivalenceMember *em = (EquivalenceMember *) lfirst(lc);
+
+        /*
+         * Note we require !bms_is_empty, else we'd accept constant
+         * expressions which are not suitable for the purpose.
+         */
+        if (bms_is_subset(em->em_relids, rel->relids) &&
+            !bms_is_empty(em->em_relids) &&
+            is_foreign_expr(root, rel, em->em_expr))
+            return em;
+    }
+
+    return NULL;
+}
+
+/*
+ * Find an EquivalenceClass member that is to be computed as a sort column
+ * in the given rel's reltarget, and is shippable.
+ *
+ * If there is more than one suitable candidate, return an arbitrary
+ * one of them.  If there is none, return NULL.
+ *
+ * This checks that the EC member expression uses only Vars from the given
+ * rel and is shippable.  Caller must separately verify that the pathkey's
+ * ordering operator is shippable.
  */
-Expr *
-find_em_expr_for_input_target(PlannerInfo *root,
-                              EquivalenceClass *ec,
-                              PathTarget *target)
+EquivalenceMember *
+find_em_for_rel_target(PlannerInfo *root, EquivalenceClass *ec,
+                       RelOptInfo *rel)
 {
+    PathTarget *target = rel->reltarget;
     ListCell   *lc1;
     int            i;

@@ -7434,15 +7478,18 @@ find_em_expr_for_input_target(PlannerInfo *root,
             while (em_expr && IsA(em_expr, RelabelType))
                 em_expr = ((RelabelType *) em_expr)->arg;

-            if (equal(em_expr, expr))
-                return em->em_expr;
+            if (!equal(em_expr, expr))
+                continue;
+
+            /* Check that expression (including relabels!) is shippable */
+            if (is_foreign_expr(root, rel, em->em_expr))
+                return em;
         }

         i++;
     }

-    elog(ERROR, "could not find pathkey item to sort");
-    return NULL;                /* keep compiler quiet */
+    return NULL;
 }

 /*
diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index 8ae79e97e4..21f2b20ce8 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -173,6 +173,9 @@ extern bool is_foreign_expr(PlannerInfo *root,
 extern bool is_foreign_param(PlannerInfo *root,
                              RelOptInfo *baserel,
                              Expr *expr);
+extern bool is_foreign_pathkey(PlannerInfo *root,
+                               RelOptInfo *baserel,
+                               PathKey *pathkey);
 extern void deparseInsertSql(StringInfo buf, RangeTblEntry *rte,
                              Index rtindex, Relation rel,
                              List *targetAttrs, bool doNothing,
@@ -215,10 +218,12 @@ extern void deparseTruncateSql(StringInfo buf,
                                DropBehavior behavior,
                                bool restart_seqs);
 extern void deparseStringLiteral(StringInfo buf, const char *val);
-extern Expr *find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel);
-extern Expr *find_em_expr_for_input_target(PlannerInfo *root,
-                                           EquivalenceClass *ec,
-                                           PathTarget *target);
+extern EquivalenceMember *find_em_for_rel(PlannerInfo *root,
+                                          EquivalenceClass *ec,
+                                          RelOptInfo *rel);
+extern EquivalenceMember *find_em_for_rel_target(PlannerInfo *root,
+                                                 EquivalenceClass *ec,
+                                                 RelOptInfo *rel);
 extern List *build_tlist_to_deparse(RelOptInfo *foreignrel);
 extern void deparseSelectStmtForRel(StringInfo buf, PlannerInfo *root,
                                     RelOptInfo *foreignrel, List *tlist,
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 95b6b7192e..6b5de89e14 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -907,6 +907,10 @@ create operator class my_op_class for type int using btree family my_op_family a
 explain (verbose, costs off)
 select array_agg(c1 order by c1 using operator(public.<^)) from ft2 where c2 = 6 and c1 < 100 group by c2;

+-- This should not be pushed either.
+explain (verbose, costs off)
+select * from ft2 order by c1 using operator(public.<^);
+
 -- Update local stats on ft2
 ANALYZE ft2;

@@ -924,6 +928,10 @@ explain (verbose, costs off)
 select array_agg(c1 order by c1 using operator(public.<^)) from ft2 where c2 = 6 and c1 < 100 group by c2;
 select array_agg(c1 order by c1 using operator(public.<^)) from ft2 where c2 = 6 and c1 < 100 group by c2;

+-- This should be pushed too.
+explain (verbose, costs off)
+select * from ft2 order by c1 using operator(public.<^);
+
 -- Remove from extension
 alter extension postgres_fdw drop operator class my_op_class using btree;
 alter extension postgres_fdw drop function my_op_cmp(a int, b int);
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index 8c6770de97..ba1931c312 100644
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -931,35 +931,6 @@ is_exprlist_member(Expr *node, List *exprs)
     return false;
 }

-/*
- * Find an equivalence class member expression, all of whose Vars, come from
- * the indicated relation.
- */
-Expr *
-find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel)
-{
-    ListCell   *lc_em;
-
-    foreach(lc_em, ec->ec_members)
-    {
-        EquivalenceMember *em = lfirst(lc_em);
-
-        if (bms_is_subset(em->em_relids, rel->relids) &&
-            !bms_is_empty(em->em_relids))
-        {
-            /*
-             * If there is more than one equivalence member whose Vars are
-             * taken entirely from this relation, we'll be content to choose
-             * any one of those.
-             */
-            return em->em_expr;
-        }
-    }
-
-    /* We didn't find any suitable equivalence class expression */
-    return NULL;
-}
-
 /*
  * relation_can_be_sorted_early
  *        Can this relation be sorted on this EC before the final output step?
diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index 0c3a0b90c8..e313eb2138 100644
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -143,7 +143,6 @@ extern EquivalenceMember *find_computable_ec_member(PlannerInfo *root,
                                                     List *exprs,
                                                     Relids relids,
                                                     bool require_parallel_safe);
-extern Expr *find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel);
 extern bool relation_can_be_sorted_early(PlannerInfo *root, RelOptInfo *rel,
                                          EquivalenceClass *ec,
                                          bool require_parallel_safe);