Обсуждение: trouble with rules
Hi! can somebody see this too? create table t1(i1 int4); create table t2(i1 int4); create table t3(i2 int4); test=> create rule rm_t1 as on delete to t1 test-> do ( delete from t2 where old.i1 = i1; test-> delete from t3 where old.i1 = i2;); pqReadData() -- backend closed the channel unexpectedly. This probably means the backend terminated abnormally beforeor while processing the request. We have lost the connection to the backend, so further processing is impossible. Terminating. OS = Linux 2.0.35, gcc 2.7.2.3, postgreSQL-6.4.2 Regards Erich
>
> Hi!
>
> can somebody see this too?
>
> create table t1(i1 int4);
> create table t2(i1 int4);
> create table t3(i2 int4);
>
> test=> create rule rm_t1 as on delete to t1
> test-> do ( delete from t2 where old.i1 = i1;
> test-> delete from t3 where old.i1 = i2;);
> pqReadData() -- backend closed the channel unexpectedly.
> This probably means the backend terminated abnormally before or
> while processing the request.
> We have lost the connection to the backend, so further processing is
> impossible. Terminating.
>
>
> OS = Linux 2.0.35, gcc 2.7.2.3, postgreSQL-6.4.2
That's courios. I can't reproduce it with v6.4 or v6.4.2
(Linux 2.1.88, gcc 2.7.2.1). Did the checks with the release
tarballs, not with the REL_6_4 tree (will check that later).
But with the current development tree I get a parse error
near delete!
I recall that there was something done in the parser about
parantheses around queries. Have to check it out and if fixed
add multiple action rules with parantheses to the regression
test to avoid breakage again in the future.
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #
Jan Wieck wrote: > > > > > Hi! > > > > can somebody see this too? > > > > create table t1(i1 int4); > > create table t2(i1 int4); > > create table t3(i2 int4); > > > > test=> create rule rm_t1 as on delete to t1 > > test-> do ( delete from t2 where old.i1 = i1; > > test-> delete from t3 where old.i1 = i2;); > > pqReadData() -- backend closed the channel unexpectedly. > > This probably means the backend terminated abnormally before or > > while processing the request. > > We have lost the connection to the backend, so further processing is > > impossible. Terminating. > > > > > > OS = Linux 2.0.35, gcc 2.7.2.3, postgreSQL-6.4.2 > > That's courios. I can't reproduce it with v6.4 or v6.4.2 > (Linux 2.1.88, gcc 2.7.2.1). Did the checks with the release > tarballs, not with the REL_6_4 tree (will check that later). CASSERT is off? Vadim
Vadim wrote:
>
> Jan Wieck wrote:
> >
> > >
> > > Hi!
> > >
> > > can somebody see this too?
> > >
> > > create table t1(i1 int4);
> > > create table t2(i1 int4);
> > > create table t3(i2 int4);
> > >
> > > test=> create rule rm_t1 as on delete to t1
> > > test-> do ( delete from t2 where old.i1 = i1;
> > > test-> delete from t3 where old.i1 = i2;);
> > > pqReadData() -- backend closed the channel unexpectedly.
> > > This probably means the backend terminated abnormally before or
> > > while processing the request.
> > > We have lost the connection to the backend, so further processing is
> > > impossible. Terminating.
> > >
> > >
> > > OS = Linux 2.0.35, gcc 2.7.2.3, postgreSQL-6.4.2
> >
> > That's courios. I can't reproduce it with v6.4 or v6.4.2
> > (Linux 2.1.88, gcc 2.7.2.1). Did the checks with the release
> > tarballs, not with the REL_6_4 tree (will check that later).
>
> CASSERT is off?
Yepp - thanks.
Fixed in REL6_4 and CURRENT.
Placed a patch in
ftp://hub.org/pub/patches/multi_action_rule.patch
Now have to look who damaged the parser in CURRENT not any
longer accepting parentheses for mutiple action rules.
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #
> > > > can somebody see this too?
> > > >
> > > > create table t1(i1 int4);
> > > > create table t2(i1 int4);
> > > > create table t3(i2 int4);
> > > >
> > > > test=> create rule rm_t1 as on delete to t1
> > > > test-> do ( delete from t2 where old.i1 = i1;
> > > > test-> delete from t3 where old.i1 = i2;);
> > > > pqReadData() -- backend closed the channel unexpectedly.
>
> Now have to look who damaged the parser in CURRENT not any
> longer accepting parentheses for mutiple action rules.
Has been commented out when INTERSECT came.
Fixed in CURRENT. I hate to but I have to comment on this:
Beeing able to put multiple actions for rules into
parentheses has been added and RELEASED with v6.4. And
this syntax is documented in the programmers manual of
v6.4.
It wasn't hard to reenable it. I just told gram.y that a
SelectStmt cannot occur in a multiple rule action block.
It looks to me, that it was taken out only to move
INTERSECT in the easy way. But this time the easy way is
IMHO the wrong way.
Removing a documented, released feature is something that
causes havy trouble for those who want to upgrade to a
new version.
Next time please keep existing syntax/features until
there is an agreement of the developers team that it has
to die.
BTW: There is 1 shift/reduce conflict in gram.y (was there
before I fixed multi action rules). Who introduced that?
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #
jwieck@debis.com (Jan Wieck) writes:
> It looks to me, that it was taken out only to move
> INTERSECT in the easy way. But this time the easy way is
> IMHO the wrong way.
> Removing a documented, released feature is something that
> causes havy trouble for those who want to upgrade to a
> new version.
> Next time please keep existing syntax/features until
> there is an agreement of the developers team that it has
> to die.
Calm down Jan ;-). I think what happened here is a slightly careless
merge of the 6.3 - based INTERSECT/EXPECT code into the current code.
Not a deliberate removal of a feature, just a foulup.
This does suggest that we need to be more careful when applying patches
developed against old system versions.
> BTW: There is 1 shift/reduce conflict in gram.y (was there
> before I fixed multi action rules). Who introduced that?
Yeah, I'm seeing that too. Same cause perhaps? It seems to have
appeared in rev 2.43, when the INTERSECT/EXPECT code was checked in.
regards, tom lane
Tom Lane wrote:
> jwieck@debis.com (Jan Wieck) writes:
> > It looks to me, that it was taken out only to move
> > INTERSECT in the easy way. But this time the easy way is
> > IMHO the wrong way.
> > Removing a documented, released feature is something that
> > causes havy trouble for those who want to upgrade to a
> > new version.
> > Next time please keep existing syntax/features until
> > there is an agreement of the developers team that it has
> > to die.
>
> Calm down Jan ;-). I think what happened here is a slightly careless
> merge of the 6.3 - based INTERSECT/EXPECT code into the current code.
> Not a deliberate removal of a feature, just a foulup.
Was my fault too. I should have added this new syntax to the
regression (as I did now). That way I would have noticed as
early as can that something disappeared.
>
> This does suggest that we need to be more careful when applying patches
> developed against old system versions.
This does suggest that we need to pay more attention that all
the nifty things we do are added to the regression suite.
Saying this I've just checked and the examples I've written
in the rule system section of the programmers manual cause
the backend to dump core.
Isn't if funny? All I'm telling could be used against me. :-)
>
> > BTW: There is 1 shift/reduce conflict in gram.y (was there
> > before I fixed multi action rules). Who introduced that?
>
> Yeah, I'm seeing that too. Same cause perhaps? It seems to have
> appeared in rev 2.43, when the INTERSECT/EXPECT code was checked in.
Hmmm - wasn't there some switch to bison that tells where it
shifts/reduces. I know most of the features of gdb, but bison
is a bit hairy for me.
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #
On Sun, 07 Feb 1999, you wrote: > Hmmm - wasn't there some switch to bison that tells where it > shifts/reduces. I know most of the features of gdb, but bison > is a bit hairy for me. bison -v will spit out a *huge* data file describing the parser. Somewhere in there it will tell you where the shift/reduce conflict is occurring. Taral
>
> On Sun, 07 Feb 1999, you wrote:
> > Hmmm - wasn't there some switch to bison that tells where it
> > shifts/reduces. I know most of the features of gdb, but bison
> > is a bit hairy for me.
>
> bison -v will spit out a *huge* data file describing the parser. Somewhere in
> there it will tell you where the shift/reduce conflict is occurring.
>
> Taral
Thanks Taral, and bingo - Tom's guess about that it came with
INTERSECT seems right.
[...]
State 269 contains 1 shift/reduce conflict.
[...]
state 269
SelectStmt -> select_w_o_sort sort_clause . for_update...
I'm currently committing the turn backs into rewriteManip.c
and the additional rule system tests.
INTERSECT IS BROKEN NOW!!! The one who's responsible for that
may contact me to help fixing it by doing the comparisions
that rely on memory addresses of Var nodes correctly
according to the requirements of the rule system. I don't
know enough about how INTERSECT/EXCEPT is expected to work.
And the regression test, which is passing here now completely
(only the 4 missing NOTICE in misc) seems not cover it.
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #
> jwieck@debis.com (Jan Wieck) writes: > > It looks to me, that it was taken out only to move > > INTERSECT in the easy way. But this time the easy way is > > IMHO the wrong way. > > Removing a documented, released feature is something that > > causes havy trouble for those who want to upgrade to a > > new version. > > Next time please keep existing syntax/features until > > there is an agreement of the developers team that it has > > to die. > > Calm down Jan ;-). I think what happened here is a slightly careless > merge of the 6.3 - based INTERSECT/EXPECT code into the current code. > Not a deliberate removal of a feature, just a foulup. > > This does suggest that we need to be more careful when applying patches > developed against old system versions. This is normally caused by Stephan's patches. His patches were originally against 6.3, and he ported them to 6.4, but he normally does lots of development without any communication with us, sends us a huge patch, and we normally have to clean up the edges somewhat. This patch actually caused fewer problems than the HAVING patch he submitted. In fact, I didn't even know he was working anymore, and then I recieve this huge patch, with a huge thesis that Thomas is merging into the docs. He is in the Army now, and probably unreachable. Basically, I think our hands are tied on this one. Not really sure we could have done anything different. The patch is usually beefy enough that is worth our effort to polish it. To his credit, he did more regression testing this time, so we have fewer problems. All his stuff has /***S*I***/ next to it, which I will remove once we resolve issues with his patch. > > BTW: There is 1 shift/reduce conflict in gram.y (was there > > before I fixed multi action rules). Who introduced that? > > Yeah, I'm seeing that too. Same cause perhaps? It seems to have > appeared in rev 2.43, when the INTERSECT/EXPECT code was checked in. Same cause. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian wrote:
> This is normally caused by Stephan's patches. His patches were
> originally against 6.3, and he ported them to 6.4, but he normally does
> lots of development without any communication with us, sends us a huge
> patch, and we normally have to clean up the edges somewhat. This patch
> actually caused fewer problems than the HAVING patch he submitted.
Porting his patch to v6.4 was not exactly what he did. He
changed the v6.5 tree in a way that his patch fit's into.
Namely he removed copyObject() in some cases. copyObject() is
an expensive function. There are only 2 reasons to call it.
One is that the object in question lives in a memory context
that could get destroyed before we are done with the object.
The other is that we are about to change the object but
others need it unchanged (in the actual case varno's got
changed).
And he reverted the possibility to group multiple rule
actions in ()'s.
One good thing it caused is, that I realize I was wrong!
LIMIT seems to never have been applied to the tree - OOOPS. I
don't know how this could have happened. Must do it before
v6.5 BETA because it's FEATURE.
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #
> Bruce Momjian wrote: > > > This is normally caused by Stephan's patches. His patches were > > originally against 6.3, and he ported them to 6.4, but he normally does > > lots of development without any communication with us, sends us a huge > > patch, and we normally have to clean up the edges somewhat. This patch > > actually caused fewer problems than the HAVING patch he submitted. > > Porting his patch to v6.4 was not exactly what he did. He > changed the v6.5 tree in a way that his patch fit's into. Yes, I understand the frustration. I had that with HAVING. I symathize. It also bothers me when things are added that are disruptive to other code, and I see he did that. Should we remove his patch? I don't know. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <maillist@candle.pha.pa.us> writes:
> Should we remove his patch? I don't know.
That seems like an overreaction. We need to look at it more carefully,
but it's a feature we want no? (I assume INTERSECT/EXCEPT are SQL92
items, not something Stefan invented out of whole cloth.) Seems like
it mostly works and we just need to find the places where it conflicts
with other post-6.3 changes.
regards, tom lane
>
> > Bruce Momjian wrote:
> >
> > > This is normally caused by Stephan's patches. His patches were
> > > originally against 6.3, and he ported them to 6.4, but he normally does
> > > lots of development without any communication with us, sends us a huge
> > > patch, and we normally have to clean up the edges somewhat. This patch
> > > actually caused fewer problems than the HAVING patch he submitted.
>
> >
> > Porting his patch to v6.4 was not exactly what he did. He
> > changed the v6.5 tree in a way that his patch fit's into.
>
> Yes, I understand the frustration. I had that with HAVING. I
> symathize. It also bothers me when things are added that are disruptive
> to other code, and I see he did that.
>
> Should we remove his patch? I don't know.
I don't think it's possible to remove it easily. Too many
things have been done after.
I've only noticed while browsing through the code why he did
comment out those things. He's comparing memoy addresses of
nodes, what doesn't work any more after copyObject(). If he's
not available right now, we must fix that part. I can help on
that, but someone else must tell what queries should produce
which expected output.
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #
jwieck@debis.com (Jan Wieck) writes:
> I've only noticed while browsing through the code why he did
> comment out those things. He's comparing memoy addresses of
> nodes, what doesn't work any more after copyObject(). If he's
> not available right now, we must fix that part.
Is there more to do than using equal() instead of a plain pointer
compare?
There might be --- for example the collapsing-UNION problem I mentioned
yesterday is a case where using equal() allows an overly aggressive
optimization. Where are these comparisons and what are they for?
regards, tom lane
>
> jwieck@debis.com (Jan Wieck) writes:
> > I've only noticed while browsing through the code why he did
> > comment out those things. He's comparing memoy addresses of
> > nodes, what doesn't work any more after copyObject(). If he's
> > not available right now, we must fix that part.
>
> Is there more to do than using equal() instead of a plain pointer
> compare?
>
> There might be --- for example the collapsing-UNION problem I mentioned
> yesterday is a case where using equal() allows an overly aggressive
> optimization. Where are these comparisons and what are they for?
rewriteHandler.c 1691 and 2908... and rewriteManip.c 175, 403
and 1068. Now that I've looked closer I see that it are
assignments. All of them have to do with sublinks and
lefttree-aggregate issues. Shouldn't be too hard to figure
out what's right and it will give us some additional queries
for the rule system checks.
So can someone please tell me how INTERSECT/EXCEPT works?
I'll deuglify the code while working on it then :-}. It's
really hard to read (must have been written in a 120 char
wide window or so).
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #
> Bruce Momjian <maillist@candle.pha.pa.us> writes: > > Should we remove his patch? I don't know. > > That seems like an overreaction. We need to look at it more carefully, > but it's a feature we want no? (I assume INTERSECT/EXCEPT are SQL92 > items, not something Stefan invented out of whole cloth.) Seems like > it mostly works and we just need to find the places where it conflicts > with other post-6.3 changes. I agree. I was just offering it as an option. Hate to make people clean up other people's mess. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
On Sun, Feb 07, 1999 at 08:41:10PM +0100, Jan Wieck wrote: > BTW: There is 1 shift/reduce conflict in gram.y (was there > before I fixed multi action rules). Who introduced that? Don't know but it was introduced with the EXCEPT and INTERSECT features. I haven't found the time yet to check where it comes from. But the same holds for ecpg since I synced both yacc files. Michael -- Michael Meskes | Go SF 49ers! Th.-Heuss-Str. 61, D-41812 Erkelenz | Go Rhein Fire! Tel.: (+49) 2431/72651 | Use Debian GNU/Linux! Email: Michael.Meskes@gmx.net | Use PostgreSQL!
> > > > jwieck@debis.com (Jan Wieck) writes: > > > I've only noticed while browsing through the code why he did > > > comment out those things. He's comparing memoy addresses of > > > nodes, what doesn't work any more after copyObject(). If he's > > > not available right now, we must fix that part. > > > > Is there more to do than using equal() instead of a plain pointer > > compare? > > > > There might be --- for example the collapsing-UNION problem I mentioned > > yesterday is a case where using equal() allows an overly aggressive > > optimization. Where are these comparisons and what are they for? > > rewriteHandler.c 1691 and 2908... and rewriteManip.c 175, 403 > and 1068. Now that I've looked closer I see that it are > assignments. All of them have to do with sublinks and > lefttree-aggregate issues. Shouldn't be too hard to figure > out what's right and it will give us some additional queries > for the rule system checks. > > So can someone please tell me how INTERSECT/EXCEPT works? Are the regression tests he supplied installed yet. Should be samples in there. > > I'll deuglify the code while working on it then :-}. It's > really hard to read (must have been written in a 120 char > wide window or so). Yes. Just run pgindent on any files you want, or I will do it if you tell me where. I ran it on some optimizer file. I can easily do a whole directory if no one else is working in there. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian wrote:
> > So can someone please tell me how INTERSECT/EXCEPT works?
>
> Are the regression tests he supplied installed yet. Should be samples
> in there.
Does not seem so, and if, they don't touch anything I
reverted.
>
> >
> > I'll deuglify the code while working on it then :-}. It's
> > really hard to read (must have been written in a 120 char
> > wide window or so).
>
>
> Yes. Just run pgindent on any files you want, or I will do it if you
> tell me where. I ran it on some optimizer file. I can easily do a
> whole directory if no one else is working in there.
I'm actually hacking on the TIME QUAL issue. Maybe it turns
out that my thought's are complete, and if Vadim agrees then
I would like to move it in. I'm sure I have to touch many
directories (parser, rewrite, executor, utils/time I have so
far). So please don't for now.
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #
Jan Wieck wrote: > > > Yes. Just run pgindent on any files you want, or I will do it if you > > tell me where. I ran it on some optimizer file. I can easily do a > > whole directory if no one else is working in there. > > I'm actually hacking on the TIME QUAL issue. Maybe it turns What's the issue you're talking about ? > out that my thought's are complete, and if Vadim agrees then > I would like to move it in. I'm sure I have to touch many > directories (parser, rewrite, executor, utils/time I have so > far). So please don't for now. pgindent should be run just before beta, not now, please. Vadim
I wrote:
>
> Tom Lane wrote:
>
> > Calm down Jan ;-). I think what happened here is a slightly careless
> > merge of the 6.3 - based INTERSECT/EXPECT code into the current code.
> > Not a deliberate removal of a feature, just a foulup.
>
> Was my fault too. I should have added this new syntax to the
> regression (as I did now). That way I would have noticed as
> early as can that something disappeared.
>
> >
> > This does suggest that we need to be more careful when applying patches
> > developed against old system versions.
>
> This does suggest that we need to pay more attention that all
> the nifty things we do are added to the regression suite.
>
> Saying this I've just checked and the examples I've written
> in the rule system section of the programmers manual cause
> the backend to dump core.
>
> Isn't if funny? All I'm telling could be used against me. :-)
No, it isn't fun any more and I'm getting angry now >:-(
I've checked it and it turns out, that due to the changes
that came in with INTERSECT/EXPECT many expressions aren't
any longer copied when they are added from one parsetree to
another. Thus, multiple parsetrees reference the same Var
nodes and if multiple rules get applied during the rule
system recursion (rewritten trees get rewritten again), later
rules mangle up the ones referenced in trees the rule system
is already done with.
I've spent night's to fix this all for v6.4. Added many
copyObject()'s around things that MUST be copied. Now I find
them commented out, because it was easier to apply v6.3 based
development onto the v6.5 sources.
I'll now revert things to do the copyObject() again where it
has to be done and will add the examples from the programmers
manual to the regression tests.
Surely this will break the INTERSECT/EXPECT code, because it
depends on nodes beeing at specific memory locations for
comparisions. But this is impossible due to the requirements
of the rule system.
Sorry for that.
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #
> No, it isn't fun any more and I'm getting angry now >:-( > > I've checked it and it turns out, that due to the changes > that came in with INTERSECT/EXPECT many expressions aren't > any longer copied when they are added from one parsetree to > another. Thus, multiple parsetrees reference the same Var > nodes and if multiple rules get applied during the rule > system recursion (rewritten trees get rewritten again), later > rules mangle up the ones referenced in trees the rule system > is already done with. > > I've spent night's to fix this all for v6.4. Added many > copyObject()'s around things that MUST be copied. Now I find > them commented out, because it was easier to apply v6.3 based > development onto the v6.5 sources. > > I'll now revert things to do the copyObject() again where it > has to be done and will add the examples from the programmers > manual to the regression tests. > > Surely this will break the INTERSECT/EXPECT code, because it > depends on nodes beeing at specific memory locations for > comparisions. But this is impossible due to the requirements > of the rule system. That is bad. He wants the Var nodes to have the same address. Why can't he use equal() like everyone else? Do you want to review his patch, and reverse anything that looks wrong in it. This may be the only way to make sure things are OK. The patch isn't that long. I had the same frustrations with HAVING six months ago. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
> That is bad. He wants the Var nodes to have the same address. Why
> can't he use equal() like everyone else? Do you want to review his
> patch, and reverse anything that looks wrong in it. This may be the
> only way to make sure things are OK. The patch isn't that long.
But we should be clear: imho INTERSECT/EXCEPT can be dumped from v6.5 if
you find it is too ugly to make work, or if you find it trashed too much
other stuff. We can re-enable it later after working out the kinks...
- Tom