Обсуждение: 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