From 84a9e656e00c0aca0e3b5a34bd6a758da7d031d9 Mon Sep 17 00:00:00 2001 From: "svoj@mysql.com/june.mysql.com" <> Date: Thu, 13 Sep 2007 15:39:16 +0500 Subject: [PATCH 01/19] BUG#30590 - delete from memory table with composite btree primary key DELETE query against memory table with btree index may remove not all matching rows. This happens only when DELETE uses index read method to find matching rows. E.g. for queries like DELETE FROM t1 WHERE a=1. Fixed by reverting fix for BUG9719 and applying proper solution. --- heap/hp_delete.c | 3 --- heap/hp_rfirst.c | 11 +++++++++++ heap/hp_rnext.c | 29 +++++++++++++++++++++++++++++ mysql-test/r/heap_btree.result | 7 +++++++ mysql-test/t/heap_btree.test | 9 +++++++++ 5 files changed, 56 insertions(+), 3 deletions(-) diff --git a/heap/hp_delete.c b/heap/hp_delete.c index 266a9da6ca3..90e537081d3 100644 --- a/heap/hp_delete.c +++ b/heap/hp_delete.c @@ -73,10 +73,7 @@ int hp_rb_delete_key(HP_INFO *info, register HP_KEYDEF *keyinfo, int res; if (flag) - { info->last_pos= NULL; /* For heap_rnext/heap_rprev */ - info->lastkey_len= 0; - } custom_arg.keyseg= keyinfo->seg; custom_arg.key_length= hp_rb_make_key(keyinfo, info->recbuf, record, recpos); diff --git a/heap/hp_rfirst.c b/heap/hp_rfirst.c index 85548fea212..6e7ee45af5f 100644 --- a/heap/hp_rfirst.c +++ b/heap/hp_rfirst.c @@ -36,6 +36,17 @@ int heap_rfirst(HP_INFO *info, byte *record, int inx) sizeof(byte*)); info->current_ptr = pos; memcpy(record, pos, (size_t)share->reclength); + /* + If we're performing index_first on a table that was taken from + table cache, info->lastkey_len is initialized to previous query. + Thus we set info->lastkey_len to proper value for subsequent + heap_rnext() calls. + This is needed for DELETE queries only, otherwise this variable is + not used. + Note that the same workaround may be needed for heap_rlast(), but + for now heap_rlast() is never used for DELETE queries. + */ + info->lastkey_len= 0; info->update = HA_STATE_AKTIV; } else diff --git a/heap/hp_rnext.c b/heap/hp_rnext.c index a1bc480333e..513e184c743 100644 --- a/heap/hp_rnext.c +++ b/heap/hp_rnext.c @@ -34,11 +34,40 @@ int heap_rnext(HP_INFO *info, byte *record) heap_rb_param custom_arg; if (info->last_pos) + { + /* + We enter this branch for non-DELETE queries after heap_rkey() + or heap_rfirst(). As last key position (info->last_pos) is available, + we only need to climb the tree using tree_search_next(). + */ pos = tree_search_next(&keyinfo->rb_tree, &info->last_pos, offsetof(TREE_ELEMENT, left), offsetof(TREE_ELEMENT, right)); + } + else if (!info->lastkey_len) + { + /* + We enter this branch only for DELETE queries after heap_rfirst(). E.g. + DELETE FROM t1 WHERE a<10. As last key position is not available + (last key is removed by heap_delete()), we must restart search as it + is done in heap_rfirst(). + + It should be safe to handle this situation without this branch. That is + branch below should find smallest element in a tree as lastkey_len is + zero. tree_search_edge() is a kind of optimisation here as it should be + faster than tree_search_key(). + */ + pos= tree_search_edge(&keyinfo->rb_tree, info->parents, + &info->last_pos, offsetof(TREE_ELEMENT, left)); + } else { + /* + We enter this branch only for DELETE queries after heap_rkey(). E.g. + DELETE FROM t1 WHERE a=10. As last key position is not available + (last key is removed by heap_delete()), we must restart search as it + is done in heap_rkey(). + */ custom_arg.keyseg = keyinfo->seg; custom_arg.key_length = info->lastkey_len; custom_arg.search_flag = SEARCH_SAME | SEARCH_FIND; diff --git a/mysql-test/r/heap_btree.result b/mysql-test/r/heap_btree.result index 72be6a3e400..639fe1ff897 100644 --- a/mysql-test/r/heap_btree.result +++ b/mysql-test/r/heap_btree.result @@ -307,4 +307,11 @@ UNIQUE USING BTREE(c1) ) ENGINE= MEMORY DEFAULT CHARSET= utf8; INSERT INTO t1 VALUES('1'), ('2'); DROP TABLE t1; +CREATE TABLE t1 (a INT, KEY USING BTREE(a)) ENGINE=MEMORY; +INSERT INTO t1 VALUES(1),(2),(2); +DELETE FROM t1 WHERE a=2; +SELECT * FROM t1; +a +1 +DROP TABLE t1; End of 4.1 tests diff --git a/mysql-test/t/heap_btree.test b/mysql-test/t/heap_btree.test index f8d6afbad04..849b7e12843 100644 --- a/mysql-test/t/heap_btree.test +++ b/mysql-test/t/heap_btree.test @@ -213,4 +213,13 @@ CREATE TABLE t1 ( INSERT INTO t1 VALUES('1'), ('2'); DROP TABLE t1; +# +# BUG#30590 - delete from memory table with composite btree primary key +# +CREATE TABLE t1 (a INT, KEY USING BTREE(a)) ENGINE=MEMORY; +INSERT INTO t1 VALUES(1),(2),(2); +DELETE FROM t1 WHERE a=2; +SELECT * FROM t1; +DROP TABLE t1; + --echo End of 4.1 tests From ef3bcaf3dd46d6a4bc7a38d7a50924eb970148c1 Mon Sep 17 00:00:00 2001 From: "anozdrin/alik@station." <> Date: Thu, 13 Sep 2007 17:30:44 +0400 Subject: [PATCH 02/19] Bug#16918: Aborted_clients > Connections. The problem was that aborted_threads variable was updated twice when a client connection had been aborted. The fix is to refactor a code to have aborted_threads updated only in one place. --- sql/mysql_priv.h | 1 - sql/sql_parse.cc | 27 +++++++++++++++++---------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index 44eb5590a28..24d02ee6948 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -726,7 +726,6 @@ pthread_handler_t handle_bootstrap(void *arg); void end_thread(THD *thd,bool put_in_cache); void flush_thread_cache(); bool mysql_execute_command(THD *thd); -bool do_command(THD *thd); bool dispatch_command(enum enum_server_command command, THD *thd, char* packet, uint packet_length); void log_slow_statement(THD *thd); diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 58f5ffc5235..99a61d95619 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -93,6 +93,8 @@ const char *xa_state_names[]={ "NON-EXISTING", "ACTIVE", "IDLE", "PREPARED" }; +static bool do_command(THD *thd); + #ifdef __WIN__ static void test_signal(int sig_ptr) { @@ -1199,23 +1201,28 @@ pthread_handler_t handle_one_connection(void *arg) } if (thd->user_connect) decrease_user_connections(thd->user_connect); + + if (thd->killed || + net->vio && net->error && net->report_error) + { + statistic_increment(aborted_threads, &LOCK_status); + } + if (net->error && net->vio != 0 && net->report_error) { if (!thd->killed && thd->variables.log_warnings > 1) - sql_print_warning(ER(ER_NEW_ABORTING_CONNECTION), + { + sql_print_warning(ER(ER_NEW_ABORTING_CONNECTION), thd->thread_id,(thd->db ? thd->db : "unconnected"), sctx->user ? sctx->user : "unauthenticated", sctx->host_or_ip, (net->last_errno ? ER(net->last_errno) : ER(ER_UNKNOWN_ERROR))); + } + net_send_error(thd, net->last_errno, NullS); - statistic_increment(aborted_threads,&LOCK_status); } - else if (thd->killed) - { - statistic_increment(aborted_threads,&LOCK_status); - } - + end_thread: close_connection(thd, 0, 1); end_thread(thd,1); @@ -1550,12 +1557,12 @@ bool do_command(THD *thd) DBUG_PRINT("info",("Got error %d reading command from socket %s", net->error, vio_description(net->vio))); + /* Check if we can continue without closing the connection */ + if (net->error != 3) - { - statistic_increment(aborted_threads,&LOCK_status); DBUG_RETURN(TRUE); // We have to close it. - } + net_send_error(thd, net->last_errno, NullS); net->error= 0; DBUG_RETURN(FALSE); From 040e520d55598170fa80d53936812f14a5d9f4fd Mon Sep 17 00:00:00 2001 From: "dkatz@damien-katzs-computer.local" <> Date: Fri, 28 Sep 2007 17:10:14 -0400 Subject: [PATCH 03/19] Bug #29772 Error compiling with option --without-geometry Added #ifdef HAVE_SPATIAL around geometry related code. --- sql/item.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sql/item.cc b/sql/item.cc index 66379d5dcf9..b44927f8da6 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -4357,12 +4357,14 @@ Field *Item::tmp_table_field_from_field_type(TABLE *table) return new Field_blob(max_length, maybe_null, name, table, collation.collation); break; // Blob handled outside of case +#ifdef HAVE_SPATIAL case MYSQL_TYPE_GEOMETRY: return new Field_geom(max_length, maybe_null, name, table, (Field::geometry_type) ((type() == Item::TYPE_HOLDER) ? ((Item_type_holder *)this)->get_geometry_type() : ((Item_geometry_func *)this)->get_geometry_type())); +#endif /* HAVE_SPATIAL */ } } @@ -6482,10 +6484,12 @@ Item_type_holder::Item_type_holder(THD *thd, Item *item) if (Field::result_merge_type(fld_type) == INT_RESULT) decimals= 0; prev_decimal_int_part= item->decimal_int_part(); +#ifdef HAVE_SPATIAL if (item->field_type() == MYSQL_TYPE_GEOMETRY) geometry_type= (item->type() == Item::FIELD_ITEM) ? ((Item_field *)item)->get_geometry_type() : (Field::geometry_type)((Item_geometry_func *)item)->get_geometry_type(); +#endif /* HAVE_SPATIAL */ } From 93fd4e200ce1541d20afa711693a626549577274 Mon Sep 17 00:00:00 2001 From: "dkatz@damien-katzs-computer.local" <> Date: Fri, 28 Sep 2007 17:13:35 -0400 Subject: [PATCH 04/19] Bug #29589 sys_innodb_max_purge_lag is defined twice in sys_variables Removed duplicate innodb variable from sys_variables array. --- sql/set_var.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/sql/set_var.cc b/sql/set_var.cc index e1246617d84..305370e6569 100644 --- a/sql/set_var.cc +++ b/sql/set_var.cc @@ -784,7 +784,6 @@ sys_var *sys_variables[]= &sys_innodb_max_purge_lag, &sys_innodb_table_locks, &sys_innodb_support_xa, - &sys_innodb_max_purge_lag, &sys_innodb_autoextend_increment, &sys_innodb_sync_spin_loops, &sys_innodb_concurrency_tickets, From 5dad55cd24e8a82ae8e4b2e63186af57204ef571 Mon Sep 17 00:00:00 2001 From: "mats@kindahl-laptop.dnsalias.net" <> Date: Mon, 1 Oct 2007 15:11:15 +0200 Subject: [PATCH 05/19] BUG#30992 (Wrong implementation of pthread_mutex_trylock()): Adding support for correct handling of pthread_mutex_trylock() on Win32 systems as well as when using the safe mutexes. --- include/my_pthread.h | 9 ++++---- mysys/my_winthread.c | 25 ++++++++++++++++++++++ mysys/thr_mutex.c | 49 +++++++++++++++++++++++++++++++++++++------- 3 files changed, 72 insertions(+), 11 deletions(-) diff --git a/include/my_pthread.h b/include/my_pthread.h index d64b5348666..e28bb9df1f3 100644 --- a/include/my_pthread.h +++ b/include/my_pthread.h @@ -135,6 +135,7 @@ struct timespec { void win_pthread_init(void); int win_pthread_setspecific(void *A,void *B,uint length); +int win_pthread_mutex_trylock(pthread_mutex_t *mutex); int pthread_create(pthread_t *,pthread_attr_t *,pthread_handler,void *); int pthread_cond_init(pthread_cond_t *cond, const pthread_condattr_t *attr); int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex); @@ -195,7 +196,7 @@ extern int pthread_mutex_destroy (pthread_mutex_t *); #else #define pthread_mutex_init(A,B) (InitializeCriticalSection(A),0) #define pthread_mutex_lock(A) (EnterCriticalSection(A),0) -#define pthread_mutex_trylock(A) (WaitForSingleObject((A), 0) == WAIT_TIMEOUT) +#define pthread_mutex_trylock(A) win_pthread_mutex_trylock((A)) #define pthread_mutex_unlock(A) LeaveCriticalSection(A) #define pthread_mutex_destroy(A) DeleteCriticalSection(A) #define my_pthread_setprio(A,B) SetThreadPriority(GetCurrentThread(), (B)) @@ -593,7 +594,7 @@ typedef struct st_safe_mutex_info_t int safe_mutex_init(safe_mutex_t *mp, const pthread_mutexattr_t *attr, const char *file, uint line); -int safe_mutex_lock(safe_mutex_t *mp,const char *file, uint line); +int safe_mutex_lock(safe_mutex_t *mp, my_bool try_lock, const char *file, uint line); int safe_mutex_unlock(safe_mutex_t *mp,const char *file, uint line); int safe_mutex_destroy(safe_mutex_t *mp,const char *file, uint line); int safe_cond_wait(pthread_cond_t *cond, safe_mutex_t *mp,const char *file, @@ -616,12 +617,12 @@ void safe_mutex_end(FILE *file); #undef pthread_cond_timedwait #undef pthread_mutex_trylock #define pthread_mutex_init(A,B) safe_mutex_init((A),(B),__FILE__,__LINE__) -#define pthread_mutex_lock(A) safe_mutex_lock((A),__FILE__,__LINE__) +#define pthread_mutex_lock(A) safe_mutex_lock((A), FALSE, __FILE__, __LINE__) #define pthread_mutex_unlock(A) safe_mutex_unlock((A),__FILE__,__LINE__) #define pthread_mutex_destroy(A) safe_mutex_destroy((A),__FILE__,__LINE__) #define pthread_cond_wait(A,B) safe_cond_wait((A),(B),__FILE__,__LINE__) #define pthread_cond_timedwait(A,B,C) safe_cond_timedwait((A),(B),(C),__FILE__,__LINE__) -#define pthread_mutex_trylock(A) pthread_mutex_lock(A) +#define pthread_mutex_trylock(A) safe_mutex_lock((A), TRUE, __FILE__, __LINE__) #define pthread_mutex_t safe_mutex_t #define safe_mutex_assert_owner(mp) \ DBUG_ASSERT((mp)->count > 0 && \ diff --git a/mysys/my_winthread.c b/mysys/my_winthread.c index 27ccaef4f23..e9fface0521 100644 --- a/mysys/my_winthread.c +++ b/mysys/my_winthread.c @@ -40,6 +40,31 @@ void win_pthread_init(void) pthread_mutex_init(&THR_LOCK_thread,MY_MUTEX_INIT_FAST); } +/** + Adapter to @c pthread_mutex_trylock() + + @retval 0 Mutex was acquired + @retval EBUSY Mutex was already locked by a thread + @retval EINVAL Mutex could not be acquired due to other error + */ +int +win_pthread_mutex_trylock(pthread_mutex_t *mutex) +{ + switch (WaitForSingleObject(mutex, 0)) { + case WAIT_TIMEOUT: + return EBUSY; + + default: + case WAIT_FAILURE: + return EINVAL; + + case WAIT_OBJECT_0: + case WAIT_ABANDONED: /* The mutex was acquired because it was + * abandoned */ + return 0; + } +} + /* ** We have tried to use '_beginthreadex' instead of '_beginthread' here ** but in this case the program leaks about 512 characters for each diff --git a/mysys/thr_mutex.c b/mysys/thr_mutex.c index 425e5fce459..53ee907e0a3 100644 --- a/mysys/thr_mutex.c +++ b/mysys/thr_mutex.c @@ -91,7 +91,7 @@ int safe_mutex_init(safe_mutex_t *mp, } -int safe_mutex_lock(safe_mutex_t *mp,const char *file, uint line) +int safe_mutex_lock(safe_mutex_t *mp, my_bool try_lock, const char *file, uint line) { int error; if (!mp->file) @@ -104,15 +104,50 @@ int safe_mutex_lock(safe_mutex_t *mp,const char *file, uint line) } pthread_mutex_lock(&mp->global); - if (mp->count > 0 && pthread_equal(pthread_self(),mp->thread)) + if (mp->count > 0) { - fprintf(stderr,"safe_mutex: Trying to lock mutex at %s, line %d, when the mutex was already locked at %s, line %d in thread %s\n", - file,line,mp->file, mp->line, my_thread_name()); - fflush(stderr); - abort(); + if (try_lock) + { + pthread_mutex_unlock(&mp->global); + return EBUSY; + } + else if (pthread_equal(pthread_self(),mp->thread)) + { + fprintf(stderr, + "safe_mutex: Trying to lock mutex at %s, line %d, when the" + " mutex was already locked at %s, line %d in thread %s\n", + file,line,mp->file, mp->line, my_thread_name()); + fflush(stderr); + abort(); + } } pthread_mutex_unlock(&mp->global); - error=pthread_mutex_lock(&mp->mutex); + + /* + If we are imitating trylock(), we need to take special + precautions. + + - We cannot use pthread_mutex_lock() only since another thread can + overtake this thread and take the lock before this thread + causing pthread_mutex_trylock() to hang. In this case, we should + just return EBUSY. Hence, we use pthread_mutex_trylock() to be + able to return immediately. + + - We cannot just use trylock() and continue execution below, since + this would generate an error and abort execution if the thread + was overtaken and trylock() returned EBUSY . In this case, we + instead just return EBUSY, since this is the expected behaviour + of trylock(). + */ + if (try_lock) + { + error= pthread_mutex_trylock(&mp->mutex); + if (error == EBUSY) + return error; + } + else + error= pthread_mutex_lock(&mp->mutex); + if (error || (error=pthread_mutex_lock(&mp->global))) { fprintf(stderr,"Got error %d when trying to lock mutex at %s, line %d\n", From 18c6118911f1f98c80a9a06af50a8c5ed02aef32 Mon Sep 17 00:00:00 2001 From: "msvensson@pilot.mysql.com" <> Date: Wed, 3 Oct 2007 21:38:32 +0200 Subject: [PATCH 06/19] Bug#30992 Wrong implementation of pthread_mutex_trylock() It's not possible to use WaitForSingleObject to wait on a CRITICAL_SECTION, instead use the TryEnterCriticalSection function. - if "mutex" was already taken => return EBUSY - if "mutex" was aquired => return 0 --- include/config-win.h | 3 +++ mysys/my_winthread.c | 22 ++++++++++------------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/include/config-win.h b/include/config-win.h index 279be7aa5e4..f0804a368bc 100644 --- a/include/config-win.h +++ b/include/config-win.h @@ -19,6 +19,9 @@ /* We have to do this define before including windows.h to get the AWE API functions */ #define _WIN32_WINNT 0x0500 +#else +/* Get NT 4.0 functions */ +#define _WIN32_WINNT 0x0400 #endif #if defined(_MSC_VER) && _MSC_VER >= 1400 diff --git a/mysys/my_winthread.c b/mysys/my_winthread.c index e9fface0521..e94369bec32 100644 --- a/mysys/my_winthread.c +++ b/mysys/my_winthread.c @@ -40,31 +40,29 @@ void win_pthread_init(void) pthread_mutex_init(&THR_LOCK_thread,MY_MUTEX_INIT_FAST); } + /** Adapter to @c pthread_mutex_trylock() @retval 0 Mutex was acquired @retval EBUSY Mutex was already locked by a thread - @retval EINVAL Mutex could not be acquired due to other error */ int win_pthread_mutex_trylock(pthread_mutex_t *mutex) { - switch (WaitForSingleObject(mutex, 0)) { - case WAIT_TIMEOUT: - return EBUSY; - - default: - case WAIT_FAILURE: - return EINVAL; - - case WAIT_OBJECT_0: - case WAIT_ABANDONED: /* The mutex was acquired because it was - * abandoned */ + if (TryEnterCriticalSection(mutex)) + { + /* Don't allow recursive lock */ + if (mutex->RecursionCount > 1){ + LeaveCriticalSection(mutex); + return EBUSY; + } return 0; } + return EBUSY; } + /* ** We have tried to use '_beginthreadex' instead of '_beginthread' here ** but in this case the program leaks about 512 characters for each From 707f067446939f8e00ac22fc1c07d62846737193 Mon Sep 17 00:00:00 2001 From: "anozdrin/alik@station." <> Date: Thu, 4 Oct 2007 17:19:14 +0400 Subject: [PATCH 07/19] Fix for BUG#31035: select from function, group by result crasher. This actually, fix for the patch for bug-27354. The problem with the patch was that Item_func_sp::used_tables() was updated, but Item_func_sp::const_item() was not. So, for Item_func_sp, we had the following inconsistency: - used_tables() returned RAND_TABLE, which means that the item can produce "random" results; - but const_item() returned TRUE, which means that the item is a constant one. The fix is to change Item_func_sp::const_item() behaviour: it must return TRUE (an item is a constant one) only if a stored function is deterministic and each of its arguments (if any) is a constant item. --- mysql-test/r/sp.result | 176 +++++++++++++++++++++++++ mysql-test/t/sp.test | 290 +++++++++++++++++++++++++++++++++++++++++ sql/item_func.cc | 13 +- 3 files changed, 477 insertions(+), 2 deletions(-) diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index a6c429a31fb..efc12b402ce 100644 --- a/mysql-test/r/sp.result +++ b/mysql-test/r/sp.result @@ -6367,4 +6367,180 @@ DROP TABLE t1; DROP PROCEDURE p1; DROP PROCEDURE p2; + +# +# Bug#31035. +# + +# +# - Prepare. +# + +DROP TABLE IF EXISTS t1; +DROP FUNCTION IF EXISTS f1; +DROP FUNCTION IF EXISTS f2; +DROP FUNCTION IF EXISTS f3; +DROP FUNCTION IF EXISTS f4; + +# +# - Create required objects. +# + +CREATE TABLE t1(c1 INT); + +INSERT INTO t1 VALUES (1), (2), (3); + +CREATE FUNCTION f1() +RETURNS INT +NOT DETERMINISTIC +RETURN 1; + +CREATE FUNCTION f2(p INT) +RETURNS INT +NOT DETERMINISTIC +RETURN 1; + +CREATE FUNCTION f3() +RETURNS INT +DETERMINISTIC +RETURN 1; + +CREATE FUNCTION f4(p INT) +RETURNS INT +DETERMINISTIC +RETURN 1; + +# +# - Check. +# + +SELECT f1() AS a FROM t1 GROUP BY a; +a +1 + +SELECT f2(@a) AS a FROM t1 GROUP BY a; +a +1 + +SELECT f3() AS a FROM t1 GROUP BY a; +a +1 + +SELECT f4(0) AS a FROM t1 GROUP BY a; +a +1 + +SELECT f4(@a) AS a FROM t1 GROUP BY a; +a +1 + +# +# - Cleanup. +# + +DROP TABLE t1; +DROP FUNCTION f1; +DROP FUNCTION f2; +DROP FUNCTION f3; +DROP FUNCTION f4; + +# +# Bug#31191. +# + +# +# - Prepare. +# + +DROP TABLE IF EXISTS t1; +DROP TABLE IF EXISTS t2; +DROP FUNCTION IF EXISTS f1; + +# +# - Create required objects. +# + +CREATE TABLE t1 ( +id INT(10) UNSIGNED NOT NULL AUTO_INCREMENT, +barcode INT(8) UNSIGNED ZEROFILL nOT NULL, +PRIMARY KEY (id), +UNIQUE KEY barcode (barcode) +); + +INSERT INTO t1 (id, barcode) VALUES (1, 12345678); +INSERT INTO t1 (id, barcode) VALUES (2, 12345679); + +CREATE TABLE test.t2 ( +id INT(10) UNSIGNED NOT NULL AUTO_INCREMENT, +barcode BIGINT(11) UNSIGNED ZEROFILL NOT NULL, +PRIMARY KEY (id) +); + +INSERT INTO test.t2 (id, barcode) VALUES (1, 12345106708); +INSERT INTO test.t2 (id, barcode) VALUES (2, 12345106709); + +CREATE FUNCTION f1(p INT(8)) +RETURNS BIGINT(11) UNSIGNED +READS SQL DATA +RETURN FLOOR(p/1000)*1000000 + 100000 + FLOOR((p MOD 1000)/10)*100 + (p MOD 10); + +# +# - Check. +# + +SELECT DISTINCT t1.barcode, f1(t1.barcode) +FROM t1 +INNER JOIN t2 +ON f1(t1.barcode) = t2.barcode +WHERE t1.barcode=12345678; +barcode f1(t1.barcode) +12345678 12345106708 + +# +# - Cleanup. +# + +DROP TABLE t1; +DROP TABLE t2; +DROP FUNCTION f1; + +# +# Bug#31226. +# + +# +# - Prepare. +# + +DROP TABLE IF EXISTS t1; +DROP FUNCTION IF EXISTS f1; + +# +# - Create required objects. +# + +CREATE TABLE t1(id INT); + +INSERT INTO t1 VALUES (1), (2), (3); + +CREATE FUNCTION f1() +RETURNS DATETIME +NOT DETERMINISTIC NO SQL +RETURN NOW(); + +# +# - Check. +# + +SELECT f1() FROM t1 GROUP BY 1; +f1() + + +# +# - Cleanup. +# + +DROP TABLE t1; +DROP FUNCTION f1; + End of 5.0 tests diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index 8dd99c2b2f1..6413cf2e89d 100644 --- a/mysql-test/t/sp.test +++ b/mysql-test/t/sp.test @@ -7354,4 +7354,294 @@ DROP TABLE t1; DROP PROCEDURE p1; DROP PROCEDURE p2; +########################################################################### + +# +# Bug#31035: select from function, group by result crasher. +# + +########################################################################### + +--echo + +--echo # +--echo # Bug#31035. +--echo # + +--echo + +--echo # +--echo # - Prepare. +--echo # + +--echo + +--disable_warnings +DROP TABLE IF EXISTS t1; +DROP FUNCTION IF EXISTS f1; +DROP FUNCTION IF EXISTS f2; +DROP FUNCTION IF EXISTS f3; +DROP FUNCTION IF EXISTS f4; +--enable_warnings + +--echo + +--echo # +--echo # - Create required objects. +--echo # + +--echo + +CREATE TABLE t1(c1 INT); + +--echo + +INSERT INTO t1 VALUES (1), (2), (3); + +--echo + +CREATE FUNCTION f1() + RETURNS INT + NOT DETERMINISTIC + RETURN 1; + +--echo + +CREATE FUNCTION f2(p INT) + RETURNS INT + NOT DETERMINISTIC + RETURN 1; + +--echo + +CREATE FUNCTION f3() + RETURNS INT + DETERMINISTIC + RETURN 1; + +--echo + +CREATE FUNCTION f4(p INT) + RETURNS INT + DETERMINISTIC + RETURN 1; + +--echo + +--echo # +--echo # - Check. +--echo # + +--echo + +# Not deterministic function, no arguments. + +SELECT f1() AS a FROM t1 GROUP BY a; + +--echo + +# Not deterministic function, non-constant argument. + +SELECT f2(@a) AS a FROM t1 GROUP BY a; + +--echo + +# Deterministic function, no arguments. + +SELECT f3() AS a FROM t1 GROUP BY a; + +--echo + +# Deterministic function, constant argument. + +SELECT f4(0) AS a FROM t1 GROUP BY a; + +--echo + +# Deterministic function, non-constant argument. + +SELECT f4(@a) AS a FROM t1 GROUP BY a; + +--echo + +--echo # +--echo # - Cleanup. +--echo # + +--echo + +DROP TABLE t1; +DROP FUNCTION f1; +DROP FUNCTION f2; +DROP FUNCTION f3; +DROP FUNCTION f4; + +--echo + +########################################################################### + +# +# Bug#31191: JOIN in combination with stored function crashes the server. +# + +########################################################################### + +--echo # +--echo # Bug#31191. +--echo # + +--echo + +--echo # +--echo # - Prepare. +--echo # + +--echo + +--disable_warnings +DROP TABLE IF EXISTS t1; +DROP TABLE IF EXISTS t2; +DROP FUNCTION IF EXISTS f1; +--enable_warnings + +--echo + +--echo # +--echo # - Create required objects. +--echo # + +--echo + +CREATE TABLE t1 ( + id INT(10) UNSIGNED NOT NULL AUTO_INCREMENT, + barcode INT(8) UNSIGNED ZEROFILL nOT NULL, + PRIMARY KEY (id), + UNIQUE KEY barcode (barcode) +); + +--echo + +INSERT INTO t1 (id, barcode) VALUES (1, 12345678); +INSERT INTO t1 (id, barcode) VALUES (2, 12345679); + +--echo + +CREATE TABLE test.t2 ( + id INT(10) UNSIGNED NOT NULL AUTO_INCREMENT, + barcode BIGINT(11) UNSIGNED ZEROFILL NOT NULL, + PRIMARY KEY (id) +); + +--echo + +INSERT INTO test.t2 (id, barcode) VALUES (1, 12345106708); +INSERT INTO test.t2 (id, barcode) VALUES (2, 12345106709); + +--echo + +CREATE FUNCTION f1(p INT(8)) + RETURNS BIGINT(11) UNSIGNED + READS SQL DATA + RETURN FLOOR(p/1000)*1000000 + 100000 + FLOOR((p MOD 1000)/10)*100 + (p MOD 10); + +--echo + +--echo # +--echo # - Check. +--echo # + +--echo + +SELECT DISTINCT t1.barcode, f1(t1.barcode) +FROM t1 +INNER JOIN t2 +ON f1(t1.barcode) = t2.barcode +WHERE t1.barcode=12345678; + +--echo + +--echo # +--echo # - Cleanup. +--echo # + +--echo + +DROP TABLE t1; +DROP TABLE t2; +DROP FUNCTION f1; + +--echo + +########################################################################### + +# +# Bug#31226: Group by function crashes mysql. +# + +########################################################################### + +--echo # +--echo # Bug#31226. +--echo # + +--echo + +--echo # +--echo # - Prepare. +--echo # + +--echo + +--disable_warnings +DROP TABLE IF EXISTS t1; +DROP FUNCTION IF EXISTS f1; +--enable_warnings + +--echo + +--echo # +--echo # - Create required objects. +--echo # + +--echo + +CREATE TABLE t1(id INT); + +--echo + +INSERT INTO t1 VALUES (1), (2), (3); + +--echo + +CREATE FUNCTION f1() + RETURNS DATETIME + NOT DETERMINISTIC NO SQL + RETURN NOW(); + +--echo + +--echo # +--echo # - Check. +--echo # + +--echo + +--replace_column 1 +SELECT f1() FROM t1 GROUP BY 1; + +--echo + +--echo # +--echo # - Cleanup. +--echo # + +--echo + +DROP TABLE t1; +DROP FUNCTION f1; + +--echo + +########################################################################### + --echo End of 5.0 tests diff --git a/sql/item_func.cc b/sql/item_func.cc index d03d497dfd0..f7103c22581 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -5583,8 +5583,13 @@ Item_func_sp::fix_fields(THD *thd, Item **ref) #endif /* ! NO_EMBEDDED_ACCESS_CHECKS */ } + if (!m_sp->m_chistics->detistic) - used_tables_cache |= RAND_TABLE_BIT; + { + used_tables_cache |= RAND_TABLE_BIT; + const_item_cache= FALSE; + } + DBUG_RETURN(res); } @@ -5592,6 +5597,10 @@ Item_func_sp::fix_fields(THD *thd, Item **ref) void Item_func_sp::update_used_tables() { Item_func::update_used_tables(); + if (!m_sp->m_chistics->detistic) - used_tables_cache |= RAND_TABLE_BIT; + { + used_tables_cache |= RAND_TABLE_BIT; + const_item_cache= FALSE; + } } From 305ebc1e21d72c54a2c3e7714b884b847da06efc Mon Sep 17 00:00:00 2001 From: "davi@moksha.local" <> Date: Thu, 4 Oct 2007 17:34:41 -0300 Subject: [PATCH 08/19] Bug#21587 FLUSH TABLES causes server crash when used with HANDLER statements This bug is a symptom of the way handler's tables are managed. The most different aspect, compared to the conventional behavior, is that the handler's tables are long lived, meaning that their lifetimes are not bounded by the duration of the command that opened them. For this effect the handler code uses its own list (handler_tables instead of open_tables) to hold open handler tables so that the tables won't be closed at the end of the command/statement. Besides the handler_tables list, there is a hash (handler_tables_hash) which is used to associate handler aliases to tables and to refresh the tables upon demand (flush tables). The current implementation doesn't work properly with refreshed tables -- more precisely when flush commands are issued by other initiators. This happens because when a handler open or read statement is being processed, the associated table has to be opened or locked and, for this matter, the open_tables and handler_tables lists are swapped so that the new table being opened is inserted into the handler_tables list. But when opening or locking the table, if the refresh version is different from the thread refresh version then all used tables in the open_tables list (now handler_tables) are refreshed. In the "refreshing" process the handler tables are flushed (closed) without being properly unlinked from the handler hash. The current implementation also fails to properly discard handlers of dropped tables, but this and other problems are going to be addressed in the fixes for bugs 31397 and 31409. The chosen approach tries to properly save and restore the table state so that no table is flushed during the table open and lock operations. The logic is almost the same as before with the list swapping, but with a working glue code. The test case for this bug is going to be committed into 5.1 because it requires a test feature only avaiable in 5.1 (wait_condition). --- sql/sql_handler.cc | 53 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc index 9aefa71647e..89090c9fa63 100644 --- a/sql/sql_handler.cc +++ b/sql/sql_handler.cc @@ -65,11 +65,6 @@ static enum enum_ha_read_modes rkey_to_rnext[]= { RNEXT_SAME, RNEXT, RPREV, RNEXT, RPREV, RNEXT, RPREV, RPREV }; -#define HANDLER_TABLES_HACK(thd) { \ - TABLE *tmp=thd->open_tables; \ - thd->open_tables=thd->handler_tables; \ - thd->handler_tables=tmp; } - static int mysql_ha_flush_table(THD *thd, TABLE **table_ptr, uint mode_flags); @@ -187,6 +182,7 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) char *db, *name, *alias; uint dblen, namelen, aliaslen, counter; int error; + TABLE *backup_open_tables, *backup_handler_tables; DBUG_ENTER("mysql_ha_open"); DBUG_PRINT("enter",("'%s'.'%s' as '%s' reopen: %d", tables->db, tables->table_name, tables->alias, @@ -215,18 +211,31 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) } } + /* save open_ and handler_ tables state */ + backup_open_tables= thd->open_tables; + backup_handler_tables= thd->handler_tables; + + /* no pre-opened tables */ + thd->open_tables= NULL; + /* to avoid flushes */ + thd->handler_tables= NULL; + /* open_tables() will set 'tables->table' if successful. It must be NULL for a real open when calling open_tables(). */ DBUG_ASSERT(! tables->table); - HANDLER_TABLES_HACK(thd); /* for now HANDLER can be used only for real TABLES */ tables->required_type= FRMTYPE_TABLE; error= open_tables(thd, &tables, &counter, 0); - HANDLER_TABLES_HACK(thd); + /* restore the state and merge the opened table into handler_tables list */ + thd->handler_tables= thd->open_tables ? + thd->open_tables->next= backup_handler_tables, + thd->open_tables : backup_handler_tables; + thd->open_tables= backup_open_tables; + if (error) goto err; @@ -351,7 +360,7 @@ bool mysql_ha_read(THD *thd, TABLE_LIST *tables, ha_rows select_limit_cnt, ha_rows offset_limit_cnt) { TABLE_LIST *hash_tables; - TABLE *table; + TABLE *table, *backup_open_tables, *backup_handler_tables; MYSQL_LOCK *lock; List list; Protocol *protocol= thd->protocol; @@ -361,7 +370,7 @@ bool mysql_ha_read(THD *thd, TABLE_LIST *tables, uint num_rows; byte *key; uint key_len; - bool not_used; + bool need_reopen; DBUG_ENTER("mysql_ha_read"); DBUG_PRINT("enter",("'%s'.'%s' as '%s'", tables->db, tables->table_name, tables->alias)); @@ -375,6 +384,7 @@ bool mysql_ha_read(THD *thd, TABLE_LIST *tables, List_iterator it(list); it++; +retry: if ((hash_tables= (TABLE_LIST*) hash_search(&thd->handler_tables_hash, (byte*) tables->alias, strlen(tables->alias) + 1))) @@ -427,9 +437,28 @@ bool mysql_ha_read(THD *thd, TABLE_LIST *tables, } tables->table=table; - HANDLER_TABLES_HACK(thd); - lock= mysql_lock_tables(thd, &tables->table, 1, 0, ¬_used); - HANDLER_TABLES_HACK(thd); + /* save open_ and handler_ tables state */ + backup_open_tables= thd->open_tables; + backup_handler_tables= thd->handler_tables; + + /* no pre-opened tables */ + thd->open_tables= NULL; + /* to avoid flushes */ + thd->handler_tables= NULL; + + lock= mysql_lock_tables(thd, &tables->table, 1, + MYSQL_LOCK_NOTIFY_IF_NEED_REOPEN, &need_reopen); + + /* restore previous context */ + thd->handler_tables= backup_handler_tables; + thd->open_tables= backup_open_tables; + + if (need_reopen) + { + mysql_ha_close_table(thd, tables); + hash_tables->table= NULL; + goto retry; + } if (!lock) goto err0; // mysql_lock_tables() printed error message already From 128772ba1ffc3ef29cb3dae718b89cc5bf5a1902 Mon Sep 17 00:00:00 2001 From: "anozdrin/alik@station." <> Date: Mon, 8 Oct 2007 17:09:28 +0400 Subject: [PATCH 09/19] Fix compilation warning. --- sql/sql_parse.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 0aa9591e95d..c3565dd9b17 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -1527,7 +1527,7 @@ int end_trans(THD *thd, enum enum_mysql_completiontype completion) 1 request of thread shutdown (see dispatch_command() description) */ -bool do_command(THD *thd) +static bool do_command(THD *thd) { char *packet= 0; ulong packet_length; From 2885d0df1556653706ee0dd70e029e6cfdb476ce Mon Sep 17 00:00:00 2001 From: "anozdrin/alik@station." <> Date: Mon, 8 Oct 2007 17:59:23 +0400 Subject: [PATCH 10/19] Get rid of compilation warning. --- sql/sql_parse.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index c3565dd9b17..8c61243e3c5 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -93,7 +93,9 @@ const char *xa_state_names[]={ "NON-EXISTING", "ACTIVE", "IDLE", "PREPARED" }; +#ifndef EMBEDDED_LIBRARY static bool do_command(THD *thd); +#endif // EMBEDDED_LIBRARY #ifdef __WIN__ static void test_signal(int sig_ptr) From 1285baf608157bd22b42e1e7ee63c935bc4fed83 Mon Sep 17 00:00:00 2001 From: "istruewing@stella.local" <> Date: Tue, 9 Oct 2007 13:10:51 +0200 Subject: [PATCH 11/19] Bug#15522 - create ... select and with merge tables Added test case. The fix for Bug#20662 does also fix this one. --- mysql-test/r/merge.result | 5 +++++ mysql-test/t/merge.test | 14 ++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/mysql-test/r/merge.result b/mysql-test/r/merge.result index 42669eeb66f..24a56fbf575 100644 --- a/mysql-test/r/merge.result +++ b/mysql-test/r/merge.result @@ -876,4 +876,9 @@ CHECK TABLE tm1; Table Op Msg_type Msg_text test.tm1 check status OK DROP TABLE tm1, t1, t2; +CREATE TABLE t1(c1 INT); +CREATE TABLE t2 (c1 INT) ENGINE=MERGE UNION=(t1) INSERT_METHOD=FIRST; +CREATE TABLE IF NOT EXISTS t1 SELECT * FROM t2; +ERROR HY000: You can't specify target table 't1' for update in FROM clause +DROP TABLE t1, t2; End of 5.0 tests diff --git a/mysql-test/t/merge.test b/mysql-test/t/merge.test index c3e5cef5e63..b5c1a01fe8e 100644 --- a/mysql-test/t/merge.test +++ b/mysql-test/t/merge.test @@ -507,4 +507,18 @@ SELECT * FROM tm1; CHECK TABLE tm1; DROP TABLE tm1, t1, t2; +# +# Bug#15522 - create ... select and with merge tables +# +# This was fixed together with Bug#20662 (Infinite loop in CREATE TABLE +# IF NOT EXISTS ... SELECT with locked tables). +# The new behavior for MERGE tables is consistent with the +# CREATE TABLE SELECT behavior for ordinary tables. +# +CREATE TABLE t1(c1 INT); +CREATE TABLE t2 (c1 INT) ENGINE=MERGE UNION=(t1) INSERT_METHOD=FIRST; +--error ER_UPDATE_TABLE_USED +CREATE TABLE IF NOT EXISTS t1 SELECT * FROM t2; +DROP TABLE t1, t2; + --echo End of 5.0 tests From 7252cbe1e0e48c8497fac35c99b113294095217e Mon Sep 17 00:00:00 2001 From: "davi@moksha.local" <> Date: Tue, 9 Oct 2007 12:02:59 -0300 Subject: [PATCH 12/19] Bug#31409 RENAME TABLE causes server crash or deadlock when used with HANDLER statements This deadlock occurs when a client issues a HANDLER ... OPEN statement that tries to open a table that has a pending name-lock on it by another client that also needs a name-lock on some other table which is already open and associated to a HANDLER instance owned by the first client. The deadlock happens because the open_table() function will back-off and wait until the name-lock goes away, causing a circular wait if some other name-lock is also pending for one of the open HANDLER tables. Such situation, for example, can be easily repeated by issuing a RENAME TABLE command in such a way that the existing table is already open as a HANDLER table by another client and this client tries to open a HANDLER to the new table name. The solution is to allow handler tables with older versions (marked for flush) to be closed before waiting for the name-lock completion. This is safe because no other name-lock can be issued between the flush and the check for pending name-locks. The test case for this bug is going to be committed into 5.1 because it requires a test feature only avaiable in 5.1 (wait_condition). --- sql/sql_base.cc | 12 +++++++++++- sql/sql_handler.cc | 47 +++++++++++++++++++++++++++------------------- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 289924ff418..905190cb9cd 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -1745,7 +1745,13 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root, DBUG_RETURN(0); } - /* close handler tables which are marked for flush */ + /* + In order for the back off and re-start process to work properly, + handler tables having old versions (due to FLUSH TABLES or pending + name-lock) MUST be closed. This is specially important if a name-lock + is pending for any table of the handler_tables list, otherwise a + deadlock may occur. + */ if (thd->handler_tables) mysql_ha_flush(thd, (TABLE_LIST*) NULL, MYSQL_HA_REOPEN_ON_USAGE, TRUE); @@ -1810,6 +1816,10 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root, table->db_stat == 0 signals wait_for_locked_table_names that the tables in question are not used any more. See table_is_used call for details. + + Notice that HANDLER tables were already taken care of by + the earlier call to mysql_ha_flush() in this same critical + section. */ close_old_data_files(thd,thd->open_tables,0,0); /* diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc index 89090c9fa63..e87381dd49c 100644 --- a/sql/sql_handler.cc +++ b/sql/sql_handler.cc @@ -182,7 +182,7 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) char *db, *name, *alias; uint dblen, namelen, aliaslen, counter; int error; - TABLE *backup_open_tables, *backup_handler_tables; + TABLE *backup_open_tables; DBUG_ENTER("mysql_ha_open"); DBUG_PRINT("enter",("'%s'.'%s' as '%s' reopen: %d", tables->db, tables->table_name, tables->alias, @@ -211,14 +211,20 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) } } - /* save open_ and handler_ tables state */ - backup_open_tables= thd->open_tables; - backup_handler_tables= thd->handler_tables; + /* + Save and reset the open_tables list so that open_tables() won't + be able to access (or know about) the previous list. And on return + from open_tables(), thd->open_tables will contain only the opened + table. - /* no pre-opened tables */ + The thd->handler_tables list is kept as-is to avoid deadlocks if + open_table(), called by open_tables(), needs to back-off because + of a pending name-lock on the table being opened. + + See open_table() back-off comments for more details. + */ + backup_open_tables= thd->open_tables; thd->open_tables= NULL; - /* to avoid flushes */ - thd->handler_tables= NULL; /* open_tables() will set 'tables->table' if successful. @@ -231,9 +237,12 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) error= open_tables(thd, &tables, &counter, 0); /* restore the state and merge the opened table into handler_tables list */ - thd->handler_tables= thd->open_tables ? - thd->open_tables->next= backup_handler_tables, - thd->open_tables : backup_handler_tables; + if (thd->open_tables) + { + thd->open_tables->next= thd->handler_tables; + thd->handler_tables= thd->open_tables; + } + thd->open_tables= backup_open_tables; if (error) @@ -360,7 +369,7 @@ bool mysql_ha_read(THD *thd, TABLE_LIST *tables, ha_rows select_limit_cnt, ha_rows offset_limit_cnt) { TABLE_LIST *hash_tables; - TABLE *table, *backup_open_tables, *backup_handler_tables; + TABLE *table, *backup_open_tables; MYSQL_LOCK *lock; List list; Protocol *protocol= thd->protocol; @@ -437,20 +446,20 @@ retry: } tables->table=table; - /* save open_ and handler_ tables state */ + /* save open_tables state */ backup_open_tables= thd->open_tables; - backup_handler_tables= thd->handler_tables; - - /* no pre-opened tables */ - thd->open_tables= NULL; - /* to avoid flushes */ - thd->handler_tables= NULL; + /* + mysql_lock_tables() needs thd->open_tables to be set correctly to + be able to handle aborts properly. When the abort happens, it's + safe to not protect thd->handler_tables because it won't close any + tables. + */ + thd->open_tables= thd->handler_tables; lock= mysql_lock_tables(thd, &tables->table, 1, MYSQL_LOCK_NOTIFY_IF_NEED_REOPEN, &need_reopen); /* restore previous context */ - thd->handler_tables= backup_handler_tables; thd->open_tables= backup_open_tables; if (need_reopen) From fd3c6b185515bc6a0ea8598e98531c5f8b326502 Mon Sep 17 00:00:00 2001 From: "davi@virtua-cwbas201-21-158-74.ctb.virtua.com.br" <> Date: Tue, 9 Oct 2007 20:46:33 -0300 Subject: [PATCH 13/19] Bug#28318 CREATE FUNCTION (UDF) requires a schema Bug#29816 Syntactically wrong query fails with misleading error message The core problem is that an SQL-invoked function name can be a that contains no , but the mysql parser insists that all stored procedures (function, procedures and triggers) must have a , which is not true for functions. This problem is especially visible when trying to create a function or when a query contains a syntax error after a function call (in the same query), both will fail with a "No database selected" message if the session is not attached to a particular schema, but the first one should succeed and the second fail with a "syntax error" message. Part of the fix is to revamp the sp name handling so that a schema name may be omitted for functions -- this means that the internal function name representation may not have a dot, which represents that the function doesn't have a schema name. The other part is to place schema checks after the type (function, trigger or procedure) of the routine is known. --- mysql-test/r/sp-error.result | 13 +++++++++++++ mysql-test/r/udf.result | 7 +++++++ mysql-test/t/sp-error.test | 20 ++++++++++++++++++++ mysql-test/t/udf.test | 14 ++++++++++++++ sql/sp.cc | 13 +++---------- sql/sp_head.cc | 31 ++++++++++++++++++++++++++++--- sql/sp_head.h | 11 +---------- sql/sql_yacc.yy | 31 ++++++++++++++++++++++++++----- 8 files changed, 112 insertions(+), 28 deletions(-) diff --git a/mysql-test/r/sp-error.result b/mysql-test/r/sp-error.result index bd0640b2b14..cc217ecd093 100644 --- a/mysql-test/r/sp-error.result +++ b/mysql-test/r/sp-error.result @@ -1452,3 +1452,16 @@ end until true end repeat retry; end// ERROR 42000: LEAVE with no matching label: retry +DROP DATABASE IF EXISTS mysqltest; +CREATE DATABASE mysqltest; +USE mysqltest; +DROP DATABASE mysqltest; +SELECT inexistent(), 1 + ,; +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1 +SELECT inexistent(); +ERROR 42000: FUNCTION inexistent does not exist +SELECT .inexistent(); +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '()' at line 1 +SELECT ..inexistent(); +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '.inexistent()' at line 1 +USE test; diff --git a/mysql-test/r/udf.result b/mysql-test/r/udf.result index 2e9cf217ed6..3e29c780ca8 100644 --- a/mysql-test/r/udf.result +++ b/mysql-test/r/udf.result @@ -296,4 +296,11 @@ Qcache_queries_in_cache 0 drop table t1; drop function metaphon; set GLOBAL query_cache_size=default; +DROP DATABASE IF EXISTS mysqltest; +CREATE DATABASE mysqltest; +USE mysqltest; +DROP DATABASE mysqltest; +CREATE FUNCTION metaphon RETURNS STRING SONAME "UDF_EXAMPLE_LIB"; +DROP FUNCTION metaphon; +USE test; End of 5.0 tests. diff --git a/mysql-test/t/sp-error.test b/mysql-test/t/sp-error.test index 240cda67edc..a1abf4852b0 100644 --- a/mysql-test/t/sp-error.test +++ b/mysql-test/t/sp-error.test @@ -2089,6 +2089,26 @@ end// delimiter ;// +# +# Bug#29816 Syntactically wrong query fails with misleading error message +# + +--disable_warnings +DROP DATABASE IF EXISTS mysqltest; +--enable_warnings +CREATE DATABASE mysqltest; +USE mysqltest; +DROP DATABASE mysqltest; +--error ER_PARSE_ERROR +SELECT inexistent(), 1 + ,; +--error ER_SP_DOES_NOT_EXIST +SELECT inexistent(); +--error ER_PARSE_ERROR +SELECT .inexistent(); +--error ER_PARSE_ERROR +SELECT ..inexistent(); +USE test; + # # BUG#NNNN: New bug synopsis # diff --git a/mysql-test/t/udf.test b/mysql-test/t/udf.test index 75af1f4be4b..6a516a29534 100644 --- a/mysql-test/t/udf.test +++ b/mysql-test/t/udf.test @@ -311,5 +311,19 @@ drop table t1; drop function metaphon; set GLOBAL query_cache_size=default; +# +# Bug#28318 CREATE FUNCTION (UDF) requires a schema +# + +--disable_warnings +DROP DATABASE IF EXISTS mysqltest; +--enable_warnings +CREATE DATABASE mysqltest; +USE mysqltest; +DROP DATABASE mysqltest; +--replace_result $UDF_EXAMPLE_LIB UDF_EXAMPLE_LIB +eval CREATE FUNCTION metaphon RETURNS STRING SONAME "$UDF_EXAMPLE_LIB"; +DROP FUNCTION metaphon; +USE test; --echo End of 5.0 tests. diff --git a/sql/sp.cc b/sql/sp.cc index 75d6fa4618f..0b84e1ad07f 100644 --- a/sql/sp.cc +++ b/sql/sp.cc @@ -1405,12 +1405,12 @@ static bool add_used_routine(LEX *lex, Query_arena *arena, { Sroutine_hash_entry *rn= (Sroutine_hash_entry *)arena->alloc(sizeof(Sroutine_hash_entry) + - key->length); + key->length + 1); if (!rn) // OOM. Error will be reported using fatal_error(). return FALSE; rn->key.length= key->length; rn->key.str= (char *)rn + sizeof(Sroutine_hash_entry); - memcpy(rn->key.str, key->str, key->length); + memcpy(rn->key.str, key->str, key->length + 1); my_hash_insert(&lex->sroutines, (byte *)rn); lex->sroutines_list.link_in_list((byte *)rn, (byte **)&rn->next); rn->belong_to_view= belong_to_view; @@ -1595,7 +1595,7 @@ sp_cache_routines_and_add_tables_aux(THD *thd, LEX *lex, for (Sroutine_hash_entry *rt= start; rt; rt= rt->next) { - sp_name name(rt->key.str, rt->key.length); + sp_name name(thd, rt->key.str, rt->key.length); int type= rt->key.str[0]; sp_head *sp; @@ -1603,13 +1603,6 @@ sp_cache_routines_and_add_tables_aux(THD *thd, LEX *lex, &thd->sp_func_cache : &thd->sp_proc_cache), &name))) { - name.m_name.str= strchr(name.m_qname.str, '.'); - name.m_db.length= name.m_name.str - name.m_qname.str; - name.m_db.str= strmake_root(thd->mem_root, name.m_qname.str, - name.m_db.length); - name.m_name.str+= 1; - name.m_name.length= name.m_qname.length - name.m_db.length - 1; - switch ((ret= db_find_routine(thd, type, &name, &sp))) { case SP_OK: diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 5ad6625efb8..69dda9ec1e8 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -369,17 +369,42 @@ sp_eval_expr(THD *thd, Field *result_field, Item **expr_item_ptr) * */ +sp_name::sp_name(THD *thd, char *key, uint key_len) +{ + m_sroutines_key.str= key; + m_sroutines_key.length= key_len; + m_qname.str= ++key; + m_qname.length= key_len - 1; + if ((m_name.str= strchr(m_qname.str, '.'))) + { + m_db.length= m_name.str - key; + m_db.str= strmake_root(thd->mem_root, key, m_db.length); + m_name.str++; + m_name.length= m_qname.length - m_db.length - 1; + } + else + { + m_name.str= m_qname.str; + m_name.length= m_qname.length; + m_db.str= 0; + m_db.length= 0; + } + m_explicit_name= false; +} + void sp_name::init_qname(THD *thd) { - m_sroutines_key.length= m_db.length + m_name.length + 2; + const uint dot= !!m_db.length; + /* m_sroutines format: m_type + [database + dot] + name + nul */ + m_sroutines_key.length= 1 + m_db.length + dot + m_name.length; if (!(m_sroutines_key.str= thd->alloc(m_sroutines_key.length + 1))) return; m_qname.length= m_sroutines_key.length - 1; m_qname.str= m_sroutines_key.str + 1; - sprintf(m_qname.str, "%.*s.%.*s", + sprintf(m_qname.str, "%.*s%.*s%.*s", m_db.length, (m_db.length ? m_db.str : ""), - m_name.length, m_name.str); + dot, ".", m_name.length, m_name.str); } diff --git a/sql/sp_head.h b/sql/sp_head.h index ebe40ce9c87..7d042367985 100644 --- a/sql/sp_head.h +++ b/sql/sp_head.h @@ -72,16 +72,7 @@ public: Creates temporary sp_name object from key, used mainly for SP-cache lookups. */ - sp_name(char *key, uint key_len) - { - m_sroutines_key.str= key; - m_sroutines_key.length= key_len; - m_name.str= m_qname.str= key + 1; - m_name.length= m_qname.length= key_len - 1; - m_db.str= 0; - m_db.length= 0; - m_explicit_name= false; - } + sp_name(THD *thd, char *key, uint key_len); // Init. the qualified name from the db and name. void init_qname(THD *thd); // thd for memroot allocation diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index e0b9ab28594..7e3c11b5122 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -1569,13 +1569,15 @@ sp_name: | ident { LEX *lex= Lex; - LEX_STRING db; + LEX_STRING db= {0,0}; + THD *thd= YYTHD; + if (check_routine_name($1)) { my_error(ER_SP_WRONG_NAME, MYF(0), $1.str); MYSQL_YYABORT; } - if (lex->copy_db_to(&db.str, &db.length)) + if (thd->db && thd->copy_db_to(&db.str, &db.length)) MYSQL_YYABORT; $$= new sp_name(db, $1, false); if ($$) @@ -1625,6 +1627,13 @@ create_function_tail: my_error(ER_SP_NO_RECURSIVE_CREATE, MYF(0), "FUNCTION"); MYSQL_YYABORT; } + + if (!lex->spname->m_db.length) + { + my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0)); + MYSQL_YYABORT; + } + /* Order is important here: new - reset - init */ sp= new sp_head(); sp->reset_thd_mem_root(thd); @@ -5194,8 +5203,8 @@ simple_expr: #endif /* HAVE_DLOPEN */ { THD *thd= lex->thd; - LEX_STRING db; - if (lex->copy_db_to(&db.str, &db.length)) + LEX_STRING db= {0,0}; + if (thd->db && thd->copy_db_to(&db.str, &db.length)) MYSQL_YYABORT; sp_name *name= new sp_name(db, $1, false); if (name) @@ -9730,7 +9739,13 @@ trigger_tail: my_error(ER_SP_NO_RECURSIVE_CREATE, MYF(0), "TRIGGER"); MYSQL_YYABORT; } - + + if (!$3->m_db.length) + { + my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0)); + MYSQL_YYABORT; + } + if (!(sp= new sp_head())) MYSQL_YYABORT; sp->reset_thd_mem_root(thd); @@ -9813,6 +9828,12 @@ sp_tail: MYSQL_YYABORT; } + if (!$3->m_db.length) + { + my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0)); + MYSQL_YYABORT; + } + lex->stmt_definition_begin= $2; /* Order is important here: new - reset - init */ From 05cf10bdf6b803e943a3348af05fa2a5a76362c4 Mon Sep 17 00:00:00 2001 From: "anozdrin/alik@station." <> Date: Wed, 10 Oct 2007 14:42:29 +0400 Subject: [PATCH 14/19] Cleanup sp.test. --- mysql-test/r/sp.result | 6 +++--- mysql-test/t/sp.test | 10 +++------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index b1120a22dfd..ef173b9661f 100644 --- a/mysql-test/r/sp.result +++ b/mysql-test/r/sp.result @@ -5667,7 +5667,6 @@ t3_id_1 t3_id_2 t4_id DROP PROCEDURE p1| DROP VIEW v1, v2| DROP TABLE t3, t4| -End of 5.0 tests DROP TABLE IF EXISTS bug23760| DROP TABLE IF EXISTS bug23760_log| DROP PROCEDURE IF EXISTS bug23760_update_log| @@ -6145,7 +6144,6 @@ Procedure sql_mode Create Procedure proc_21513 CREATE DEFINER=`root`@`localhost` PROCEDURE `proc_21513`() `my_label`:BEGIN END drop procedure proc_21513| -End of 5.0 tests. drop table t1,t2; CREATE TABLE t1 (a int auto_increment primary key) engine=MyISAM; CREATE TABLE t2 (a int auto_increment primary key, b int) engine=innodb; @@ -6565,4 +6563,6 @@ f1() DROP TABLE t1; DROP FUNCTION f1; -End of 5.0 tests +# ------------------------------------------------------------------ +# -- End of 5.0 tests +# ------------------------------------------------------------------ diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index 465585a693e..501d96c842a 100644 --- a/mysql-test/t/sp.test +++ b/mysql-test/t/sp.test @@ -6642,9 +6642,6 @@ DROP VIEW v1, v2| DROP TABLE t3, t4| ---echo End of 5.0 tests - - # # BUG#23760: ROW_COUNT() and store procedure not owrking together # @@ -7076,9 +7073,6 @@ show create procedure proc_21513| drop procedure proc_21513| -### ---echo End of 5.0 tests. - # # BUG#NNNN: New bug synopsis # @@ -7677,4 +7671,6 @@ DROP FUNCTION f1; ########################################################################### ---echo End of 5.0 tests +--echo # ------------------------------------------------------------------ +--echo # -- End of 5.0 tests +--echo # ------------------------------------------------------------------ From b92f3309f1c020ab146d0d5e685249535018c1fc Mon Sep 17 00:00:00 2001 From: "davi@moksha.com.br" <> Date: Fri, 12 Oct 2007 10:55:46 -0300 Subject: [PATCH 15/19] Bug#31409 RENAME TABLE causes server crash or deadlock when used with HANDLER statements If mysql_lock_tables fails because the lock was aborted, we need to reset thd->some_tables_delete, otherwise we might loop indefinitely because handler's tables are not closed in a standard way, meaning that close_thread_tables() (which resets some_tables_deleted) is not used. This patch fixes sporadical failures of handler_myisam/innodb tests which were introduced by previous fix for this bug. --- sql/sql_handler.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc index e87381dd49c..822f2b2c419 100644 --- a/sql/sql_handler.cc +++ b/sql/sql_handler.cc @@ -466,6 +466,12 @@ retry: { mysql_ha_close_table(thd, tables); hash_tables->table= NULL; + /* + The lock might have been aborted, we need to manually reset + thd->some_tables_deleted because handler's tables are closed + in a non-standard way. Otherwise we might loop indefinitely. + */ + thd->some_tables_deleted= 0; goto retry; } From 99a270ba0f86ed5a163b7f570a0808eb6e4eb8cb Mon Sep 17 00:00:00 2001 From: "malff@lambda.hsd1.co.comcast.net." <> Date: Mon, 15 Oct 2007 19:15:38 -0600 Subject: [PATCH 16/19] Bug#28318 (CREATE FUNCTION (UDF) requires a schema) -- part II The root cause of the issue was that the CREATE FUNCTION grammar, for User Defined Functions, was using the sp_name rule. The sp_name rule is intended for fully qualified stored procedure names, like either ident.ident, or just ident but with a default database implicitly selected. A UDF does not have a fully qualified name, only a name (ident), and should not use the sp_name grammar fragment during parsing. The fix is to re-organize the CREATE FUNCTION grammar, to better separate: - creating UDF (no definer, can have AGGREGATE, simple ident) - creating Stored Functions (definer, no AGGREGATE, fully qualified name) With the test case provided, another issue was exposed which is also fixed: the DROP FUNCTION statement was using sp_name and also failing when no database is implicitly selected, when droping UDF functions. The fix is also to change the grammar so that DROP FUNCTION works with both the ident.ident syntax (to drop a stored function), or just the ident syntax (to drop either a UDF or a Stored Function, in the current database) --- mysql-test/r/sp-error.result | 4 +- mysql-test/r/udf.result | 4 +- mysql-test/t/sp-error.test | 7 +- mysql-test/t/udf.test | 4 +- sql/sql_parse.cc | 6 - sql/sql_yacc.yy | 412 +++++++++++++++++++---------------- 6 files changed, 233 insertions(+), 204 deletions(-) diff --git a/mysql-test/r/sp-error.result b/mysql-test/r/sp-error.result index cc217ecd093..b4bcfbdc7f7 100644 --- a/mysql-test/r/sp-error.result +++ b/mysql-test/r/sp-error.result @@ -1211,7 +1211,7 @@ ERROR 42S02: Unknown table 'c' in field list drop procedure bug15091; drop function if exists bug16896; create aggregate function bug16896() returns int return 1; -ERROR 42000: AGGREGATE is not supported for stored functions +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '() returns int return 1' at line 1 DROP PROCEDURE IF EXISTS bug14702; CREATE IF NOT EXISTS PROCEDURE bug14702() BEGIN @@ -1457,7 +1457,7 @@ CREATE DATABASE mysqltest; USE mysqltest; DROP DATABASE mysqltest; SELECT inexistent(), 1 + ,; -ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1 +ERROR 42000: FUNCTION inexistent does not exist SELECT inexistent(); ERROR 42000: FUNCTION inexistent does not exist SELECT .inexistent(); diff --git a/mysql-test/r/udf.result b/mysql-test/r/udf.result index 3e29c780ca8..4a12e8e6d81 100644 --- a/mysql-test/r/udf.result +++ b/mysql-test/r/udf.result @@ -95,10 +95,10 @@ FR DROP TABLE bug19904; CREATE DEFINER=CURRENT_USER() FUNCTION should_not_parse RETURNS STRING SONAME "should_not_parse.so"; -ERROR HY000: Incorrect usage of SONAME and DEFINER +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'RETURNS STRING SONAME "should_not_parse.so"' at line 2 CREATE DEFINER=someone@somewhere FUNCTION should_not_parse RETURNS STRING SONAME "should_not_parse.so"; -ERROR HY000: Incorrect usage of SONAME and DEFINER +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'RETURNS STRING SONAME "should_not_parse.so"' at line 2 create table t1(f1 int); insert into t1 values(1),(2); explain select myfunc_int(f1) from t1 order by 1; diff --git a/mysql-test/t/sp-error.test b/mysql-test/t/sp-error.test index a1abf4852b0..8133a2271a1 100644 --- a/mysql-test/t/sp-error.test +++ b/mysql-test/t/sp-error.test @@ -1744,7 +1744,7 @@ drop procedure bug15091; drop function if exists bug16896; --enable_warnings ---error ER_SP_NO_AGGREGATE +--error ER_PARSE_ERROR create aggregate function bug16896() returns int return 1; @@ -2099,7 +2099,10 @@ DROP DATABASE IF EXISTS mysqltest; CREATE DATABASE mysqltest; USE mysqltest; DROP DATABASE mysqltest; ---error ER_PARSE_ERROR +# Both ER_SP_DOES_NOT_EXIST and ER_PARSE_ERROR are valid here, +# the result is implementation dependent: +# See Bug#29816 for details +--error ER_SP_DOES_NOT_EXIST SELECT inexistent(), 1 + ,; --error ER_SP_DOES_NOT_EXIST SELECT inexistent(); diff --git a/mysql-test/t/udf.test b/mysql-test/t/udf.test index 6a516a29534..14aef3361e4 100644 --- a/mysql-test/t/udf.test +++ b/mysql-test/t/udf.test @@ -113,11 +113,11 @@ DROP TABLE bug19904; # Bug#21269: DEFINER-clause is allowed for UDF-functions # ---error ER_WRONG_USAGE +--error ER_PARSE_ERROR CREATE DEFINER=CURRENT_USER() FUNCTION should_not_parse RETURNS STRING SONAME "should_not_parse.so"; ---error ER_WRONG_USAGE +--error ER_PARSE_ERROR CREATE DEFINER=someone@somewhere FUNCTION should_not_parse RETURNS STRING SONAME "should_not_parse.so"; # diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 8c61243e3c5..a111208cbf9 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -4016,12 +4016,6 @@ end_with_restore_list: if (check_access(thd,INSERT_ACL,"mysql",0,1,0,0)) break; #ifdef HAVE_DLOPEN - if (sp_find_routine(thd, TYPE_ENUM_FUNCTION, lex->spname, - &thd->sp_func_cache, FALSE)) - { - my_error(ER_UDF_EXISTS, MYF(0), lex->spname->m_name.str); - goto error; - } if (!(res = mysql_create_function(thd, &lex->udf))) send_ok(thd); #else diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 9c07add98d4..c1012b66bf4 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -1121,8 +1121,6 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong *yystacksize); %type cast_type -%type udf_func_type - %type FUNC_ARG0 FUNC_ARG1 FUNC_ARG2 FUNC_ARG3 keyword keyword_sp %type user grant_user @@ -1181,11 +1179,12 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong *yystacksize); statement sp_suid sp_c_chistics sp_a_chistics sp_chistic sp_c_chistic xa load_data opt_field_or_var_spec fields_or_vars opt_load_data_set_spec - definer view_replace_or_algorithm view_replace view_algorithm_opt - view_algorithm view_or_trigger_or_sp view_or_trigger_or_sp_tail + view_replace_or_algorithm view_replace view_algorithm_opt + view_algorithm view_or_trigger_or_sp definer_tail view_suid view_tail view_list_opt view_list view_select - view_check_option trigger_tail sp_tail + view_check_option trigger_tail sp_tail sf_tail udf_tail case_stmt_specification simple_case_stmt searched_case_stmt + definer_opt no_definer definer END_OF_INPUT %type call sp_proc_stmts sp_proc_stmts1 sp_proc_stmt @@ -1570,15 +1569,14 @@ sp_name: | ident { LEX *lex= Lex; - LEX_STRING db= {0,0}; - THD *thd= YYTHD; + LEX_STRING db; if (check_routine_name($1)) { my_error(ER_SP_WRONG_NAME, MYF(0), $1.str); MYSQL_YYABORT; } - if (thd->db && thd->copy_db_to(&db.str, &db.length)) + if (lex->copy_db_to(&db.str, &db.length)) MYSQL_YYABORT; $$= new sp_name(db, $1, false); if ($$) @@ -1586,131 +1584,6 @@ sp_name: } ; -create_function_tail: - RETURNS_SYM udf_type UDF_SONAME_SYM TEXT_STRING_sys - { - LEX *lex=Lex; - if (lex->definer != NULL) - { - /* - DEFINER is a concept meaningful when interpreting SQL code. - UDF functions are compiled. - Using DEFINER with UDF has therefore no semantic, - and is considered a parsing error. - */ - my_error(ER_WRONG_USAGE, MYF(0), "SONAME", "DEFINER"); - MYSQL_YYABORT; - } - lex->sql_command = SQLCOM_CREATE_FUNCTION; - lex->udf.name = lex->spname->m_name; - lex->udf.returns=(Item_result) $2; - lex->udf.dl=$4.str; - } - | '(' - { - THD *thd= YYTHD; - LEX *lex= thd->lex; - Lex_input_stream *lip= thd->m_lip; - sp_head *sp; - - /* - First check if AGGREGATE was used, in that case it's a - syntax error. - */ - if (lex->udf.type == UDFTYPE_AGGREGATE) - { - my_error(ER_SP_NO_AGGREGATE, MYF(0)); - MYSQL_YYABORT; - } - - if (lex->sphead) - { - my_error(ER_SP_NO_RECURSIVE_CREATE, MYF(0), "FUNCTION"); - MYSQL_YYABORT; - } - - if (!lex->spname->m_db.length) - { - my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0)); - MYSQL_YYABORT; - } - - /* Order is important here: new - reset - init */ - sp= new sp_head(); - sp->reset_thd_mem_root(thd); - sp->init(lex); - sp->init_sp_name(thd, lex->spname); - - sp->m_type= TYPE_ENUM_FUNCTION; - lex->sphead= sp; - /* - * We have to turn of CLIENT_MULTI_QUERIES while parsing a - * stored procedure, otherwise yylex will chop it into pieces - * at each ';'. - */ - sp->m_old_cmq= thd->client_capabilities & CLIENT_MULTI_QUERIES; - thd->client_capabilities &= ~CLIENT_MULTI_QUERIES; - lex->sphead->m_param_begin= lip->tok_start+1; - } - sp_fdparam_list ')' - { - THD *thd= YYTHD; - LEX *lex= thd->lex; - Lex_input_stream *lip= thd->m_lip; - - lex->sphead->m_param_end= lip->tok_start; - } - RETURNS_SYM - { - LEX *lex= Lex; - lex->charset= NULL; - lex->length= lex->dec= NULL; - lex->interval_list.empty(); - lex->type= 0; - } - type - { - LEX *lex= Lex; - sp_head *sp= lex->sphead; - - if (sp->fill_field_definition(YYTHD, lex, - (enum enum_field_types) $8, - &sp->m_return_field_def)) - MYSQL_YYABORT; - - bzero((char *)&lex->sp_chistics, sizeof(st_sp_chistics)); - } - sp_c_chistics - { - THD *thd= YYTHD; - LEX *lex= thd->lex; - Lex_input_stream *lip= thd->m_lip; - - lex->sphead->m_chistics= &lex->sp_chistics; - lex->sphead->m_body_begin= lip->tok_start; - } - sp_proc_stmt - { - LEX *lex= Lex; - sp_head *sp= lex->sphead; - - if (sp->is_not_allowed_in_function("function")) - MYSQL_YYABORT; - - lex->sql_command= SQLCOM_CREATE_SPFUNCTION; - sp->init_strings(YYTHD, lex); - if (!(sp->m_flags & sp_head::HAS_RETURN)) - { - my_error(ER_SP_NORETURN, MYF(0), sp->m_qname.str); - MYSQL_YYABORT; - } - /* Restore flag if it was cleared above */ - if (sp->m_old_cmq) - YYTHD->client_capabilities |= CLIENT_MULTI_QUERIES; - sp->restore_thd_mem_root(YYTHD); - } - ; - sp_a_chistics: /* Empty */ {} | sp_a_chistics sp_chistic {} @@ -3011,10 +2884,6 @@ opt_select_from: opt_limit_clause {} | select_from select_lock_type; -udf_func_type: - /* empty */ { $$ = UDFTYPE_FUNCTION; } - | AGGREGATE_SYM { $$ = UDFTYPE_AGGREGATE; }; - udf_type: STRING_SYM {$$ = (int) STRING_RESULT; } | REAL {$$ = (int) REAL_RESULT; } @@ -3678,7 +3547,7 @@ alter: lex->sql_command= SQLCOM_ALTER_FUNCTION; lex->spname= $3; } - | ALTER view_algorithm_opt definer view_suid + | ALTER view_algorithm_opt definer_opt view_suid VIEW_SYM table_ident { THD *thd= YYTHD; @@ -5204,9 +5073,31 @@ simple_expr: #endif /* HAVE_DLOPEN */ { THD *thd= lex->thd; - LEX_STRING db= {0,0}; - if (thd->db && thd->copy_db_to(&db.str, &db.length)) + LEX_STRING db; + if (! thd->db && ! lex->sphead) + { + /* + The proper error message should be in the lines of: + Can't resolve () to a function call, + because this function: + - is not a native function, + - is not a user defined function, + - can not match a stored function since no database is selected. + Reusing ER_SP_DOES_NOT_EXIST have a message consistent with + the case when a default database exist, see below. + */ + my_error(ER_SP_DOES_NOT_EXIST, MYF(0), + "FUNCTION", $1); MYSQL_YYABORT; + } + + if (lex->copy_db_to(&db.str, &db.length)) + MYSQL_YYABORT; + + /* + From here, the parser assumes () is a stored function, + as a last choice. This later can lead to ER_SP_DOES_NOT_EXIST. + */ sp_name *name= new sp_name(db, $1, false); if (name) name->init_qname(thd); @@ -6508,9 +6399,11 @@ drop: lex->drop_if_exists=$3; lex->name=$4.str; } - | DROP FUNCTION_SYM if_exists sp_name + | DROP FUNCTION_SYM if_exists ident '.' ident { - LEX *lex=Lex; + THD *thd= YYTHD; + LEX *lex= thd->lex; + sp_name *spname; if (lex->sphead) { my_error(ER_SP_NO_DROP_SP, MYF(0), "FUNCTION"); @@ -6518,7 +6411,28 @@ drop: } lex->sql_command = SQLCOM_DROP_FUNCTION; lex->drop_if_exists= $3; - lex->spname= $4; + spname= new sp_name($4, $6, true); + spname->init_qname(thd); + lex->spname= spname; + } + | DROP FUNCTION_SYM if_exists ident + { + THD *thd= YYTHD; + LEX *lex= thd->lex; + LEX_STRING db= {0, 0}; + sp_name *spname; + if (lex->sphead) + { + my_error(ER_SP_NO_DROP_SP, MYF(0), "FUNCTION"); + MYSQL_YYABORT; + } + if (thd->db && lex->copy_db_to(&db.str, &db.length)) + MYSQL_YYABORT; + lex->sql_command = SQLCOM_DROP_FUNCTION; + lex->drop_if_exists= $3; + spname= new sp_name(db, $4, false); + spname->init_qname(thd); + lex->spname= spname; } | DROP PROCEDURE if_exists sp_name { @@ -9567,19 +9481,27 @@ subselect_end: **************************************************************************/ view_or_trigger_or_sp: - definer view_or_trigger_or_sp_tail - {} - | view_replace_or_algorithm definer view_tail - {} + definer definer_tail + {} + | no_definer no_definer_tail + {} + | view_replace_or_algorithm definer_opt view_tail + {} ; -view_or_trigger_or_sp_tail: - view_tail - {} +definer_tail: + view_tail | trigger_tail - {} | sp_tail - {} + | sf_tail + ; + +no_definer_tail: + view_tail + | trigger_tail + | sp_tail + | sf_tail + | udf_tail ; /************************************************************************** @@ -9588,23 +9510,31 @@ view_or_trigger_or_sp_tail: **************************************************************************/ +definer_opt: + no_definer + | definer + ; + +no_definer: + /* empty */ + { + /* + We have to distinguish missing DEFINER-clause from case when + CURRENT_USER specified as definer explicitly in order to properly + handle CREATE TRIGGER statements which come to replication thread + from older master servers (i.e. to create non-suid trigger in this + case). + */ + YYTHD->lex->definer= 0; + } + ; + definer: - /* empty */ - { - /* - We have to distinguish missing DEFINER-clause from case when - CURRENT_USER specified as definer explicitly in order to properly - handle CREATE TRIGGER statements which come to replication thread - from older master servers (i.e. to create non-suid trigger in this - case). - */ - YYTHD->lex->definer= 0; - } - | DEFINER_SYM EQ user - { - YYTHD->lex->definer= get_current_user(YYTHD, $3); - } - ; + DEFINER_SYM EQ user + { + YYTHD->lex->definer= get_current_user(YYTHD, $3); + } +; /************************************************************************** @@ -9755,12 +9685,6 @@ trigger_tail: MYSQL_YYABORT; } - if (!$3->m_db.length) - { - my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0)); - MYSQL_YYABORT; - } - if (!(sp= new sp_head())) MYSQL_YYABORT; sp->reset_thd_mem_root(thd); @@ -9822,17 +9746,131 @@ trigger_tail: **************************************************************************/ +udf_tail: + AGGREGATE_SYM remember_name FUNCTION_SYM ident + RETURNS_SYM udf_type UDF_SONAME_SYM TEXT_STRING_sys + { + LEX *lex=Lex; + lex->sql_command = SQLCOM_CREATE_FUNCTION; + lex->udf.type= UDFTYPE_AGGREGATE; + lex->stmt_definition_begin= $2; + lex->udf.name = $4; + lex->udf.returns=(Item_result) $6; + lex->udf.dl=$8.str; + } + | remember_name FUNCTION_SYM ident + RETURNS_SYM udf_type UDF_SONAME_SYM TEXT_STRING_sys + { + LEX *lex=Lex; + lex->sql_command = SQLCOM_CREATE_FUNCTION; + lex->udf.type= UDFTYPE_FUNCTION; + lex->stmt_definition_begin= $1; + lex->udf.name = $3; + lex->udf.returns=(Item_result) $5; + lex->udf.dl=$7.str; + } + ; + +sf_tail: + remember_name /* $1 */ + FUNCTION_SYM /* $2 */ + sp_name /* $3 */ + '(' /* 44 */ + { /* $5 */ + THD *thd= YYTHD; + LEX *lex= thd->lex; + Lex_input_stream *lip= thd->m_lip; + sp_head *sp; + + lex->stmt_definition_begin= $1; + lex->spname= $3; + + if (lex->sphead) + { + my_error(ER_SP_NO_RECURSIVE_CREATE, MYF(0), "FUNCTION"); + MYSQL_YYABORT; + } + + /* Order is important here: new - reset - init */ + sp= new sp_head(); + sp->reset_thd_mem_root(thd); + sp->init(lex); + sp->init_sp_name(thd, lex->spname); + + sp->m_type= TYPE_ENUM_FUNCTION; + lex->sphead= sp; + /* + * We have to turn of CLIENT_MULTI_QUERIES while parsing a + * stored procedure, otherwise yylex will chop it into pieces + * at each ';'. + */ + sp->m_old_cmq= thd->client_capabilities & CLIENT_MULTI_QUERIES; + thd->client_capabilities &= ~CLIENT_MULTI_QUERIES; + lex->sphead->m_param_begin= lip->tok_start+1; + } + sp_fdparam_list /* $6 */ + ')' /* $7 */ + { /* $8 */ + THD *thd= YYTHD; + LEX *lex= thd->lex; + Lex_input_stream *lip= thd->m_lip; + + lex->sphead->m_param_end= lip->tok_start; + } + RETURNS_SYM /* $9 */ + { /* $10 */ + LEX *lex= Lex; + lex->charset= NULL; + lex->length= lex->dec= NULL; + lex->interval_list.empty(); + lex->type= 0; + } + type /* $11 */ + { /* $12 */ + LEX *lex= Lex; + sp_head *sp= lex->sphead; + + if (sp->fill_field_definition(YYTHD, lex, + (enum enum_field_types) $11, + &sp->m_return_field_def)) + MYSQL_YYABORT; + + bzero((char *)&lex->sp_chistics, sizeof(st_sp_chistics)); + } + sp_c_chistics /* $13 */ + { /* $14 */ + THD *thd= YYTHD; + LEX *lex= thd->lex; + Lex_input_stream *lip= thd->m_lip; + + lex->sphead->m_chistics= &lex->sp_chistics; + lex->sphead->m_body_begin= lip->tok_start; + } + sp_proc_stmt /* $15 */ + { + LEX *lex= Lex; + sp_head *sp= lex->sphead; + + if (sp->is_not_allowed_in_function("function")) + MYSQL_YYABORT; + + lex->sql_command= SQLCOM_CREATE_SPFUNCTION; + sp->init_strings(YYTHD, lex); + if (!(sp->m_flags & sp_head::HAS_RETURN)) + { + my_error(ER_SP_NORETURN, MYF(0), sp->m_qname.str); + MYSQL_YYABORT; + } + /* Restore flag if it was cleared above */ + if (sp->m_old_cmq) + YYTHD->client_capabilities |= CLIENT_MULTI_QUERIES; + sp->restore_thd_mem_root(YYTHD); + } + ; + + sp_tail: - udf_func_type remember_name FUNCTION_SYM sp_name - { - LEX *lex=Lex; - lex->udf.type= $1; - lex->stmt_definition_begin= $2; - lex->spname= $4; - } - create_function_tail - {} - | PROCEDURE remember_name sp_name + PROCEDURE remember_name sp_name { LEX *lex= Lex; sp_head *sp; @@ -9843,12 +9881,6 @@ sp_tail: MYSQL_YYABORT; } - if (!$3->m_db.length) - { - my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0)); - MYSQL_YYABORT; - } - lex->stmt_definition_begin= $2; /* Order is important here: new - reset - init */ From a9c6ed46e2c76fd73744c28564cfc216206bb469 Mon Sep 17 00:00:00 2001 From: "malff@lambda.hsd1.co.comcast.net." <> Date: Tue, 16 Oct 2007 11:16:31 -0600 Subject: [PATCH 17/19] Implementing code review comments --- mysql-test/r/sp.result | 15 +++++++++++++++ mysql-test/t/sp.test | 29 +++++++++++++++++++++++++++++ sql/sql_udf.cc | 4 ++-- 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index ef173b9661f..061bbafd9a1 100644 --- a/mysql-test/r/sp.result +++ b/mysql-test/r/sp.result @@ -6563,6 +6563,21 @@ f1() DROP TABLE t1; DROP FUNCTION f1; +DROP PROCEDURE IF EXISTS db28318_a.t1; +DROP PROCEDURE IF EXISTS db28318_b.t2; +DROP DATABASE IF EXISTS db28318_a; +DROP DATABASE IF EXISTS db28318_b; +CREATE DATABASE db28318_a; +CREATE DATABASE db28318_b; +CREATE PROCEDURE db28318_a.t1() SELECT "db28318_a.t1"; +CREATE PROCEDURE db28318_b.t2() CALL t1(); +use db28318_a; +CALL db28318_b.t2(); +ERROR 42000: PROCEDURE db28318_b.t1 does not exist +DROP PROCEDURE db28318_a.t1; +DROP PROCEDURE db28318_b.t2; +DROP DATABASE db28318_a; +DROP DATABASE db28318_b; # ------------------------------------------------------------------ # -- End of 5.0 tests # ------------------------------------------------------------------ diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index 501d96c842a..785e7e3793c 100644 --- a/mysql-test/t/sp.test +++ b/mysql-test/t/sp.test @@ -7671,6 +7671,35 @@ DROP FUNCTION f1; ########################################################################### +# +# Bug#28318 (CREATE FUNCTION (UDF) requires a schema) +# + +--disable_warnings +DROP PROCEDURE IF EXISTS db28318_a.t1; +DROP PROCEDURE IF EXISTS db28318_b.t2; +DROP DATABASE IF EXISTS db28318_a; +DROP DATABASE IF EXISTS db28318_b; +--enable_warnings + +CREATE DATABASE db28318_a; +CREATE DATABASE db28318_b; + +CREATE PROCEDURE db28318_a.t1() SELECT "db28318_a.t1"; +CREATE PROCEDURE db28318_b.t2() CALL t1(); + +use db28318_a; + +# In db28318_b.t2, t1 refers to db28318_b.t1 +--error ER_SP_DOES_NOT_EXIST +CALL db28318_b.t2(); + +DROP PROCEDURE db28318_a.t1; +DROP PROCEDURE db28318_b.t2; +DROP DATABASE db28318_a; +DROP DATABASE db28318_b; + + --echo # ------------------------------------------------------------------ --echo # -- End of 5.0 tests --echo # ------------------------------------------------------------------ diff --git a/sql/sql_udf.cc b/sql/sql_udf.cc index 077660f0bb9..ce6cd43eace 100644 --- a/sql/sql_udf.cc +++ b/sql/sql_udf.cc @@ -426,14 +426,14 @@ int mysql_create_function(THD *thd,udf_func *udf) } if (udf->name.length > NAME_LEN) { - my_error(ER_TOO_LONG_IDENT, MYF(0), udf->name); + my_error(ER_TOO_LONG_IDENT, MYF(0), udf->name.str); DBUG_RETURN(1); } rw_wrlock(&THR_LOCK_udf); if ((hash_search(&udf_hash,(byte*) udf->name.str, udf->name.length))) { - my_error(ER_UDF_EXISTS, MYF(0), udf->name); + my_error(ER_UDF_EXISTS, MYF(0), udf->name.str); goto err; } if (!(dl = find_udf_dl(udf->dl))) From f3a6483945ff8af19bacfc25ea613d4968b491ba Mon Sep 17 00:00:00 2001 From: "malff@lambda.hsd1.co.comcast.net." <> Date: Tue, 16 Oct 2007 14:27:20 -0600 Subject: [PATCH 18/19] Fixed broken call to my_error --- sql/sql_yacc.yy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index c1012b66bf4..3fd8d339f31 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -5087,7 +5087,7 @@ simple_expr: the case when a default database exist, see below. */ my_error(ER_SP_DOES_NOT_EXIST, MYF(0), - "FUNCTION", $1); + "FUNCTION", $1.str); MYSQL_YYABORT; } From 86082dfcefb83f8bb81f675bfbf7c234449d1d51 Mon Sep 17 00:00:00 2001 From: "dkatz@damien-katzs-computer.local" <> Date: Wed, 17 Oct 2007 17:54:11 -0400 Subject: [PATCH 19/19] Bug #29804 UDF parameters don't contain correct string length Previously, UDF *_init functions were passed constant strings with erroneous lengths. The length came from the containing variable's size, not the length of the value itself. Now the *_init functions get the constant as a null terminated string with the correct length supplied too. --- mysql-test/r/udf.result | 24 ++++++++++++++++++++++++ mysql-test/t/udf.test | 36 ++++++++++++++++++++++++++++++++++++ sql/item_func.cc | 3 ++- sql/udf_example.c | 35 +++++++++++++++++++++++++++++++++++ sql/udf_example.def | 2 ++ 5 files changed, 99 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/udf.result b/mysql-test/r/udf.result index 4a12e8e6d81..e6797796ea0 100644 --- a/mysql-test/r/udf.result +++ b/mysql-test/r/udf.result @@ -303,4 +303,28 @@ DROP DATABASE mysqltest; CREATE FUNCTION metaphon RETURNS STRING SONAME "UDF_EXAMPLE_LIB"; DROP FUNCTION metaphon; USE test; +CREATE TABLE const_len_bug ( +str_const varchar(4000), +result1 varchar(4000), +result2 varchar(4000) +); +CREATE TRIGGER check_const_len_trigger BEFORE INSERT ON const_len_bug FOR EACH ROW BEGIN +set NEW.str_const = 'bar'; +set NEW.result2 = check_const_len(NEW.str_const); +END | +CREATE PROCEDURE check_const_len_sp (IN str_const VARCHAR(4000)) +BEGIN +DECLARE result VARCHAR(4000); +SET result = check_const_len(str_const); +insert into const_len_bug values(str_const, result, ""); +END | +CREATE FUNCTION check_const_len RETURNS string SONAME "UDF_EXAMPLE_LIB"; +CALL check_const_len_sp("foo"); +SELECT * from const_len_bug; +str_const result1 result2 +bar Correct length Correct length +DROP FUNCTION check_const_len; +DROP PROCEDURE check_const_len_sp; +DROP TRIGGER check_const_len_trigger; +DROP TABLE const_len_bug; End of 5.0 tests. diff --git a/mysql-test/t/udf.test b/mysql-test/t/udf.test index 14aef3361e4..22b8ed10a49 100644 --- a/mysql-test/t/udf.test +++ b/mysql-test/t/udf.test @@ -326,4 +326,40 @@ eval CREATE FUNCTION metaphon RETURNS STRING SONAME "$UDF_EXAMPLE_LIB"; DROP FUNCTION metaphon; USE test; +# +# Bug #29804 UDF parameters don't contain correct string length +# + +CREATE TABLE const_len_bug ( + str_const varchar(4000), + result1 varchar(4000), + result2 varchar(4000) +); + +DELIMITER |; +CREATE TRIGGER check_const_len_trigger BEFORE INSERT ON const_len_bug FOR EACH ROW BEGIN + set NEW.str_const = 'bar'; + set NEW.result2 = check_const_len(NEW.str_const); +END | + +CREATE PROCEDURE check_const_len_sp (IN str_const VARCHAR(4000)) +BEGIN +DECLARE result VARCHAR(4000); +SET result = check_const_len(str_const); +insert into const_len_bug values(str_const, result, ""); +END | +DELIMITER ;| + +--replace_result $UDF_EXAMPLE_LIB UDF_EXAMPLE_LIB +eval CREATE FUNCTION check_const_len RETURNS string SONAME "$UDF_EXAMPLE_LIB"; + +CALL check_const_len_sp("foo"); + +SELECT * from const_len_bug; + +DROP FUNCTION check_const_len; +DROP PROCEDURE check_const_len_sp; +DROP TRIGGER check_const_len_trigger; +DROP TABLE const_len_bug; + --echo End of 5.0 tests. diff --git a/sql/item_func.cc b/sql/item_func.cc index f7103c22581..e4d370bbdf2 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -2924,7 +2924,8 @@ udf_handler::fix_fields(THD *thd, Item_result_field *func, String *res= arguments[i]->val_str(&buffers[i]); if (arguments[i]->null_value) continue; - f_args.args[i]= (char*) res->ptr(); + f_args.args[i]= (char*) res->c_ptr(); + f_args.lengths[i]= res->length(); break; } case INT_RESULT: diff --git a/sql/udf_example.c b/sql/udf_example.c index 0f28c2a14b0..df3a69755ad 100644 --- a/sql/udf_example.c +++ b/sql/udf_example.c @@ -1106,4 +1106,39 @@ char * is_const(UDF_INIT *initid, UDF_ARGS *args __attribute__((unused)), } + +my_bool check_const_len_init(UDF_INIT *initid, UDF_ARGS *args, char *message) +{ + if (args->arg_count != 1) + { + strmov(message, "CHECK_CONST_LEN accepts only one argument"); + return 1; + } + if (args->args[0] == 0) + { + initid->ptr= (char*)"Not constant"; + } + else if(strlen(args->args[0]) == args->lengths[0]) + { + initid->ptr= (char*)"Correct length"; + } + else + { + initid->ptr= (char*)"Wrong length"; + } + initid->max_length = 100; + return 0; +} + +char * check_const_len(UDF_INIT *initid, UDF_ARGS *args __attribute__((unused)), + char *result, unsigned long *length, + char *is_null, char *error __attribute__((unused))) +{ + strmov(result, initid->ptr); + *length= strlen(result); + *is_null= 0; + return result; +} + + #endif /* HAVE_DLOPEN */ diff --git a/sql/udf_example.def b/sql/udf_example.def index 7a87147d7b6..3d569941cc8 100644 --- a/sql/udf_example.def +++ b/sql/udf_example.def @@ -23,3 +23,5 @@ EXPORTS avgcost is_const is_const_init + check_const_len + check_const_len_init