Commit 476ca52e authored by unknown's avatar unknown

Fix for BUG#11211 "GROUP BY doesn't work correctly"

  
When the GROUP BY clause contains a column reference that can be resolved to
both an aliased column in the SELECT list, and to a column in the FROM clause,
the group column is resolved to the column in the FROM clause (for ANSI conformance).
However, it may be so that the user's intent is just the other way around, and he/she
gets the query results grouped by a completely different column than expexted.
This patch adds a warning in such cases that tells the user that there is potential
ambiguity in the group column.

sql/sql_select.cc
- Added a warning when a GROUP column is ambiguous due to that there is a
  column reference with the same name both in the SELECT and FROM clauses.
  In this case we resolve to the column in FROM clause and warn the user
  of a possible ambiguity.
- More extensive comments.
- Changed the function to return bool instead of int (as in other places).

mysql-test/t/group_by.test
  Added test for BUG#11211.

mysql-test/r/group_by.result
  Added test for BUG#11211.


mysql-test/r/group_by.result:
  Import patch 11211.diff
mysql-test/t/group_by.test:
  Import patch 11211.diff
sql/sql_select.cc:
  Import patch 11211.diff
BitKeeper/etc/ignore:
  Added ndb/src/dummy.cpp to the ignore list
mysql-test/r/alias.result:
  Added warning for potentially ambiguous column.
mysql-test/r/having.result:
  Added warning for potentially ambiguous column.
