Обсуждение: Conditional NOTIFY is not implemented
Hi,
Upgrading from 7.0.2 to 7.1.3 (great job, btw!), brought us
some problems. Watch the following:
| versuch=# create table chkntfy (
| versuch(# NUMMER integer default 0 not null,
| versuch(# TEXT varchar(255) default '?' not null
| versuch(# );
| CREATE
| versuch=#
| versuch=# CREATE RULE ru_u_chkntfy AS ON UPDATE TO chkntfy DO NOTIFY CHKNTFY;
| CREATE
| versuch=# insert into chkntfy (nummer,text) values ('1','eins');
| INSERT 110522 1
| versuch=# update chkntfy set nummer=10 where nummer = 1;
| ERROR: Conditional NOTIFY is not implemented
| versuch=#
Matthew Copeland noted the same in pgsql-admin and was told to use triggers
instead of rules. Well, it's a little nuisance, since we don't have statement
triggers (you'd get a notify for each row modified, which in our case can
be a killer.)
Somehow the notify seems to take up the `where' qualifier of the query
triggering the rule (you don't get the error message if you use an unqualified
query).
Is this considered a bug? Is a fix in sight?
Regards
-- tomas
tomas@fabula.de writes:
> versuch=# CREATE RULE ru_u_chkntfy AS ON UPDATE TO chkntfy DO NOTIFY CHKNTFY;
> CREATE
> versuch=# update chkntfy set nummer=10 where nummer = 1;
> ERROR: Conditional NOTIFY is not implemented
> Somehow the notify seems to take up the `where' qualifier of the query
> triggering the rule (you don't get the error message if you use an
> unqualified query).
> Is this considered a bug? Is a fix in sight?
Before 7.1, the system simply failed to account for the condition that
should be applied to the notify --- the notify ended up being fired all
the time, even if it shouldn't have been. In this case, the notify
should only occur if there are rows in chkntfy with nummer = 1 --- but
pre-7.1, it'd occur regardless. (We were rather fortunate to avoid a
crash, actually, since the code would attach a condition to a NOTIFY
querytree that should never have had one ... but then ignore it.)
Present behavior is to error out if the rewriter tries to attach a
nonempty condition to a NOTIFY query.
It'd be a simple code change to revert to the pre-7.1 behavior (notify
fires unconditionally), but actually making it work *correctly* is a
lot harder. NOTIFYs don't normally have any plan associated with them
at all, so there's no way to test a query condition.
Since we've seen several complaints about the new behavior, whereas
no one ever complained about excess NOTIFYs pre-7.1, perhaps the best
solution in the short run is to revert to the old behavior. We could
just document that NOTIFYs in rules are fired unconditionally.
Comments?
regards, tom lane
On Thu, Sep 06, 2001 at 03:16:26PM -0400, Tom Lane wrote: > tomas@fabula.de writes: > > versuch=# CREATE RULE ru_u_chkntfy AS ON UPDATE TO chkntfy DO NOTIFY CHKNTFY; > > CREATE > > versuch=# update chkntfy set nummer=10 where nummer = 1; > > ERROR: Conditional NOTIFY is not implemented > > > Somehow the notify seems to take up the `where' qualifier of the query > > triggering the rule (you don't get the error message if you use an > > unqualified query). > > > Is this considered a bug? Is a fix in sight? > > Before 7.1, the system simply failed to account for the condition that > should be applied to the notify --- the notify ended up being fired all > the time, even if it shouldn't have been. In this case, the notify > should only occur if there are rows in chkntfy with nummer = 1 --- but > pre-7.1, it'd occur regardless. (We were rather fortunate to avoid a > crash, actually, since the code would attach a condition to a NOTIFY > querytree that should never have had one ... but then ignore it.) > > Present behavior is to error out if the rewriter tries to attach a > nonempty condition to a NOTIFY query. > > It'd be a simple code change to revert to the pre-7.1 behavior (notify > fires unconditionally), but actually making it work *correctly* is a > lot harder. NOTIFYs don't normally have any plan associated with them > at all, so there's no way to test a query condition. > > Since we've seen several complaints about the new behavior, whereas > no one ever complained about excess NOTIFYs pre-7.1, perhaps the best > solution in the short run is to revert to the old behavior. We could > just document that NOTIFYs in rules are fired unconditionally. > > Comments? Thanks for the clear explanation. I see now. Well -- I think either behavior is a little strange, so I'd go for the one you think best for the moment and stick with it, putting on a big red warning sign ;-) My pattern of use for ``CREATE RULE... NOTIFY...'' was, up to now, to get a notice when anything changed on a table and then go look what happened; a `poor man's statement level trigger' if you wish. Thus, the old behavior didn't bother me that much. I don't know how others are using it. Come to think of it, the CREATE RULE ``triggers'' on statements anyway -- it looks at the parsed statement and is independent of the actual content of the database: so the old behaviour seems a bit more natural to me: ``Look: someone has called an UPDATE on this table: I don't know whether it is going to hit any records, but...'' the CREATE RULE acts then as a kind of `qualifier barrier' and therefore the NOTIFY doesn't see it. What do you think? Thanks again for your great work Cheers -- tomas
tomas@fabula.de writes:
> My pattern of use for ``CREATE RULE... NOTIFY...'' was, up to now, to get
> a notice when anything changed on a table and then go look what happened;
> a `poor man's statement level trigger' if you wish. Thus, the old behavior
> didn't bother me that much. I don't know how others are using it.
Yeah, that is the normal and recommended usage pattern for NOTIFY, so
getting a NOTIFY when nothing actually happened is fairly harmless.
(Undoubtedly that's why no one complained before.)
Changing the rewriter to error out when it couldn't really Do The Right
Thing seemed like a good idea at the time, but now it seems clear that
this didn't do anything to enhance the usefulness of the system. Unless
someone objects, I'll change it back for 7.2.
regards, tom lane
On Fri, Sep 07, 2001 at 12:30:44AM -0400, Tom Lane wrote: > tomas@fabula.de writes: > > My pattern of use for ``CREATE RULE... NOTIFY...'' was [...] > Yeah, that is the normal and recommended usage pattern for NOTIFY, so > getting a NOTIFY when nothing actually happened is fairly harmless. > (Undoubtedly that's why no one complained before.) > > Changing the rewriter to error out when it couldn't really Do The Right > Thing seemed like a good idea at the time, but now it seems clear that > this didn't do anything to enhance the usefulness of the system. Unless > someone objects, I'll change it back for 7.2. > > regards, tom lane I won't object ;-) Thank you again -- tomas
I said:
> Changing the rewriter to error out when it couldn't really Do The Right
> Thing seemed like a good idea at the time, but now it seems clear that
> this didn't do anything to enhance the usefulness of the system. Unless
> someone objects, I'll change it back for 7.2.
Not having seen any objections, I've committed the change. If you need
a fix in place before 7.2, it's really a trivial change: just replace
the elog calls (there are two) by "return". See attached patch against
current sources.
regards, tom lane
*** /home/postgres/pgsql/src/backend/rewrite/rewriteManip.c.orig Wed Apr 18 16:42:55 2001
--- /home/postgres/pgsql/src/backend/rewrite/rewriteManip.c Fri Sep 7 16:52:31 2001
***************
*** 592,606 ****
if (parsetree->commandType == CMD_UTILITY)
{
-
/*
! * Noplace to put the qual on a utility statement.
*
! * For now, we expect utility stmt to be a NOTIFY, so give a specific
! * error message for that case.
*/
if (parsetree->utilityStmt && IsA(parsetree->utilityStmt, NotifyStmt))
! elog(ERROR, "Conditional NOTIFY is not implemented");
else
elog(ERROR, "Conditional utility statements are not implemented");
}
--- 592,612 ----
if (parsetree->commandType == CMD_UTILITY)
{
/*
! * There's noplace to put the qual on a utility statement.
! *
! * If it's a NOTIFY, silently ignore the qual; this means that the
! * NOTIFY will execute, whether or not there are any qualifying rows.
! * While clearly wrong, this is much more useful than refusing to
! * execute the rule at all, and extra NOTIFY events are harmless for
! * typical uses of NOTIFY.
*
! * If it isn't a NOTIFY, error out, since unconditional execution
! * of other utility stmts is unlikely to be wanted. (This case is
! * not currently allowed anyway, but keep the test for safety.)
*/
if (parsetree->utilityStmt && IsA(parsetree->utilityStmt, NotifyStmt))
! return;
else
elog(ERROR, "Conditional utility statements are not implemented");
}
***************
*** 634,648 ****
if (parsetree->commandType == CMD_UTILITY)
{
-
/*
! * Noplace to put the qual on a utility statement.
*
! * For now, we expect utility stmt to be a NOTIFY, so give a specific
! * error message for that case.
*/
if (parsetree->utilityStmt && IsA(parsetree->utilityStmt, NotifyStmt))
! elog(ERROR, "Conditional NOTIFY is not implemented");
else
elog(ERROR, "Conditional utility statements are not implemented");
}
--- 640,652 ----
if (parsetree->commandType == CMD_UTILITY)
{
/*
! * There's noplace to put the qual on a utility statement.
*
! * See comments in AddQual for motivation.
*/
if (parsetree->utilityStmt && IsA(parsetree->utilityStmt, NotifyStmt))
! return;
else
elog(ERROR, "Conditional utility statements are not implemented");
}
On Fri, 7 Sep 2001 tomas@fabula.de wrote: > On Fri, Sep 07, 2001 at 12:30:44AM -0400, Tom Lane wrote: > > tomas@fabula.de writes: > > > My pattern of use for ``CREATE RULE... NOTIFY...'' was [...] > > > Yeah, that is the normal and recommended usage pattern for NOTIFY, so > > getting a NOTIFY when nothing actually happened is fairly harmless. > > (Undoubtedly that's why no one complained before.) > > > > Changing the rewriter to error out when it couldn't really Do The Right > > Thing seemed like a good idea at the time, but now it seems clear that > > this didn't do anything to enhance the usefulness of the system. Unless > > someone objects, I'll change it back for 7.2. > > > > regards, tom lane > > I won't object ;-) > > Thank you again > -- tomas I wouldn't really mind that either. My employer wouldn't allow me to change the current set of RULE/NOTIFY's to triggers so that we could upgrade to 7.1.3 from 7.0.3, so having it act the same way in 7.2 as it did in 7.0.3 would be nice for me. :) Matthew M. Copeland