From 0569a827ea322fc0cd391b7086d1d98e745d1924 Mon Sep 17 00:00:00 2001 From: Georgi Kodinov Date: Tue, 2 Feb 2010 18:30:23 +0200 Subject: Bug #45989 take 2 : memory leak after explain encounters an error in the query. Fixes a leak after materializing a GROUP BY subquery to a temp table when the subquery has a blob column in the SELECT list. Fixed by correctly destructing temporary buffers for re-usable queries --- sql/sql_select.h | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) (limited to 'sql/sql_select.h') diff --git a/sql/sql_select.h b/sql/sql_select.h index dd99d358bac..bfff0a0ffa2 100644 --- a/sql/sql_select.h +++ b/sql/sql_select.h @@ -354,7 +354,25 @@ public: */ bool no_const_tables; - JOIN *tmp_join; ///< copy of this JOIN to be used with temporary tables + /** + Copy of this JOIN to be used with temporary tables. + + tmp_join is used when the JOIN needs to be "reusable" (e.g. in a subquery + that gets re-executed several times) and we know will use temporary tables + for materialization. The materialization to a temporary table overwrites the + JOIN structure to point to the temporary table after the materialization is + done. This is where tmp_join is used : it's a copy of the JOIN before the + materialization and is used in restoring before re-execution by overwriting + the current JOIN structure with the saved copy. + Because of this we should pay extra care of not freeing up helper structures + that are referenced by the original contents of the JOIN. We can check for + this by making sure the "current" join is not the temporary copy, e.g. + !tmp_join || tmp_join != join + + We should free these sub-structures at JOIN::destroy() if the "current" join + has a copy is not that copy. + */ + JOIN *tmp_join; ROLLUP rollup; ///< Used with rollup bool select_distinct; ///< Set if SELECT DISTINCT -- cgit v1.2.1 From e5a38da7ff68b174b254fb18d4f16977cebe1617 Mon Sep 17 00:00:00 2001 From: Sergey Glukhov Date: Wed, 10 Feb 2010 18:56:47 +0400 Subject: Bug#45195 valgrind warnings about uninitialized values in store_record_in_cache() The problem becomes apparent only if HAVE_purify is undefined. It related to the part of code placed in open_table_from_share() fuction where we initialize record buffer only if HAVE_purify is enabled. So in case of HAVE_purify=OFF record buffer is not initialized on open table stage. Next we read key, find NULL value and update appropriate null bit but do not update record buffer. After that the record is stored in the join cache(store_record_in_cache). For CHAR fields we strip trailing spaces and in our case this procedure uses uninitialized record buffer. The fix is to skip stripping space procedure in case of null values for CHAR fields(partially based on 6.0 JOIN_CACHE implementation). --- sql/sql_select.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'sql/sql_select.h') diff --git a/sql/sql_select.h b/sql/sql_select.h index bfff0a0ffa2..ef43f9b049c 100644 --- a/sql/sql_select.h +++ b/sql/sql_select.h @@ -95,6 +95,10 @@ typedef struct st_table_ref } TABLE_REF; + +#define CACHE_BLOB 1 /* blob field */ +#define CACHE_STRIPPED 2 /* field stripped of trailing spaces */ + /** CACHE_FIELD and JOIN_CACHE is used on full join to cache records in outer table @@ -103,8 +107,8 @@ typedef struct st_table_ref typedef struct st_cache_field { uchar *str; uint length, blob_length; - Field_blob *blob_field; - bool strip; + Field *field; + uint type; /**< category of the of the copied field (CACHE_BLOB et al.) */ } CACHE_FIELD; -- cgit v1.2.1 From 5107f6b9b47e41c03d2c75f9d4d70b25dc0aab08 Mon Sep 17 00:00:00 2001 From: Georgi Kodinov Date: Thu, 11 Feb 2010 19:41:53 +0200 Subject: Addendum to bug #46175 : use and check for the correct error values when converting to a enumerated type. --- sql/sql_select.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'sql/sql_select.h') diff --git a/sql/sql_select.h b/sql/sql_select.h index ef43f9b049c..8d3520bf9c8 100644 --- a/sql/sql_select.h +++ b/sql/sql_select.h @@ -736,10 +736,11 @@ public: we need to check for errors executing it and react accordingly */ if (!res && table->in_use->is_error()) - res= 2; + res= 1; /* STORE_KEY_FATAL */ dbug_tmp_restore_column_map(table->write_set, old_map); null_key= to_field->is_null() || item->null_value; - return (err != 0 || res > 2 ? STORE_KEY_FATAL : (store_key_result) res); + return ((err != 0 || res < 0 || res > 2) ? STORE_KEY_FATAL : + (store_key_result) res); } }; @@ -775,10 +776,10 @@ protected: we need to check for errors executing it and react accordingly */ if (!err && to_field->table->in_use->is_error()) - err= 2; + err= 1; /* STORE_KEY_FATAL */ } null_key= to_field->is_null() || item->null_value; - return (err > 2 ? STORE_KEY_FATAL : (store_key_result) err); + return ((err < 0 || err > 2) ? STORE_KEY_FATAL : (store_key_result) err); } }; -- cgit v1.2.1 From 780566d4c5491f51cd9d058a8a2294462e3c5bbe Mon Sep 17 00:00:00 2001 From: Georgi Kodinov Date: Mon, 15 Feb 2010 10:54:27 +0200 Subject: Addendum 2 for bug #46175: NULL read_view and consistent read assertion Fixed a compilation warning. --- sql/sql_select.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'sql/sql_select.h') diff --git a/sql/sql_select.h b/sql/sql_select.h index 8d3520bf9c8..b39827ef61b 100644 --- a/sql/sql_select.h +++ b/sql/sql_select.h @@ -769,7 +769,7 @@ protected: if ((res= item->save_in_field(to_field, 1))) { if (!err) - err= res; + err= res < 0 ? 1 : res; /* 1=STORE_KEY_FATAL */ } /* Item::save_in_field() may call Item::val_xxx(). And if this is a subquery @@ -779,7 +779,7 @@ protected: err= 1; /* STORE_KEY_FATAL */ } null_key= to_field->is_null() || item->null_value; - return ((err < 0 || err > 2) ? STORE_KEY_FATAL : (store_key_result) err); + return (err > 2 ? STORE_KEY_FATAL : (store_key_result) err); } }; -- cgit v1.2.1