DELETE/UPDATE FOR PORTION OF with rule system is not working

Started by jian he18 days ago9 messageshackers
Jump to latest
#1jian he
jian.universality@gmail.com

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.

--
jian
https://www.enterprisedb.com/

#2Kirill Reshke
reshkekirill@gmail.com
In reply to: jian he (#1)
Re: DELETE/UPDATE FOR PORTION OF with rule system is not working

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: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.

+1 for banning

[0]: https://git.postgresql.org/cgit/postgresql.git/commit/?id=8e72d914c52876525a90b28444453de8085c866f

--
Best regards,
Kirill Reshke

#3jian he
jian.universality@gmail.com
In reply to: Kirill Reshke (#2)
Re: DELETE/UPDATE FOR PORTION OF with rule system is not working

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.

--
jian
https://www.enterprisedb.com/

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
#4Kirill Reshke
reshkekirill@gmail.com
In reply to: jian he (#3)
Re: DELETE/UPDATE FOR PORTION OF with rule system is not working

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.

--
jian
https://www.enterprisedb.com/

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

#5Kirill Reshke
reshkekirill@gmail.com
In reply to: Kirill Reshke (#4)
Re: DELETE/UPDATE FOR PORTION OF with rule system is not working

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

#6Paul Jungwirth
pj@illuminatedcomputing.com
In reply to: jian he (#3)
Re: DELETE/UPDATE FOR PORTION OF with rule system is not working

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

#7jian he
jian.universality@gmail.com
In reply to: jian he (#1)
Re: DELETE/UPDATE FOR PORTION OF with rule system is not working

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.

--
jian
https://www.enterprisedb.com/

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
#8Paul Jungwirth
pj@illuminatedcomputing.com
In reply to: jian he (#7)
Re: DELETE/UPDATE FOR PORTION OF with rule system is not working

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

#9Peter Eisentraut
peter_e@gmx.net
In reply to: Paul Jungwirth (#8)
Re: DELETE/UPDATE FOR PORTION OF with rule system is not working

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