Re: BUG #19827: postgres_fdw: Assert in add_foreign_ordered_paths() with redundant ORDER BY in set-op input

From: Robins Tharakan <tharakan(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: pgsql-bugs(at)lists(dot)postgresql(dot)org, Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
Subject: Re: BUG #19827: postgres_fdw: Assert in add_foreign_ordered_paths() with redundant ORDER BY in set-op input
Date: 2026-11-03 03:39:59
Message-ID: CAEP4nAxw-ArnX0Xqe7b_3M-v7s8m2c=MdGHGtwNS72xDGw1GuQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Hi David,

Thanks for taking a look.

On Tue, 3 Nov 2026 at 11:44, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> On Tue, 3 Nov 2026 at 05:09, Robins Tharakan <tharakan(at)gmail(dot)com> wrote:
> >
> > Here's the patch that fixes the issue.
> > It applies from HEAD through REL_17_STABLE. make check passes too.
>
> (I don't seem to have any emails to see what you're replying to here,
> and [1] doesn't seem to have any matches apart from your email.)

It is odd that despite receiving an email via the web-form submission process,
it is missing from the pgsql-bugs web-portal today. At this point, it seems like
a bug with the web-form / mailing system.

Anyway, below is the verbatim copy of what I received from the
pgsql-bugs web-form submission that has my original triaging and
testing details.

"
PG Bug reporting form <noreply(at)postgresql(dot)org>
02:14 (11 hours ago)
to pgsql-bugs, me

The following bug has been logged on the website:

Bug reference: 19827
Logged by: Robins Tharakan
Email address: tharakan(at)gmail(dot)com
PostgreSQL version: 19rc1
Operating system: Debian 13 (x86_64)
Description:

A set-operation input that scans a postgres_fdw foreign table and has
an ORDER BY whose keys are all redundant (a constant, or a column the
WHERE clause equates to a constant) fails this assertion:

TRAP: failed Assert("root->query_pathkeys == root->sort_pathkeys"),
File: "postgres_fdw.c", Line: 8280

Release builds return correct rows (checked against the local table).

add_foreign_ordered_paths() asserts that query_pathkeys is
sort_pathkeys for a base or join rel. Since 66c0185a3,
standard_qp_callback() falls back to setop_pathkeys when sort_pathkeys
is NIL, for an input of a set operation. A redundant ORDER BY leaves
sort_pathkeys NIL while the query still has a sortClause, so
create_ordered_paths() still calls the FDW for UPPERREL_ORDERED.

It fails on master and REL_17_STABLE to REL_19_STABLE; 16 has no
setop_pathkeys. On 17 only UNION inputs trip it; from 18 EXCEPT does
too (and INTERSECT, tried on master). A patch follows in a reply.

SQL
===
CREATE EXTENSION postgres_fdw;
DO $$
BEGIN
EXECUTE format('CREATE SERVER loopback FOREIGN DATA WRAPPER postgres_fdw
OPTIONS (dbname %L, port %L)',
current_database(), current_setting('port'));
END
$$;
CREATE USER MAPPING FOR CURRENT_USER SERVER loopback;
CREATE TABLE t (a int, b bool);
INSERT INTO t VALUES (1, true), (2, false);
CREATE FOREIGN TABLE ft (a int, b bool)
SERVER loopback OPTIONS (table_name 't');

(SELECT a, b FROM ft WHERE a = 1 ORDER BY a) UNION (SELECT 2, true);

-- from 18, also:
(SELECT 1 AS x, b FROM ft ORDER BY 1) EXCEPT (SELECT 1, true);

Commit
======
6cda9dfc4f97 (master) — fails here
Reproduced starting from 12933dc6048, a re-commit of 66c0185a3.
Built with --enable-cassert and -O2 (gcc 14.2, Debian 13, x86_64).

Backtrace
=========
#4 add_foreign_ordered_paths (...) at postgres_fdw.c:8280
#5 postgresGetForeignUpperPaths (root=..., stage=UPPERREL_ORDERED, ...)
at postgres_fdw.c:8119
#6 create_ordered_paths (...) at planner.c:5784
#7 grouping_planner (...) at planner.c:2160
#8 subquery_planner (..., plan_name="setop_1", ...) at planner.c:1367
#9 recurse_set_operations (...) at prepunion.c:252
#10 generate_nonunion_paths (...) at prepunion.c:1091
#11 recurse_set_operations (...) at prepunion.c:287
#12 plan_set_operations (root=...) at prepunion.c:164
(EXCEPT case; UNION goes through generate_union_paths)

Patch
=====
The patch (in a reply to this report), is against master 6cda9dfc4f97,
and applies through to REL_17_STABLE. In add_foreign_ordered_paths(),
the base/join branch now handles the set-operation case first:

+ if (root->query_pathkeys != root->sort_pathkeys)
+ {
+ Assert(root->sort_pathkeys == NIL &&
+ root->query_pathkeys ==
root->setop_pathkeys);
+ fpinfo->pushdown_safe = true;
+ return;
+ }

Why that patch looks correct:
- For a base or join rel, query_pathkeys can differ from sort_pathkeys
only through the setop_pathkeys fallback: GROUP BY, window functions
or DISTINCT would make the ordered stage's input an upper rel. The
new Assert checks that.
- With sort_pathkeys NIL there's no final sort, so the ordered rel is
safe to push down, as the grouping-rel path below already decides
when sort_pathkeys is NIL.
- Release builds took pushdown_safe from qp_is_pushdown_safe, computed
for setop_pathkeys, so a LIMIT in the input was pushed down only if
the set operation's order could be. Results were right either way.
The one plan change: such an input with a LIMIT whose set-operation
order can't be shipped now pushes the LIMIT down without ORDER BY,
which is fine since every row ties on the redundant keys.
- A top-level query with an all-redundant ORDER BY is unchanged:
query_pathkeys is NIL there too.

The test adds five queries to postgres_fdw.sql: UNION inputs over a
base rel, over a join and with a LIMIT, an EXCEPT input and an
INTERSECT input. They check rows, not plans, so one expected output
fits every branch.

To keep back-branch plans exactly as they are, set pushdown_safe from
qp_is_pushdown_safe in the new block instead; it's a one-line change.

Testing
=======
On the tips of master (6cda9dfc4f97), REL_19_STABLE (4942dc745cf1),
REL_18_STABLE (19a2002b5db9) and REL_17_STABLE (9c44922d125c), with
assertions: the patch applies and compiles with no warning in
postgres_fdw.c, the core regression tests and postgres_fdw's pass, and
with the postgres_fdw.c change reverted, the new test hits this
assertion.

By hand, plans match an unpatched release build's except in that one
case. For

(SELECT a, b::text COLLATE "C" AS c FROM ft
WHERE a = 1 ORDER BY a LIMIT 1) UNION (SELECT 2, 'x');

the patched build sends

SELECT a, b FROM public.t WHERE ((a = 1)) LIMIT 1::bigint

where the unpatched build applies the LIMIT locally. The rows are the
same.

Found using SQLsmith and Claude.

-
Robins Tharakan

"

-
Robins
Databricks

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message shihao zhong 2026-11-03 03:40:47 Re: [Bug] Heap use-after-free in COPY FROM error context after ON_ERROR ignore skips a row
Previous Message shihao zhong 2026-11-03 03:29:24 Re: data race in libpq process-global variables (static_client_encoding / static_std_strings) when connections are established concurrently