Fix rewrite code so that rules are in fact executed in order by name,
rather than being reordered according to INSTEAD attribute for implementation convenience. Also, increase compiled-in recursion depth limit from 10 to 100 rewrite cycles. 10 seems pretty marginal for situations where multiple rules exist for the same query. There was a complaint about this recently, so I'm going to bump it up. (Perhaps we should make the limit a GUC parameter, but that's too close to being a new feature to do in beta.)
This commit is contained in:
Tom Lane
parent
757b98fda8
commit
c1f91a38e2
@ -1,4 +1,4 @@
|
||||
<!-- $Header: /cvsroot/pgsql/doc/src/sgml/rules.sgml,v 1.25 2002/10/14 22:14:34 tgl Exp $ -->
|
||||
<!-- $Header: /cvsroot/pgsql/doc/src/sgml/rules.sgml,v 1.26 2002/10/19 19:00:47 tgl Exp $ -->
|
||||
|
||||
<Chapter Id="rules">
|
||||
<Title>The Rule System</Title>
|
||||
@ -1043,8 +1043,8 @@ CREATE RULE rule_name AS ON event
|
||||
in more or less parse trees.
|
||||
So the parse trees in the rule actions must have either another command type
|
||||
or another result relation. Otherwise this recursive process will end up in a loop.
|
||||
There is a compiled in recursion limit of currently 10 iterations.
|
||||
If after 10 iterations there are still update rules to apply the
|
||||
There is a compiled-in recursion limit of currently 100 iterations.
|
||||
If after 100 iterations there are still update rules to apply the
|
||||
rule system assumes a loop over multiple rule definitions and reports
|
||||
an error.
|
||||
</Para>
|
||||
|
||||
@ -7,7 +7,7 @@
|
||||
* Portions Copyright (c) 1994, Regents of the University of California
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.111 2002/10/14 22:14:35 tgl Exp $
|
||||
* $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.112 2002/10/19 19:00:47 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
@ -503,7 +503,7 @@ matchLocks(CmdType event,
|
||||
int varno,
|
||||
Query *parsetree)
|
||||
{
|
||||
List *real_locks = NIL;
|
||||
List *matching_locks = NIL;
|
||||
int nlocks;
|
||||
int i;
|
||||
|
||||
@ -529,11 +529,11 @@ matchLocks(CmdType event,
|
||||
rangeTableEntry_used((Node *) parsetree, varno, 0) :
|
||||
attribute_used((Node *) parsetree,
|
||||
varno, oneLock->attrno, 0)))
|
||||
real_locks = lappend(real_locks, oneLock);
|
||||
matching_locks = lappend(matching_locks, oneLock);
|
||||
}
|
||||
}
|
||||
|
||||
return real_locks;
|
||||
return matching_locks;
|
||||
}
|
||||
|
||||
|
||||
@ -841,36 +841,6 @@ fireRIRrules(Query *parsetree)
|
||||
}
|
||||
|
||||
|
||||
/*
|
||||
* idea is to fire regular rules first, then qualified instead
|
||||
* rules and unqualified instead rules last. Any lemming is counted for.
|
||||
*/
|
||||
static List *
|
||||
orderRules(List *locks)
|
||||
{
|
||||
List *regular = NIL;
|
||||
List *instead_rules = NIL;
|
||||
List *instead_qualified = NIL;
|
||||
List *i;
|
||||
|
||||
foreach(i, locks)
|
||||
{
|
||||
RewriteRule *rule_lock = (RewriteRule *) lfirst(i);
|
||||
|
||||
if (rule_lock->isInstead)
|
||||
{
|
||||
if (rule_lock->qual == NULL)
|
||||
instead_rules = lappend(instead_rules, rule_lock);
|
||||
else
|
||||
instead_qualified = lappend(instead_qualified, rule_lock);
|
||||
}
|
||||
else
|
||||
regular = lappend(regular, rule_lock);
|
||||
}
|
||||
return nconc(nconc(regular, instead_qualified), instead_rules);
|
||||
}
|
||||
|
||||
|
||||
/*
|
||||
* Modify the given query by adding 'AND NOT rule_qual' to its qualification.
|
||||
* This is used to generate suitable "else clauses" for conditional INSTEAD
|
||||
@ -908,84 +878,89 @@ CopyAndAddQual(Query *parsetree,
|
||||
}
|
||||
|
||||
|
||||
|
||||
/*
|
||||
* fireRules -
|
||||
* Iterate through rule locks applying rules.
|
||||
* All rules create their own parsetrees. Instead rules
|
||||
* with rule qualification save the original parsetree
|
||||
* and add their negated qualification to it. Real instead
|
||||
* rules finally throw away the original parsetree.
|
||||
*
|
||||
* remember: reality is for dead birds -- glass
|
||||
* Input arguments:
|
||||
* parsetree - original query
|
||||
* rt_index - RT index of result relation in original query
|
||||
* event - type of rule event
|
||||
* locks - list of rules to fire
|
||||
* Output arguments:
|
||||
* *instead_flag - set TRUE if any unqualified INSTEAD rule is found
|
||||
* (must be initialized to FALSE)
|
||||
* *qual_product - filled with modified original query if any qualified
|
||||
* INSTEAD rule is found (must be initialized to NULL)
|
||||
* Return value:
|
||||
* list of rule actions adjusted for use with this query
|
||||
*
|
||||
* Qualified INSTEAD rules generate their action with the qualification
|
||||
* condition added. They also generate a modified version of the original
|
||||
* query with the negated qualification added, so that it will run only for
|
||||
* rows that the qualified action doesn't act on. (If there are multiple
|
||||
* qualified INSTEAD rules, we AND all the negated quals onto a single
|
||||
* modified original query.) We won't execute the original, unmodified
|
||||
* query if we find either qualified or unqualified INSTEAD rules. If
|
||||
* we find both, the modified original query is discarded too.
|
||||
*/
|
||||
static List *
|
||||
fireRules(Query *parsetree,
|
||||
int rt_index,
|
||||
CmdType event,
|
||||
bool *instead_flag,
|
||||
List *locks,
|
||||
List **qual_products)
|
||||
bool *instead_flag,
|
||||
Query **qual_product)
|
||||
{
|
||||
List *results = NIL;
|
||||
List *i;
|
||||
|
||||
/* choose rule to fire from list of rules */
|
||||
if (locks == NIL)
|
||||
return NIL;
|
||||
|
||||
locks = orderRules(locks); /* real instead rules last */
|
||||
|
||||
foreach(i, locks)
|
||||
{
|
||||
RewriteRule *rule_lock = (RewriteRule *) lfirst(i);
|
||||
Node *event_qual;
|
||||
List *actions;
|
||||
Node *event_qual = rule_lock->qual;
|
||||
List *actions = rule_lock->actions;
|
||||
QuerySource qsrc;
|
||||
List *r;
|
||||
|
||||
/* multiple rule action time */
|
||||
*instead_flag = rule_lock->isInstead;
|
||||
event_qual = rule_lock->qual;
|
||||
actions = rule_lock->actions;
|
||||
|
||||
/* Determine correct QuerySource value for actions */
|
||||
if (rule_lock->isInstead)
|
||||
{
|
||||
if (event_qual != NULL)
|
||||
qsrc = QSRC_QUAL_INSTEAD_RULE;
|
||||
else
|
||||
{
|
||||
qsrc = QSRC_INSTEAD_RULE;
|
||||
*instead_flag = true; /* report unqualified INSTEAD */
|
||||
}
|
||||
}
|
||||
else
|
||||
qsrc = QSRC_NON_INSTEAD_RULE;
|
||||
|
||||
if (qsrc == QSRC_QUAL_INSTEAD_RULE)
|
||||
{
|
||||
Query *qual_product;
|
||||
|
||||
/*
|
||||
* If there are instead rules with qualifications, the
|
||||
* If there are INSTEAD rules with qualifications, the
|
||||
* original query is still performed. But all the negated rule
|
||||
* qualifications of the instead rules are added so it does
|
||||
* qualifications of the INSTEAD rules are added so it does
|
||||
* its actions only in cases where the rule quals of all
|
||||
* instead rules are false. Think of it as the default action
|
||||
* in a case. We save this in *qual_products so
|
||||
* INSTEAD rules are false. Think of it as the default action
|
||||
* in a case. We save this in *qual_product so
|
||||
* deepRewriteQuery() can add it to the query list after we
|
||||
* mangled it up enough.
|
||||
*
|
||||
* If we have already found an unqualified INSTEAD rule,
|
||||
* then *qual_product won't be used, so don't bother building it.
|
||||
*/
|
||||
if (*qual_products == NIL)
|
||||
qual_product = parsetree;
|
||||
else
|
||||
qual_product = (Query *) lfirst(*qual_products);
|
||||
|
||||
qual_product = CopyAndAddQual(qual_product,
|
||||
event_qual,
|
||||
rt_index,
|
||||
event);
|
||||
|
||||
*qual_products = makeList1(qual_product);
|
||||
if (! *instead_flag)
|
||||
{
|
||||
if (*qual_product == NULL)
|
||||
*qual_product = parsetree;
|
||||
*qual_product = CopyAndAddQual(*qual_product,
|
||||
event_qual,
|
||||
rt_index,
|
||||
event);
|
||||
}
|
||||
}
|
||||
|
||||
/* Now process the rule's actions and add them to the result list */
|
||||
@ -1003,22 +978,20 @@ fireRules(Query *parsetree,
|
||||
|
||||
results = lappend(results, rule_action);
|
||||
}
|
||||
|
||||
/*
|
||||
* If this was an unqualified instead rule, throw away an
|
||||
* eventually saved 'default' parsetree
|
||||
*/
|
||||
if (qsrc == QSRC_INSTEAD_RULE)
|
||||
*qual_products = NIL;
|
||||
}
|
||||
|
||||
return results;
|
||||
}
|
||||
|
||||
|
||||
|
||||
/*
|
||||
* One pass of rewriting a single query.
|
||||
*
|
||||
* parsetree is the input query. Return value and output parameters
|
||||
* are defined the same as for fireRules, above.
|
||||
*/
|
||||
static List *
|
||||
RewriteQuery(Query *parsetree, bool *instead_flag, List **qual_products)
|
||||
RewriteQuery(Query *parsetree, bool *instead_flag, Query **qual_product)
|
||||
{
|
||||
CmdType event;
|
||||
List *product_queries = NIL;
|
||||
@ -1083,9 +1056,9 @@ RewriteQuery(Query *parsetree, bool *instead_flag, List **qual_products)
|
||||
product_queries = fireRules(parsetree,
|
||||
result_relation,
|
||||
event,
|
||||
instead_flag,
|
||||
locks,
|
||||
qual_products);
|
||||
instead_flag,
|
||||
qual_product);
|
||||
}
|
||||
|
||||
heap_close(rt_entry_relation, NoLock); /* keep lock! */
|
||||
@ -1099,7 +1072,7 @@ RewriteQuery(Query *parsetree, bool *instead_flag, List **qual_products)
|
||||
* can be rewritten. Detecting cycles is left for the reader as an exercise.
|
||||
*/
|
||||
#ifndef REWRITE_INVOKE_MAX
|
||||
#define REWRITE_INVOKE_MAX 10
|
||||
#define REWRITE_INVOKE_MAX 100
|
||||
#endif
|
||||
|
||||
static int numQueryRewriteInvoked = 0;
|
||||
@ -1111,20 +1084,17 @@ static int numQueryRewriteInvoked = 0;
|
||||
static List *
|
||||
deepRewriteQuery(Query *parsetree)
|
||||
{
|
||||
List *n;
|
||||
List *rewritten = NIL;
|
||||
List *result;
|
||||
bool instead;
|
||||
List *qual_products = NIL;
|
||||
bool instead = false;
|
||||
Query *qual_product = NULL;
|
||||
List *n;
|
||||
|
||||
if (++numQueryRewriteInvoked > REWRITE_INVOKE_MAX)
|
||||
{
|
||||
elog(ERROR, "query rewritten %d times, may contain cycles",
|
||||
numQueryRewriteInvoked - 1);
|
||||
}
|
||||
|
||||
instead = false;
|
||||
result = RewriteQuery(parsetree, &instead, &qual_products);
|
||||
result = RewriteQuery(parsetree, &instead, &qual_product);
|
||||
|
||||
foreach(n, result)
|
||||
{
|
||||
@ -1132,8 +1102,7 @@ deepRewriteQuery(Query *parsetree)
|
||||
List *newstuff;
|
||||
|
||||
newstuff = deepRewriteQuery(pt);
|
||||
if (newstuff != NIL)
|
||||
rewritten = nconc(rewritten, newstuff);
|
||||
rewritten = nconc(rewritten, newstuff);
|
||||
}
|
||||
|
||||
/*
|
||||
@ -1141,39 +1110,30 @@ deepRewriteQuery(Query *parsetree)
|
||||
* it is done last. This is needed because update and delete rule
|
||||
* actions might not do anything if they are invoked after the update
|
||||
* or delete is performed. The command counter increment between the
|
||||
* query execution makes the deleted (and maybe the updated) tuples
|
||||
* query executions makes the deleted (and maybe the updated) tuples
|
||||
* disappear so the scans for them in the rule actions cannot find
|
||||
* them.
|
||||
*
|
||||
* If we found any unqualified INSTEAD, the original query is not
|
||||
* done at all, in any form. Otherwise, we add the modified form
|
||||
* if qualified INSTEADs were found, else the unmodified form.
|
||||
*/
|
||||
if (parsetree->commandType == CMD_INSERT)
|
||||
if (!instead)
|
||||
{
|
||||
/*
|
||||
* qual_products are the original query with the negated rule
|
||||
* qualification of an INSTEAD rule
|
||||
*/
|
||||
if (qual_products != NIL)
|
||||
rewritten = nconc(qual_products, rewritten);
|
||||
|
||||
/*
|
||||
* Add the unmodified original query, if no INSTEAD rule was seen.
|
||||
*/
|
||||
if (!instead)
|
||||
rewritten = lcons(parsetree, rewritten);
|
||||
}
|
||||
else
|
||||
{
|
||||
/*
|
||||
* qual_products are the original query with the negated rule
|
||||
* qualification of an INSTEAD rule
|
||||
*/
|
||||
if (qual_products != NIL)
|
||||
rewritten = nconc(rewritten, qual_products);
|
||||
|
||||
/*
|
||||
* Add the unmodified original query, if no INSTEAD rule was seen.
|
||||
*/
|
||||
if (!instead)
|
||||
rewritten = lappend(rewritten, parsetree);
|
||||
if (parsetree->commandType == CMD_INSERT)
|
||||
{
|
||||
if (qual_product != NULL)
|
||||
rewritten = lcons(qual_product, rewritten);
|
||||
else
|
||||
rewritten = lcons(parsetree, rewritten);
|
||||
}
|
||||
else
|
||||
{
|
||||
if (qual_product != NULL)
|
||||
rewritten = lappend(rewritten, qual_product);
|
||||
else
|
||||
rewritten = lappend(rewritten, parsetree);
|
||||
}
|
||||
}
|
||||
|
||||
return rewritten;
|
||||
|
||||
@ -83,22 +83,25 @@ create rule rtest_t6_ins as on insert to rtest_t6
|
||||
--
|
||||
-- Tables and rules for the rule fire order test
|
||||
--
|
||||
-- As of PG 7.3, the rules should fire in order by name, regardless
|
||||
-- of INSTEAD attributes or creation order.
|
||||
--
|
||||
create table rtest_order1 (a int4);
|
||||
create table rtest_order2 (a int4, b int4, c text);
|
||||
create sequence rtest_seq;
|
||||
create rule rtest_order_r3 as on insert to rtest_order1 do instead
|
||||
insert into rtest_order2 values (new.a, nextval('rtest_seq'),
|
||||
'rule 3 - this should run 3rd or 4th');
|
||||
'rule 3 - this should run 3rd');
|
||||
create rule rtest_order_r4 as on insert to rtest_order1
|
||||
where a < 100 do instead
|
||||
insert into rtest_order2 values (new.a, nextval('rtest_seq'),
|
||||
'rule 4 - this should run 2nd');
|
||||
'rule 4 - this should run 4th');
|
||||
create rule rtest_order_r2 as on insert to rtest_order1 do
|
||||
insert into rtest_order2 values (new.a, nextval('rtest_seq'),
|
||||
'rule 2 - this should run 1st');
|
||||
'rule 2 - this should run 2nd');
|
||||
create rule rtest_order_r1 as on insert to rtest_order1 do instead
|
||||
insert into rtest_order2 values (new.a, nextval('rtest_seq'),
|
||||
'rule 1 - this should run 3rd or 4th');
|
||||
'rule 1 - this should run 1st');
|
||||
--
|
||||
-- Tables and rules for the instead nothing test
|
||||
--
|
||||
@ -644,12 +647,12 @@ select * from rtest_t8;
|
||||
--
|
||||
insert into rtest_order1 values (1);
|
||||
select * from rtest_order2;
|
||||
a | b | c
|
||||
---+---+-------------------------------------
|
||||
1 | 1 | rule 2 - this should run 1st
|
||||
1 | 2 | rule 4 - this should run 2nd
|
||||
1 | 3 | rule 1 - this should run 3rd or 4th
|
||||
1 | 4 | rule 3 - this should run 3rd or 4th
|
||||
a | b | c
|
||||
---+---+------------------------------
|
||||
1 | 1 | rule 1 - this should run 1st
|
||||
1 | 2 | rule 2 - this should run 2nd
|
||||
1 | 3 | rule 3 - this should run 3rd
|
||||
1 | 4 | rule 4 - this should run 4th
|
||||
(4 rows)
|
||||
|
||||
--
|
||||
@ -1321,10 +1324,10 @@ SELECT tablename, rulename, definition FROM pg_rules
|
||||
rtest_nothn1 | rtest_nothn_r2 | CREATE RULE rtest_nothn_r2 AS ON INSERT TO rtest_nothn1 WHERE ((new.a >= 30) AND (new.a < 40)) DO INSTEAD NOTHING;
|
||||
rtest_nothn2 | rtest_nothn_r3 | CREATE RULE rtest_nothn_r3 AS ON INSERT TO rtest_nothn2 WHERE (new.a >= 100) DO INSTEAD INSERT INTO rtest_nothn3 (a, b) VALUES (new.a, new.b);
|
||||
rtest_nothn2 | rtest_nothn_r4 | CREATE RULE rtest_nothn_r4 AS ON INSERT TO rtest_nothn2 DO INSTEAD NOTHING;
|
||||
rtest_order1 | rtest_order_r1 | CREATE RULE rtest_order_r1 AS ON INSERT TO rtest_order1 DO INSTEAD INSERT INTO rtest_order2 (a, b, c) VALUES (new.a, nextval('rtest_seq'::text), 'rule 1 - this should run 3rd or 4th'::text);
|
||||
rtest_order1 | rtest_order_r2 | CREATE RULE rtest_order_r2 AS ON INSERT TO rtest_order1 DO INSERT INTO rtest_order2 (a, b, c) VALUES (new.a, nextval('rtest_seq'::text), 'rule 2 - this should run 1st'::text);
|
||||
rtest_order1 | rtest_order_r3 | CREATE RULE rtest_order_r3 AS ON INSERT TO rtest_order1 DO INSTEAD INSERT INTO rtest_order2 (a, b, c) VALUES (new.a, nextval('rtest_seq'::text), 'rule 3 - this should run 3rd or 4th'::text);
|
||||
rtest_order1 | rtest_order_r4 | CREATE RULE rtest_order_r4 AS ON INSERT TO rtest_order1 WHERE (new.a < 100) DO INSTEAD INSERT INTO rtest_order2 (a, b, c) VALUES (new.a, nextval('rtest_seq'::text), 'rule 4 - this should run 2nd'::text);
|
||||
rtest_order1 | rtest_order_r1 | CREATE RULE rtest_order_r1 AS ON INSERT TO rtest_order1 DO INSTEAD INSERT INTO rtest_order2 (a, b, c) VALUES (new.a, nextval('rtest_seq'::text), 'rule 1 - this should run 1st'::text);
|
||||
rtest_order1 | rtest_order_r2 | CREATE RULE rtest_order_r2 AS ON INSERT TO rtest_order1 DO INSERT INTO rtest_order2 (a, b, c) VALUES (new.a, nextval('rtest_seq'::text), 'rule 2 - this should run 2nd'::text);
|
||||
rtest_order1 | rtest_order_r3 | CREATE RULE rtest_order_r3 AS ON INSERT TO rtest_order1 DO INSTEAD INSERT INTO rtest_order2 (a, b, c) VALUES (new.a, nextval('rtest_seq'::text), 'rule 3 - this should run 3rd'::text);
|
||||
rtest_order1 | rtest_order_r4 | CREATE RULE rtest_order_r4 AS ON INSERT TO rtest_order1 WHERE (new.a < 100) DO INSTEAD INSERT INTO rtest_order2 (a, b, c) VALUES (new.a, nextval('rtest_seq'::text), 'rule 4 - this should run 4th'::text);
|
||||
rtest_person | rtest_pers_del | CREATE RULE rtest_pers_del AS ON DELETE TO rtest_person DO DELETE FROM rtest_admin WHERE (rtest_admin.pname = old.pname);
|
||||
rtest_person | rtest_pers_upd | CREATE RULE rtest_pers_upd AS ON UPDATE TO rtest_person DO UPDATE rtest_admin SET pname = new.pname WHERE (rtest_admin.pname = old.pname);
|
||||
rtest_system | rtest_sys_del | CREATE RULE rtest_sys_del AS ON DELETE TO rtest_system DO (DELETE FROM rtest_interface WHERE (rtest_interface.sysname = old.sysname); DELETE FROM rtest_admin WHERE (rtest_admin.sysname = old.sysname); );
|
||||
|
||||
@ -100,6 +100,9 @@ create rule rtest_t6_ins as on insert to rtest_t6
|
||||
--
|
||||
-- Tables and rules for the rule fire order test
|
||||
--
|
||||
-- As of PG 7.3, the rules should fire in order by name, regardless
|
||||
-- of INSTEAD attributes or creation order.
|
||||
--
|
||||
create table rtest_order1 (a int4);
|
||||
create table rtest_order2 (a int4, b int4, c text);
|
||||
|
||||
@ -107,20 +110,20 @@ create sequence rtest_seq;
|
||||
|
||||
create rule rtest_order_r3 as on insert to rtest_order1 do instead
|
||||
insert into rtest_order2 values (new.a, nextval('rtest_seq'),
|
||||
'rule 3 - this should run 3rd or 4th');
|
||||
'rule 3 - this should run 3rd');
|
||||
|
||||
create rule rtest_order_r4 as on insert to rtest_order1
|
||||
where a < 100 do instead
|
||||
insert into rtest_order2 values (new.a, nextval('rtest_seq'),
|
||||
'rule 4 - this should run 2nd');
|
||||
'rule 4 - this should run 4th');
|
||||
|
||||
create rule rtest_order_r2 as on insert to rtest_order1 do
|
||||
insert into rtest_order2 values (new.a, nextval('rtest_seq'),
|
||||
'rule 2 - this should run 1st');
|
||||
'rule 2 - this should run 2nd');
|
||||
|
||||
create rule rtest_order_r1 as on insert to rtest_order1 do instead
|
||||
insert into rtest_order2 values (new.a, nextval('rtest_seq'),
|
||||
'rule 1 - this should run 3rd or 4th');
|
||||
'rule 1 - this should run 1st');
|
||||
|
||||
--
|
||||
-- Tables and rules for the instead nothing test
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user