6.4 Aggregate Bug

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)

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 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)

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

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.