Re: Postgres Planner Bug

Here is an email from Gavin showing a problem with subqueries and casts
causing errors when they shouldn't.

---------------------------------------------------------------------------

Gavin Sherry wrote:
> Hi Bruce,
> 
> Thanks to a user query (handle: lltd, IRC) I came across a bug in the
> planner. The query was:
> 
> ---
> select o1.timestamp::date as date, count(*), (select sum(oi.price) from
> "order" o2, "order_item" oi where oi.order_id = o2.id and
> o2.timestamp::date = o1.timestamp::date and o2.timestamp is not
> null) as total from "order" o1 where o1.timestamp is not null
> group by o1.timestamp::date order by o1.timestamp::date desc;
> ---
> 
> The error he was receiving:
> 
> ---
> ERROR:  Sub-SELECT uses un-GROUPed attribute o1.timestamp from outer query
> ---
> 
> After a bit of looking around, I determined that the cast in the order by
> clause was causing the error, not the fact that the query had an ungrouped
> attribute in the outer query.
> 
> I have come up with a simpler demonstration which works under 7.1 and CVS.
> 
> create table a ( i int);
> insert into a values(1);
> insert into a values(1);
> insert into a values(1);
> insert into a values(1);
> insert into a values(1);
> insert into a values(2);
> insert into a values(2);
> insert into a values(2);
> insert into a values(2);
> insert into a values(3);
> insert into a values(3);
> insert into a values(3);
> insert into a values(3);
> 
> --- NO ERROR ---
> 
> select o1.i::smallint,count(*),(select sum(o2.i) from a o2 where
> o2.i=o1.i::smallint) as sum from a o1 group by o1.i;
> 
> --- ERROR ---
> select o1.i::smallint,count(*),(select sum(o2.i) from a o2 where
> o2.i=o1.i::smallint) as sum from a o1 group by o1.i::smallint;
> 
> ----
> 
> Notice that the difference is only the cast in the order by clause. Here
> are my results:
> 
> template1=# select version();
>                                 version
> ------------------------------------------------------------------------
>  PostgreSQL 7.3devel on i686-pc-linux-gnu, compiled by GCC egcs-2.91.66
> (1 row)
> 
> template1=# \d a
>        Table "public.a"
>  Column |  Type   | Modifiers
> --------+---------+-----------
>  i      | integer |
> 
> template1=# select * from a;
>  i
> ---
>  1
>  1
>  1
>  1
>  1
>  2
>  2
>  2
>  2
>  3
>  3
>  3
>  3
> (13 rows)
> 
> te1=# select o1.i::smallint,count(*),(select sum(o2.i) from a o2 where
> o2.i=o1.i::smallint) as sum from a o1 group by o1.i;
>  i | count | sum
> ---+-------+-----
>  1 |     5 |   5
>  2 |     4 |   8
>  3 |     4 |  12
> (3 rows)
> 
> template1=# select o1.i::smallint,count(*),(select sum(o2.i) from a o2
> where o2.i=o1.i::smallint) as sum from a o1 group by o1.i::smallint;
> ERROR:  Sub-SELECT uses un-GROUPed attribute o1.i from outer query
> 
> [under patched version]
> 
> template1=# select o1.i::smallint,count(*),(select sum(o2.i) from a o2
> where o2.i=o1.i::smallint) as sum from a o1 group by o1.i;
>  i | count | sum
> ---+-------+-----
>  1 |     5 |   5
>  2 |     4 |   8
>  3 |     4 |  12
> (3 rows)
> 
> template1=# select o1.i::smallint,count(*),(select sum(o2.i) from a o2
> where o2.i=o1.i::smallint) as sum from a o1 group by o1.i::smallint;
>  i | count | sum
> ---+-------+-----
>  1 |     5 |   5
>  2 |     4 |   8
>  3 |     4 |  12
> (3 rows)
> 
> 
> As it works out, the bug is caused by these lines in
> optimizer/util/clauses.c
> 
>                 if (equal(thisarg, lfirst(gl)))
>                 {
>                     contained_in_group_clause = true;
>                     break;
>                 }
> 
> 'thisarg' is an item from the args list used by Expr *. We only access
> this code inside check_subplans_for_ungrouped_vars_walker() if the node is
> a subplan. The problem is that equal() is not sufficiently intelligent to
> consider the equality of 'thisarg' and lfirst(gl) (an arg from the group
> by clause) considering that thisarg and lfirst(gl) are not necessarily of
> the same node type. This means we fail out in equal():
> 
>     /*
>      * are they the same type of nodes?
>      */
>     if (nodeTag(a) != nodeTag(b))
>         return false;
> 
> 
> The patch below 'fixes' this (and possibly breaks everything else). I
> haven't tested it rigorously and it *just* special cases group by
> clauses with functions in them. Here's the patch:
> 
> Index: ./src/backend/optimizer/util/clauses.c
> ===================================================================
> RCS
> file: /projects/cvsroot/pgsql-server/src/backend/optimizer/util/clauses.c,v
> retrieving revision 1.107
> diff -2 -c -r1.107 clauses.c
> *** ./src/backend/optimizer/util/clauses.c  2002/08/31 22:10:43 1.107
> --- ./src/backend/optimizer/util/clauses.c  2002/09/30 15:02:47
> ***************
> *** 703,706 ****
> --- 703,718 ----
>                     contained_in_group_clause = true;
>                     break;
> +               } else {
> +                   if(IsA(lfirst(gl),Expr) &&
> +                           length(((Expr *)lfirst(gl))->args) == 1 &&
> +                           IsA(lfirst(((Expr *)lfirst(gl))->args),Var) ) {
> +
> +                       Var *tvar = (Var *) lfirst(((Expr *)lfirst(gl))->args);
> +                       if(var->varattno == tvar->varattno) {
> +                           contained_in_group_clause = true;
> +                           break;
> +                       }
> +
> +                   }
>                 }
>             }
> 
> ----
> 
> There are two assumptions here: 1) the only time this bug occurs is when
> the group by clause argument is an expression and a function at that (even
> though I do no test for this correctly) 2) We can see whether thisarg ==
> lfirst(gl) by looking at the varattno of each and comparing. It occurs to
> me that this is just plain wrong and works only for the specific query.
> 
> The reason why I've sent this email to you and not to the list is I do not
> have time to follow through on this -- as much as I would like to. I
> simply do no have the time. :-(
> 
> Gavin
> 
> 

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073