parent 894fd6ae
...@@ -1116,3 +1116,4 @@ vio/test-ssl ...@@ -1116,3 +1116,4 @@ vio/test-ssl
vio/test-sslclient vio/test-sslclient
vio/test-sslserver vio/test-sslserver
vio/viotest-ssl vio/viotest-ssl
ndb/src/dummy.cpp
...@@ -58,6 +58,8 @@ INSERT INTO t1 VALUES (3359362,406,3359362,'Mustermann Musterfrau',7001,'2000-05 ...@@ -58,6 +58,8 @@ INSERT INTO t1 VALUES (3359362,406,3359362,'Mustermann Musterfrau',7001,'2000-05
SELECT ELT(FIELD(kundentyp,'PP','PPA','PG','PGA','FK','FKA','FP','FPA','K','KA','V','VA',''), 'Privat (Private Nutzung)','Privat (Private Nutzung) Sitz im Ausland','Privat (geschaeftliche Nutzung)','Privat (geschaeftliche Nutzung) Sitz im Ausland','Firma (Kapitalgesellschaft)','Firma (Kapitalgesellschaft) Sitz im Ausland','Firma (Personengesellschaft)','Firma (Personengesellschaft) Sitz im Ausland','oeff. rechtl. Koerperschaft','oeff. rechtl. Koerperschaft Sitz im Ausland','Eingetragener Verein','Eingetragener Verein Sitz im Ausland','Typ unbekannt') AS Kundentyp ,kategorie FROM t1 WHERE hdl_nr < 2000000 AND kategorie IN ('Prepaid','Mobilfunk') AND st_klasse = 'Workflow' GROUP BY kundentyp ORDER BY kategorie; SELECT ELT(FIELD(kundentyp,'PP','PPA','PG','PGA','FK','FKA','FP','FPA','K','KA','V','VA',''), 'Privat (Private Nutzung)','Privat (Private Nutzung) Sitz im Ausland','Privat (geschaeftliche Nutzung)','Privat (geschaeftliche Nutzung) Sitz im Ausland','Firma (Kapitalgesellschaft)','Firma (Kapitalgesellschaft) Sitz im Ausland','Firma (Personengesellschaft)','Firma (Personengesellschaft) Sitz im Ausland','oeff. rechtl. Koerperschaft','oeff. rechtl. Koerperschaft Sitz im Ausland','Eingetragener Verein','Eingetragener Verein Sitz im Ausland','Typ unbekannt') AS Kundentyp ,kategorie FROM t1 WHERE hdl_nr < 2000000 AND kategorie IN ('Prepaid','Mobilfunk') AND st_klasse = 'Workflow' GROUP BY kundentyp ORDER BY kategorie;
Kundentyp kategorie Kundentyp kategorie
Privat (Private Nutzung) Mobilfunk Privat (Private Nutzung) Mobilfunk
Warnings:
Warning 1052 Column 'kundentyp' in group statement is ambiguous
drop table t1; drop table t1;
CREATE TABLE t1 ( CREATE TABLE t1 (
AUFNR varchar(12) NOT NULL default '', AUFNR varchar(12) NOT NULL default '',
......
...@@ -722,3 +722,27 @@ WHERE hostname LIKE '%aol%' ...@@ -722,3 +722,27 @@ WHERE hostname LIKE '%aol%'
GROUP BY hostname; GROUP BY hostname;
hostname no hostname no
cache-dtc-af05.proxy.aol.com 1 cache-dtc-af05.proxy.aol.com 1
drop table if exists t1, t2;
Warnings:
Note 1051 Unknown table 't2'
create table t1 (c1 char(3), c2 char(3));
create table t2 (c3 char(3), c4 char(3));
insert into t1 values ('aaa', 'bb1'), ('aaa', 'bb2');
insert into t2 values ('aaa', 'bb1'), ('aaa', 'bb2');
select t1.c1 as c2 from t1, t2 where t1.c2 = t2.c4
group by c2;
c2
aaa
aaa
Warnings:
Warning 1052 Column 'c2' in group statement is ambiguous
show warnings;
Level Code Message
Warning 1052 Column 'c2' in group statement is ambiguous
select t1.c1 as c2 from t1, t2 where t1.c2 = t2.c4
group by t1.c1;
c2
aaa
show warnings;
Level Code Message
drop table t1, t2;
...@@ -304,6 +304,7 @@ s1 ...@@ -304,6 +304,7 @@ s1
0 0
0 0
Warnings: Warnings:
Warning 1052 Column 's1' in group statement is ambiguous
Warning 1052 Column 's1' in having clause is ambiguous Warning 1052 Column 's1' in having clause is ambiguous
select s1*0 from t1 group by s1 having s1 = 0; select s1*0 from t1 group by s1 having s1 = 0;
s1*0 s1*0
......
...@@ -539,3 +539,26 @@ SELECT hostname, COUNT(DISTINCT user_id) as no FROM t1 ...@@ -539,3 +539,26 @@ SELECT hostname, COUNT(DISTINCT user_id) as no FROM t1
WHERE hostname LIKE '%aol%' WHERE hostname LIKE '%aol%'
GROUP BY hostname; GROUP BY hostname;
#
# Bug#11211: Ambiguous column reference in GROUP BY.
#
drop table if exists t1, t2;
create table t1 (c1 char(3), c2 char(3));
create table t2 (c3 char(3), c4 char(3));
insert into t1 values ('aaa', 'bb1'), ('aaa', 'bb2');
insert into t2 values ('aaa', 'bb1'), ('aaa', 'bb2');
# query with ambiguous column reference 'c2'
select t1.c1 as c2 from t1, t2 where t1.c2 = t2.c4
group by c2;
show warnings;
# this query has no ambiguity
select t1.c1 as c2 from t1, t2 where t1.c2 = t2.c4
group by t1.c1;
show warnings;
drop table t1, t2;
...@@ -11790,11 +11790,11 @@ cp_buffer_from_ref(THD *thd, TABLE_REF *ref) ...@@ -11790,11 +11790,11 @@ cp_buffer_from_ref(THD *thd, TABLE_REF *ref)
ref_pointer_array. ref_pointer_array.
RETURN RETURN
0 if OK FALSE if OK
1 if error occurred TRUE if error occurred
*/ */
static int static bool
find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables, find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,
ORDER *order, List<Item> &fields, List<Item> &all_fields, ORDER *order, List<Item> &fields, List<Item> &all_fields,
bool is_group_field) bool is_group_field)
...@@ -11811,13 +11811,13 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables, ...@@ -11811,13 +11811,13 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,
{ {
my_error(ER_BAD_FIELD_ERROR, MYF(0), my_error(ER_BAD_FIELD_ERROR, MYF(0),
order_item->full_name(), thd->where); order_item->full_name(), thd->where);
return 1; return TRUE;
} }
order->item= ref_pointer_array + count - 1; order->item= ref_pointer_array + count - 1;
order->in_field_list= 1; order->in_field_list= 1;
order->counter= count; order->counter= count;
order->counter_used= 1; order->counter_used= 1;
return 0; return FALSE;
} }
/* Lookup the current GROUP/ORDER field in the SELECT clause. */ /* Lookup the current GROUP/ORDER field in the SELECT clause. */
uint counter; uint counter;
...@@ -11825,7 +11825,7 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables, ...@@ -11825,7 +11825,7 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,
select_item= find_item_in_list(order_item, fields, &counter, select_item= find_item_in_list(order_item, fields, &counter,
REPORT_EXCEPT_NOT_FOUND, &unaliased); REPORT_EXCEPT_NOT_FOUND, &unaliased);
if (!select_item) if (!select_item)
return 1; /* Some error occured. */ return TRUE; /* The item is not unique, or some other error occured. */
/* Check whether the resolved field is not ambiguos. */ /* Check whether the resolved field is not ambiguos. */
...@@ -11839,7 +11839,7 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables, ...@@ -11839,7 +11839,7 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,
*/ */
if (unaliased && !order_item->fixed && order_item->fix_fields(thd, tables, if (unaliased && !order_item->fixed && order_item->fix_fields(thd, tables,
order->item)) order->item))
return 1; return TRUE;
/* Lookup the current GROUP field in the FROM clause. */ /* Lookup the current GROUP field in the FROM clause. */
order_item_type= order_item->type(); order_item_type= order_item->type();
...@@ -11879,27 +11879,42 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables, ...@@ -11879,27 +11879,42 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,
{ {
order->item= ref_pointer_array + counter; order->item= ref_pointer_array + counter;
order->in_field_list=1; order->in_field_list=1;
return 0; return FALSE;
} }
else
/*
There is a field with the same name in the FROM clause. This is the field
that will be chosen. In this case we issue a warning so the user knows
that the field from the FROM clause overshadows the column reference from
the SELECT list.
*/
push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, ER_NON_UNIQ_ERROR,
ER(ER_NON_UNIQ_ERROR), from_field->field_name,
current_thd->where);
} }
order->in_field_list=0; order->in_field_list=0;
/* /*
The call to order_item->fix_fields() means that here we resolve 'order_item'
to a column from a table in the list 'tables', or to a column in some outer
query. Exactly because of the second case we come to this point even if
(select_item == not_found_item), inspite of that fix_fields() calls
find_item_in_list() one more time.
We check order_item->fixed because Item_func_group_concat can put We check order_item->fixed because Item_func_group_concat can put
arguments for which fix_fields already was called. arguments for which fix_fields already was called.
'it' reassigned in if condition because fix_field can change it.
*/ */
if (!order_item->fixed && if (!order_item->fixed &&
(order_item->fix_fields(thd, tables, order->item) || (order_item->fix_fields(thd, tables, order->item) ||
(order_item= *order->item)->check_cols(1) || (order_item= *order->item)->check_cols(1) ||
thd->is_fatal_error)) thd->is_fatal_error))
return 1; // Wrong field return TRUE; /* Wrong field. */
uint el= all_fields.elements; uint el= all_fields.elements;
all_fields.push_front(order_item); // Add new field to field list all_fields.push_front(order_item); /* Add new field to field list. */
ref_pointer_array[el]= order_item; ref_pointer_array[el]= order_item;
order->item= ref_pointer_array + el; order->item= ref_pointer_array + el;
return 0; return FALSE;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment