aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2014-12-12 12:41:49 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2014-12-12 12:41:49 -0500
commit8ec8760fc87ecde0516e511f1c55aec627b01ea7 (patch)
tree5c7130c19b17f0b9e5f56f1d64969e961bdb6d54
parent237a8824430c607fce1eafde0c625744d50a455c (diff)
downloadpostgresql-8ec8760fc87ecde0516e511f1c55aec627b01ea7.tar.gz
postgresql-8ec8760fc87ecde0516e511f1c55aec627b01ea7.zip
Revert misguided change to postgres_fdw FOR UPDATE/SHARE code.
In commit 462bd95705a0c23ba0b0ba60a78d32566a0384c1, 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.
-rw-r--r--contrib/postgres_fdw/expected/postgres_fdw.out33
-rw-r--r--contrib/postgres_fdw/postgres_fdw.c17
-rw-r--r--contrib/postgres_fdw/sql/postgres_fdw.sql5
3 files changed, 45 insertions, 10 deletions
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index c7ba042cb7f..a335e56a9df 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -237,6 +237,39 @@ SELECT * FROM ft1 t1 WHERE t1.c1 = 101 AND t1.c6 = '1' AND t1.c7 >= '1';
101 | 1 | 00101 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo
(1 row)
+-- with FOR UPDATE/SHARE
+EXPLAIN (VERBOSE, COSTS false) SELECT * FROM ft1 t1 WHERE c1 = 101 FOR UPDATE;
+ QUERY PLAN
+----------------------------------------------------------------------------------------------------------------
+ LockRows
+ Output: c1, c2, c3, c4, c5, c6, c7, c8, t1.*
+ -> Foreign Scan on public.ft1 t1
+ Output: c1, c2, c3, c4, c5, c6, c7, c8, t1.*
+ Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE (("C 1" = 101)) FOR UPDATE
+(5 rows)
+
+SELECT * FROM ft1 t1 WHERE c1 = 101 FOR UPDATE;
+ c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
+-----+----+-------+------------------------------+--------------------------+----+------------+-----
+ 101 | 1 | 00101 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo
+(1 row)
+
+EXPLAIN (VERBOSE, COSTS false) SELECT * FROM ft1 t1 WHERE c1 = 102 FOR SHARE;
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------------------
+ LockRows
+ Output: c1, c2, c3, c4, c5, c6, c7, c8, t1.*
+ -> Foreign Scan on public.ft1 t1
+ Output: c1, c2, c3, c4, c5, c6, c7, c8, t1.*
+ Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE (("C 1" = 102)) FOR SHARE
+(5 rows)
+
+SELECT * FROM ft1 t1 WHERE c1 = 102 FOR SHARE;
+ c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
+-----+----+-------+------------------------------+--------------------------+----+------------+-----
+ 102 | 2 | 00102 | Sat Jan 03 00:00:00 1970 PST | Sat Jan 03 00:00:00 1970 | 2 | 2 | foo
+(1 row)
+
-- aggregate
SELECT COUNT(*) FROM ft1 t1;
count
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index f00fdbfb238..c3039a6480e 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -822,7 +822,7 @@ postgresGetForeignPlan(PlannerInfo *root,
}
else
{
- PlanRowMark *rc = get_plan_rowmark(root->rowMarks, baserel->relid);
+ RowMarkClause *rc = get_parse_rowmark(root->parse, baserel->relid);
if (rc)
{
@@ -835,18 +835,15 @@ postgresGetForeignPlan(PlannerInfo *root,
* complete information about, and (b) it wouldn't work anyway on
* older remote servers. Likewise, we don't worry about NOWAIT.
*/
- switch (rc->markType)
+ switch (rc->strength)
{
- case ROW_MARK_EXCLUSIVE:
- case ROW_MARK_NOKEYEXCLUSIVE:
- appendStringInfoString(&sql, " FOR UPDATE");
- break;
- case ROW_MARK_SHARE:
- case ROW_MARK_KEYSHARE:
+ case LCS_FORKEYSHARE:
+ case LCS_FORSHARE:
appendStringInfoString(&sql, " FOR SHARE");
break;
- default:
- /* nothing needed */
+ case LCS_FORNOKEYUPDATE:
+ case LCS_FORUPDATE:
+ appendStringInfoString(&sql, " FOR UPDATE");
break;
}
}
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 01491f886c1..fcfb10d29ea 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -151,6 +151,11 @@ SELECT * FROM ft1 WHERE false;
-- with WHERE clause
EXPLAIN (VERBOSE, COSTS false) SELECT * FROM ft1 t1 WHERE t1.c1 = 101 AND t1.c6 = '1' AND t1.c7 >= '1';
SELECT * FROM ft1 t1 WHERE t1.c1 = 101 AND t1.c6 = '1' AND t1.c7 >= '1';
+-- with FOR UPDATE/SHARE
+EXPLAIN (VERBOSE, COSTS false) SELECT * FROM ft1 t1 WHERE c1 = 101 FOR UPDATE;
+SELECT * FROM ft1 t1 WHERE c1 = 101 FOR UPDATE;
+EXPLAIN (VERBOSE, COSTS false) SELECT * FROM ft1 t1 WHERE c1 = 102 FOR SHARE;
+SELECT * FROM ft1 t1 WHERE c1 = 102 FOR SHARE;
-- aggregate
SELECT COUNT(*) FROM ft1 t1;
-- join two tables