DELETE/UPDATE FOR PORTION OF with rule system is not working
Hi.
https://git.postgresql.org/cgit/postgresql.git/commit/?id=8e72d914c52876525a90b28444453de8085c866f
DELETE/UPDATE FOR PORTION OF with rule system is not working, and
there are no RULE related regession tests.
in gram.y, RuleStmt, RuleActionList, RuleActionStmt
RuleActionStmt:
SelectStmt
| InsertStmt
| UpdateStmt
| DeleteStmt
| NotifyStmt
;
So far, I found 2 errors, and one crash.
There might be more bugs, I didn't try all the cases.
drop table if exists fpo_rule;
create table fpo_rule (f1 bigint, f2 int4range);
INSERT INTO fpo_rule values (1, '[1, 10]');
CREATE RULE rule3 AS ON INSERT TO fpo_rule DO INSTEAD UPDATE fpo_rule
FOR PORTION OF f2 FROM 1 to 4 SET F1 = 2;
INSERT INTO fpo_rule values (2, '[2, 12]');
ERROR: range types do not match
\errverbose
ERROR: XX000: range types do not match
LOCATION: range_minus_multi, rangetypes.c:1260
CREATE RULE rule4 AS ON DELETE TO fpo_rule DO INSTEAD UPDATE fpo_rule
FOR PORTION OF f2 FROM 1 to 4 SET F1 = 2;
DELETE FROM fpo_rule;
ERROR: no relation entry for relid 3
\errverbose
ERROR: XX000: no relation entry for relid 3
LOCATION: find_base_rel, relnode.c:556
DROP RULE rule4 ON fpo_rule;
CREATE RULE rule5 AS ON UPDATE TO fpo_rule DO INSTEAD DELETE FROM
fpo_rule FOR PORTION OF f2 FROM 1 to 4;
UPDATE fpo_rule FOR PORTION OF f2 FROM 1 to 4 SET F1 = 2; -- server crash
As of now, we should try to ban CREATE ROLE with UPDATE/DELETE FOR PORTION OF.
On Mon, 13 Apr 2026 at 06:45, jian he <jian.universality@gmail.com> wrote:
Hi.
https://git.postgresql.org/cgit/postgresql.git/commit/?id=8e72d914c52876525a90b28444453de8085c866f
DELETE/UPDATE FOR PORTION OF with rule system is not working, and
there are no RULE related regession tests.
Thanks! +cc Paul & Peter as authors of [0]https://git.postgresql.org/cgit/postgresql.git/commit/?id=8e72d914c52876525a90b28444453de8085c866f
in gram.y, RuleStmt, RuleActionList, RuleActionStmt
RuleActionStmt:
SelectStmt
| InsertStmt
| UpdateStmt
| DeleteStmt
| NotifyStmt
;So far, I found 2 errors, and one crash.
There might be more bugs, I didn't try all the cases.drop table if exists fpo_rule;
create table fpo_rule (f1 bigint, f2 int4range);
INSERT INTO fpo_rule values (1, '[1, 10]');
CREATE RULE rule3 AS ON INSERT TO fpo_rule DO INSTEAD UPDATE fpo_rule
FOR PORTION OF f2 FROM 1 to 4 SET F1 = 2;INSERT INTO fpo_rule values (2, '[2, 12]');
ERROR: range types do not match
\errverbose
ERROR: XX000: range types do not match
LOCATION: range_minus_multi, rangetypes.c:1260CREATE RULE rule4 AS ON DELETE TO fpo_rule DO INSTEAD UPDATE fpo_rule
FOR PORTION OF f2 FROM 1 to 4 SET F1 = 2;
DELETE FROM fpo_rule;
ERROR: no relation entry for relid 3
\errverbose
ERROR: XX000: no relation entry for relid 3
LOCATION: find_base_rel, relnode.c:556DROP RULE rule4 ON fpo_rule;
CREATE RULE rule5 AS ON UPDATE TO fpo_rule DO INSTEAD DELETE FROM
fpo_rule FOR PORTION OF f2 FROM 1 to 4;
UPDATE fpo_rule FOR PORTION OF f2 FROM 1 to 4 SET F1 = 2; -- server crashAs of now, we should try to ban CREATE ROLE with UPDATE/DELETE FOR PORTION OF.
+1 for banning
[0]: https://git.postgresql.org/cgit/postgresql.git/commit/?id=8e72d914c52876525a90b28444453de8085c866f
--
Best regards,
Kirill Reshke
hi.
Actually it's supported.
The issue mentioned in the first email is caused by:
https://git.postgresql.org/cgit/postgresql.git/commit/?id=8e72d914c52876525a90b28444453de8085c866f
https://git.postgresql.org/cgit/postgresql.git/diff/src/backend/nodes/nodeFuncs.c?id=8e72d914c52876525a90b28444453de8085c866f
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -2579,6 +2579,20 @@ expression_tree_walker_impl(Node *node,
return true;
}
break;
+ case T_ForPortionOfExpr:
+ {
+ ForPortionOfExpr *forPortionOf = (ForPortionOfExpr *) node;
+
+ if (WALK(forPortionOf->targetFrom))
+ return true;
+ if (WALK(forPortionOf->targetTo))
+ return true;
+ if (WALK(forPortionOf->targetRange))
+ return true;
+ if (WALK(forPortionOf->overlapsExpr))
+ return true;
+ }
+ break;
We forgot to WALK (expression_tree_walker_impl)
ForPortionOfExpr->rangeVar and ForPortionOfExpr->rangeTargetList.
We need to WALK those two fields of ForPortionOfExpr in
rewriteRuleAction (ChangeVarNodes,
ReplaceVarsFromTargetList, etc.), and maybe elsewhere.
i am surprised that nothing else has broken because of this.
Attachments:
v1-0001-fix-DELETE-UPDATE-FOR-PORTION-OF-with-rule.patchtext/x-patch; charset=US-ASCII; name=v1-0001-fix-DELETE-UPDATE-FOR-PORTION-OF-with-rule.patchDownload+66-1
On Mon, 13 Apr 2026 at 14:01, jian he <jian.universality@gmail.com> wrote:
hi.
Actually it's supported.The issue mentioned in the first email is caused by:
https://git.postgresql.org/cgit/postgresql.git/commit/?id=8e72d914c52876525a90b28444453de8085c866f
https://git.postgresql.org/cgit/postgresql.git/diff/src/backend/nodes/nodeFuncs.c?id=8e72d914c52876525a90b28444453de8085c866f--- a/src/backend/nodes/nodeFuncs.c +++ b/src/backend/nodes/nodeFuncs.c @@ -2579,6 +2579,20 @@ expression_tree_walker_impl(Node *node, return true; } break; + case T_ForPortionOfExpr: + { + ForPortionOfExpr *forPortionOf = (ForPortionOfExpr *) node; + + if (WALK(forPortionOf->targetFrom)) + return true; + if (WALK(forPortionOf->targetTo)) + return true; + if (WALK(forPortionOf->targetRange)) + return true; + if (WALK(forPortionOf->overlapsExpr)) + return true; + } + break;We forgot to WALK (expression_tree_walker_impl)
ForPortionOfExpr->rangeVar and ForPortionOfExpr->rangeTargetList.
We need to WALK those two fields of ForPortionOfExpr in
rewriteRuleAction (ChangeVarNodes,
ReplaceVarsFromTargetList, etc.), and maybe elsewhere.i am surprised that nothing else has broken because of this.
This fix looks valid for me.
Also, are all 4 new test cases really needed? If yes, why are we
missing ON DELETE TO ... DO INSTEAD INSERT ?
--
Best regards,
Kirill Reshke
On Mon, 13 Apr 2026 at 20:10, Kirill Reshke <reshkekirill@gmail.com> wrote:
If yes, why are we
missing ON DELETE TO ... DO INSTEAD INSERT ?
Oh sorry... i mean ON UPDATE TO DO INSTEAD UPDATE FROM FOR PORTION OF
--
Best regards,
Kirill Reshke
On Mon, Apr 13, 2026 at 2:01 AM jian he <jian.universality@gmail.com> wrote:
hi.
Actually it's supported.
I also don't see any reason not to support this.
We forgot to WALK (expression_tree_walker_impl)
ForPortionOfExpr->rangeVar and ForPortionOfExpr->rangeTargetList.
We need to WALK those two fields of ForPortionOfExpr in
rewriteRuleAction (ChangeVarNodes,
ReplaceVarsFromTargetList, etc.), and maybe elsewhere.i am surprised that nothing else has broken because of this.
Thank you for investigating and sending a fix.
The patch looks fine to me. I like the diversity of tests; I don't
think we need any more.
+-- UPDATE/DELETE FOR PORTION OF with RULEs
+CREATE TABLE fpo_rule (f1 bigint, f2 int4range);
+INSERT INTO fpo_rule VALUES (1, '[1, 10]');
+
+CREATE RULE fpo_rule1 AS ON INSERT TO fpo_rule DO INSTEAD UPDATE
fpo_rule FOR PORTION OF f2 FROM 1 TO 4 SET f1 = 2;
+INSERT INTO fpo_rule VALUES (1, '[1, 10]');
+SELECT * FROM fpo_rule ORDER BY f1;
I only have two small suggestions:
Please use '[1, 11)' syntax to match the other tests.
Breaking these long lines would be nice. For example:
+CREATE RULE fpo_rule1 AS ON INSERT TO fpo_rule
+ DO INSTEAD UPDATE fpo_rule FOR PORTION OF f2 FROM 1 TO 4 SET f1 = 2;
Yours,
--
Paul ~{:-)
pj@illuminatedcomputing.com
On Thu, Apr 16, 2026 at 6:40 AM Paul A Jungwirth
<pj@illuminatedcomputing.com> wrote:
I only have two small suggestions:
Please use '[1, 11)' syntax to match the other tests.
Breaking these long lines would be nice. For example:
+CREATE RULE fpo_rule1 AS ON INSERT TO fpo_rule + DO INSTEAD UPDATE fpo_rule FOR PORTION OF f2 FROM 1 TO 4 SET f1 = 2;
Please check the attached v2.
V1 only has DO INSTEAD rules, adding one DO ALSO rule would make the
test coverage more robust.
Attachments:
v2-0001-fix-DELETE-UPDATE-FOR-PORTION-OF-with-rules.patchtext/x-patch; charset=US-ASCII; name=v2-0001-fix-DELETE-UPDATE-FOR-PORTION-OF-with-rules.patchDownload+92-1
Import Notes
Reply to msg id not found: CA+renyXkcs=5P3dd=OoyM+A=UKA7iR_zS4TAQ42F9NTv+2RNiA@mail.gmail.com
On Thu, Apr 16, 2026 at 8:20 PM jian he <jian.universality@gmail.com> wrote:
On Thu, Apr 16, 2026 at 6:40 AM Paul A Jungwirth
<pj@illuminatedcomputing.com> wrote:I only have two small suggestions:
Please use '[1, 11)' syntax to match the other tests.
Breaking these long lines would be nice. For example:
+CREATE RULE fpo_rule1 AS ON INSERT TO fpo_rule + DO INSTEAD UPDATE fpo_rule FOR PORTION OF f2 FROM 1 TO 4 SET f1 = 2;Please check the attached v2.
V1 only has DO INSTEAD rules, adding one DO ALSO rule would make the
test coverage more robust.
Thanks for those changes. This looks great to me!
Yours,
--
Paul ~{:-)
pj@illuminatedcomputing.com
On 19.04.26 20:07, Paul A Jungwirth wrote:
On Thu, Apr 16, 2026 at 8:20 PM jian he <jian.universality@gmail.com> wrote:
On Thu, Apr 16, 2026 at 6:40 AM Paul A Jungwirth
<pj@illuminatedcomputing.com> wrote:I only have two small suggestions:
Please use '[1, 11)' syntax to match the other tests.
Breaking these long lines would be nice. For example:
+CREATE RULE fpo_rule1 AS ON INSERT TO fpo_rule + DO INSTEAD UPDATE fpo_rule FOR PORTION OF f2 FROM 1 TO 4 SET f1 = 2;Please check the attached v2.
V1 only has DO INSTEAD rules, adding one DO ALSO rule would make the
test coverage more robust.Thanks for those changes. This looks great to me!
committed