Обсуждение: 6.4 Aggregate Bug

Поиск
Список
Период
Сортировка

6.4 Aggregate Bug

От
David Hartwig
Дата:
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;

Re: [HACKERS] 6.4 Aggregate Bug

От
Edmund Mergl
Дата:
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

Re: [HACKERS] 6.4 Aggregate Bug

От
"Thomas G. Lockhart"
Дата:
> 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)

Re: [HACKERS] 6.4 Aggregate Bug

От
Bruce Momjian
Дата:
> 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)

Re: [HACKERS] 6.4 Aggregate Bug

От
Bruce Momjian
Дата:
> > 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)

Re: [HACKERS] 6.4 Aggregate Bug

От
David Hartwig
Дата:
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.


Re: [HACKERS] 6.4 Aggregate Bug

От
"Thomas G. Lockhart"
Дата:
> > > 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

Re: [HACKERS] 6.4 Aggregate Bug

От
"Thomas G. Lockhart"
Дата:
> 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)
                  {

Re: [HACKERS] 6.4 Aggregate Bug

От
David Hartwig
Дата:

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.