Обсуждение: 6.4 Aggregate Bug
While testing my 6.4 patch to allow functions/expressions to be
specified in the ORDER/GROUP BY clause (and not in the target list) I
came across a nasty little bug. A segmentation fault gets thrown
somewhere in replace_agg_clause() when using aggregates, in combination
with a function or expression. (I am still tracking down the
offending lines of code. Sorry, the Linux/GCC environment is still new
to me.)
I backed out my patch, and discovered the bug was still present. The
bug does not exist in version 6.3.2. Here is an example:
-- This crashes the backend
select upper(a) as x, count(k) from t group by x;
-- This works fine
select upper(a) as x, count(a) from t group by x;
Notice how in the first query, (the one that does not work) upper() has
a different argument than count(). And in the second query (the one
that works) upper() has the same argument as count(). When using
count(*) it will always fail.
This is the the pattern that I have observed. If the arguments in the
aggregate and non-aggregate functions are the same, it runs; if the
arguments in the aggregate and non-aggregate functions are different, it
crashes.
I have attached a test script for anyone able to help with (or verify)
this problem.
create table t (
j integer,
k integer,
a varchar
);
insert into t values (1, 1, 'a');
insert into t values (2, 2, 'b');
insert into t values (2, 3, 'c');
insert into t values (3, 4, 'A');
insert into t values (3, 5, 'B');
insert into t values (3, 6, 'C');
insert into t values (4, 7, 'a');
insert into t values (4, 8, 'b');
insert into t values (4, 9, 'c');
insert into t values (4, 0, 'a');
-- OK
select upper(a) as x, count(a) from t group by x;
-- OK
select k/2 as x, max(k) from t group by x;
-- OK
-- select k as x, max(j) from t group by x;
-- OK
select upper(a) as x, count(k), count(a) from t group by x;
-- CRASH
select k/2 as x, max(j) from t group by x;
-- CRASH
select upper(a) as x, count(k) from t group by x;
-- CRASH
select upper(a) as x, count(xmin) from t group by x;
-- CRASH
select upper(a) as x, count(oid) from t group by x;
-- CRASH
select upper(a) as x, count(*) from t group by x;
David Hartwig wrote: > > While testing my 6.4 patch to allow functions/expressions to be > specified in the ORDER/GROUP BY clause (and not in the target list) I will this patch allow the following syntax : select count(SUBSTR(var,1,5)), SUBSTR(var,1,5) from t group by SUBSTR(var,1,5); This is important for writing database-independent code (e.g. it's the only syntax Oracle understands). Edmund -- Edmund Mergl mailto:E.Mergl@bawue.de Im Haldenhau 9 http://www.bawue.de/~mergl 70565 Stuttgart fon: +49 711 747503 Germany
> While testing my 6.4 patch to allow functions/expressions to be
> specified in the ORDER/GROUP BY clause (and not in the target list) I
> came across a nasty little bug.
> I backed out my patch, and discovered the bug was still present. The
> bug does not exist in version 6.3.2.
Nor in my cvs tree from around the second week in July (on or before
July 9, with a few patches committed to the Postgres tree afterwards).
Haven't tried a current source tree...
- Tom
>
> -- This crashes the backend
> select upper(a) as x, count(k) from t group by x;
tgl=> create table t (a text, k int);
CREATE
tgl=> select upper(a) as x, count(k) from t group by x;
x|count
-+-----
| 0
(1 row)
tgl=> insert into t values ('one', 1);
INSERT 643434 1
tgl=> insert into t values ('two', 2);
INSERT 643435 1
tgl=> insert into t values ('two', 2);
INSERT 643436 1
tgl=> select upper(a) as x, count(k) from t group by x;
x |count
---+-----
ONE| 1
TWO| 2
(2 rows)
And with your test case instead:
tgl=> select upper(a) as x, count(k) from t group by x;
x|count
-+-----
A| 4
B| 3
C| 3
(3 rows)
> While testing my 6.4 patch to allow functions/expressions to be
> specified in the ORDER/GROUP BY clause (and not in the target list) I
> came across a nasty little bug. A segmentation fault gets thrown
> somewhere in replace_agg_clause() when using aggregates, in combination
> with a function or expression. (I am still tracking down the
> offending lines of code. Sorry, the Linux/GCC environment is still new
> to me.)
>
> I backed out my patch, and discovered the bug was still present. The
> bug does not exist in version 6.3.2. Here is an example:
>
> -- This crashes the backend
> select upper(a) as x, count(k) from t group by x;
>
> -- This works fine
> select upper(a) as x, count(a) from t group by x;
>
> Notice how in the first query, (the one that does not work) upper() has
> a different argument than count(). And in the second query (the one
> that works) upper() has the same argument as count(). When using
> count(*) it will always fail.
Here is my initial analysis of the crash. It appears the types do not
match. Not sure why.
---------------------------------------------------------------------------
POSTGRES backend interactive interface
$Revision: 1.80 $ $Date: 1998/07/18 18:34:09 $
> select upper(a) as x, count(xmin) from t group by x;
StartTransactionCommand() at Sat Aug 1 14:55:16 1998
Breakpoint 1, match_varid (test_var=0x305010, tlist=0x303750) at
tlist.c:270
270 type_var = (Oid) test_var->vartype;
(gdb)
(gdb) cont
Continuing.
Breakpoint 1, match_varid (test_var=0x305110, tlist=0x303750) at
tlist.c:270
270 type_var = (Oid) test_var->vartype;
(gdb) cont
Continuing.
Breakpoint 1, match_varid (test_var=0x3048d0, tlist=0x303ad0) at
tlist.c:270
270 type_var = (Oid) test_var->vartype;
(gdb) l
265 match_varid(Var *test_var, List *tlist)
266 {
267 List *tl;
268 Oid type_var;
269
270 type_var = (Oid) test_var->vartype;
271
272 Assert(test_var->varlevelsup == 0);
273 foreach(tl, tlist)
274 {
(gdb) call pprint(test_var)
{ VAR
:varno 1
:varattno 3
:vartype 1043
:vartypmod -1
:varlevelsup 0
:varnoold 1
:varoattno 3
}
(gdb) call pprint(tlist)
(
{ TARGETENTRY
:resdom
{ RESDOM
:resno 1
:restype 25
:restypmod -1
:resname "x"
:reskey 1
:reskeyop 740
:resjunk 0
}
:expr
{ VAR
:varno 1
:varattno 1
:vartype 25
:vartypmod -1
:varlevelsup 0
:varnoold -1
:varoattno 1
}
}
{ TARGETENTRY
:resdom
{ RESDOM
:resno 2
:restype 28
:restypmod -1
:resname "(null)
"
:reskey 0
:reskeyop 0
:resjunk 0
}
:expr
{ VAR
:varno 1
:varattno 2
:vartype 28
:vartypmod -1
:varlevelsup 0
:varnoold 1
:varoattno -3
}
}
)
--
Bruce Momjian | 830 Blythe Avenue
maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026
+ If your life is a hard drive, | (610) 353-9879(w)
+ Christ can be your backup. | (610) 853-3000(h)
> > While testing my 6.4 patch to allow functions/expressions to be > > specified in the ORDER/GROUP BY clause (and not in the target list) I > > came across a nasty little bug. > > I backed out my patch, and discovered the bug was still present. The > > bug does not exist in version 6.3.2. > > Nor in my cvs tree from around the second week in July (on or before > July 9, with a few patches committed to the Postgres tree afterwards). > Haven't tried a current source tree... Can someone CVS back in time to find the date it broke? -- Bruce Momjian | 830 Blythe Avenue maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026 + If your life is a hard drive, | (610) 353-9879(w) + Christ can be your backup. | (610) 853-3000(h)
Edmund Mergl wrote: > David Hartwig wrote: > > > > While testing my 6.4 patch to allow functions/expressions to be > > specified in the ORDER/GROUP BY clause (and not in the target list) I > > will this patch allow the following syntax : > > select count(SUBSTR(var,1,5)), SUBSTR(var,1,5) from t group by SUBSTR(var,1,5); > > This is important for writing database-independent code (e.g. it's the only > syntax Oracle understands). > > YES. It also handles expressions like "x / 2". Also, the functions (or expressions) need not be in the target list.
> > > While testing my 6.4 patch I
> > > came across a nasty little bug.
> > Nor in my cvs tree from around the second week in July...
> > Haven't tried a current source tree...
> Can someone CVS back in time to find the date it broke?
I haven't done that yet. I _did_ do a CVSup just now and in the parser
area I do have an uncommitted patch for parse_clause.c in the
transformSortClause() routine. It was to help with type matching on
whatever Bruce and I were fixing a couple of weeks ago, before the docs
and Applix/iodbc became a higher priority. Anyway, don't know if that is
helping David's symptoms or not, or whether the breakage is in another
part of the code. Nothing else is any different in the parser area of
the code (except for gram.c, which shouldn't be an issue?).
I'll go ahead and commit the code tonight or tomorrow and then David or
you can see if it helps. Will post some e-mail when it happens...
- Tom
> I'll go ahead and commit the code tonight or tomorrow...
OK, I committed the patches. They fix the
SELECT NULL ORDER BY 1
query problem, but do assign types for cases where they aren't assigned
in the transformSortClause() procedure so may touch what David is
doing??
Patch enclosed, but the cvs tree already has it too.
- Tom
*** parse_clause.c.orig Sat Jul 11 15:16:44 1998
--- parse_clause.c Wed Jul 15 16:59:59 1998
***************
*** 317,322 ****
--- 317,326 ----
{
List *s = NIL;
+ #ifdef PARSEDEBUG
+ printf("transformSortClause: entering\n");
+ #endif
+
while (orderlist != NIL)
{
SortGroupBy *sortby = lfirst(orderlist);
***************
*** 326,332 ****
--- 330,346 ----
restarget = find_targetlist_entry(pstate, sortby, targetlist);
+ #ifdef PARSEDEBUG
+ printf("transformSortClause: find sorting operator for type %d\n",
+ restarget->resdom->restype);
+ #endif
+
sortcl->resdom = resdom = restarget->resdom;
+
+ /* if we have InvalidOid, then this is a NULL field and don't need to sort */
+ if (resdom->restype == InvalidOid)
+ resdom->restype = INT4OID;
+
sortcl->opoid = oprid(oper(sortby->useOp,
resdom->restype,
resdom->restype, false));
***************
*** 389,394 ****
--- 403,416 ----
/* not a member of the sortclauses yet */
SortClause *sortcl = makeNode(SortClause);
+ #ifdef PARSEDEBUG
+ printf("transformSortClause: (2) find sorting operator for type %d\n",
+ tlelt->resdom->restype);
+ #endif
+
+ if (tlelt->resdom->restype == InvalidOid)
+ tlelt->resdom->restype = INT4OID;
+
sortcl->resdom = tlelt->resdom;
sortcl->opoid = any_ordering_op(tlelt->resdom->restype);
***************
*** 423,428 ****
--- 445,455 ----
/* not a member of the sortclauses yet */
SortClause *sortcl = makeNode(SortClause);
+ #ifdef PARSEDEBUG
+ printf("transformSortClause: try sorting type %d\n",
+ tlelt->resdom->restype);
+ #endif
+
sortcl->resdom = tlelt->resdom;
sortcl->opoid = any_ordering_op(tlelt->resdom->restype);
***************
*** 485,490 ****
--- 512,524 ----
{
((TargetEntry *)lfirst(prev_target))->resdom->restype = itype;
}
+ #if FALSE
+ else
+ {
+ ((TargetEntry *)lfirst(prev_target))->resdom->restype = UNKNOWNOID;
+ ((TargetEntry *)lfirst(next_target))->resdom->restype = UNKNOWNOID;
+ }
+ #endif
}
else if (itype == InvalidOid)
{
Thomas G. Lockhart wrote: > > I'll go ahead and commit the code tonight or tomorrow... > > OK, I committed the patches. They fix the > > SELECT NULL ORDER BY 1 > > query problem, but do assign types for cases where they aren't assigned > in the transformSortClause() procedure so may touch what David is > doing?? Well if you meant that maybe it would fix the aggragate bug...no such luck. :( I will go ahead and submit my patch soon. I would like to get it out for review and use. (Right now, I am busy updating my system to its knees) BTW, The bug has nothing to do with my patch. Except it does steal some of its thunder, by rendering a significant portion of the newly available queries unusable.