From bb65b800144b9d4bfeba88a441b5c9f19ae8f9f4 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 20 Jul 2005 22:52:15 -0700 Subject: [PATCH 1/6] Adding support for opteron optimization in build files. --- BUILD/check-cpu | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/BUILD/check-cpu b/BUILD/check-cpu index b970a4b9a5b..dc894c91cbd 100755 --- a/BUILD/check-cpu +++ b/BUILD/check-cpu @@ -90,6 +90,9 @@ case "$cpu_family--$model_name" in *Athlon*) cpu_arg="athlon"; ;; + *Opteron*) + cpu_arg="opteron"; + ;; # Intel ia64 *Itanium*) @@ -147,6 +150,9 @@ case "$cc_ver--$cc_verno" in ppc-*) check_cpu_args='-mcpu=$cpu_arg -mtune=$cpu_arg' ;; + x86_64-*) + check_cpu_args='-mtune=$cpu_arg' + ;; *) check_cpu_cflags="" return From ee793d030e4221a5226ea6337deecd84b4337f63 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 17 Aug 2005 01:18:13 +0200 Subject: [PATCH 2/6] mtr_process.pl: Bug#11792: Check all of status code, to catch a crash as a failure mtr_cases.pl: Code cleanup for skipped/disabled handling mtr_process.pl: In debug mode, report if mysqladmin did not at least make the server stop listening to the port. Increased the time waiting for terminating to 20 seconds, to wait for really slow slave shutdown. Added call to start_reap_all(), to avoid zombies. mtr_report.pl: Removed prototype for unused function mtr_report_test_disabled() mysql-test/lib/mtr_report.pl: Removed prototype for unused function mtr_report_test_disabled() mysql-test/lib/mtr_cases.pl: Code cleanup for skipped/disabled handling mysql-test/lib/mtr_process.pl: Bug#11792: Check all of status code, to catch a crash as a failure --- mysql-test/lib/mtr_cases.pl | 27 ++++++++++++------------- mysql-test/lib/mtr_process.pl | 37 ++++++++++++++++++++++++----------- mysql-test/lib/mtr_report.pl | 1 - 3 files changed, 39 insertions(+), 26 deletions(-) diff --git a/mysql-test/lib/mtr_cases.pl b/mysql-test/lib/mtr_cases.pl index 12714ddc1ad..158fd602ef8 100644 --- a/mysql-test/lib/mtr_cases.pl +++ b/mysql-test/lib/mtr_cases.pl @@ -53,21 +53,20 @@ sub collect_test_cases ($) { else { # ---------------------------------------------------------------------- - # Skip some tests listed in disabled.def + # Disable some tests listed in disabled.def # ---------------------------------------------------------------------- - my %skiplist; - my $skipfile= "$testdir/disabled.def"; - if ( open(SKIPFILE, $skipfile) ) + my %disabled; + if ( open(DISABLED, "$testdir/disabled.def" ) ) { - while ( ) + while ( ) { chomp; if ( /^\s*(\S+)\s*:\s*(.*?)\s*$/ ) { - $skiplist{$1}= $2; + $disabled{$1}= $2; } } - close SKIPFILE; + close DISABLED; } foreach my $elem ( sort readdir(TESTDIR) ) { @@ -75,7 +74,7 @@ sub collect_test_cases ($) { next if ! defined $tname; next if $::opt_do_test and ! defined mtr_match_prefix($elem,$::opt_do_test); - collect_one_test_case($testdir,$resdir,$tname,$elem,$cases,\%skiplist); + collect_one_test_case($testdir,$resdir,$tname,$elem,$cases,\%disabled); } closedir TESTDIR; } @@ -119,7 +118,7 @@ sub collect_one_test_case($$$$$$) { my $tname= shift; my $elem= shift; my $cases= shift; - my $skiplist=shift; + my $disabled=shift; my $path= "$testdir/$elem"; @@ -188,7 +187,7 @@ sub collect_one_test_case($$$$$$) { my $slave_mi_file= "$testdir/$tname.slave-mi"; my $master_sh= "$testdir/$tname-master.sh"; my $slave_sh= "$testdir/$tname-slave.sh"; - my $disabled= "$testdir/$tname.disabled"; + my $disabled_file= "$testdir/$tname.disabled"; $tinfo->{'master_opt'}= $::glob_win32 ? ["--default-time-zone=+3:00"] : []; $tinfo->{'slave_opt'}= $::glob_win32 ? ["--default-time-zone=+3:00"] : []; @@ -292,18 +291,18 @@ sub collect_one_test_case($$$$$$) { } # FIXME why this late? - if ( $skiplist->{$tname} ) + if ( $disabled->{$tname} ) { $tinfo->{'skip'}= 1; $tinfo->{'disable'}= 1; # Sub type of 'skip' - $tinfo->{'comment'}= $skiplist->{$tname} if $skiplist->{$tname}; + $tinfo->{'comment'}= $disabled->{$tname} if $disabled->{$tname}; } - if ( -f $disabled ) + if ( -f $disabled_file ) { $tinfo->{'skip'}= 1; $tinfo->{'disable'}= 1; # Sub type of 'skip' - $tinfo->{'comment'}= mtr_fromfile($disabled); + $tinfo->{'comment'}= mtr_fromfile($disabled_file); } # We can't restart a running server that may be in use diff --git a/mysql-test/lib/mtr_process.pl b/mysql-test/lib/mtr_process.pl index e4cdaff1e77..1f18968031c 100644 --- a/mysql-test/lib/mtr_process.pl +++ b/mysql-test/lib/mtr_process.pl @@ -186,8 +186,8 @@ sub spawn_parent_impl { if ( $mode eq 'run' or $mode eq 'test' ) { my $exit_value= -1; - my $signal_num= 0; - my $dumped_core= 0; +# my $signal_num= 0; +# my $dumped_core= 0; if ( $mode eq 'run' ) { @@ -199,9 +199,10 @@ sub spawn_parent_impl { mtr_error("$path ($pid) got lost somehow"); } - $exit_value= $? >> 8; - $signal_num= $? & 127; - $dumped_core= $? & 128; + $exit_value= $?; +# $exit_value= $? >> 8; +# $signal_num= $? & 127; +# $dumped_core= $? & 128; return $exit_value; } @@ -229,9 +230,10 @@ sub spawn_parent_impl { if ( $ret_pid == $pid ) { # We got termination of mysqltest, we are done - $exit_value= $? >> 8; - $signal_num= $? & 127; - $dumped_core= $? & 128; + $exit_value= $?; +# $exit_value= $? >> 8; +# $signal_num= $? & 127; +# $dumped_core= $? & 128; last; } @@ -473,6 +475,7 @@ sub mtr_stop_mysqld_servers ($) { } else { + # Server is dead, we remove the pidfile if any # Race, could have been removed between I tested with -f # and the unlink() below, so I better check again with -f @@ -502,10 +505,12 @@ sub mtr_stop_mysqld_servers ($) { # that for true Win32 processes, kill(0,$pid) will not return 1. # ---------------------------------------------------------------------- + start_reap_all(); # Avoid zombies + SIGNAL: foreach my $sig (15,9) { - my $retries= 10; # 10 seconds + my $retries= 20; # FIXME 20 seconds, this is silly! kill($sig, keys %mysqld_pids); while ( $retries-- and kill(0, keys %mysqld_pids) ) { @@ -514,6 +519,8 @@ sub mtr_stop_mysqld_servers ($) { } } + stop_reap_all(); # Get into control again + # ---------------------------------------------------------------------- # Now, we check if all we can find using kill(0,$pid) are dead, # and just assume the rest are. We cleanup socket and PID files. @@ -632,7 +639,8 @@ sub mtr_mysqladmin_shutdown () { $mysql_admin_pids{$pid}= 1; } - # We wait blocking, we wait for the last one anyway + # As mysqladmin is such a simple program, we trust it to terminate. + # I.e. we wait blocking, and wait wait for them all before we go on. while (keys %mysql_admin_pids) { foreach my $pid (keys %mysql_admin_pids) @@ -651,7 +659,8 @@ sub mtr_mysqladmin_shutdown () { my $timeout= 20; # 20 seconds max my $res= 1; # If we just fall through, we are done - + # in the sense that the servers don't + # listen to their ports any longer TIME: while ( $timeout-- ) { @@ -669,6 +678,8 @@ sub mtr_mysqladmin_shutdown () { last; # If we got here, we are done } + $timeout or mtr_debug("At least one server is still listening to its port"); + sleep(5) if $::glob_win32; # FIXME next startup fails if no sleep return $res; @@ -794,8 +805,12 @@ sub sleep_until_file_created ($$$) { # ############################################################################## +# FIXME something is wrong, we sometimes terminate with "Hangup" written +# to tty, and no STDERR output telling us why. + sub mtr_exit ($) { my $code= shift; +# cluck("Called mtr_exit()"); local $SIG{HUP} = 'IGNORE'; kill('HUP', -$$); exit($code); diff --git a/mysql-test/lib/mtr_report.pl b/mysql-test/lib/mtr_report.pl index 0af34d11a3f..b9dab6b8d32 100644 --- a/mysql-test/lib/mtr_report.pl +++ b/mysql-test/lib/mtr_report.pl @@ -10,7 +10,6 @@ sub mtr_report_test_name($); sub mtr_report_test_passed($); sub mtr_report_test_failed($); sub mtr_report_test_skipped($); -sub mtr_report_test_disabled($); sub mtr_show_failed_diff ($); sub mtr_report_stats ($); From 0f7bb92df9198f9541b53e32cdf1324fe58a606c Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 17 Aug 2005 03:35:50 +0200 Subject: [PATCH 3/6] mysql-test-run.pl: Bug#11884: Corrected --start-and-exit, start the server as if the default/specified test case would have been run mysql-test/mysql-test-run.pl: Bug#11884: Corrected --start-and-exit, start the server as if the default/specified test case would have been run --- mysql-test/mysql-test-run.pl | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl index 523f31f5a5f..e976242e726 100755 --- a/mysql-test/mysql-test-run.pl +++ b/mysql-test/mysql-test-run.pl @@ -341,7 +341,6 @@ sub main () { if ( ! $glob_use_running_server ) { - if ( $opt_start_dirty ) { kill_running_server(); @@ -356,7 +355,7 @@ sub main () { } } - if ( $opt_start_and_exit or $opt_start_dirty ) + if ( $opt_start_dirty ) { if ( ndbcluster_start() ) { @@ -371,16 +370,13 @@ sub main () { mtr_error("Can't start the mysqld server"); } } + elsif ( $opt_bench ) + { + run_benchmarks(shift); # Shift what? Extra arguments?! + } else { - if ( $opt_bench ) - { - run_benchmarks(shift); # Shift what? Extra arguments?! - } - else - { - run_tests(); - } + run_tests(); } mtr_exit(0); @@ -1490,6 +1486,16 @@ sub run_testcase ($) { } } + # ---------------------------------------------------------------------- + # If --start-and-exit given, stop here to let user manually run tests + # ---------------------------------------------------------------------- + + if ( $opt_start_and_exit ) + { + mtr_report("\nServers started, exiting"); + exit(0); + } + # ---------------------------------------------------------------------- # Run the test case # ---------------------------------------------------------------------- @@ -2248,7 +2254,8 @@ Misc options script-debug Debug this script itself compress Use the compressed protocol between client and server timer Show test case execution time - start-and-exit Only initiate and start the "mysqld" servers + start-and-exit Only initiate and start the "mysqld" servers, use the startup + settings for the specified test case if any start-dirty Only start the "mysqld" servers without initiation fast Don't try to cleanup from earlier runs reorder Reorder tests to get less server restarts From 3eac44950bf8bd2887927bab15f1e670cefc26a2 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 17 Aug 2005 13:46:36 +0300 Subject: [PATCH 4/6] WL#2486 - Process NATURAL and USING joins according to SQL:2003 - Applied Monty's patch after his review of sql_base.cc sql/sql_base.cc: Applied Monty's patch after his review of WL#2486. --- sql/sql_base.cc | 301 +++++++++++++++++++++++++----------------------- 1 file changed, 158 insertions(+), 143 deletions(-) diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 4c1b8347466..57bd6985579 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -2600,30 +2600,35 @@ find_field_in_view(THD *thd, TABLE_LIST *table_list, NATURAL/USING joins RETURN - - Pointer to the found Field - - NULL if the field was not found - - WRONG_GRANT if no access rights to the found field + NULL if the field was not found + WRONG_GRANT if no access rights to the found field + # Pointer to the found Field */ static Field * find_field_in_natural_join(THD *thd, TABLE_LIST *table_ref, const char *name, const char *table_name, const char *db_name, uint length, Item **ref, bool check_grants, - bool register_tree_change, TABLE_LIST **actual_table) + bool register_tree_change, + TABLE_LIST **actual_table) { - DBUG_ENTER("find_field_in_natural_join"); - DBUG_PRINT("enter", ("natural join, field name: '%s', ref 0x%lx", - name, (ulong) ref)); - DBUG_ASSERT(table_ref->is_natural_join && table_ref->join_columns); List_iterator_fast field_it(*(table_ref->join_columns)); - Natural_join_column *nj_col= NULL; - Field *found_field= NULL; + Natural_join_column *nj_col; + Field *found_field; + DBUG_ENTER("find_field_in_natural_join"); + DBUG_PRINT("enter", ("field name: '%s', ref 0x%lx", + name, (ulong) ref)); + DBUG_ASSERT(table_ref->is_natural_join && table_ref->join_columns); + DBUG_ASSERT(*actual_table == NULL); - *actual_table= NULL; + LINT_INIT(found_field); - while ((nj_col= field_it++)) + for (;;) { + if (!(nj_col= field_it++)) + DBUG_RETURN(NULL); + if (table_name) { /* @@ -2640,7 +2645,7 @@ find_field_in_natural_join(THD *thd, TABLE_LIST *table_ref, const char *name, if (db_name && db_name[0]) { const char *cur_db_name= nj_col->db_name(); - if (cur_db_name && cur_db_name && strcmp(db_name, cur_db_name)) + if (cur_db_name && strcmp(db_name, cur_db_name)) continue; } } @@ -2649,9 +2654,6 @@ find_field_in_natural_join(THD *thd, TABLE_LIST *table_ref, const char *name, break; } - if (!nj_col) - DBUG_RETURN(NULL); - #ifndef NO_EMBEDDED_ACCESS_CHECKS if (check_grants && nj_col->check_grants(thd, name, length)) DBUG_RETURN(WRONG_GRANT); @@ -2668,13 +2670,14 @@ find_field_in_natural_join(THD *thd, TABLE_LIST *table_ref, const char *name, DBUG_RETURN(NULL); DBUG_ASSERT(nj_col->table_field == NULL); if (nj_col->table_ref->schema_table_reformed) + { /* Translation table items are always Item_fields and fixed already('mysql_schema_table' function). So we can return ->field. It is used only for 'show & where' commands. */ DBUG_RETURN(((Item_field*) (nj_col->view_field->item))->field); - + } if (register_tree_change) thd->change_item_tree(ref, item); else @@ -2710,8 +2713,8 @@ find_field_in_natural_join(THD *thd, TABLE_LIST *table_ref, const char *name, lookup for fields in prepared tables) RETURN - 0 field is not found - # pointer to field + 0 field is not found + # pointer to field */ Field * @@ -2719,10 +2722,10 @@ find_field_in_table(THD *thd, TABLE *table, const char *name, uint length, bool check_grants, bool allow_rowid, uint *cached_field_index_ptr) { - DBUG_ENTER("find_field_in_table"); - DBUG_PRINT("enter", ("table: '%s', field name: '%s'", table->alias, name)); Field **field_ptr, *field; uint cached_field_index= *cached_field_index_ptr; + DBUG_ENTER("find_field_in_table"); + DBUG_PRINT("enter", ("table: '%s', field name: '%s'", table->alias, name)); /* We assume here that table->field < NO_CACHED_FIELD_INDEX = UINT_MAX */ if (cached_field_index < table->s->fields && @@ -2760,7 +2763,7 @@ find_field_in_table(THD *thd, TABLE *table, const char *name, uint length, if (check_grants && check_grant_column(thd, &table->grant, table->s->db, table->s->table_name, name, length)) - DBUG_RETURN(WRONG_GRANT); + field= WRONG_GRANT; #endif DBUG_RETURN(field); } @@ -2819,22 +2822,23 @@ find_field_in_table_ref(THD *thd, TABLE_LIST *table_list, simply compare the qualifying table and database names with the ones of 'table_list' because each field in such a join may originate from a different table. - TODO: Ensure that db and tables->db always points to something ! + TODO: Ensure that table_name, db_name and tables->db always points to + something ! */ if (!table_list->is_natural_join && - (table_name && table_name[0] && - my_strcasecmp(table_alias_charset, table_list->alias, table_name) || + table_name && table_name[0] && + (my_strcasecmp(table_alias_charset, table_list->alias, table_name) || (db_name && db_name[0] && table_list->db && table_list->db[0] && strcmp(db_name, table_list->db)))) DBUG_RETURN(0); + *actual_table= NULL; if (table_list->field_translation) { if ((fld= find_field_in_view(thd, table_list, name, item_name, length, - ref, check_grants_view, register_tree_change))) + ref, check_grants_view, + register_tree_change))) *actual_table= table_list; - else - *actual_table= NULL; } else if (table_list->is_natural_join) fld= find_field_in_natural_join(thd, table_list, name, table_name, @@ -2848,8 +2852,6 @@ find_field_in_table_ref(THD *thd, TABLE_LIST *table_list, check_grants_table, allow_rowid, cached_field_index_ptr))) *actual_table= table_list; - else - *actual_table= NULL; #ifndef NO_EMBEDDED_ACCESS_CHECKS /* check for views with temporary table algorithm */ if (check_grants_view && table_list->view && @@ -2858,7 +2860,7 @@ find_field_in_table_ref(THD *thd, TABLE_LIST *table_list, table_list->view_db.str, table_list->view_name.str, name, length)) - DBUG_RETURN(WRONG_GRANT); + fld= WRONG_GRANT; #endif } @@ -2982,18 +2984,15 @@ find_field_in_tables(THD *thd, Item_ident *item, db= name_buff; } + if (last_table) + last_table= last_table->next_name_resolution_table; + /* The field we search for is qualified with a table name and optional db. */ if (table_name && table_name[0]) { - bool found_table=0; - for ( ; - (cur_table && - (last_table ? - (cur_table != last_table->next_name_resolution_table) : TRUE)); + for (; cur_table != last_table ; cur_table= cur_table->next_name_resolution_table) { - DBUG_ASSERT(cur_table); - found_table= 1; Field *cur_field= find_field_in_table_ref(thd, cur_table, name, item->name, table_name, db, length, ref, @@ -3002,8 +3001,8 @@ find_field_in_tables(THD *thd, Item_ident *item, want_privilege) && check_privileges), (test(cur_table->grant. - want_privilege) - && check_privileges), + want_privilege) && + check_privileges), 1, &(item->cached_field_index), register_tree_change, &actual_table); @@ -3032,8 +3031,12 @@ find_field_in_tables(THD *thd, Item_ident *item, } if (found) return found; - if (!found_table && (report_error == REPORT_ALL_ERRORS || - report_error == REPORT_EXCEPT_NON_UNIQUE)) + /* + If there were no tables to search, we wouldn't go through the loop and + cur_table wouldn't be updated by the loop increment part. + */ + if (cur_table == first_table && (report_error == REPORT_ALL_ERRORS || + report_error == REPORT_EXCEPT_NON_UNIQUE)) { char buff[NAME_LEN*2+1]; if (db && db[0]) @@ -3054,13 +3057,9 @@ find_field_in_tables(THD *thd, Item_ident *item, /* The field we search for is not qualified. */ allow_rowid= cur_table && !cur_table->next_local; - for ( ; - (cur_table && - (last_table ? - (cur_table != last_table->next_name_resolution_table) : TRUE)); - cur_table= cur_table->next_name_resolution_table) + for (; cur_table != last_table ; + cur_table= cur_table->next_name_resolution_table) { - DBUG_ASSERT(cur_table); Field *cur_field= find_field_in_table_ref(thd, cur_table, name, item->name, NULL, NULL, length, ref, (cur_table->table && @@ -3359,20 +3358,21 @@ test_if_string_in_list(const char *find, List *str_list) is resolved only the supplied 'table_ref'. RETURN - FALSE - if all OK - TRUE - otherwise + FALSE if all OK + TRUE otherwise */ static bool set_new_item_local_context(THD *thd, Item_ident *item, TABLE_LIST *table_ref) { Name_resolution_context *context; + if (!(context= (Name_resolution_context*) thd->calloc(sizeof(Name_resolution_context)))) return TRUE; context->init(); - context->first_name_resolution_table= table_ref; - context->last_name_resolution_table= table_ref; + context->first_name_resolution_table= + context->last_name_resolution_table= table_ref; item->context= context; return FALSE; } @@ -3405,8 +3405,8 @@ set_new_item_local_context(THD *thd, Item_ident *item, TABLE_LIST *table_ref) called for the previous level of NATURAL/USING joins. RETURN - TRUE - if error when some common column is non-unique, or out of memory - FALSE - if OK + TRUE error when some common column is non-unique, or out of memory + FALSE OK */ static bool @@ -3415,16 +3415,16 @@ mark_common_columns(THD *thd, TABLE_LIST *table_ref_1, TABLE_LIST *table_ref_2, { Field_iterator_table_ref it_1, it_2; Natural_join_column *nj_col_1, *nj_col_2; - const char *field_name_1, *field_name_2; - *found_using_fields= 0; - bool add_columns= TRUE; + const char *field_name_1; Query_arena *arena, backup; + bool add_columns= TRUE; bool result= TRUE; DBUG_ENTER("mark_common_columns"); - DBUG_PRINT("info", ("operand_1: %s, operand_2: %s", + DBUG_PRINT("info", ("operand_1: %s operand_2: %s", table_ref_1->alias, table_ref_2->alias)); + *found_using_fields= 0; arena= thd->change_arena_if_needed(&backup); /* @@ -3447,25 +3447,31 @@ mark_common_columns(THD *thd, TABLE_LIST *table_ref_1, TABLE_LIST *table_ref_2, for (it_1.set(table_ref_1); !it_1.end_of_fields(); it_1.next()) { bool is_created_1; + bool found= FALSE; if (!(nj_col_1= it_1.get_or_create_column_ref(thd, &is_created_1))) goto err; field_name_1= nj_col_1->name(); - bool found= FALSE; /* If nj_col_1 was just created add it to the list of join columns. */ if (is_created_1) table_ref_1->join_columns->push_back(nj_col_1); - /* Find a field with the same name in table_ref_2. */ + /* + Find a field with the same name in table_ref_2. + + Note that for the second loop, it_2.set() will iterate over + table_ref_2->join_columns and not generate any new elements or + lists. + */ nj_col_2= NULL; - field_name_2= NULL; for (it_2.set(table_ref_2); !it_2.end_of_fields(); it_2.next()) { bool is_created_2; Natural_join_column *cur_nj_col_2; + const char *cur_field_name_2; if (!(cur_nj_col_2= it_2.get_or_create_column_ref(thd, &is_created_2))) goto err; - const char *cur_field_name_2= cur_nj_col_2->name(); + cur_field_name_2= cur_nj_col_2->name(); /* If nj_col_1 was just created add it to the list of join columns. */ if (add_columns && is_created_2) @@ -3480,14 +3486,14 @@ mark_common_columns(THD *thd, TABLE_LIST *table_ref_1, TABLE_LIST *table_ref_2, goto err; } nj_col_2= cur_nj_col_2; - field_name_2= cur_field_name_2; found= TRUE; } } + /* Force it_2.set() to use table_ref_2->join_columns. */ table_ref_2->is_join_columns_complete= TRUE; add_columns= FALSE; if (!found) - continue; + continue; // No matching field /* field_1 and field_2 have the same names. Check if they are in the USING @@ -3496,8 +3502,7 @@ mark_common_columns(THD *thd, TABLE_LIST *table_ref_1, TABLE_LIST *table_ref_2, */ if (nj_col_2 && (!using_fields || - (using_fields && - test_if_string_in_list(field_name_1, using_fields)))) + test_if_string_in_list(field_name_1, using_fields))) { Item *item_1= nj_col_1->create_item(thd); Item *item_2= nj_col_2->create_item(thd); @@ -3505,12 +3510,17 @@ mark_common_columns(THD *thd, TABLE_LIST *table_ref_1, TABLE_LIST *table_ref_2, Field *field_2= nj_col_2->field(); Item_ident *item_ident_1, *item_ident_2; Name_resolution_context *context_1, *context_2; + Item_func_eq *eq_cond; + DBUG_PRINT("info", ("new equi-join condition: %s.%s = %s.%s", table_ref_1->alias, field_1->field_name, table_ref_2->alias, field_2->field_name)); + if (!item_1 || !item_2) + goto err; // out of memory + /* - The first assert guarantees that the two created items are of + The following assert checks that the two created items are of type Item_ident. */ DBUG_ASSERT(!thd->lex->current_select->no_wrap_view_item); @@ -3535,24 +3545,21 @@ mark_common_columns(THD *thd, TABLE_LIST *table_ref_1, TABLE_LIST *table_ref_2, resolution of these items, and to enable proper name resolution of the items during the execute phase of PS. */ - if (set_new_item_local_context(thd, item_ident_1, table_ref_1)) - goto err; - if (set_new_item_local_context(thd, item_ident_2, table_ref_2)) + if (set_new_item_local_context(thd, item_ident_1, table_ref_1) || + set_new_item_local_context(thd, item_ident_2, table_ref_2)) goto err; - Item_func_eq *eq_cond= new Item_func_eq(item_ident_1, item_ident_2); - if (!eq_cond) - goto err; /* Out of memory. */ + if (!(eq_cond= new Item_func_eq(item_ident_1, item_ident_2))) + goto err; /* Out of memory. */ /* Add the new equi-join condition to the ON clause. Notice that fix_fields() is applied to all ON conditions in setup_conds() so we don't do it here. */ - if (table_ref_1->outer_join & JOIN_TYPE_RIGHT) - add_join_on(table_ref_1, eq_cond); - else - add_join_on(table_ref_2, eq_cond); + add_join_on((table_ref_1->outer_join & JOIN_TYPE_RIGHT ? + table_ref_1 : table_ref_2), + eq_cond); nj_col_1->is_common= nj_col_2->is_common= TRUE; nj_col_1->is_coalesced= nj_col_2->is_coalesced= TRUE; @@ -3624,8 +3631,8 @@ err: for the join that is being processed. RETURN - TRUE - if error when some common column is ambiguous - FALSE - if OK + TRUE error: Some common column is ambiguous + FALSE OK */ static bool @@ -3640,16 +3647,15 @@ store_natural_using_join_columns(THD *thd, TABLE_LIST *natural_using_join, bool is_created; Query_arena *arena, backup; bool result= TRUE; - + List *non_join_columns; DBUG_ENTER("store_natural_using_join_columns"); + DBUG_ASSERT(!natural_using_join->join_columns); + arena= thd->change_arena_if_needed(&backup); - List *non_join_columns; - if (!(non_join_columns= new List)) - goto err; - DBUG_ASSERT(!natural_using_join->join_columns); - if (!(natural_using_join->join_columns= new List)) + if (!(non_join_columns= new List) || + !(natural_using_join->join_columns= new List)) goto err; /* Append the columns of the first join operand. */ @@ -3657,6 +3663,10 @@ store_natural_using_join_columns(THD *thd, TABLE_LIST *natural_using_join, { if (!(nj_col_1= it_1.get_or_create_column_ref(thd, &is_created))) goto err; + /* + The following assert checks that mark_common_columns() was run and + we created the list table_ref_1->join_columns. + */ DBUG_ASSERT(!is_created); if (nj_col_1->is_common) { @@ -3679,23 +3689,23 @@ store_natural_using_join_columns(THD *thd, TABLE_LIST *natural_using_join, List_iterator_fast using_fields_it(*using_fields); while ((using_field_name= using_fields_it++)) { - const char *using_field_name_ptr= using_field_name->ptr(); + const char *using_field_name_ptr= using_field_name->c_ptr(); List_iterator_fast it(*(natural_using_join->join_columns)); Natural_join_column *common_field; - bool found= FALSE; - while ((common_field= it++)) + + for (;;) { + /* If reached the end of fields, and none was found, report error. */ + if (!(common_field= it++)) + { + my_error(ER_BAD_FIELD_ERROR, MYF(0), using_field_name_ptr, + current_thd->where); + goto err; + } if (!my_strcasecmp(system_charset_info, common_field->name(), using_field_name_ptr)) - found= TRUE; - } - if (!found) - { - my_error(ER_BAD_FIELD_ERROR, MYF(0), using_field_name_ptr, - current_thd->where); - delete non_join_columns; - goto err; + break; // Found match } } } @@ -3705,22 +3715,24 @@ store_natural_using_join_columns(THD *thd, TABLE_LIST *natural_using_join, { if (!(nj_col_2= it_2.get_or_create_column_ref(thd, &is_created))) goto err; + /* + The following assert checks that mark_common_columns() was run and + we created the list table_ref_2->join_columns. + */ DBUG_ASSERT(!is_created); if (!nj_col_2->is_common) non_join_columns->push_back(nj_col_2); else + { /* Reset the common columns for the next call to mark_common_columns. */ nj_col_2->is_common= FALSE; - + } } if (non_join_columns->elements > 0) natural_using_join->join_columns->concat(non_join_columns); - else - delete non_join_columns; natural_using_join->is_join_columns_complete= TRUE; - result= FALSE; err: @@ -3729,6 +3741,7 @@ err: DBUG_RETURN(result); } + /* Precompute and store the row types of the top-most NATURAL/USING joins. @@ -3755,8 +3768,8 @@ err: from the right to the left in the FROM clause. RETURN - TRUE - if error - FALSE - if OK + TRUE Error + FALSE OK */ static bool @@ -3765,22 +3778,22 @@ store_top_level_join_columns(THD *thd, TABLE_LIST *table_ref, TABLE_LIST *right_neighbor) { DBUG_ENTER("store_top_level_join_columns"); + /* Call the procedure recursively for each nested table reference. */ if (table_ref->nested_join) { List_iterator_fast nested_it(table_ref->nested_join->join_list); - TABLE_LIST *cur_table_ref; TABLE_LIST *cur_left_neighbor= nested_it++; TABLE_LIST *cur_right_neighbor= NULL; + while (cur_left_neighbor) { - cur_table_ref= cur_left_neighbor; + TABLE_LIST *cur_table_ref= cur_left_neighbor; cur_left_neighbor= nested_it++; - if (cur_table_ref->nested_join && - store_top_level_join_columns(thd, cur_table_ref, - cur_left_neighbor, cur_right_neighbor)) - DBUG_RETURN(TRUE); - cur_right_neighbor= cur_table_ref; + if (cur_table_ref->nested_join && + store_top_level_join_columns(thd, cur_table_ref, + cur_left_neighbor, cur_right_neighbor)) + DBUG_RETURN(TRUE); cur_right_neighbor= cur_table_ref; } } @@ -3800,8 +3813,6 @@ store_top_level_join_columns(THD *thd, TABLE_LIST *table_ref, */ TABLE_LIST *table_ref_2= operand_it++; /* Second NATURAL join operand.*/ TABLE_LIST *table_ref_1= operand_it++; /* First NATURAL join operand. */ - TABLE_LIST *last_leaf_on_the_left= NULL; - TABLE_LIST *first_leaf_on_the_right= NULL; List *using_fields= table_ref->join_using_fields; uint found_using_fields; @@ -3838,11 +3849,13 @@ store_top_level_join_columns(THD *thd, TABLE_LIST *table_ref, /* Change this table reference to become a leaf for name resolution. */ if (left_neighbor) { + TABLE_LIST *last_leaf_on_the_left; last_leaf_on_the_left= left_neighbor->last_leaf_for_name_resolution(); last_leaf_on_the_left->next_name_resolution_table= table_ref; } if (right_neighbor) { + TABLE_LIST *first_leaf_on_the_right; first_leaf_on_the_right= right_neighbor->first_leaf_for_name_resolution(); table_ref->next_name_resolution_table= first_leaf_on_the_right; } @@ -3874,10 +3887,11 @@ store_top_level_join_columns(THD *thd, TABLE_LIST *table_ref, to the left in the FROM clause. RETURN - TRUE - if error - FALSE - if OK + TRUE Error + FALSE OK */ -static bool setup_natural_join_row_types(THD *thd, List *from_clause, +static bool setup_natural_join_row_types(THD *thd, + List *from_clause, Name_resolution_context *context) { thd->where= "from clause"; @@ -3891,11 +3905,12 @@ static bool setup_natural_join_row_types(THD *thd, List *from_clause List_iterator_fast table_ref_it(*from_clause); TABLE_LIST *table_ref; /* Current table reference. */ /* Table reference to the left of the current. */ - TABLE_LIST *left_neighbor= table_ref_it++; + TABLE_LIST *left_neighbor; /* Table reference to the right of the current. */ TABLE_LIST *right_neighbor= NULL; - while (left_neighbor) + /* Note that tables in the list are in reversed order */ + for (left_neighbor= table_ref_it++; left_neighbor ; ) { table_ref= left_neighbor; left_neighbor= table_ref_it++; @@ -3914,7 +3929,7 @@ static bool setup_natural_join_row_types(THD *thd, List *from_clause /* Store the top-most, left-most NATURAL/USING join, so that we start the search from that one instead of context->table_list. At this point - right_neigbor points to the left-most top-level table reference in the + right_neighbor points to the left-most top-level table reference in the FROM clause. */ DBUG_ASSERT(right_neighbor); @@ -4249,8 +4264,7 @@ bool get_key_map_from_key_list(key_map *map, TABLE *table, for all columns 1 If any privilege is ok RETURN - 0 ok - 'it' is updated to point at last inserted + 0 ok 'it' is updated to point at last inserted 1 error. Error message is generated but not sent to client */ @@ -4263,8 +4277,7 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name, bool found; char name_buff[NAME_LEN+1]; DBUG_ENTER("insert_fields"); - DBUG_PRINT("arena", ("insert_fields: current arena: 0x%lx", - (ulong)thd->current_arena)); + DBUG_PRINT("arena", ("current arena: 0x%lx", (ulong)thd->current_arena)); if (db_name && lower_case_table_names) { @@ -4297,8 +4310,8 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name, */ if (!tables->is_natural_join) { - if (table_name && my_strcasecmp(table_alias_charset, table_name, tables->alias) - || + if (table_name && my_strcasecmp(table_alias_charset, table_name, + tables->alias) || (db_name && strcmp(tables->db,db_name))) continue; } @@ -4312,7 +4325,8 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name, { field_iterator.set(tables); if (check_grant_all_columns(thd, SELECT_ACL, field_iterator.grant(), - field_iterator.db_name(), field_iterator.table_name(), + field_iterator.db_name(), + field_iterator.table_name(), &field_iterator)) DBUG_RETURN(TRUE); } @@ -4336,21 +4350,19 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name, for (; !field_iterator.end_of_fields(); field_iterator.next()) { - Item *not_used_item; - uint not_used_field_index= NO_CACHED_FIELD_INDEX; - const char *field_name= field_iterator.name(); Item *item; - /* If this is a column of a NATURAL/USING join, and the star was qualified - with a table (and database) name, check if the column is not a coalesced - one, and if not, that is belongs to the same table. + /* + If this is a column of a NATURAL/USING join, and the star was + qualified with a table (and database) name, check if the + column is not a coalesced one, and if not, that is belongs to + the same table. */ if (tables->is_natural_join && table_name) { - if (field_iterator.is_coalesced() - || - my_strcasecmp(table_alias_charset, table_name, field_iterator.table_name()) - || + if (field_iterator.is_coalesced() || + my_strcasecmp(table_alias_charset, table_name, + field_iterator.table_name()) || (db_name && strcmp(db_name, field_iterator.db_name()))) continue; } @@ -4360,8 +4372,8 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name, if (!found) { - it->replace(item); /* Replace '*' with the first found item. */ found= TRUE; + it->replace(item); /* Replace '*' with the first found item. */ } else it->after(item); /* Add 'item' to the SELECT list. */ @@ -4371,8 +4383,9 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name, Set privilege information for the fields of newly created views. We have that (any_priviliges == TRUE) if and only if we are creating a view. In the time of view creation we can't use the MERGE algorithm, - therefore if 'tables' is itself a view, it is represented by a temporary - table. Thus in this case we can be sure that 'item' is an Item_field. + therefore if 'tables' is itself a view, it is represented by a + temporary table. Thus in this case we can be sure that 'item' is an + Item_field. */ if (any_privileges) { @@ -4381,6 +4394,7 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name, DBUG_ASSERT(item->type() == Item::FIELD_ITEM); Item_field *fld= (Item_field*) item; const char *table_name= field_iterator.table_name(); + if (!tables->schema_table && !(fld->have_privileges= (get_column_grant(thd, field_iterator.grant(), @@ -4414,11 +4428,12 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name, bool is_created; TABLE *field_table; /* - In this case we are shure that the column ref will not be created + In this case we are sure that the column ref will not be created because it was already created and stored with the natural join. - */ + */ Natural_join_column *nj_col; - if (!(nj_col= field_iterator.get_or_create_column_ref(thd, &is_created))) + if (!(nj_col= field_iterator.get_or_create_column_ref(thd, + &is_created))) DBUG_RETURN(TRUE); DBUG_ASSERT(nj_col->table_field && !is_created); field_table= nj_col->table_ref->table; @@ -4450,9 +4465,9 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name, DBUG_RETURN(FALSE); /* - TODO: in the case when we skipped all columns because there was a qualified - '*', and all columns were coalesced, we have to give a more meaningful message - than ER_BAD_TABLE_ERROR. + TODO: in the case when we skipped all columns because there was a + qualified '*', and all columns were coalesced, we have to give a more + meaningful message than ER_BAD_TABLE_ERROR. */ if (!table_name) my_message(ER_NO_TABLES_USED, ER(ER_NO_TABLES_USED), MYF(0)); From 06ab6b88764afc6cfcc7358296d402a29de7bc79 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 17 Aug 2005 14:20:01 +0300 Subject: [PATCH 5/6] Fixed code formatting. --- sql/sql_base.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sql/sql_base.cc b/sql/sql_base.cc index bb3209d56c5..4d4f9ae48da 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -3793,7 +3793,8 @@ store_top_level_join_columns(THD *thd, TABLE_LIST *table_ref, if (cur_table_ref->nested_join && store_top_level_join_columns(thd, cur_table_ref, cur_left_neighbor, cur_right_neighbor)) - DBUG_RETURN(TRUE); cur_right_neighbor= cur_table_ref; + DBUG_RETURN(TRUE); + cur_right_neighbor= cur_table_ref; } } From d33b968f0304161bdd0c74764f71630862e33a9a Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 17 Aug 2005 17:19:31 +0300 Subject: [PATCH 6/6] WL#2486 - natural and using join according to SQL:2003 - fixed a problem with RIGHT JOIN ON and enabled corresponding tests in select.test - fixed a memory leak mysql-test/r/select.result: Fixed a problem with RIGHT JOIN ON queries, enabling the corresponding tests. mysql-test/t/select.test: Fixed a problem with RIGHT JOIN ON queries, enabling the corresponding tests. sql/sql_base.cc: Fixed a problem with RIGHT JOINs that have operand(s) which are NATURAL JOIN(s). sql/table.h: Inherit from Sql_alloc for proper memory allocation. The change fixes a memory leak. --- mysql-test/r/select.result | 17 +++++++++++++++++ mysql-test/t/select.test | 6 ++---- sql/sql_base.cc | 27 +++++++++++++++++++++++---- sql/table.h | 2 +- 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/mysql-test/r/select.result b/mysql-test/r/select.result index 580ccc44a7c..1b35df534a0 100644 --- a/mysql-test/r/select.result +++ b/mysql-test/r/select.result @@ -2218,6 +2218,23 @@ a 1 2 3 +select * from (t1 as t2 left join t1 as t3 using (a)) right join t1 on t1.a>1; +a a +NULL 1 +1 2 +2 2 +3 2 +1 3 +2 3 +3 3 +select * from t1 right join (t1 as t2 left join t1 as t3 using (a)) on t1.a>1; +a a +2 1 +3 1 +2 2 +3 2 +2 3 +3 3 select * from (t1 as t2 left join t1 as t3 using (a)) right outer join t1 using ( a ); a 1 diff --git a/mysql-test/t/select.test b/mysql-test/t/select.test index e0c4d66633b..390c4372f16 100644 --- a/mysql-test/t/select.test +++ b/mysql-test/t/select.test @@ -1850,10 +1850,8 @@ select * from t1 left join (t1 as t2 left join t1 as t3 using (a)) using ( a ); select * from (t1 as t2 left join t1 as t3 using (a)) natural left join t1; select * from t1 natural left join (t1 as t2 left join t1 as t3 using (a)); # right join on -# TODO: WL#2486 - there is a problem in the order of tables in RIGHT JOIN -# check how we set next_name_resolution_table -# select * from (t1 as t2 left join t1 as t3 using (a)) right join t1 on t1.a>1; -# select * from t1 right join (t1 as t2 left join t1 as t3 using (a)) on t1.a>1; +select * from (t1 as t2 left join t1 as t3 using (a)) right join t1 on t1.a>1; +select * from t1 right join (t1 as t2 left join t1 as t3 using (a)) on t1.a>1; # right [outer] joing using select * from (t1 as t2 left join t1 as t3 using (a)) right outer join t1 using ( a ); select * from t1 right outer join (t1 as t2 left join t1 as t3 using (a)) using ( a ); diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 4d4f9ae48da..98ce12eb7de 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -3790,10 +3790,29 @@ store_top_level_join_columns(THD *thd, TABLE_LIST *table_ref, { TABLE_LIST *cur_table_ref= cur_left_neighbor; cur_left_neighbor= nested_it++; - if (cur_table_ref->nested_join && - store_top_level_join_columns(thd, cur_table_ref, - cur_left_neighbor, cur_right_neighbor)) - DBUG_RETURN(TRUE); + /* + The order of RIGHT JOIN operands is reversed in 'join list' to + transform it into a LEFT JOIN. However, in this procedure we need + the join operands in their lexical order, so below we reverse the + join operands. Notice that this happens only in the first loop, and + not in the second one, as in the second loop cur_left_neighbor == NULL. + This is the correct behavior, because the second loop + sets cur_table_ref reference correctly after the join operands are + swapped in the first loop. + */ + if (cur_left_neighbor && + cur_table_ref->outer_join & JOIN_TYPE_RIGHT) + { + DBUG_ASSERT(cur_table_ref); + /* This can happen only for JOIN ... ON. */ + DBUG_ASSERT(table_ref->nested_join->join_list.elements == 2); + swap_variables(TABLE_LIST*, cur_left_neighbor, cur_table_ref); + } + + if (cur_table_ref->nested_join && + store_top_level_join_columns(thd, cur_table_ref, + cur_left_neighbor, cur_right_neighbor)) + DBUG_RETURN(TRUE); cur_right_neighbor= cur_table_ref; } } diff --git a/sql/table.h b/sql/table.h index bb9de5dc86d..3d4f02e389b 100644 --- a/sql/table.h +++ b/sql/table.h @@ -374,7 +374,7 @@ struct Field_translator Field (for tables), or a Field_translator (for views). */ -class Natural_join_column +class Natural_join_column: public Sql_alloc { public: Field_translator *view_field; /* Column reference of merge view. */