On Thu, 7 Mar 2024 at 19:09, Michał Kłeczek <mic...@kleczek.org> wrote: > > The following query: > > SELECT * FROM ( > SELECT 2023 AS year, * FROM remote_table_1 > UNION ALL > SELECT 2022 AS year, * FROM remote_table_2 > ) > ORDER BY year DESC; > > yields the following remote query: > > SELECT [columns] FROM remote_table_1 ORDER BY 2023 DESC > > and subsequently fails remote execution. > > > Not really sure where the problem is - the planner or postgres_fdw. > I guess it is postgres_fdw not filtering out ordering keys.
Interesting. I've attached a self-contained recreator for the casual passerby. I think the fix should go in appendOrderByClause(). It's at that point we look for the EquivalenceMember for the relation and can easily discover if the em_expr is a Const. I think we can safely just skip doing any ORDER BY <const> stuff and not worry about if the literal format of the const will appear as a reference to an ordinal column position in the ORDER BY clause. Something like the attached patch I think should work. I wonder if we need a test... David
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 8fc66fa11c..2f4ed33173 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -3914,11 +3914,11 @@ appendOrderByClause(List *pathkeys, bool has_final_sort,
int nestlevel;
const char *delim = " ";
StringInfo buf = context->buf;
+ bool gotone = false;
/* Make sure any constants in the exprs are printed portably */
nestlevel = set_transmission_modes();
- appendStringInfoString(buf, " ORDER BY");
foreach(lcell, pathkeys)
{
PathKey *pathkey = lfirst(lcell);
@@ -3949,6 +3949,21 @@ appendOrderByClause(List *pathkeys, bool has_final_sort,
if (em == NULL)
elog(ERROR, "could not find pathkey item to sort");
+ /*
+ * If the member just has a Const expression then we needn't
add it to
+ * the ORDER BY clause. This can happen in UNION ALL queries
where the
+ * union child targetlist has a Const. Adding these would be
wasteful,
+ * but also, for INT columns, putting an integer literal will
be seen
+ * as an ordinal column position rather than a value to sort
by, so we
+ * must skip these.
+ */
+ if (IsA(em->em_expr, Const))
+ continue;
+
+ if (!gotone)
+ appendStringInfoString(buf, " ORDER BY");
+ gotone = true;
+
em_expr = em->em_expr;
/*
demo_of_postgres_fdw_order_by_const_bug.sql
Description: Binary data
