Thread: inherited GROUP BY is busted ... I need some help here
I've been chasing Chris Bitmead's coredump report from earlier today. I find that it can be reproduced very easily. For example: regression=> select f1 from int4_tbl group by f1; < no problem > regression=> select f1 from int4_tbl* group by f1; < core dump > (You may get unstable behavior rather than a reliable core dump if you are not configured --enable-cassert.) The problem seems to be in optimizer/plan/planner.c, which is responsible for creating the Sort and Group plan nodes needed to implement GROUP BY. It also has to mark the lower plan's targetlist items with resdom->reskey numbers so that the executor will know which items to use for sort keys (cf. FormSortKeys in executor/nodeSort.c). The trouble is that that latter marking is done in planner.c's make_subplanTargetList(), which *is never invoked* for a query that involves inheritance. union_planner() only calls it if the given plan involves neither UNION nor inheritance. In the UNION case, recursion into union_planner does the right thing, but not so in the inheritance case. I rewrote some of this code a couple months ago, but I find that 6.4.2 has similar problems, so at least I can say I didn't break it ;-). It seems clear that at least some of the processing that union_planner does in the simple case (the "else" part of its first big if-then-else) also needs to be done in the inheritance case (and perhaps also in the UNION case?). But I'm not sure exactly what. There's a lot going on in this chunk of code, and I don't understand very much of it. I could really use some advice... regards, tom lane
I wrote: > I've been chasing Chris Bitmead's coredump report from earlier today. > I find that it can be reproduced very easily. For example: > regression=> select f1 from int4_tbl group by f1; > < no problem > > regression=> select f1 from int4_tbl* group by f1; > < core dump > We had tentatively agreed not to fix this for 6.5, but I got more worried about it when I noticed that this particular simple case worked in 6.4.2. I don't like regressing ... so I dug into it and have just committed a fix. It turns out that pretty much *anything* involving grouping or aggregation would fail if the query used inheritance, because the necessary preprocessing wasn't getting done in that case. I think that's a big enough bug to justify fixing at this late date. (Besides, the fixes do not change the non-inheritance case, so I don't think I could have broken anything that was working...) regards, tom lane
Tom, I can dig into this with you, if it is not already fixed. > I've been chasing Chris Bitmead's coredump report from earlier today. > I find that it can be reproduced very easily. For example: > regression=> select f1 from int4_tbl group by f1; > < no problem > > regression=> select f1 from int4_tbl* group by f1; > < core dump > > > (You may get unstable behavior rather than a reliable core dump > if you are not configured --enable-cassert.) > > The problem seems to be in optimizer/plan/planner.c, which is > responsible for creating the Sort and Group plan nodes needed to > implement GROUP BY. It also has to mark the lower plan's targetlist > items with resdom->reskey numbers so that the executor will know which > items to use for sort keys (cf. FormSortKeys in executor/nodeSort.c). > The trouble is that that latter marking is done in planner.c's > make_subplanTargetList(), which *is never invoked* for a query that > involves inheritance. union_planner() only calls it if the given plan > involves neither UNION nor inheritance. In the UNION case, recursion > into union_planner does the right thing, but not so in the inheritance > case. > > I rewrote some of this code a couple months ago, but I find that 6.4.2 > has similar problems, so at least I can say I didn't break it ;-). > > It seems clear that at least some of the processing that union_planner > does in the simple case (the "else" part of its first big if-then-else) > also needs to be done in the inheritance case (and perhaps also in > the UNION case?). But I'm not sure exactly what. There's a lot going > on in this chunk of code, and I don't understand very much of it. > I could really use some advice... > > regards, tom lane > > -- 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, is this still an open item? > I've been chasing Chris Bitmead's coredump report from earlier today. > I find that it can be reproduced very easily. For example: > regression=> select f1 from int4_tbl group by f1; > < no problem > > regression=> select f1 from int4_tbl* group by f1; > < core dump > > > (You may get unstable behavior rather than a reliable core dump > if you are not configured --enable-cassert.) > > The problem seems to be in optimizer/plan/planner.c, which is > responsible for creating the Sort and Group plan nodes needed to > implement GROUP BY. It also has to mark the lower plan's targetlist > items with resdom->reskey numbers so that the executor will know which > items to use for sort keys (cf. FormSortKeys in executor/nodeSort.c). > The trouble is that that latter marking is done in planner.c's > make_subplanTargetList(), which *is never invoked* for a query that > involves inheritance. union_planner() only calls it if the given plan > involves neither UNION nor inheritance. In the UNION case, recursion > into union_planner does the right thing, but not so in the inheritance > case. > > I rewrote some of this code a couple months ago, but I find that 6.4.2 > has similar problems, so at least I can say I didn't break it ;-). > > It seems clear that at least some of the processing that union_planner > does in the simple case (the "else" part of its first big if-then-else) > also needs to be done in the inheritance case (and perhaps also in > the UNION case?). But I'm not sure exactly what. There's a lot going > on in this chunk of code, and I don't understand very much of it. > I could really use some advice... > > regards, tom lane > > -- 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: > Tom, is this still an open item? That particular coredump seems to be fixed. There might be some other problems lurking with inherited queries, but this thread can be written off I think... >> I've been chasing Chris Bitmead's coredump report from earlier today. >> I find that it can be reproduced very easily. For example: >> regression=> select f1 from int4_tbl group by f1; >> < no problem > >> regression=> select f1 from int4_tbl* group by f1; >> < core dump > >> >> (You may get unstable behavior rather than a reliable core dump >> if you are not configured --enable-cassert.) regards, tom lane