Re: [HACKERS] Postgres Planner Bug

2002-09-30 Thread Bruce Momjian


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 

Re: [HACKERS] Postgres Planner Bug

2002-09-30 Thread Gavin Sherry


  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

Mistake here. It relates to the group by clause, not order by.

Gavin


---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster



Re: [HACKERS] Postgres Planner Bug

2002-09-30 Thread Tom Lane

Bruce Momjian [EMAIL PROTECTED] writes:
 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.

Surely this cure is worse than the disease.

The general problem is that we don't attempt to match
arbitrary-expression GROUP BY clauses against arbitrary subexpressions
of sub-SELECTs.  While that could certainly be done, I'm concerned about
the cycles that we'd expend trying to match everything against
everything else.  This would be an exponential cost imposed on every
group-by-with-subselect query whether it needed the feature or not.

Given that GROUP BY is restricted to a simple column reference in both
SQL92 and SQL99, is it worth a large performance hit on unrelated
queries to support this feature?  What other DBs support it?

regards, tom lane

---(end of broadcast)---
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly