8000 Revert misguided change to postgres_fdw FOR UPDATE/SHARE code. · rflynn/postgres@1a1fb46 · GitHub
[go: up one dir, main page]
Skip to content
10000

Commit 1a1fb46

Browse files
tglsfdctglsfdc
committed
Revert misguided change to postgres_fdw FOR UPDATE/SHARE code.
In commit 462bd95, I changed postgres_fdw to rely on get_plan_rowmark() instead of get_parse_rowmark(). I still think that's a good idea in the long run, but as Etsuro Fujita pointed out, it doesn't work today because planner.c forces PlanRowMarks to have markType = ROW_MARK_COPY for all foreign tables. There's no urgent reason to change this in the back branches, so let's just revert that part of yesterday's commit rather than trying to design a better solution under time pressure. Also, add a regression test case showing what postgres_fdw does with FOR UPDATE/SHARE. I'd blithely assumed there was one already, else I'd have realized yesterday that this code didn't work.
1 parent f2e2054 commit 1a1fb46

File tree

3 files changed

+45
-10
lines changed

3 files changed

+45
-10
lines changed

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,39 @@ SELECT * FROM ft1 t1 WHERE t1.c1 = 101 AND t1.c6 = '1' AND t1.c7 >= '1';
231231
101 | 1 | 00101 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo
232232
(1 row)
233233

234+
-- with FOR UPDATE/SHARE
235+
EXPLAIN (VERBOSE, COSTS false) SELECT * FROM ft1 t1 WHERE c1 = 101 FOR UPDATE;
236+
QUERY PLAN
237+
----------------------------------------------------------------------------------------------------------------
238+
LockRows
239+
Output: c1, c2, c3, c4, c5, c6, c7, c8, t1.*
240+
-> Foreign Scan on public.ft1 t1
241+
Output: c1, c2, c3, c4, c5, c6, c7, c8, t1.*
242+
Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE (("C 1" = 101)) FOR UPDATE
243+
(5 rows)
244+
245+
SELECT * FROM ft1 t1 WHERE c1 = 101 FOR UPDATE;
246+
c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
247+
-----+----+-------+------------------------------+--------------------------+----+------------+-----
248+
101 | 1 | 00101 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo
249+
(1 row)
250+
10000 251+
EXPLAIN (VERBOSE, COSTS false) SELECT * FROM ft1 t1 WHERE c1 = 102 FOR SHARE;
252+
QUERY PLAN
253+
---------------------------------------------------------------------------------------------------------------
254+
LockRows
255+
Output: c1, c2, c3, c4, c5, c6, c7, c8, t1.*
256+
-> Foreign Scan on public.ft1 t1
257+
Output: c1, c2, c3, c4, c5, c6, c7, c8, t1.*
258+
Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE (("C 1" = 102)) FOR SHARE
259+
(5 rows)
260+
261+
SELECT * FROM ft1 t1 WHERE c1 = 102 FOR SHARE;
262+
c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
263+
-----+----+-------+------------------------------+--------------------------+----+------------+-----
264+
102 | 2 | 00102 | Sat Jan 03 00:00:00 1970 PST | Sat Jan 03 00:00:00 1970 | 2 | 2 | foo
265+
(1 row)
266+
234267
-- aggregate
235268
SELECT COUNT(*) FROM ft1 t1;
236269
count

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,7 @@ postgresGetForeignPlan(PlannerInfo *root,
817817
}
818818
else
819819
{
820-
PlanRowMark *rc = get_plan_rowmark(root->rowMarks, baserel->relid);
820+
RowMarkClause *rc = get_parse_rowmark(root->parse, baserel->relid);
821821

822822
if (rc)
823823
{
@@ -830,18 +830,15 @@ postgresGetForeignPlan(PlannerInfo *root,
830830
* complete information about, and (b) it wouldn't work anyway on
831831
* older remote servers. Likewise, we don't worry about NOWAIT.
832832
*/
833-
switch (rc->markType)
833+
switch (rc->strength)
834834
{
835-
case ROW_MARK_EXCLUSIVE:
836-
case ROW_MARK_NOKEYEXCLUSIVE:
837-
appendStringInfoString(&sql, " FOR UPDATE");
838-
break;
839-
case ROW_MARK_SHARE:
840-
case ROW_MARK_KEYSHARE:
835+
case LCS_FORKEYSHARE:
836+
case LCS_FORSHARE:
841837
appendStringInfoString(&sql, " FOR SHARE");
842838
break;
843-
default:
844-
/* nothing needed */
839+
case LCS_FORNOKEYUPDATE:
840+
case LCS_FORUPDATE:
841+
appendStringInfoString(&sql, " FOR UPDATE");
845842
break;
846843
}
847844
}

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,11 @@ SELECT * FROM ft1 WHERE false;
145145
-- with WHERE clause
146146
EXPLAIN (VERBOSE, COSTS false) SELECT * FROM ft1 t1 WHERE t1.c1 = 101 AND t1.c6 = '1' AND t1.c7 >= '1';
147147
SELECT * FROM ft1 t1 WHERE t1.c1 = 101 AND t1.c6 = '1' AND t1.c7 >= '1';
148+
-- with FOR UPDATE/SHARE
149+
EXPLAIN (VERBOSE, COSTS false) SELECT * FROM ft1 t1 WHERE c1 = 101 FOR UPDATE;
150+
SELECT * FROM ft1 t1 WHERE c1 = 101 FOR UPDATE;
151+
EXPLAIN (VERBOSE, COSTS false) SELECT * FROM ft1 t1 WHERE c1 = 102 FOR SHARE;
152+
SELECT * FROM ft1 t1 WHERE c1 = 102 FOR SHARE;
148153
-- aggregate
149154
SELECT COUNT(*) FROM ft1 t1;
150155
-- join two tables

0 commit comments

Comments
 (0)
0