Re: Different results between PostgreSQL and Oracle for "for update" statement - Mailing list pgsql-hackers
| From | Andy Fan |
|---|---|
| Subject | Re: Different results between PostgreSQL and Oracle for "for update" statement |
| Date | |
| Msg-id | CAKU4AWrN4HRgxN_VXw0Uxx0Wq6M37_yHaAweNFuKBKsoU=0CDg@mail.gmail.com Whole thread Raw |
| In response to | Re: Different results between PostgreSQL and Oracle for "for update" statement (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: Different results between PostgreSQL and Oracle for "for update" statement
|
| List | pgsql-hackers |
On Thu, Nov 19, 2020 at 11:49 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Andy Fan <zhihui.fan1213@gmail.com> writes:
> create table su (a int, b int);
> insert into su values(1, 1);
> - session 1:
> begin;
> update su set b = 2 where b = 1;
> - sess 2:
> select * from su where a in (select a from su where b = 1) for update;
This'd probably work the way you expect if there were "for update"
in the sub-select as well. As is, the sub-select will happily
return "1".
regards, tom lane
Thank you for your attention. Your suggestion would fix the issue. However
The difference will cause some risks when users move their application from Oracleto PostgreSQL. So I'd like to think which behavior is more reasonable.
In our current pg strategy, we would get
select * from su where a in (select a from su where b = 1) for update;
a | b
---+---
1 | 2
(1 row)
The data is obtained from 4 steps:
1. In the subquery, it gets su(a=1, b=1).
2. in the outer query, it is blocked.
3. session 1 is committed. sess 2 can continue.
4. outer query gets su(a=1, b=2).
By comparing the result in step 1 & step 4 in the same query, it
looks like we are using 2 different snapshots for the same query.
I think it is a bit strange.
If we think it is an issue, I think we can fix it with something like this:
+/*
+ * We should use the same RowMarkType for the RangeTblEntry
+ * if the underline relation is the same. Doesn't handle SubQuery for now.
+ */
+static RowMarkType
+select_rowmark_type_for_rangetable(RangeTblEntry *rte,
+ List *rowmarks,
+ LockClauseStrength strength)
+{
+ ListCell *lc;
+ foreach(lc, rowmarks)
+ {
+ PlanRowMark *rc = lfirst_node(PlanRowMark, lc);
+ if (rc->relid == rte->relid)
+ return rc->markType;
+ }
+ return select_rowmark_type(rte, strength);
+}
+
/*
* preprocess_rowmarks - set up PlanRowMarks if needed
*/
@@ -2722,6 +2743,7 @@ preprocess_rowmarks(PlannerInfo *root)
newrc->strength = rc->strength;
newrc->waitPolicy = rc->waitPolicy;
newrc->isParent = false;
+ newrc->relid = rte->relid;
prowmarks = lappend(prowmarks, newrc);
}
@@ -2742,18 +2764,18 @@ preprocess_rowmarks(PlannerInfo *root)
newrc = makeNode(PlanRowMark);
newrc->rti = newrc->prti = i;
newrc->rowmarkId = ++(root->glob->lastRowMarkId);
- newrc->markType = select_rowmark_type(rte, LCS_NONE);
+ newrc->markType = select_rowmark_type_for_rangetable(rte, prowmarks, LCS_NONE);
newrc->allMarkTypes = (1 << newrc->markType);
newrc->strength = LCS_NONE;
newrc->waitPolicy = LockWaitBlock; /* doesn't matter */
newrc->isParent = false;
-
+ newrc->relid = rte->relid;
prowmarks = lappend(prowmarks, newrc);
}
-
root->rowMarks = prowmarks;
}
+
/*
* Select RowMarkType to use for a given table
*/
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index ac5685da64..926086a69a 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -1103,6 +1103,7 @@ typedef struct PlanRowMark
LockClauseStrength strength; /* LockingClause's strength, or LCS_NONE */
LockWaitPolicy waitPolicy; /* NOWAIT and SKIP LOCKED options */
bool isParent; /* true if this is a "dummy" parent entry */
+ Oid relid; /* relation oid */
} PlanRowMark;
Do you think it is a bug and the above method is the right way to go?
--
If we think it is an issue, I think we can fix it with something like this:
+/*
+ * We should use the same RowMarkType for the RangeTblEntry
+ * if the underline relation is the same. Doesn't handle SubQuery for now.
+ */
+static RowMarkType
+select_rowmark_type_for_rangetable(RangeTblEntry *rte,
+ List *rowmarks,
+ LockClauseStrength strength)
+{
+ ListCell *lc;
+ foreach(lc, rowmarks)
+ {
+ PlanRowMark *rc = lfirst_node(PlanRowMark, lc);
+ if (rc->relid == rte->relid)
+ return rc->markType;
+ }
+ return select_rowmark_type(rte, strength);
+}
+
/*
* preprocess_rowmarks - set up PlanRowMarks if needed
*/
@@ -2722,6 +2743,7 @@ preprocess_rowmarks(PlannerInfo *root)
newrc->strength = rc->strength;
newrc->waitPolicy = rc->waitPolicy;
newrc->isParent = false;
+ newrc->relid = rte->relid;
prowmarks = lappend(prowmarks, newrc);
}
@@ -2742,18 +2764,18 @@ preprocess_rowmarks(PlannerInfo *root)
newrc = makeNode(PlanRowMark);
newrc->rti = newrc->prti = i;
newrc->rowmarkId = ++(root->glob->lastRowMarkId);
- newrc->markType = select_rowmark_type(rte, LCS_NONE);
+ newrc->markType = select_rowmark_type_for_rangetable(rte, prowmarks, LCS_NONE);
newrc->allMarkTypes = (1 << newrc->markType);
newrc->strength = LCS_NONE;
newrc->waitPolicy = LockWaitBlock; /* doesn't matter */
newrc->isParent = false;
-
+ newrc->relid = rte->relid;
prowmarks = lappend(prowmarks, newrc);
}
-
root->rowMarks = prowmarks;
}
+
/*
* Select RowMarkType to use for a given table
*/
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index ac5685da64..926086a69a 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -1103,6 +1103,7 @@ typedef struct PlanRowMark
LockClauseStrength strength; /* LockingClause's strength, or LCS_NONE */
LockWaitPolicy waitPolicy; /* NOWAIT and SKIP LOCKED options */
bool isParent; /* true if this is a "dummy" parent entry */
+ Oid relid; /* relation oid */
} PlanRowMark;
Do you think it is a bug and the above method is the right way to go?
Best Regards
Andy Fan
pgsql-hackers by date: