From f7cf8e57c12f435980bafb4f4b9c657a722345a4 Mon Sep 17 00:00:00 2001 From: Alexey Kopytov Date: Fri, 27 Feb 2009 11:26:06 +0200 Subject: [PATCH 1/2] Fix for bug #40552: Race condition around default_directories in load_defaults() load_defaults(), my_search_option_files() and my_print_default_files() utilized a global variable containing a pointer to thread local memory. This could lead to race conditions when those functions were called with high concurrency. Fixed by changing the interface of the said functions to avoid the necessity for using a global variable. Since we cannot change load_defaults() prototype for API compatibility reasons, it was renamed my_load_defaults(). Now load_defaults() is a thread-unsafe wrapper around a thread-safe version, my_load_defaults(). mysys/default.c: 1. Added a thread-safe version of load_defaults(), changed load_defaults() with the old interface to be a thread-unsafe wrapper around the thread-safe version. 2. Always use a private MEM_ROOT in my_print_default_files, don't use a global variable. sql-common/client.c: Use a thread-safe version of load_defaults(). --- include/my_sys.h | 7 +- mysys/default.c | 67 ++++++++++++++----- server-tools/instance-manager/instance_map.cc | 3 +- server-tools/instance-manager/options.cc | 4 +- server-tools/instance-manager/options.h | 3 + sql-common/client.c | 2 +- 6 files changed, 66 insertions(+), 20 deletions(-) diff --git a/include/my_sys.h b/include/my_sys.h index 5335b65822f..180bfe0f07d 100644 --- a/include/my_sys.h +++ b/include/my_sys.h @@ -842,14 +842,17 @@ extern void *memdup_root(MEM_ROOT *root,const void *str, size_t len); extern int get_defaults_options(int argc, char **argv, char **defaults, char **extra_defaults, char **group_suffix); +extern int my_load_defaults(const char *conf_file, const char **groups, + int *argc, char ***argv, const char ***); extern int load_defaults(const char *conf_file, const char **groups, - int *argc, char ***argv); + int *argc, char ***argv); extern int modify_defaults_file(const char *file_location, const char *option, const char *option_value, const char *section_name, int remove_option); extern int my_search_option_files(const char *conf_file, int *argc, char ***argv, uint *args_used, - Process_option_func func, void *func_ctx); + Process_option_func func, void *func_ctx, + const char **default_directories); extern void free_defaults(char **argv); extern void my_print_default_files(const char *conf_file); extern void print_defaults(const char *conf_file, const char **groups); diff --git a/mysys/default.c b/mysys/default.c index 6b2b31d43ec..cdd9462c1d1 100644 --- a/mysys/default.c +++ b/mysys/default.c @@ -151,7 +151,7 @@ static char *remove_end_comment(char *ptr); int my_search_option_files(const char *conf_file, int *argc, char ***argv, uint *args_used, Process_option_func func, - void *func_ctx) + void *func_ctx, const char **default_directories) { const char **dirs, *forced_default_file, *forced_extra_defaults; int error= 0; @@ -359,9 +359,8 @@ int get_defaults_options(int argc, char **argv, return org_argc - argc; } - /* - Read options from configurations files + Wrapper around my_load_defaults() for interface compatibility. SYNOPSIS load_defaults() @@ -372,6 +371,35 @@ int get_defaults_options(int argc, char **argv, argc Pointer to argc of original program argv Pointer to argv of original program + NOTES + + This function is NOT thread-safe as it uses a global pointer internally. + See also notes for my_load_defaults(). + + RETURN + 0 ok + 1 The given conf_file didn't exists +*/ +int load_defaults(const char *conf_file, const char **groups, + int *argc, char ***argv) +{ + return my_load_defaults(conf_file, groups, argc, argv, &default_directories); +} + +/* + Read options from configurations files + + SYNOPSIS + my_load_defaults() + conf_file Basename for configuration file to search for. + If this is a path, then only this file is read. + groups Which [group] entrys to read. + Points to an null terminated array of pointers + argc Pointer to argc of original program + argv Pointer to argv of original program + default_directories Pointer to a location where a pointer to the list + of default directories will be stored + IMPLEMENTATION Read options from configuration files and put them BEFORE the arguments @@ -386,13 +414,18 @@ int get_defaults_options(int argc, char **argv, that was put in *argv RETURN - 0 ok - 1 The given conf_file didn't exists + - If successful, 0 is returned. If 'default_directories' is not NULL, + a pointer to the array of default directory paths is stored to a location + it points to. That stored value must be passed to my_search_option_files() + later. + + - 1 is returned if the given conf_file didn't exist. In this case, the + value pointed to by default_directories is undefined. */ -int load_defaults(const char *conf_file, const char **groups, - int *argc, char ***argv) +int my_load_defaults(const char *conf_file, const char **groups, + int *argc, char ***argv, const char ***default_directories) { DYNAMIC_ARRAY args; TYPELIB group; @@ -402,10 +435,11 @@ int load_defaults(const char *conf_file, const char **groups, MEM_ROOT alloc; char *ptr,**res; struct handle_option_ctx ctx; + const char **dirs; DBUG_ENTER("load_defaults"); init_alloc_root(&alloc,512,0); - if ((default_directories= init_default_directories(&alloc)) == NULL) + if ((dirs= init_default_directories(&alloc)) == NULL) goto err; /* Check if the user doesn't want any default option processing @@ -426,6 +460,8 @@ int load_defaults(const char *conf_file, const char **groups, (*argc)--; *argv=res; *(MEM_ROOT*) ptr= alloc; /* Save alloc root for free */ + if (default_directories) + *default_directories= dirs; DBUG_RETURN(0); } @@ -444,7 +480,8 @@ int load_defaults(const char *conf_file, const char **groups, ctx.group= &group; error= my_search_option_files(conf_file, argc, argv, &args_used, - handle_default_option, (void *) &ctx); + handle_default_option, (void *) &ctx, + dirs); /* Here error contains <> 0 only if we have a fully specified conf_file or a forced default file @@ -490,6 +527,10 @@ int load_defaults(const char *conf_file, const char **groups, puts(""); exit(0); } + + if (error == 0 && default_directories) + *default_directories= dirs; + DBUG_RETURN(error); err: @@ -895,15 +936,11 @@ void my_print_default_files(const char *conf_file) fputs(conf_file,stdout); else { - /* - If default_directories is already initialized, use it. Otherwise, - use a private MEM_ROOT. - */ - const char **dirs = default_directories; + const char **dirs; MEM_ROOT alloc; init_alloc_root(&alloc,512,0); - if (!dirs && (dirs= init_default_directories(&alloc)) == NULL) + if ((dirs= init_default_directories(&alloc)) == NULL) { fputs("Internal error initializing default directories list", stdout); } diff --git a/server-tools/instance-manager/instance_map.cc b/server-tools/instance-manager/instance_map.cc index d7328d51cfe..b137370b50a 100644 --- a/server-tools/instance-manager/instance_map.cc +++ b/server-tools/instance-manager/instance_map.cc @@ -536,7 +536,8 @@ int Instance_map::load() */ if (my_search_option_files(Options::Main::config_file, &argc, (char ***) &argv, &args_used, - process_option, (void*) this)) + process_option, (void*) this, + Options::default_directories)) log_info("Falling back to compiled-in defaults."); return complete_initialization(); diff --git a/server-tools/instance-manager/options.cc b/server-tools/instance-manager/options.cc index 7eba3187dd9..6f084e7c63e 100644 --- a/server-tools/instance-manager/options.cc +++ b/server-tools/instance-manager/options.cc @@ -86,6 +86,7 @@ const char *Options::Main::bind_address= NULL; /* No default value */ uint Options::Main::monitoring_interval= DEFAULT_MONITORING_INTERVAL; uint Options::Main::port_number= DEFAULT_PORT; my_bool Options::Main::mysqld_safe_compatible= FALSE; +const char **Options::default_directories= NULL; /* Options::User_management */ @@ -439,7 +440,8 @@ int Options::load(int argc, char **argv) log_info("Loading config file '%s'...", (const char *) Main::config_file); - load_defaults(Main::config_file, default_groups, &argc, &saved_argv); + my_load_defaults(Main::config_file, default_groups, &argc, + &saved_argv, &default_directories); if ((handle_options(&argc, &saved_argv, my_long_options, get_one_option))) return ERR_INVALID_USAGE; diff --git a/server-tools/instance-manager/options.h b/server-tools/instance-manager/options.h index 0202ca271c9..5d4df51faae 100644 --- a/server-tools/instance-manager/options.h +++ b/server-tools/instance-manager/options.h @@ -91,6 +91,9 @@ struct Options #endif public: + /* Array of paths to be passed to my_search_option_files() later */ + static const char **default_directories; + static int load(int argc, char **argv); static void cleanup(); diff --git a/sql-common/client.c b/sql-common/client.c index 63c746a3f5a..91a47b6a6f8 100644 --- a/sql-common/client.c +++ b/sql-common/client.c @@ -1021,7 +1021,7 @@ void mysql_read_default_options(struct st_mysql_options *options, argc=1; argv=argv_buff; argv_buff[0]= (char*) "client"; groups[0]= (char*) "client"; groups[1]= (char*) group; groups[2]=0; - load_defaults(filename, groups, &argc, &argv); + my_load_defaults(filename, groups, &argc, &argv, NULL); if (argc != 1) /* If some default option */ { char **option=argv; From 0fd7a359da836ab3233023dcd1b7c6ab09c90be6 Mon Sep 17 00:00:00 2001 From: Ramil Kalimullin Date: Mon, 16 Mar 2009 09:02:10 +0400 Subject: [PATCH 2/2] Fix for bug #42957: no results from select where .. (col=col and col=col) or ... (false expression) Problem: optimizer didn't take into account a singular case when we eliminated all the predicates at the AND level of WHERE. That may lead to wrong results. Fix: replace (a=a AND a=a...) with TRUE if we eliminated all the predicates. mysql-test/r/select.result: Fix for bug #42957: no results from select where .. (col=col and col=col) or ... (false expression) - test result. mysql-test/t/select.test: Fix for bug #42957: no results from select where .. (col=col and col=col) or ... (false expression) - test case. sql/sql_select.cc: Fix for bug #42957: no results from select where .. (col=col and col=col) or ... (false expression) - replacing equality predicates by multiple equality items check if we eliminate all the predicates at the AND level and replace them with TRUE if so. --- mysql-test/r/select.result | 28 ++++++++++++++++++++++++++++ mysql-test/t/select.test | 16 ++++++++++++++++ sql/sql_select.cc | 9 ++++++++- 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/select.result b/mysql-test/r/select.result index 621c11906cb..0771c7fb370 100644 --- a/mysql-test/r/select.result +++ b/mysql-test/r/select.result @@ -4416,4 +4416,32 @@ date_nokey Warnings: Warning 1292 Incorrect date value: '10:41:7' for column 'date_nokey' at row 1 DROP TABLE A,C; +CREATE TABLE t1 (a INT NOT NULL, b INT); +INSERT INTO t1 VALUES (1, 1); +EXPLAIN EXTENDED SELECT * FROM t1 WHERE (a=a AND a=a) OR b > 2; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 SIMPLE t1 system NULL NULL NULL NULL 1 100.00 +Warnings: +Note 1003 select '1' AS `a`,'1' AS `b` from `test`.`t1` where 1 +SELECT * FROM t1 WHERE (a=a AND a=a) OR b > 2; +a b +1 1 +DROP TABLE t1; +CREATE TABLE t1 (a INT NOT NULL, b INT NOT NULL, c INT NOT NULL); +EXPLAIN EXTENDED SELECT * FROM t1 WHERE (a=a AND b=b AND c=c) OR b > 20; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 SIMPLE t1 system NULL NULL NULL NULL 0 0.00 const row not found +Warnings: +Note 1003 select '0' AS `a`,'0' AS `b`,'0' AS `c` from `test`.`t1` where 1 +EXPLAIN EXTENDED SELECT * FROM t1 WHERE (a=a AND a=a AND b=b) OR b > 20; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 SIMPLE t1 system NULL NULL NULL NULL 0 0.00 const row not found +Warnings: +Note 1003 select '0' AS `a`,'0' AS `b`,'0' AS `c` from `test`.`t1` where 1 +EXPLAIN EXTENDED SELECT * FROM t1 WHERE (a=a AND b=b AND a=a) OR b > 20; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 SIMPLE t1 system NULL NULL NULL NULL 0 0.00 const row not found +Warnings: +Note 1003 select '0' AS `a`,'0' AS `b`,'0' AS `c` from `test`.`t1` where 1 +DROP TABLE t1; End of 5.1 tests diff --git a/mysql-test/t/select.test b/mysql-test/t/select.test index ccdb53ec11f..8981ddbe2e4 100644 --- a/mysql-test/t/select.test +++ b/mysql-test/t/select.test @@ -3769,4 +3769,20 @@ SELECT date_nokey FROM C DROP TABLE A,C; +# +# Bug #42957: no results from +# select where .. (col=col and col=col) or ... (false expression) +# +CREATE TABLE t1 (a INT NOT NULL, b INT); +INSERT INTO t1 VALUES (1, 1); +EXPLAIN EXTENDED SELECT * FROM t1 WHERE (a=a AND a=a) OR b > 2; +SELECT * FROM t1 WHERE (a=a AND a=a) OR b > 2; +DROP TABLE t1; + +CREATE TABLE t1 (a INT NOT NULL, b INT NOT NULL, c INT NOT NULL); +EXPLAIN EXTENDED SELECT * FROM t1 WHERE (a=a AND b=b AND c=c) OR b > 20; +EXPLAIN EXTENDED SELECT * FROM t1 WHERE (a=a AND a=a AND b=b) OR b > 20; +EXPLAIN EXTENDED SELECT * FROM t1 WHERE (a=a AND b=b AND a=a) OR b > 20; +DROP TABLE t1; + --echo End of 5.1 tests diff --git a/sql/sql_select.cc b/sql/sql_select.cc index bea748562eb..3bf94bc828f 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -7625,7 +7625,7 @@ static COND *build_equal_items_for_cond(THD *thd, COND *cond, if (and_level) { /* - Retrieve all conjucts of this level detecting the equality + Retrieve all conjuncts of this level detecting the equality that are subject to substitution by multiple equality items and removing each such predicate from the conjunction after having found/created a multiple equality whose inference the predicate is. @@ -7641,6 +7641,13 @@ static COND *build_equal_items_for_cond(THD *thd, COND *cond, li.remove(); } + /* + Check if we eliminated all the predicates of the level, e.g. + (a=a AND b=b AND a=a) + */ + if (!(args->elements + cond_equal.current_level.elements + eq_list.elements)) + return new Item_int((longlong) 1,1); + List_iterator_fast it(cond_equal.current_level); while ((item_equal= it++)) {