From dc9af8bd9d59d3b885c81f065c4e6257abfe809e Mon Sep 17 00:00:00 2001 From: Sagar Shedge Date: Sun, 6 Sep 2026 07:09:55 +0530 Subject: [PATCH v2] postgres_fdw: Push down WITH TIES for known remote servers Previously, FETCH FIRST ... WITH TIES was always kept local because the remote server might predate version 13, which added support for the clause. Plain LIMIT cannot preserve the additional tied rows. Use an existing cached connection to determine whether the remote server supports WITH TIES, without opening a connection or doing network I/O for this version check. Remote estimates may already have populated the cache during planning. Keep the restriction local when no suitable mapping or cached version is available, or when the relation does not belong to a single foreign server. Require nonempty remote sort keys as well as shippable ordering. The planner can remove every sort key as redundant, for example when a WHERE clause fixes the ordering expression. In that case the deparser emits no ORDER BY and a remote WITH TIES clause would be invalid. Emit FETCH FIRST ... WITH TIES instead of LIMIT, with OFFSET first in SQL-standard order. Parenthesize the count and offset expressions so that casts added by the deparser are valid in these grammar positions. Add regression coverage for cached-version pushdown, local fallback, OFFSET, redundant sort keys and EXPLAIN without a user mapping. Co-authored-by: Jinqing Kuang Discussion: https://postgr.es/m/CAPhYifHu_Nd+YoAg0iWfCCO+6eGo5nzbQNyOm=uxXTcvKatccw@mail.gmail.com --- contrib/postgres_fdw/connection.c | 30 ++++ contrib/postgres_fdw/deparse.c | 39 ++++- .../postgres_fdw/expected/postgres_fdw.out | 151 +++++++++++++++++- contrib/postgres_fdw/postgres_fdw.c | 40 ++++- contrib/postgres_fdw/postgres_fdw.h | 1 + contrib/postgres_fdw/sql/postgres_fdw.sql | 60 ++++++- 6 files changed, 303 insertions(+), 18 deletions(-) diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c index b5d4cf3dccc..05198279ef2 100644 --- a/contrib/postgres_fdw/connection.c +++ b/contrib/postgres_fdw/connection.c @@ -1030,6 +1030,36 @@ ReleaseConnection(PGconn *conn) */ } +/* + * Return the server version number of the already-cached connection for + * "user", if one exists, or 0 if there is none (in which case the caller + * must not assume anything about the remote server's version). + * + * This never establishes a new connection and never does any network I/O: + * it only consults the connection cache and, if a live entry is found, + * reads the version number libpq already recorded during that connection's + * startup handshake. Planning can use this information without opening + * another connection. + */ +int +GetCachedConnectionVersion(UserMapping *user) +{ + bool found; + ConnCacheKey key; + ConnCacheEntry *entry; + + if (ConnectionHash == NULL) + return 0; + + key = user->umid; + entry = (ConnCacheEntry *) hash_search(ConnectionHash, &key, HASH_FIND, + &found); + if (!found || entry->conn == NULL || entry->invalidated) + return 0; + + return PQserverVersion(entry->conn); +} + /* * Assign a "unique" number for a cursor. * diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c index ff9fe0f87e4..74beadc418b 100644 --- a/contrib/postgres_fdw/deparse.c +++ b/contrib/postgres_fdw/deparse.c @@ -4232,15 +4232,44 @@ appendLimitClause(deparse_expr_cxt *context) /* Make sure any constants in the exprs are printed portably */ nestlevel = set_transmission_modes(); - if (root->parse->limitCount) + if (root->parse->limitOption == LIMIT_OPTION_WITH_TIES) { - appendStringInfoString(buf, " LIMIT "); + /* + * Plain LIMIT has no way to express WITH TIES, so use the + * SQL-standard FETCH clause instead. Emit OFFSET before FETCH as + * required by the SQL standard. + * + * Unlike LIMIT/OFFSET, the value in this position is restricted to + * "c_expr" rather than a full "a_expr" (see select_fetch_first_value + * in gram.y), which notably disallows the "::type" cast decoration + * deparseExpr() adds to constants for portability. Parenthesize the + * value to work around that; c_expr explicitly allows a parenthesized + * a_expr, so this is valid regardless of what kind of expression it + * turns out to be. + */ + if (root->parse->limitOffset) + { + appendStringInfoString(buf, " OFFSET ("); + deparseExpr((Expr *) root->parse->limitOffset, context); + appendStringInfoString(buf, ") ROWS"); + } + Assert(root->parse->limitCount); + appendStringInfoString(buf, " FETCH FIRST ("); deparseExpr((Expr *) root->parse->limitCount, context); + appendStringInfoString(buf, ") ROWS WITH TIES"); } - if (root->parse->limitOffset) + else { - appendStringInfoString(buf, " OFFSET "); - deparseExpr((Expr *) root->parse->limitOffset, context); + if (root->parse->limitCount) + { + appendStringInfoString(buf, " LIMIT "); + deparseExpr((Expr *) root->parse->limitCount, context); + } + if (root->parse->limitOffset) + { + appendStringInfoString(buf, " OFFSET "); + deparseExpr((Expr *) root->parse->limitOffset, context); + } } reset_transmission_modes(nestlevel); diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index a6295674daf..8519d4bdccb 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -1087,17 +1087,17 @@ SELECT * FROM ft1 t1 WHERE t1.c1 === t1.c2 order by t1.c2 limit 1; 1 | 1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo (1 row) --- Ensure we don't ship FETCH FIRST .. WITH TIES +-- Ensure we ship FETCH FIRST .. WITH TIES once the remote server's version +-- is known (i.e., a connection to it is already cached in this session, as +-- is the case here due to preceding tests) EXPLAIN (VERBOSE, COSTS OFF) SELECT t1.c2 FROM ft1 t1 WHERE t1.c1 > 960 ORDER BY t1.c2 FETCH FIRST 2 ROWS WITH TIES; - QUERY PLAN -------------------------------------------------------------------------------------------------- - Limit + QUERY PLAN +---------------------------------------------------------------------------------------------------------------------------------- + Foreign Scan on public.ft1 t1 Output: c2 - -> Foreign Scan on public.ft1 t1 - Output: c2 - Remote SQL: SELECT c2 FROM "S 1"."T 1" WHERE (("C 1" > 960)) ORDER BY c2 ASC NULLS LAST -(5 rows) + Remote SQL: SELECT c2 FROM "S 1"."T 1" WHERE (("C 1" > 960)) ORDER BY c2 ASC NULLS LAST FETCH FIRST (2::bigint) ROWS WITH TIES +(3 rows) SELECT t1.c2 FROM ft1 t1 WHERE t1.c1 > 960 ORDER BY t1.c2 FETCH FIRST 2 ROWS WITH TIES; c2 @@ -1108,6 +1108,141 @@ SELECT t1.c2 FROM ft1 t1 WHERE t1.c1 > 960 ORDER BY t1.c2 FETCH FIRST 2 ROWS WIT 0 (4 rows) +-- Same, but combined with OFFSET, emitted before FETCH FIRST in SQL-standard +-- order. Skipping into the middle of a tied group must not drop any of the +-- remaining ties. +EXPLAIN (VERBOSE, COSTS OFF) +SELECT t1.c2 FROM ft1 t1 WHERE t1.c1 > 960 ORDER BY t1.c2 OFFSET 1 FETCH FIRST 2 ROWS WITH TIES; + QUERY PLAN +---------------------------------------------------------------------------------------------------------------------------------------------------------- + Foreign Scan on public.ft1 t1 + Output: c2 + Remote SQL: SELECT c2 FROM "S 1"."T 1" WHERE (("C 1" > 960)) ORDER BY c2 ASC NULLS LAST OFFSET (1::bigint) ROWS FETCH FIRST (2::bigint) ROWS WITH TIES +(3 rows) + +SELECT t1.c2 FROM ft1 t1 WHERE t1.c1 > 960 ORDER BY t1.c2 OFFSET 1 FETCH FIRST 2 ROWS WITH TIES; + c2 +---- + 0 + 0 + 0 +(3 rows) + +-- Ensure we never ship FETCH FIRST .. WITH TIES for a query whose result +-- combines rows from more than one foreign server (here, a join between +-- ft5 on "loopback" and ft6 on "loopback2"), regardless of whether either +-- server's version is known; there's no single remote query to push the +-- FETCH clause into, so it must stay local +EXPLAIN (VERBOSE, COSTS OFF) +SELECT ft5.c1, ft5.c2 FROM ft5 JOIN ft6 USING (c1) + ORDER BY ft5.c2 FETCH FIRST 2 ROWS WITH TIES; + QUERY PLAN +------------------------------------------------------------------------------------- + Limit + Output: ft5.c1, ft5.c2 + -> Nested Loop + Output: ft5.c1, ft5.c2 + Join Filter: (ft5.c1 = ft6.c1) + -> Foreign Scan on public.ft5 + Output: ft5.c1, ft5.c2, ft5.c3 + Remote SQL: SELECT c1, c2 FROM "S 1"."T 4" ORDER BY c2 ASC NULLS LAST + -> Materialize + Output: ft6.c1 + -> Foreign Scan on public.ft6 + Output: ft6.c1 + Remote SQL: SELECT c1 FROM "S 1"."T 4" +(13 rows) + +-- Two independently limited scans on different foreign servers, combined +-- locally via UNION ALL: each side's FETCH FIRST .. WITH TIES pushdown +-- decision is made independently based on its own server's cached +-- connection, with no coordination needed between them. ft5's server +-- (loopback) is already warmed up by many earlier tests, so that side +-- pushes the FETCH clause down; ft6's server (loopback2) has not been +-- connected to yet, so that side falls back to a local Limit. +EXPLAIN (VERBOSE, COSTS OFF) +(SELECT c1, c2 FROM ft6 ORDER BY c2 FETCH FIRST 2 ROWS WITH TIES) +UNION ALL +(SELECT c1, c2 FROM ft5 ORDER BY c2 FETCH FIRST 2 ROWS WITH TIES); + QUERY PLAN +---------------------------------------------------------------------------------------------------------------------- + Append + -> Limit + Output: ft6.c1, ft6.c2 + -> Foreign Scan on public.ft6 + Output: ft6.c1, ft6.c2 + Remote SQL: SELECT c1, c2 FROM "S 1"."T 4" ORDER BY c2 ASC NULLS LAST + -> Foreign Scan on public.ft5 + Output: ft5.c1, ft5.c2 + Remote SQL: SELECT c1, c2 FROM "S 1"."T 4" ORDER BY c2 ASC NULLS LAST FETCH FIRST (2::bigint) ROWS WITH TIES +(9 rows) + +-- Keep WITH TIES local when all ORDER BY keys are redundant. ft2 uses +-- remote estimates, so invalid remote SQL would fail during planning. +EXPLAIN (VERBOSE, COSTS OFF) +SELECT c2, count(*) FROM ft2 GROUP BY c2 + ORDER BY (1+1) FETCH FIRST 2 ROWS WITH TIES; + QUERY PLAN +------------------------------------------------------------------------ + Limit + Output: c2, (count(*)), 2 + -> Foreign Scan + Output: c2, (count(*)), 2 + Relations: Aggregate on (public.ft2) + Remote SQL: SELECT c2, count(*), 2 FROM "S 1"."T 1" GROUP BY 1 +(6 rows) + +SELECT count(*) FROM ( + SELECT c2, count(*) FROM ft2 GROUP BY c2 + ORDER BY (1+1) FETCH FIRST 2 ROWS WITH TIES +) s; + count +------- + 10 +(1 row) + +-- A restriction can also make the ORDER BY key redundant. All four +-- matching groups tie, and OFFSET must still skip one of them. +EXPLAIN (VERBOSE, COSTS OFF) +SELECT c1, count(*) FROM ft2 WHERE c1 > 960 AND c2 = 1 GROUP BY c1, c2 + ORDER BY c2 OFFSET 1 FETCH FIRST 2 ROWS WITH TIES; + QUERY PLAN +-------------------------------------------------------------------------------------------------------------------------------- + Limit + Output: c1, (count(*)), c2 + -> GroupAggregate + Output: c1, count(*), c2 + Group Key: ft2.c1 + -> Foreign Scan on public.ft2 + Output: c1, c2 + Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" WHERE (("C 1" > 960)) AND ((c2 = 1)) ORDER BY "C 1" ASC NULLS LAST +(8 rows) + +SELECT count(*) FROM ( + SELECT c1, count(*) FROM ft2 WHERE c1 > 960 AND c2 = 1 GROUP BY c1, c2 + ORDER BY c2 OFFSET 1 FETCH FIRST 2 ROWS WITH TIES +) s; + count +------- + 3 +(1 row) + +-- EXPLAIN with local estimates does not require a user mapping. +CREATE SERVER no_mapping FOREIGN DATA WRAPPER postgres_fdw; +CREATE FOREIGN TABLE ft_no_mapping (a int) SERVER no_mapping; +EXPLAIN (VERBOSE, COSTS OFF) +SELECT a FROM ft_no_mapping ORDER BY a FETCH FIRST 2 ROWS WITH TIES; + QUERY PLAN +---------------------------------------------------------------------------------- + Limit + Output: a + -> Foreign Scan on public.ft_no_mapping + Output: a + Remote SQL: SELECT a FROM public.ft_no_mapping ORDER BY a ASC NULLS LAST +(5 rows) + +DROP FOREIGN TABLE ft_no_mapping; +DROP SERVER no_mapping; -- Test CASE pushdown EXPLAIN (VERBOSE, COSTS OFF) SELECT c1,c2,c3 FROM ft2 WHERE CASE WHEN c1 > 990 THEN c1 END < 1000 ORDER BY c1; diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index c731ee199e2..9069317a83c 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -8514,12 +8514,44 @@ add_foreign_final_paths(PlannerInfo *root, RelOptInfo *input_rel, * determined to be safe to push down before we get here. So in that case * the FETCH clause is safe to push down with ORDER BY if the remote * server is v13 or later, but if not, the remote query will fail entirely - * for lack of support for it. Since we do not currently have a way to do - * a remote-version check (without accessing the remote server), disable - * pushing the FETCH clause for now. + * for lack of support for it. Do not open a connection just to check the + * remote server's version. If a connection is already cached, perhaps + * from an earlier query or remote estimates during this planning, its + * version is available without additional network I/O. Push the FETCH + * clause down only when the cached version confirms support; otherwise + * keep it local. */ if (parse->limitOption == LIMIT_OPTION_WITH_TIES) - return; + { + Oid pushdown_userid; + UserMapping *user; + + /* + * All sort keys might have been removed as redundant. Without an + * ORDER BY clause in the remote query, WITH TIES is not valid. + */ + if (pathkeys == NIL) + return; + + /* + * 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; + + pushdown_userid = OidIsValid(final_rel->userid) ? + final_rel->userid : GetUserId(); + /* EXPLAIN without remote estimates need not have a user mapping. */ + user = GetUserMappingExtended(pushdown_userid, final_rel->serverid, + DEBUG1); + + if (user == NULL || GetCachedConnectionVersion(user) < 130000) + return; + } /* * Also, the LIMIT/OFFSET cannot be pushed down, if their expressions are diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h index da7da1c2ea9..c2d03266607 100644 --- a/contrib/postgres_fdw/postgres_fdw.h +++ b/contrib/postgres_fdw/postgres_fdw.h @@ -170,6 +170,7 @@ extern void process_pending_request(AsyncRequest *areq); extern PGconn *GetConnection(UserMapping *user, bool will_prep_stmt, PgFdwConnState **state); extern void ReleaseConnection(PGconn *conn); +extern int GetCachedConnectionVersion(UserMapping *user); extern unsigned int GetCursorNumber(PGconn *conn); extern unsigned int GetPrepStmtNumber(PGconn *conn); extern void do_sql_command(PGconn *conn, const char *sql); diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index eaeb90485e8..10835eaa4b0 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -442,11 +442,69 @@ EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE t1.c1 === t1.c2 order by t1.c2 limit 1; SELECT * FROM ft1 t1 WHERE t1.c1 === t1.c2 order by t1.c2 limit 1; --- Ensure we don't ship FETCH FIRST .. WITH TIES +-- Ensure we ship FETCH FIRST .. WITH TIES once the remote server's version +-- is known (i.e., a connection to it is already cached in this session, as +-- is the case here due to preceding tests) EXPLAIN (VERBOSE, COSTS OFF) SELECT t1.c2 FROM ft1 t1 WHERE t1.c1 > 960 ORDER BY t1.c2 FETCH FIRST 2 ROWS WITH TIES; SELECT t1.c2 FROM ft1 t1 WHERE t1.c1 > 960 ORDER BY t1.c2 FETCH FIRST 2 ROWS WITH TIES; +-- Same, but combined with OFFSET, emitted before FETCH FIRST in SQL-standard +-- order. Skipping into the middle of a tied group must not drop any of the +-- remaining ties. +EXPLAIN (VERBOSE, COSTS OFF) +SELECT t1.c2 FROM ft1 t1 WHERE t1.c1 > 960 ORDER BY t1.c2 OFFSET 1 FETCH FIRST 2 ROWS WITH TIES; +SELECT t1.c2 FROM ft1 t1 WHERE t1.c1 > 960 ORDER BY t1.c2 OFFSET 1 FETCH FIRST 2 ROWS WITH TIES; + +-- Ensure we never ship FETCH FIRST .. WITH TIES for a query whose result +-- combines rows from more than one foreign server (here, a join between +-- ft5 on "loopback" and ft6 on "loopback2"), regardless of whether either +-- server's version is known; there's no single remote query to push the +-- FETCH clause into, so it must stay local +EXPLAIN (VERBOSE, COSTS OFF) +SELECT ft5.c1, ft5.c2 FROM ft5 JOIN ft6 USING (c1) + ORDER BY ft5.c2 FETCH FIRST 2 ROWS WITH TIES; + +-- Two independently limited scans on different foreign servers, combined +-- locally via UNION ALL: each side's FETCH FIRST .. WITH TIES pushdown +-- decision is made independently based on its own server's cached +-- connection, with no coordination needed between them. ft5's server +-- (loopback) is already warmed up by many earlier tests, so that side +-- pushes the FETCH clause down; ft6's server (loopback2) has not been +-- connected to yet, so that side falls back to a local Limit. +EXPLAIN (VERBOSE, COSTS OFF) +(SELECT c1, c2 FROM ft6 ORDER BY c2 FETCH FIRST 2 ROWS WITH TIES) +UNION ALL +(SELECT c1, c2 FROM ft5 ORDER BY c2 FETCH FIRST 2 ROWS WITH TIES); + +-- Keep WITH TIES local when all ORDER BY keys are redundant. ft2 uses +-- remote estimates, so invalid remote SQL would fail during planning. +EXPLAIN (VERBOSE, COSTS OFF) +SELECT c2, count(*) FROM ft2 GROUP BY c2 + ORDER BY (1+1) FETCH FIRST 2 ROWS WITH TIES; +SELECT count(*) FROM ( + SELECT c2, count(*) FROM ft2 GROUP BY c2 + ORDER BY (1+1) FETCH FIRST 2 ROWS WITH TIES +) s; + +-- A restriction can also make the ORDER BY key redundant. All four +-- matching groups tie, and OFFSET must still skip one of them. +EXPLAIN (VERBOSE, COSTS OFF) +SELECT c1, count(*) FROM ft2 WHERE c1 > 960 AND c2 = 1 GROUP BY c1, c2 + ORDER BY c2 OFFSET 1 FETCH FIRST 2 ROWS WITH TIES; +SELECT count(*) FROM ( + SELECT c1, count(*) FROM ft2 WHERE c1 > 960 AND c2 = 1 GROUP BY c1, c2 + ORDER BY c2 OFFSET 1 FETCH FIRST 2 ROWS WITH TIES +) s; + +-- EXPLAIN with local estimates does not require a user mapping. +CREATE SERVER no_mapping FOREIGN DATA WRAPPER postgres_fdw; +CREATE FOREIGN TABLE ft_no_mapping (a int) SERVER no_mapping; +EXPLAIN (VERBOSE, COSTS OFF) +SELECT a FROM ft_no_mapping ORDER BY a FETCH FIRST 2 ROWS WITH TIES; +DROP FOREIGN TABLE ft_no_mapping; +DROP SERVER no_mapping; + -- Test CASE pushdown EXPLAIN (VERBOSE, COSTS OFF) SELECT c1,c2,c3 FROM ft2 WHERE CASE WHEN c1 > 990 THEN c1 END < 1000 ORDER BY c1; base-commit: 7beefa8d46978341aed7ba14cc82bf37c0564948 -- 2.43.0