postgres_fdw: push down FETCH FIRST .. WITH TIES when server version allows

Started by Sagar Shedge11 days ago6 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

appliessuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253692
psql -h localhost -U postgres

Built from patchset v6 (message #6), September 16, 2026 at 02:00 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t253692_6 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t253692_6 && git checkout t253692_6

Patchset v6 (message #6) is on t253692_6

Jump to latest
#1Sagar Shedge
sagar.shedge92@gmail.com

Hi Hackers,

add_foreign_final_paths() currently disables pushing down FETCH FIRST
.. WITH TIES entirely, because doing so requires knowing whether the
remote server is v13+ (which added support for the clause), and
checking that would mean opening a connection during planning (see
the discussion at /messages/by-id/18467-7bb89084ff03a08d@postgresql.org
which led to the current behavior).

Attached patch fills in that one remaining gap. postgres_fdw already
keeps a connection cache alive for the session's lifetime; if a
connection to the relevant foreign server already exists in that cache
(from an earlier query in the same session), its version is known for
free, with no additional network access. GetCachedConnectionVersion()
lookup into that cache and retun cached version. This information used in
add_foreign_final_paths() to allow the pushdown only when a cached
connection reports version 13 or later. The relation's server/user
mapping are read from RelOptInfo's own serverid/userid fields, which
are InvalidOid whenever the relation spans more than one foreign server
(a cross-server join, or a sharded partitioned table). So the pushdown
correctly stays disabled in those cases.

appendLimitClause() is updated to emit the SQL-standard FETCH FIRST
clause (with OFFSET ahead of it, per the grammar) instead of plain
LIMIT/OFFSET when WITH TIES is in use. The value in that position is
parsed as c_expr rather than a_expr, which does not accept the
"::type" cast decoration deparseExpr() normally emits for constants;
the patch parenthesizes it, which c_expr explicitly allows.

Regarding the collation/tie-semantics concern raised in the original
thread: by the time add_foreign_final_paths() runs, ORDER BY has
already been determined safe to push down by an earlier check. Ties are
just rows that compare equal under that same, already-vetted comparison.
So no new risk is introduced by additionallyfetching the tied rows.

Tested against a loopback foreign server, including: 1/ cold-cache
sessions correctly falling back to local evaluation; 2/ warm-cache
sessions pushing the FETCH clause down with results matching the
non-FDW reference, both with and without OFFSET 3/ cross-server
joins/unions correctly never attempting the pushdown. New regression
tests added to postgres_fdw.sql/expected covering all of the above.
make check passes.

Regards,
Sagar Shedge
Multigres Engineer, Supabase

Attachments:

t253692_1
0001-postgres_fdw-fetch-first-with-ties.patchapplication/octet-stream; name=0001-postgres_fdw-fetch-first-with-ties.patchDownload+201-16
#2Jinqing Kuang
kuangjinqingcn@gmail.com
In reply to: Sagar Shedge (#1)
Re: postgres_fdw: push down FETCH FIRST .. WITH TIES when server version allows

On Sep 6, 2026, at 10:39, Sagar Shedge <sagar.shedge92@gmail.com> wrote:

Hi Hackers,

add_foreign_final_paths() currently disables pushing down FETCH FIRST
.. WITH TIES entirely, because doing so requires knowing whether the
remote server is v13+ (which added support for the clause), and
checking that would mean opening a connection during planning (see
the discussion at /messages/by-id/18467-7bb89084ff03a08d@postgresql.org
which led to the current behavior).

Attached patch fills in that one remaining gap. postgres_fdw already
keeps a connection cache alive for the session's lifetime; if a
connection to the relevant foreign server already exists in that cache
(from an earlier query in the same session), its version is known for
free, with no additional network access. GetCachedConnectionVersion()
lookup into that cache and retun cached version. This information used in
add_foreign_final_paths() to allow the pushdown only when a cached
connection reports version 13 or later. The relation's server/user
mapping are read from RelOptInfo's own serverid/userid fields, which
are InvalidOid whenever the relation spans more than one foreign server
(a cross-server join, or a sharded partitioned table). So the pushdown
correctly stays disabled in those cases.

appendLimitClause() is updated to emit the SQL-standard FETCH FIRST
clause (with OFFSET ahead of it, per the grammar) instead of plain
LIMIT/OFFSET when WITH TIES is in use. The value in that position is
parsed as c_expr rather than a_expr, which does not accept the
"::type" cast decoration deparseExpr() normally emits for constants;
the patch parenthesizes it, which c_expr explicitly allows.

Regarding the collation/tie-semantics concern raised in the original
thread: by the time add_foreign_final_paths() runs, ORDER BY has
already been determined safe to push down by an earlier check. Ties are
just rows that compare equal under that same, already-vetted comparison.
So no new risk is introduced by additionallyfetching the tied rows.

Tested against a loopback foreign server, including: 1/ cold-cache
sessions correctly falling back to local evaluation; 2/ warm-cache
sessions pushing the FETCH clause down with results matching the
non-FDW reference, both with and without OFFSET 3/ cross-server
joins/unions correctly never attempting the pushdown. New regression
tests added to postgres_fdw.sql/expected covering all of the above.
make check passes.

Regards,
Sagar Shedge
Multigres Engineer, Supabase

<0001-postgres_fdw-fetch-first-with-ties.patch>

Hi Sagar,

I found two regressions in the patch.

With use_remote_estimate=true, this fails during planning:

SELECT a, count(*) FROM ft
WHERE b = 1 GROUP BY a, b
ORDER BY b FETCH FIRST 2 ROWS WITH TIES;

The planner removes b from the sort keys because WHERE fixes its value.
The remote query then has WITH TIES without ORDER BY:

ERROR: WITH TIES cannot be specified without ORDER BY clause

ORDER BY (1+1) has the same problem on grouped queries. I’ve kept
WITH TIES local when pathkeys is empty.

Ordinary EXPLAIN also fails with local estimates when the server has
neither a user-specific nor a PUBLIC mapping:

CREATE SERVER no_mapping FOREIGN DATA WRAPPER postgres_fdw;
CREATE FOREIGN TABLE ft_no_mapping (a int) SERVER no_mapping;
EXPLAIN (VERBOSE, COST OFF)
SELECT a FROM ft_no_mapping ORDER BY a
FETCH FIRST 2 ROWS WITH TIES;

GetUserMapping() errors before the cache lookup can fall back. I used
GetUserMappingExtended(..., DEBUG1) so a missing mapping keeps the limit
local. Existing mapping checks for remote estimates and execution still
apply.

I’ve attached v2 with fixes for both cases on top of your original patch,
along with regression tests.

Regards,
Jinqing

Attachments:

t253692_2
v2-0001-postgres_fdw-fetch-first-with-ties.patchapplication/octet-stream; name=v2-0001-postgres_fdw-fetch-first-with-ties.patch; x-unix-mode=0644Download+303-16
#3Jeevan Chalke
jeevan.chalke@enterprisedb.com
In reply to: Jinqing Kuang (#2)
Re: postgres_fdw: push down FETCH FIRST .. WITH TIES when server version allows

Hello,

On Thu, Sep 10, 2026 at 7:05 AM Jinqing Kuang <kuangjinqingcn@gmail.com>
wrote:

On Sep 6, 2026, at 10:39, Sagar Shedge <sagar.shedge92@gmail.com> wrote:

Hi Hackers,

add_foreign_final_paths() currently disables pushing down FETCH FIRST
.. WITH TIES entirely, because doing so requires knowing whether the
remote server is v13+ (which added support for the clause), and
checking that would mean opening a connection during planning (see
the discussion at

/messages/by-id/18467-7bb89084ff03a08d@postgresql.org

which led to the current behavior).

Attached patch fills in that one remaining gap. postgres_fdw already
keeps a connection cache alive for the session's lifetime; if a
connection to the relevant foreign server already exists in that cache
(from an earlier query in the same session), its version is known for
free, with no additional network access. GetCachedConnectionVersion()
lookup into that cache and retun cached version. This information used in
add_foreign_final_paths() to allow the pushdown only when a cached
connection reports version 13 or later. The relation's server/user
mapping are read from RelOptInfo's own serverid/userid fields, which
are InvalidOid whenever the relation spans more than one foreign server
(a cross-server join, or a sharded partitioned table). So the pushdown
correctly stays disabled in those cases.

appendLimitClause() is updated to emit the SQL-standard FETCH FIRST
clause (with OFFSET ahead of it, per the grammar) instead of plain
LIMIT/OFFSET when WITH TIES is in use. The value in that position is
parsed as c_expr rather than a_expr, which does not accept the
"::type" cast decoration deparseExpr() normally emits for constants;
the patch parenthesizes it, which c_expr explicitly allows.

Regarding the collation/tie-semantics concern raised in the original
thread: by the time add_foreign_final_paths() runs, ORDER BY has
already been determined safe to push down by an earlier check. Ties are
just rows that compare equal under that same, already-vetted comparison.
So no new risk is introduced by additionallyfetching the tied rows.

Tested against a loopback foreign server, including: 1/ cold-cache
sessions correctly falling back to local evaluation; 2/ warm-cache
sessions pushing the FETCH clause down with results matching the
non-FDW reference, both with and without OFFSET 3/ cross-server
joins/unions correctly never attempting the pushdown. New regression
tests added to postgres_fdw.sql/expected covering all of the above.
make check passes.

Regards,
Sagar Shedge
Multigres Engineer, Supabase

<0001-postgres_fdw-fetch-first-with-ties.patch>

Hi Sagar,

I found two regressions in the patch.

With use_remote_estimate=true, this fails during planning:

SELECT a, count(*) FROM ft
WHERE b = 1 GROUP BY a, b
ORDER BY b FETCH FIRST 2 ROWS WITH TIES;

The planner removes b from the sort keys because WHERE fixes its value.
The remote query then has WITH TIES without ORDER BY:

ERROR: WITH TIES cannot be specified without ORDER BY clause

ORDER BY (1+1) has the same problem on grouped queries. I’ve kept
WITH TIES local when pathkeys is empty.

Ordinary EXPLAIN also fails with local estimates when the server has
neither a user-specific nor a PUBLIC mapping:

CREATE SERVER no_mapping FOREIGN DATA WRAPPER postgres_fdw;
CREATE FOREIGN TABLE ft_no_mapping (a int) SERVER no_mapping;
EXPLAIN (VERBOSE, COST OFF)
SELECT a FROM ft_no_mapping ORDER BY a
FETCH FIRST 2 ROWS WITH TIES;

GetUserMapping() errors before the cache lookup can fall back. I used
GetUserMappingExtended(..., DEBUG1) so a missing mapping keeps the limit
local. Existing mapping checks for remote estimates and execution still
apply.

I’ve attached v2 with fixes for both cases on top of your original patch,
along with regression tests.

I gave the patch a quick review. It applies cleanly, builds, and make check
in
contrib/postgres_fdw passes, including the new tests. The logic looks
correct
to me, and I couldn't find a case where the pushdown produces different
results
than the local fallback.

One thing worth discussing explicitly rather than leaving implicit is that
the
pushdown decision in add_foreign_final_paths() depends entirely on whatever
connection happens to already be cached for that user mapping at plan time:

if (user == NULL || GetCachedConnectionVersion(user) < 130000)
return;

That means the exact same query, planned twice in the same backend, can end
up
with two different plans purely because of unrelated activity in between:

- First time a given foreign server is touched in a session (no cached
connection yet) => WITH TIES stays local, *no pushdown*, the full result
set
for the ORDER BY gets fetched.
- Any later query against that server in the same backend, once anything has
opened a connection to it => *pushed down*.

So EXPLAIN on the same statement can show a Foreign Scan with FETCH FIRST
...
WITH TIES folded into the remote SQL on one run, and a local LIMIT node on
another, with nothing about the query itself having changed. Someone
diagnosing
a slow query by comparing EXPLAIN output across sessions could easily
mistake
this for a bug.

To be clear, I don't think this makes the patch wrong, but since it
introduces
a new source of connection-history-dependent plan shape in postgres_fdw, I
think it's worth either:

- a note in the code comment above the check (right now the comment explains
why we use the cache, but not that this makes the pushdown decision
session-history-dependent), and/or
- a line in the commit message/release notes calling it out explicitly, so
it
doesn't surprise someone debugging plan differences later.

Curious whether this tradeoff was already considered and just not written
down,
or whether there's a reason it's not worth documenting.

Thanks

Regards,
Jinqing

--
*Jeevan Chalke*
*Senior Principal Engineer, Engineering Manager*
*Product Development*

enterprisedb.com <https://www.enterprisedb.com&gt;

#4Sagar Shedge
sagar.shedge92@gmail.com
In reply to: Jeevan Chalke (#3)
Re: postgres_fdw: push down FETCH FIRST .. WITH TIES when server version allows

Thanks Jinqing for handling regressions. I did one more round of testing
with
different flags and scenarios.

Jeevan,

To be clear, I don't think this makes the patch wrong, but since it

introduces

a new source of connection-history-dependent plan shape in postgres_fdw, I
think it's worth either:

- a note in the code comment above the check (right now the comment

explains

why we use the cache, but not that this makes the pushdown decision
session-history-dependent), and/or
- a line in the commit message/release notes calling it out explicitly,

so it

doesn't surprise someone debugging plan differences later.

Curious whether this tradeoff was already considered and just not written

down,

or whether there's a reason it's not worth documenting.

Good catch. I had considered it but hadn't written it down. While thinking
it through, Postgres already has similar behavior for custom vs. generic
plans which differ across executions where the optimizer's estimates lead
to different plans.
Thanks for pushing on that. It makes sense to highlight both in the code
comment and the commit message.

Attached updated patch.

On Thu, Sep 10, 2026 at 4:12 PM Jeevan Chalke <
jeevan.chalke@enterprisedb.com> wrote:

Hello,

On Thu, Sep 10, 2026 at 7:05 AM Jinqing Kuang <kuangjinqingcn@gmail.com>
wrote:

On Sep 6, 2026, at 10:39, Sagar Shedge <sagar.shedge92@gmail.com>

wrote:

Hi Hackers,

add_foreign_final_paths() currently disables pushing down FETCH FIRST
.. WITH TIES entirely, because doing so requires knowing whether the
remote server is v13+ (which added support for the clause), and
checking that would mean opening a connection during planning (see
the discussion at

/messages/by-id/18467-7bb89084ff03a08d@postgresql.org

which led to the current behavior).

Attached patch fills in that one remaining gap. postgres_fdw already
keeps a connection cache alive for the session's lifetime; if a
connection to the relevant foreign server already exists in that cache
(from an earlier query in the same session), its version is known for
free, with no additional network access. GetCachedConnectionVersion()
lookup into that cache and retun cached version. This information used

in

add_foreign_final_paths() to allow the pushdown only when a cached
connection reports version 13 or later. The relation's server/user
mapping are read from RelOptInfo's own serverid/userid fields, which
are InvalidOid whenever the relation spans more than one foreign server
(a cross-server join, or a sharded partitioned table). So the pushdown
correctly stays disabled in those cases.

appendLimitClause() is updated to emit the SQL-standard FETCH FIRST
clause (with OFFSET ahead of it, per the grammar) instead of plain
LIMIT/OFFSET when WITH TIES is in use. The value in that position is
parsed as c_expr rather than a_expr, which does not accept the
"::type" cast decoration deparseExpr() normally emits for constants;
the patch parenthesizes it, which c_expr explicitly allows.

Regarding the collation/tie-semantics concern raised in the original
thread: by the time add_foreign_final_paths() runs, ORDER BY has
already been determined safe to push down by an earlier check. Ties are
just rows that compare equal under that same, already-vetted comparison.
So no new risk is introduced by additionallyfetching the tied rows.

Tested against a loopback foreign server, including: 1/ cold-cache
sessions correctly falling back to local evaluation; 2/ warm-cache
sessions pushing the FETCH clause down with results matching the
non-FDW reference, both with and without OFFSET 3/ cross-server
joins/unions correctly never attempting the pushdown. New regression
tests added to postgres_fdw.sql/expected covering all of the above.
make check passes.

Regards,
Sagar Shedge
Multigres Engineer, Supabase

<0001-postgres_fdw-fetch-first-with-ties.patch>

Hi Sagar,

I found two regressions in the patch.

With use_remote_estimate=true, this fails during planning:

SELECT a, count(*) FROM ft
WHERE b = 1 GROUP BY a, b
ORDER BY b FETCH FIRST 2 ROWS WITH TIES;

The planner removes b from the sort keys because WHERE fixes its value.
The remote query then has WITH TIES without ORDER BY:

ERROR: WITH TIES cannot be specified without ORDER BY clause

ORDER BY (1+1) has the same problem on grouped queries. I’ve kept
WITH TIES local when pathkeys is empty.

Ordinary EXPLAIN also fails with local estimates when the server has
neither a user-specific nor a PUBLIC mapping:

CREATE SERVER no_mapping FOREIGN DATA WRAPPER postgres_fdw;
CREATE FOREIGN TABLE ft_no_mapping (a int) SERVER no_mapping;
EXPLAIN (VERBOSE, COST OFF)
SELECT a FROM ft_no_mapping ORDER BY a
FETCH FIRST 2 ROWS WITH TIES;

GetUserMapping() errors before the cache lookup can fall back. I used
GetUserMappingExtended(..., DEBUG1) so a missing mapping keeps the limit
local. Existing mapping checks for remote estimates and execution still
apply.

I’ve attached v2 with fixes for both cases on top of your original patch,
along with regression tests.

I gave the patch a quick review. It applies cleanly, builds, and make
check in
contrib/postgres_fdw passes, including the new tests. The logic looks
correct
to me, and I couldn't find a case where the pushdown produces different
results
than the local fallback.

One thing worth discussing explicitly rather than leaving implicit is that
the
pushdown decision in add_foreign_final_paths() depends entirely on
whatever
connection happens to already be cached for that user mapping at plan time:

if (user == NULL || GetCachedConnectionVersion(user) < 130000)
return;

That means the exact same query, planned twice in the same backend, can
end up
with two different plans purely because of unrelated activity in between:

- First time a given foreign server is touched in a session (no cached
connection yet) => WITH TIES stays local, *no pushdown*, the full
result set
for the ORDER BY gets fetched.
- Any later query against that server in the same backend, once anything
has
opened a connection to it => *pushed down*.

So EXPLAIN on the same statement can show a Foreign Scan with FETCH FIRST
...
WITH TIES folded into the remote SQL on one run, and a local LIMIT node on
another, with nothing about the query itself having changed. Someone
diagnosing
a slow query by comparing EXPLAIN output across sessions could easily
mistake
this for a bug.

To be clear, I don't think this makes the patch wrong, but since it
introduces
a new source of connection-history-dependent plan shape in postgres_fdw, I
think it's worth either:

- a note in the code comment above the check (right now the comment
explains
why we use the cache, but not that this makes the pushdown decision
session-history-dependent), and/or
- a line in the commit message/release notes calling it out explicitly, so
it
doesn't surprise someone debugging plan differences later.

Curious whether this tradeoff was already considered and just not written
down,
or whether there's a reason it's not worth documenting.

Thanks

Regards,
Jinqing

--
*Jeevan Chalke*
*Senior Principal Engineer, Engineering Manager*
*Product Development*

enterprisedb.com <https://www.enterprisedb.com&gt;

--
Sagar Dilip Shedge,
Pune.

With Regards.

Attachments:

t253692_4
v3-0001-postgres_fdw-Push-down-WITH-TIES-for-known-remote.patchapplication/octet-stream; name=v3-0001-postgres_fdw-Push-down-WITH-TIES-for-known-remote.patchDownload+313-16
#5Jeevan Chalke
jeevan.chalke@enterprisedb.com
In reply to: Sagar Shedge (#4)
Re: postgres_fdw: push down FETCH FIRST .. WITH TIES when server version allows

On Fri, Sep 11, 2026 at 9:17 AM Sagar Shedge <sagar.shedge92@gmail.com>
wrote:

Thanks Jinqing for handling regressions. I did one more round of testing
with
different flags and scenarios.

Jeevan,

To be clear, I don't think this makes the patch wrong, but since it

introduces

a new source of connection-history-dependent plan shape in postgres_fdw,

I

think it's worth either:

- a note in the code comment above the check (right now the comment

explains

why we use the cache, but not that this makes the pushdown decision
session-history-dependent), and/or
- a line in the commit message/release notes calling it out explicitly,

so it

doesn't surprise someone debugging plan differences later.

Curious whether this tradeoff was already considered and just not

written down,

or whether there's a reason it's not worth documenting.

Good catch. I had considered it but hadn't written it down. While thinking
it through, Postgres already has similar behavior for custom vs. generic
plans which differ across executions where the optimizer's estimates lead
to different plans.
Thanks for pushing on that. It makes sense to highlight both in the code
comment and the commit message.

Attached updated patch.

Thanks for the changes. Looking deeper into the code, I noticed this:

  +       /*
  +        * final_rel->serverid is set only if the whole relation belongs
to a
  +        * single FDW (see grouping_planner()); this is InvalidOid for,
e.g.,
  +        * a join or partitioned scan spanning more than one foreign
server,
  +        * in which case there's no single remote query to push the FETCH
  +        * clause into.
  +        */
  +       if (!OidIsValid(final_rel->serverid))
  +           return;

This check also guards a case beyond what the comment describes: a
partitioned/inherited relation whose partitions are all on the same foreign
server. There, final_rel->serverid is still InvalidOid (it's a
multi-relation
Merge Append, not a single foreign relation), so this correctly forces
WITH TIES to stay local. That matters because pushing FETCH FIRST ... WITH
TIES
independently into each partition's own scan would be an actual correctness
bug.
Ties have to be evaluated against the globally merged ordering across all
partitions, not per-partition. This if already prevents that, but the
comment
currently frames the check only in terms of "no single remote query to push
into," not the correctness hazard it happens to also rule out.

Worth calling that out explicitly in the comment, and adding a regression
test
for the same-server multi-partition case, so it's clear this isn't just a
missing-optimization corner but a case that would silently return wrong
results
if this check were ever relaxed or bypassed.

Rest all looks good to me.

Thanks

On Thu, Sep 10, 2026 at 4:12 PM Jeevan Chalke <
jeevan.chalke@enterprisedb.com> wrote:

Hello,

On Thu, Sep 10, 2026 at 7:05 AM Jinqing Kuang <kuangjinqingcn@gmail.com>
wrote:

On Sep 6, 2026, at 10:39, Sagar Shedge <sagar.shedge92@gmail.com>

wrote:

Hi Hackers,

add_foreign_final_paths() currently disables pushing down FETCH FIRST
.. WITH TIES entirely, because doing so requires knowing whether the
remote server is v13+ (which added support for the clause), and
checking that would mean opening a connection during planning (see
the discussion at

/messages/by-id/18467-7bb89084ff03a08d@postgresql.org

which led to the current behavior).

Attached patch fills in that one remaining gap. postgres_fdw already
keeps a connection cache alive for the session's lifetime; if a
connection to the relevant foreign server already exists in that cache
(from an earlier query in the same session), its version is known for
free, with no additional network access. GetCachedConnectionVersion()
lookup into that cache and retun cached version. This information used

in

add_foreign_final_paths() to allow the pushdown only when a cached
connection reports version 13 or later. The relation's server/user
mapping are read from RelOptInfo's own serverid/userid fields, which
are InvalidOid whenever the relation spans more than one foreign server
(a cross-server join, or a sharded partitioned table). So the pushdown
correctly stays disabled in those cases.

appendLimitClause() is updated to emit the SQL-standard FETCH FIRST
clause (with OFFSET ahead of it, per the grammar) instead of plain
LIMIT/OFFSET when WITH TIES is in use. The value in that position is
parsed as c_expr rather than a_expr, which does not accept the
"::type" cast decoration deparseExpr() normally emits for constants;
the patch parenthesizes it, which c_expr explicitly allows.

Regarding the collation/tie-semantics concern raised in the original
thread: by the time add_foreign_final_paths() runs, ORDER BY has
already been determined safe to push down by an earlier check. Ties are
just rows that compare equal under that same, already-vetted

comparison.

So no new risk is introduced by additionallyfetching the tied rows.

Tested against a loopback foreign server, including: 1/ cold-cache
sessions correctly falling back to local evaluation; 2/ warm-cache
sessions pushing the FETCH clause down with results matching the
non-FDW reference, both with and without OFFSET 3/ cross-server
joins/unions correctly never attempting the pushdown. New regression
tests added to postgres_fdw.sql/expected covering all of the above.
make check passes.

Regards,
Sagar Shedge
Multigres Engineer, Supabase

<0001-postgres_fdw-fetch-first-with-ties.patch>

Hi Sagar,

I found two regressions in the patch.

With use_remote_estimate=true, this fails during planning:

SELECT a, count(*) FROM ft
WHERE b = 1 GROUP BY a, b
ORDER BY b FETCH FIRST 2 ROWS WITH TIES;

The planner removes b from the sort keys because WHERE fixes its value.
The remote query then has WITH TIES without ORDER BY:

ERROR: WITH TIES cannot be specified without ORDER BY clause

ORDER BY (1+1) has the same problem on grouped queries. I’ve kept
WITH TIES local when pathkeys is empty.

Ordinary EXPLAIN also fails with local estimates when the server has
neither a user-specific nor a PUBLIC mapping:

CREATE SERVER no_mapping FOREIGN DATA WRAPPER postgres_fdw;
CREATE FOREIGN TABLE ft_no_mapping (a int) SERVER no_mapping;
EXPLAIN (VERBOSE, COST OFF)
SELECT a FROM ft_no_mapping ORDER BY a
FETCH FIRST 2 ROWS WITH TIES;

GetUserMapping() errors before the cache lookup can fall back. I used
GetUserMappingExtended(..., DEBUG1) so a missing mapping keeps the limit
local. Existing mapping checks for remote estimates and execution still
apply.

I’ve attached v2 with fixes for both cases on top of your original patch,
along with regression tests.

I gave the patch a quick review. It applies cleanly, builds, and make
check in
contrib/postgres_fdw passes, including the new tests. The logic looks
correct
to me, and I couldn't find a case where the pushdown produces different
results
than the local fallback.

One thing worth discussing explicitly rather than leaving implicit is
that the
pushdown decision in add_foreign_final_paths() depends entirely on
whatever
connection happens to already be cached for that user mapping at plan
time:

if (user == NULL || GetCachedConnectionVersion(user) < 130000)
return;

That means the exact same query, planned twice in the same backend, can
end up
with two different plans purely because of unrelated activity in between:

- First time a given foreign server is touched in a session (no cached
connection yet) => WITH TIES stays local, *no pushdown*, the full
result set
for the ORDER BY gets fetched.
- Any later query against that server in the same backend, once anything
has
opened a connection to it => *pushed down*.

So EXPLAIN on the same statement can show a Foreign Scan with FETCH
FIRST ...
WITH TIES folded into the remote SQL on one run, and a local LIMIT node
on
another, with nothing about the query itself having changed. Someone
diagnosing
a slow query by comparing EXPLAIN output across sessions could easily
mistake
this for a bug.

To be clear, I don't think this makes the patch wrong, but since it
introduces
a new source of connection-history-dependent plan shape in postgres_fdw, I
think it's worth either:

- a note in the code comment above the check (right now the comment
explains
why we use the cache, but not that this makes the pushdown decision
session-history-dependent), and/or
- a line in the commit message/release notes calling it out explicitly,
so it
doesn't surprise someone debugging plan differences later.

Curious whether this tradeoff was already considered and just not written
down,
or whether there's a reason it's not worth documenting.

Thanks

Regards,
Jinqing

--
*Jeevan Chalke*
*Senior Principal Engineer, Engineering Manager*
*Product Development*

enterprisedb.com <https://www.enterprisedb.com&gt;

--
Sagar Dilip Shedge,
Pune.

With Regards.

--
*Jeevan Chalke*
*Senior Principal Engineer, Engineering Manager*
*Product Development*

enterprisedb.com <https://www.enterprisedb.com&gt;

#6Jinqing Kuang
kuangjinqingcn@gmail.com
In reply to: Jeevan Chalke (#5)
Re: postgres_fdw: push down FETCH FIRST .. WITH TIES when server version allows

On Sep 11, 2026, at 22:36, Jeevan Chalke <jeevan.chalke@enterprisedb.com> wrote:

On Fri, Sep 11, 2026 at 9:17 AM Sagar Shedge <sagar.shedge92@gmail.com> wrote:
Thanks Jinqing for handling regressions. I did one more round of testing with
different flags and scenarios.

Jeevan,

To be clear, I don't think this makes the patch wrong, but since it introduces
a new source of connection-history-dependent plan shape in postgres_fdw, I
think it's worth either:

- a note in the code comment above the check (right now the comment explains
why we use the cache, but not that this makes the pushdown decision
session-history-dependent), and/or
- a line in the commit message/release notes calling it out explicitly, so it
doesn't surprise someone debugging plan differences later.

Curious whether this tradeoff was already considered and just not written down,
or whether there's a reason it's not worth documenting.

Good catch. I had considered it but hadn't written it down. While thinking
it through, Postgres already has similar behavior for custom vs. generic
plans which differ across executions where the optimizer's estimates lead
to different plans.
Thanks for pushing on that. It makes sense to highlight both in the code
comment and the commit message.

Attached updated patch.

Thanks for the changes. Looking deeper into the code, I noticed this:

+       /*
+        * final_rel->serverid is set only if the whole relation belongs to a
+        * single FDW (see grouping_planner()); this is InvalidOid for, e.g.,
+        * a join or partitioned scan spanning more than one foreign server,
+        * in which case there's no single remote query to push the FETCH
+        * clause into.
+        */
+       if (!OidIsValid(final_rel->serverid))
+           return;

This check also guards a case beyond what the comment describes: a
partitioned/inherited relation whose partitions are all on the same foreign
server. There, final_rel->serverid is still InvalidOid (it's a multi-relation
Merge Append, not a single foreign relation), so this correctly forces
WITH TIES to stay local. That matters because pushing FETCH FIRST ... WITH TIES
independently into each partition's own scan would be an actual correctness bug.
Ties have to be evaluated against the globally merged ordering across all
partitions, not per-partition. This if already prevents that, but the comment
currently frames the check only in terms of "no single remote query to push
into," not the correctness hazard it happens to also rule out.

Worth calling that out explicitly in the comment, and adding a regression test
for the same-server multi-partition case, so it's clear this isn't just a
missing-optimization corner but a case that would silently return wrong results
if this check were ever relaxed or bypassed.

Rest all looks good to me.

Thanks

On Thu, Sep 10, 2026 at 4:12 PM Jeevan Chalke <jeevan.chalke@enterprisedb.com> wrote:
Hello,

On Thu, Sep 10, 2026 at 7:05 AM Jinqing Kuang <kuangjinqingcn@gmail.com> wrote:

On Sep 6, 2026, at 10:39, Sagar Shedge <sagar.shedge92@gmail.com> wrote:

Hi Hackers,

add_foreign_final_paths() currently disables pushing down FETCH FIRST
.. WITH TIES entirely, because doing so requires knowing whether the
remote server is v13+ (which added support for the clause), and
checking that would mean opening a connection during planning (see
the discussion at /messages/by-id/18467-7bb89084ff03a08d@postgresql.org
which led to the current behavior).

Attached patch fills in that one remaining gap. postgres_fdw already
keeps a connection cache alive for the session's lifetime; if a
connection to the relevant foreign server already exists in that cache
(from an earlier query in the same session), its version is known for
free, with no additional network access. GetCachedConnectionVersion()
lookup into that cache and retun cached version. This information used in
add_foreign_final_paths() to allow the pushdown only when a cached
connection reports version 13 or later. The relation's server/user
mapping are read from RelOptInfo's own serverid/userid fields, which
are InvalidOid whenever the relation spans more than one foreign server
(a cross-server join, or a sharded partitioned table). So the pushdown
correctly stays disabled in those cases.

appendLimitClause() is updated to emit the SQL-standard FETCH FIRST
clause (with OFFSET ahead of it, per the grammar) instead of plain
LIMIT/OFFSET when WITH TIES is in use. The value in that position is
parsed as c_expr rather than a_expr, which does not accept the
"::type" cast decoration deparseExpr() normally emits for constants;
the patch parenthesizes it, which c_expr explicitly allows.

Regarding the collation/tie-semantics concern raised in the original
thread: by the time add_foreign_final_paths() runs, ORDER BY has
already been determined safe to push down by an earlier check. Ties are
just rows that compare equal under that same, already-vetted comparison.
So no new risk is introduced by additionallyfetching the tied rows.

Tested against a loopback foreign server, including: 1/ cold-cache
sessions correctly falling back to local evaluation; 2/ warm-cache
sessions pushing the FETCH clause down with results matching the
non-FDW reference, both with and without OFFSET 3/ cross-server
joins/unions correctly never attempting the pushdown. New regression
tests added to postgres_fdw.sql/expected covering all of the above.
make check passes.

Regards,
Sagar Shedge
Multigres Engineer, Supabase

<0001-postgres_fdw-fetch-first-with-ties.patch>

Hi Sagar,

I found two regressions in the patch.

With use_remote_estimate=true, this fails during planning:

SELECT a, count(*) FROM ft
WHERE b = 1 GROUP BY a, b
ORDER BY b FETCH FIRST 2 ROWS WITH TIES;

The planner removes b from the sort keys because WHERE fixes its value.
The remote query then has WITH TIES without ORDER BY:

ERROR: WITH TIES cannot be specified without ORDER BY clause

ORDER BY (1+1) has the same problem on grouped queries. I’ve kept
WITH TIES local when pathkeys is empty.

Ordinary EXPLAIN also fails with local estimates when the server has
neither a user-specific nor a PUBLIC mapping:

CREATE SERVER no_mapping FOREIGN DATA WRAPPER postgres_fdw;
CREATE FOREIGN TABLE ft_no_mapping (a int) SERVER no_mapping;
EXPLAIN (VERBOSE, COST OFF)
SELECT a FROM ft_no_mapping ORDER BY a
FETCH FIRST 2 ROWS WITH TIES;

GetUserMapping() errors before the cache lookup can fall back. I used
GetUserMappingExtended(..., DEBUG1) so a missing mapping keeps the limit
local. Existing mapping checks for remote estimates and execution still
apply.

I’ve attached v2 with fixes for both cases on top of your original patch,
along with regression tests.

I gave the patch a quick review. It applies cleanly, builds, and make check in
contrib/postgres_fdw passes, including the new tests. The logic looks correct
to me, and I couldn't find a case where the pushdown produces different results
than the local fallback.

One thing worth discussing explicitly rather than leaving implicit is that the
pushdown decision in add_foreign_final_paths() depends entirely on whatever
connection happens to already be cached for that user mapping at plan time:

if (user == NULL || GetCachedConnectionVersion(user) < 130000)
return;

That means the exact same query, planned twice in the same backend, can end up
with two different plans purely because of unrelated activity in between:

- First time a given foreign server is touched in a session (no cached
connection yet) => WITH TIES stays local, no pushdown, the full result set
for the ORDER BY gets fetched.
- Any later query against that server in the same backend, once anything has
opened a connection to it => pushed down.

So EXPLAIN on the same statement can show a Foreign Scan with FETCH FIRST ...
WITH TIES folded into the remote SQL on one run, and a local LIMIT node on
another, with nothing about the query itself having changed. Someone diagnosing
a slow query by comparing EXPLAIN output across sessions could easily mistake
this for a bug.

To be clear, I don't think this makes the patch wrong, but since it introduces
a new source of connection-history-dependent plan shape in postgres_fdw, I
think it's worth either:

- a note in the code comment above the check (right now the comment explains
why we use the cache, but not that this makes the pushdown decision
session-history-dependent), and/or
- a line in the commit message/release notes calling it out explicitly, so it
doesn't surprise someone debugging plan differences later.

Curious whether this tradeoff was already considered and just not written down,
or whether there's a reason it's not worth documenting.

Thanks

Regards,
Jinqing

--
Jeevan Chalke
Senior Principal Engineer, Engineering Manager
Product Development

enterprisedb.com

--
Sagar Dilip Shedge,
Pune.
With Regards.

--
Jeevan Chalke
Senior Principal Engineer, Engineering Manager
Product Development

enterprisedb.com

Thanks for taking another look. I’ve added tests for the same-server
partition case, covering ties across partitions and OFFSET into the
tied group, with the connection already cached.

While looking into this case, I noticed that the partitioned parent has
no fdwroutine, so grouping_planner() doesn’t call GetForeignUpperPaths()
for it. This means the global Limit stays local without reaching the
server-id check. I’ve clarified that in the comment.

Attached is v4 based on Sagar’s v3.

Regards,
Jinqing

Attachments:

t253692_6
v4-0001-postgres_fdw-fetch-first-with-ties.patchapplication/octet-stream; name=v4-0001-postgres_fdw-fetch-first-with-ties.patch; x-unix-mode=0644Download+404-18