Thread: Major breakage?
Is anyone else seeing major breakage of the regression tests with
today's (Monday's) CVS checkins? Or did I break something myself?
I'm seeing wrong answers in tests numerology and select_having;
plus coredumps in opr_sanity, subselect and rules. Also the same
unexpected messages in union and misc as were there a few days ago.
I've been making what I thought were perfectly safe changes, so
I was surprised when things blew up in my face just before I was
ready to check in. Noting the scope of what other people committed
today, I'd like to believe it's someone else's fault...
regards, tom lane
Tom Lane wrote: > > Is anyone else seeing major breakage of the regression tests with > today's (Monday's) CVS checkins? Or did I break something myself? > > I'm seeing wrong answers in tests numerology and select_having; > plus coredumps in opr_sanity, subselect and rules. Also the same > unexpected messages in union and misc as were there a few days ago. > > I've been making what I thought were perfectly safe changes, so > I was surprised when things blew up in my face just before I was > ready to check in. Noting the scope of what other people committed > today, I'd like to believe it's someone else's fault... Try gmake clean + initdb. At least RULES were affected by my changes... I forgot to say, sorry. Vadim
I wrote:
>> Is anyone else seeing major breakage of the regression tests with
>> today's (Monday's) CVS checkins? Or did I break something myself?
Nope, Vadim broke something. It looks like anything with a subplan
will coredump in Monday's sources. executor/nodeSubPlan.c has
bool
ExecInitSubPlan(SubPlan *node, EState *estate, Plan *parent)
{ ... ExecCheckPerms(CMD_SELECT, 0, node->rtable, (Query *) NULL);
^^^^^^^^^^^^^^
(and has had that for a long time, evidently). One of the additions
Vadim checked in yesterday extends ExecCheckPerms() to try to use
its parseTree argument --- unconditionally. Guaranteed null-pointer
dereference.
Perhaps ExecInitSubPlan is in error to pass a null parseTree; if not,
then ExecCheckPerms needs to be modified to cope. I don't understand
either routine enough to fix it correctly.
This bug is the cause of the opr_sanity coredump I'm seeing.
I don't have time to investigate the other test failures right now,
but very possibly they are the same thing.
BTW, anyone who is *not* seeing regression test coredumps with the
current CVS sources must have their compile/link options set so that
dereferencing a null pointer isn't fatal. I think that's a very bad
choice for software development --- you want to hear about it, loud
and clear, if your code tries to use a null pointer.
regards, tom lane
> I wrote:
> >> Is anyone else seeing major breakage of the regression tests with
> >> today's (Monday's) CVS checkins? Or did I break something myself?
>
> Nope, Vadim broke something. It looks like anything with a subplan
> will coredump in Monday's sources. executor/nodeSubPlan.c has
>
> bool
> ExecInitSubPlan(SubPlan *node, EState *estate, Plan *parent)
> {
> ...
> ExecCheckPerms(CMD_SELECT, 0, node->rtable, (Query *) NULL);
> ^^^^^^^^^^^^^^
>
> (and has had that for a long time, evidently). One of the additions
> Vadim checked in yesterday extends ExecCheckPerms() to try to use
> its parseTree argument --- unconditionally. Guaranteed null-pointer
> dereference.
>
> Perhaps ExecInitSubPlan is in error to pass a null parseTree; if not,
> then ExecCheckPerms needs to be modified to cope. I don't understand
> either routine enough to fix it correctly.
I caused the 'having' problems. I am working on a fix.
-- 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
Tom Lane wrote:
>
> Nope, Vadim broke something. It looks like anything with a subplan
> will coredump in Monday's sources. executor/nodeSubPlan.c has
>
> bool
> ExecInitSubPlan(SubPlan *node, EState *estate, Plan *parent)
> {
> ...
> ExecCheckPerms(CMD_SELECT, 0, node->rtable, (Query *) NULL);
> ^^^^^^^^^^^^^^
>
> (and has had that for a long time, evidently). One of the additions
> Vadim checked in yesterday extends ExecCheckPerms() to try to use
> its parseTree argument --- unconditionally. Guaranteed null-pointer
> dereference.
>
> Perhaps ExecInitSubPlan is in error to pass a null parseTree; if not, ^^^^^^^^^^^^^^^^^^^^^^^^^^^
No.
> then ExecCheckPerms needs to be modified to cope. I don't understand
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Yes.
> either routine enough to fix it correctly.
Thanks!
Unfortunately, I can't fix this in CVS - I'm changing
execMain.c now to support READ COMMITTED mode. Could someone
add check in ExecCheckPerms ?
Sorry.
Vadim