Fix rules's command contains for-update
Hi hackers,
I run the following sql on the latest master branch (head commit:
6f0e19005) of Postgres:
```sql
gpadmin=# create table t(c int);
CREATE TABLE
gpadmin=# create rule myrule as on insert to t do instead select * from t
for update;
CREATE RULE
gpadmin=# insert into t values (1);
psql: ERROR: no relation entry for relid 1
```
It throws an error.
After some investigation, I found that:
1. in the function `transformRuleStmt`, it creates a new ParseState
`sub_pstate` to transform
actions. And in this `sub_pstate`, it is initially contains two
rangetblentry, "old" and "new".
2. in the function `transformSelectStmt`, it will invoke
`transformLockingClause` to handle `for update`. And it loops all the
entries in rtables.
I think for a CreateRuleStmt, its command part if is a select-for-update
statement, the for-update clause should skip the two "new", "old"
RangeTblEntry.
How to fix this:
1. forbid the syntax: rule's command cannot be a select-for-update
2. skip new and old: I have a patch to show this idea, please see the
attachment.
Any thoughts? Thanks!
Best Regards,
Zhenghua Lyu
Attachments:
0001-Fix-applying-rules-whose-command-contains-lockingCla.patchapplication/octet-stream; name=0001-Fix-applying-rules-whose-command-contains-lockingCla.patchDownload
From 551a4e43c1b2d2a6ad63afb4b20567370d715f15 Mon Sep 17 00:00:00 2001
From: Zhenghua Lyu <zlv@pivotal.io>
Date: Thu, 4 Apr 2019 16:10:10 +0800
Subject: [PATCH] Fix applying rules whose command contains lockingClause
---
src/backend/parser/analyze.c | 5 +++++
src/backend/parser/parse_utilcmd.c | 2 ++
src/include/parser/parse_node.h | 2 ++
3 files changed, 9 insertions(+)
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 400558b..63f37ec 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -2773,11 +2773,16 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc,
{
/* all regular tables used in query */
i = 0;
+
foreach(rt, qry->rtable)
{
RangeTblEntry *rte = (RangeTblEntry *) lfirst(rt);
++i;
+
+ if (pstate->p_contain_old_new_rte && i <= PRS2_NEW_VARNO)
+ continue;
+
switch (rte->rtekind)
{
case RTE_RELATION:
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 674f4b9..191b800 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -2778,6 +2778,8 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString,
addRTEtoQuery(sub_pstate, oldrte, false, true, false);
addRTEtoQuery(sub_pstate, newrte, false, true, false);
+ sub_pstate->p_contain_old_new_rte = true;
+
/* Transform the rule action statement */
top_subqry = transformStmt(sub_pstate,
(Node *) copyObject(action));
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index 3d8039a..9c0fbfa 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -206,6 +206,8 @@ struct ParseState
bool p_hasSubLinks;
bool p_hasModifyingCTE;
+ bool p_contain_old_new_rte;
+
Node *p_last_srf; /* most recent set-returning func/op found */
/*
--
2.7.4