From 0aa589d0998b41f3e0f55e1f62e86ec8d9d17ba9 Mon Sep 17 00:00:00 2001 From: "dlenev@brandersnatch.localdomain" <> Date: Thu, 30 Sep 2004 16:28:17 +0400 Subject: [PATCH 1/4] Final solution for bug# 4302 "Ambiguos order by when renamed column is identical to another in result" According to SQL standard queries like "select t1.a as col from t1, t2 order by a" should return an error if both tables contain field a. --- mysql-test/r/order_by.result | 9 +++----- mysql-test/t/order_by.test | 5 +++-- sql/item.cc | 16 ++++++++------ sql/mysql_priv.h | 3 ++- sql/sql_base.cc | 43 +++++++++++++++++++++++------------- sql/sql_select.cc | 26 ++++++++++++++-------- 6 files changed, 62 insertions(+), 40 deletions(-) diff --git a/mysql-test/r/order_by.result b/mysql-test/r/order_by.result index 69ce69ad499..94d56bbc2fa 100644 --- a/mysql-test/r/order_by.result +++ b/mysql-test/r/order_by.result @@ -680,6 +680,9 @@ order by col; ERROR 23000: Column 'col' in order clause is ambiguous select col1 from t1, t2 where t1.col1=t2.col2 order by col; ERROR 23000: Column 'col' in order clause is ambiguous +select t1.col as t1_col, t2.col2 from t1, t2 where t1.col1=t2.col2 +order by col; +ERROR 23000: Column 'col' in order clause is ambiguous select t1.col as t1_col, t2.col from t1, t2 where t1.col1=t2.col2 order by col; t1_col col @@ -696,12 +699,6 @@ col col2 1 3 2 2 3 1 -select t1.col as t1_col, t2.col2 from t1, t2 where t1.col1=t2.col2 -order by col; -t1_col col2 -1 1 -2 2 -3 3 select t2.col2, t2.col, t2.col from t2 order by col; col2 col col 3 1 1 diff --git a/mysql-test/t/order_by.test b/mysql-test/t/order_by.test index 1d65ce9003a..988c106bf21 100644 --- a/mysql-test/t/order_by.test +++ b/mysql-test/t/order_by.test @@ -472,13 +472,14 @@ select t1.col as c1, t2.col as c2 from t1, t2 where t1.col1=t2.col2 order by col; --error 1052 select col1 from t1, t2 where t1.col1=t2.col2 order by col; +--error 1052 +select t1.col as t1_col, t2.col2 from t1, t2 where t1.col1=t2.col2 + order by col; select t1.col as t1_col, t2.col from t1, t2 where t1.col1=t2.col2 order by col; select col2 as c, col as c from t2 order by col; select col2 as col, col as col2 from t2 order by col; -select t1.col as t1_col, t2.col2 from t1, t2 where t1.col1=t2.col2 - order by col; select t2.col2, t2.col, t2.col from t2 order by col; select t2.col2 as col from t2 order by t2.col; diff --git a/sql/item.cc b/sql/item.cc index 14136435a50..3c9b54074dc 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -1246,6 +1246,7 @@ bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref) TABLE_LIST *table_list; Item **refer= (Item **)not_found_item; uint counter; + bool not_used; // Prevent using outer fields in subselects, that is not supported now SELECT_LEX *cursel= (SELECT_LEX *) thd->lex->current_select; if (cursel->master_unit()->first_select()->linkage != DERIVED_TABLE_TYPE) @@ -1288,7 +1289,8 @@ bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref) } if (sl->resolve_mode == SELECT_LEX::SELECT_MODE && (refer= find_item_in_list(this, sl->item_list, &counter, - REPORT_EXCEPT_NOT_FOUND)) != + REPORT_EXCEPT_NOT_FOUND, + ¬_used)) != (Item **) not_found_item) { if (*refer && (*refer)->fixed) // Avoid crash in case of error @@ -1889,6 +1891,7 @@ bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables, Item **reference) { DBUG_ASSERT(fixed == 0); uint counter; + bool not_used; if (!ref) { TABLE_LIST *where= 0, *table_list; @@ -1908,13 +1911,13 @@ bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables, Item **reference) first_select()->linkage != DERIVED_TABLE_TYPE) ? REPORT_EXCEPT_NOT_FOUND : - REPORT_ALL_ERRORS))) == + REPORT_ALL_ERRORS ), ¬_used)) == (Item **)not_found_item) { upward_lookup= 1; Field *tmp= (Field*) not_found_field; /* - We can't find table field in table list of current select, + We can't find table field in select list of current select, consequently we have to find it in outer subselect(s). We can't join lists of outer & current select, because of scope of view rules. For example if both tables (outer & current) have @@ -1929,8 +1932,8 @@ bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables, Item **reference) Item_subselect *prev_subselect_item= prev_unit->item; if (sl->resolve_mode == SELECT_LEX::SELECT_MODE && (ref= find_item_in_list(this, sl->item_list, - &counter, - REPORT_EXCEPT_NOT_FOUND)) != + &counter, REPORT_EXCEPT_NOT_FOUND, + ¬_used)) != (Item **)not_found_item) { if (*ref && (*ref)->fixed) // Avoid crash in case of error @@ -1989,8 +1992,7 @@ bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables, Item **reference) // Call to report error find_item_in_list(this, *(thd->lex->current_select->get_item_list()), - &counter, - REPORT_ALL_ERRORS); + &counter, REPORT_ALL_ERRORS, ¬_used); } ref= 0; return 1; diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index e47807dd36e..28aec2f9448 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -705,7 +705,8 @@ enum find_item_error_report_type {REPORT_ALL_ERRORS, REPORT_EXCEPT_NOT_FOUND, IGNORE_ERRORS}; extern const Item **not_found_item; Item ** find_item_in_list(Item *item, List &items, uint *counter, - find_item_error_report_type report_error); + find_item_error_report_type report_error, + bool *unaliased); bool get_key_map_from_key_list(key_map *map, TABLE *table, List *index_list); bool insert_fields(THD *thd,TABLE_LIST *tables, diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 75eb5753e1e..7464523aad4 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -2082,14 +2082,17 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables, return not_found_item, report other errors, return 0 IGNORE_ERRORS Do not report errors, return 0 if error - + unaliased Set to true if item is field which was found + by original field name and not by its alias + in item list. Set to false otherwise. + RETURN VALUES 0 Item is not found or item is not unique, error message is reported not_found_item Function was called with report_error == REPORT_EXCEPT_NOT_FOUND and item was not found. No error message was reported - found field + found field */ // Special Item pointer for find_item_in_list returning @@ -2098,7 +2101,7 @@ const Item **not_found_item= (const Item**) 0x1; Item ** find_item_in_list(Item *find, List &items, uint *counter, - find_item_error_report_type report_error) + find_item_error_report_type report_error, bool *unaliased) { List_iterator li(items); Item **found=0, **found_unaliased= 0, *item; @@ -2107,6 +2110,9 @@ find_item_in_list(Item *find, List &items, uint *counter, const char *table_name=0; bool found_unaliased_non_uniq= 0; uint unaliased_counter; + + *unaliased= FALSE; + if (find->type() == Item::FIELD_ITEM || find->type() == Item::REF_ITEM) { field_name= ((Item_ident*) find)->field_name; @@ -2134,17 +2140,18 @@ find_item_in_list(Item *find, List &items, uint *counter, /* If table name is specified we should find field 'field_name' in table 'table_name'. According to SQL-standard we should ignore - aliases in this case. Note that we should prefer fields from the - select list over other fields from the tables participating in - this select in case of ambiguity. + aliases in this case. + + Since we should NOT prefer fields from the select list over + other fields from the tables participating in this select in + case of ambiguity we have to do extra check outside this function. We use strcmp for table names and database names as these may be - case sensitive. - In cases where they are not case sensitive, they are always in lower - case. + case sensitive. In cases where they are not case sensitive, they + are always in lower case. item_field->field_name and item_field->table_name can be 0x0 if - item is not fix fielded yet. + item is not fix_field()'ed yet. */ if (item_field->field_name && item_field->table_name && !my_strcasecmp(system_charset_info, item_field->field_name, @@ -2153,17 +2160,22 @@ find_item_in_list(Item *find, List &items, uint *counter, (!db_name || (item_field->db_name && !strcmp(item_field->db_name, db_name)))) { - if (found) + if (found_unaliased) { - if ((*found)->eq(item, 0)) - continue; // Same field twice + if ((*found_unaliased)->eq(item, 0)) + continue; + /* + Two matching fields in select list. + We already can bail out because we are searching through + unaliased names only and will have duplicate error anyway. + */ if (report_error != IGNORE_ERRORS) my_printf_error(ER_NON_UNIQ_ERROR, ER(ER_NON_UNIQ_ERROR), MYF(0), find->full_name(), current_thd->where); return (Item**) 0; } - found= li.ref(); - *counter= i; + found_unaliased= li.ref(); + unaliased_counter= i; if (db_name) break; // Perfect match } @@ -2235,6 +2247,7 @@ find_item_in_list(Item *find, List &items, uint *counter, { found= found_unaliased; *counter= unaliased_counter; + *unaliased= TRUE; } } if (found) diff --git a/sql/sql_select.cc b/sql/sql_select.cc index e8e111a9a37..1112e073cb1 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -7946,15 +7946,14 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,ORDER *order, List &fields, List &all_fields) { - Item *itemptr=*order->item; - if (itemptr->type() == Item::INT_ITEM) + Item *it= *order->item; + if (it->type() == Item::INT_ITEM) { /* Order by position */ - uint count= (uint) itemptr->val_int(); + uint count= (uint) it->val_int(); if (!count || count > fields.elements) { my_printf_error(ER_BAD_FIELD_ERROR,ER(ER_BAD_FIELD_ERROR), - MYF(0),itemptr->full_name(), - thd->where); + MYF(0), it->full_name(), thd->where); return 1; } order->item= ref_pointer_array + count-1; @@ -7962,20 +7961,28 @@ find_order_in_list(THD *thd, Item **ref_pointer_array, return 0; } uint counter; - Item **item= find_item_in_list(itemptr, fields, &counter, - REPORT_EXCEPT_NOT_FOUND); + bool unaliased; + Item **item= find_item_in_list(it, fields, &counter, + REPORT_EXCEPT_NOT_FOUND, &unaliased); if (!item) return 1; if (item != (Item **)not_found_item) { + /* + If we have found field not by its alias in select list but by its + original field name, we should additionaly check if we have conflict + for this name (in case if we would perform lookup in all tables). + */ + if (unaliased && !it->fixed && it->fix_fields(thd, tables, order->item)) + return 1; + order->item= ref_pointer_array + counter; order->in_field_list=1; return 0; } order->in_field_list=0; - Item *it= *order->item; /* We check it->fixed because Item_func_group_concat can put arguments for which fix_fields already was called. @@ -8104,10 +8111,11 @@ setup_new_fields(THD *thd,TABLE_LIST *tables,List &fields, thd->set_query_id=1; // Not really needed, but... uint counter; + bool not_used; for (; new_field ; new_field= new_field->next) { if ((item= find_item_in_list(*new_field->item, fields, &counter, - IGNORE_ERRORS))) + IGNORE_ERRORS, ¬_used))) new_field->item=item; /* Change to shared Item */ else { From b04573650885e5a8dbcce367fe5e49a74dae85b4 Mon Sep 17 00:00:00 2001 From: "kent@mysql.com" <> Date: Thu, 30 Sep 2004 19:40:33 +0200 Subject: [PATCH 2/4] mysql_protocols.test: --exec now checks return code, make all command lines succeed --- mysql-test/t/mysql_protocols.test | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mysql-test/t/mysql_protocols.test b/mysql-test/t/mysql_protocols.test index 942ba2722d8..e5158586124 100644 --- a/mysql-test/t/mysql_protocols.test +++ b/mysql-test/t/mysql_protocols.test @@ -4,7 +4,7 @@ --exec echo "select ' ok' as ''" | $MYSQL --exec echo "select ' ok' as 'TCP'" | $MYSQL --protocol=TCP --exec echo "select ' ok' as 'SOCKET'" | $MYSQL --protocol=SOCKET ---exec echo "select ' ok' as 'PIPE'" | $MYSQL --protocol=PIPE 2>&1 ---exec echo "select ' ok' as 'MEMORY'" | $MYSQL --protocol=MEMORY 2>&1 ---exec echo "select ' ok' as 'NullS'" | $MYSQL --protocol=NullS 2>&1 +--exec echo "select ' ok' as 'PIPE'" | $MYSQL --protocol=PIPE 2>&1 || true +--exec echo "select ' ok' as 'MEMORY'" | $MYSQL --protocol=MEMORY 2>&1 || true +--exec echo "select ' ok' as 'NullS'" | $MYSQL --protocol=NullS 2>&1 || true From 49b81a610eed30873056ab70ec353a03227aa91c Mon Sep 17 00:00:00 2001 From: "tomas@poseidon.ndb.mysql.com" <> Date: Thu, 30 Sep 2004 21:12:46 +0000 Subject: [PATCH 3/4] bug#5824 --- ndb/src/mgmclient/CommandInterpreter.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/ndb/src/mgmclient/CommandInterpreter.cpp b/ndb/src/mgmclient/CommandInterpreter.cpp index fbb74d7c151..41d642b3b23 100644 --- a/ndb/src/mgmclient/CommandInterpreter.cpp +++ b/ndb/src/mgmclient/CommandInterpreter.cpp @@ -611,10 +611,12 @@ CommandInterpreter::executeHelp(char* parameters) << endl; ndbout << " = "; - for(int i = 0; i Date: Fri, 1 Oct 2004 00:32:42 +0200 Subject: [PATCH 4/4] client_test.test: Use TESTS_BINDIR to point out bin dir for "client_test" mysql-test-run.sh: Export TESTS_BINDIR to be used from --exec --- mysql-test/mysql-test-run.sh | 4 +++- mysql-test/t/client_test.test | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/mysql-test/mysql-test-run.sh b/mysql-test/mysql-test-run.sh index 5cb491b43c4..c447e96590f 100644 --- a/mysql-test/mysql-test-run.sh +++ b/mysql-test/mysql-test-run.sh @@ -507,6 +507,7 @@ if [ x$SOURCE_DIST = x1 ] ; then fi CLIENT_BINDIR="$BASEDIR/client" + TESTS_BINDIR="$BASEDIR/tests" MYSQLADMIN="$CLIENT_BINDIR/mysqladmin" WAIT_PID="$BASEDIR/extra/mysql_waitpid" MYSQL_MANAGER_CLIENT="$CLIENT_BINDIR/mysqlmanagerc" @@ -525,6 +526,7 @@ else MYSQLD="$VALGRIND $BASEDIR/bin/mysqld" fi CLIENT_BINDIR="$BASEDIR/bin" + TESTS_BINDIR="$BASEDIR/tests" MYSQL_TEST="$CLIENT_BINDIR/mysqltest" MYSQL_DUMP="$CLIENT_BINDIR/mysqldump" MYSQL_BINLOG="$CLIENT_BINDIR/mysqlbinlog" @@ -582,7 +584,7 @@ MYSQL_DUMP="$MYSQL_DUMP --no-defaults -uroot --socket=$MASTER_MYSOCK --password= MYSQL_BINLOG="$MYSQL_BINLOG --no-defaults --local-load=$MYSQL_TMP_DIR $EXTRA_MYSQLBINLOG_OPT" MYSQL_FIX_SYSTEM_TABLES="$MYSQL_FIX_SYSTEM_TABLES --no-defaults --host=localhost --port=$MASTER_MYPORT --socket=$MASTER_MYSOCK --user=root --password=$DBPASSWD --basedir=$BASEDIR --bindir=$CLIENT_BINDIR --verbose" MYSQL="$MYSQL --host=localhost --port=$MASTER_MYPORT --socket=$MASTER_MYSOCK --user=root --password=$DBPASSWD" -export MYSQL MYSQL_DUMP MYSQL_BINLOG MYSQL_FIX_SYSTEM_TABLES CLIENT_BINDIR +export MYSQL MYSQL_DUMP MYSQL_BINLOG MYSQL_FIX_SYSTEM_TABLES CLIENT_BINDIR TESTS_BINDIR MYSQL_TEST_ARGS="--no-defaults --socket=$MASTER_MYSOCK --database=$DB \ --user=$DBUSER --password=$DBPASSWD --silent -v --skip-safemalloc \ diff --git a/mysql-test/t/client_test.test b/mysql-test/t/client_test.test index eb053b6c902..50230d1dd16 100644 --- a/mysql-test/t/client_test.test +++ b/mysql-test/t/client_test.test @@ -1,2 +1,2 @@ -- disable_result_log ---exec ../tests/client_test --testcase --user=root --socket=var/tmp/master.sock --port=$MYSQL_TCP_PORT +--exec $TESTS_BINDIR/client_test --testcase --user=root --socket=var/tmp/master.sock --port=$MYSQL_TCP_PORT