Hello Igor, Plese find the comments below. On Sat, Oct 30, 2010 at 12:56:10PM -0700, Igor Babaev wrote:

At file:///home/igor/maria/maria-5.3-mwl128-bug668290/

------------------------------------------------------------ revno: 2840 revision-id: igor@askmonty.org-20101030195609-h53be2cdzjlz38bz parent: igor@askmonty.org-20101030130745-563ypxtkcwv4vbfg committer: Igor Babaev <igor@askmonty.org> branch nick: maria-5.3-mwl128-bug668290 timestamp: Sat 2010-10-30 12:56:09 -0700 message: Fixed LP bug #668290. Prohibited to use hash join algorithm BNLH if join attributes need non-binary collations. It has to be done because BNLH does not support join for such attributes yet. Later this limitations will be lifted.

Changed default collations for the schemes of some test cases to preserve the old execution plans. ... === modified file 'sql/field.h' --- a/sql/field.h 2010-09-23 22:00:32 +0000 +++ b/sql/field.h 2010-10-30 19:56:09 +0000 @@ -585,6 +585,12 @@ } /* Hash value */ virtual void hash(ulong *nr, ulong *nr2); + + virtual bool hash_join_is_possible(bool globally_allowed) + { + return globally_allowed; + } Is there any partiular reason why have this globally_allowed parameter?

In my understanding, a field either allows hash or not, and that is regardless of whether it was "globally allowed", so the check of whether hash join is globally allowed should be done outside of the function. Also, it would be nice to have here a comment specifying the meaning of this function. (I know all this is supposed to be temporary, but expirience shows that often supposedly temporary things stay for a long time, that's why it's nice to put a comment).

+ friend bool reopen_table(THD *,struct st_table *,bool); friend int cre_myisam(char * name, register TABLE *form, uint options, ulonglong auto_increment_value); @@ -760,6 +766,12 @@ my_decimal *val_decimal(my_decimal *); virtual bool str_needs_quotes() { return TRUE; } uint is_equal(Create_field *new_field); + + bool hash_join_is_possible(bool globally_allowed) + { + /* TODO: support hash joins for non-binary collations */ + return globally_allowed && (flags & BINARY_FLAG); + } };

@@ -1904,6 +1916,7 @@ uint size_of() const { return sizeof(*this); } int reset(void) { return !maybe_null() || Field_blob::reset(); } geometry_type get_geometry_type() { return geom_type; }; + bool hash_join_is_possible(bool globally_allowed) { return FALSE; } }; #endif /*HAVE_SPATIAL*/

=== modified file 'sql/sql_select.cc' --- a/sql/sql_select.cc 2010-10-30 13:07:45 +0000 +++ b/sql/sql_select.cc 2010-10-30 19:56:09 +0000 @@ -3547,7 +3547,7 @@ /* Additional optimization: if we're processing "t.key BETWEEN c1 AND c1" then proceed as if we were processing - "t.key = c1". + "t.key = c1".sis TODO: This is a very limited fix. A more generic fix is possible. Is the above a meaningful change?

There are 2 options: A) Make equality propagation code be able to handle BETWEEN @@ -5946,6 +5946,44 @@ }

+/** + @brief + Check whether hash join algorithm can be used to join this table + + @param is_allowed usage of hash join is allowed + + @details + This function finds out whether the ref items that have been chosen + by the planner to access this table can be used for hash join algorithms. + The answer depends on a certain property of the the fields of the + joined tables on which the hash join key is built. If hash join is + allowed for all these fields the answer is positive unless hash join + algorithms are not allowed for the query at all. + + @note + The function is supposed to be called now only after the function + get_best_combination has been called. + + @retval TRUE it's possible to use hash join to join this table + @retval FALSE otherwise +*/ + +bool JOIN_TAB::hash_join_is_possible(bool is_allowed) +{ + if (!is_allowed) + return FALSE;

Same as with globally_allowed. I think the parameter is redundant and rather confusing.

+ if (type != JT_REF && type != JT_EQ_REF) + return FALSE; + KEY *keyinfo= &table->key_info[ref.key]; + for (uint i= 0; i < ref.key_parts; i++) + { + if (!(keyinfo->key_part[i].field->hash_join_is_possible(TRUE))) + return FALSE; + } + return TRUE; +} + + static uint cache_record_length(JOIN *join,uint idx) { @@ -7649,8 +7687,10 @@ &bufsz, &flags, &cost);

if ((cache_level <=4 && !no_hashed_cache) || no_bka_cache || - (flags & HA_MRR_NO_ASSOCIATION) && cache_level <=6) + ((flags & HA_MRR_NO_ASSOCIATION) && cache_level <=6)) { + if (!tab->hash_join_is_possible(TRUE)) + goto no_join_cache; if (cache_level == 3) prev_cache= 0; if ((tab->cache= new JOIN_CACHE_BNLH(join, tab, prev_cache)) &&

=== modified file 'sql/sql_select.h' --- a/sql/sql_select.h 2010-10-30 13:07:45 +0000 +++ b/sql/sql_select.h 2010-10-30 19:56:09 +0000 @@ -391,6 +391,7 @@ return max_used_fieldlength; } double get_partial_join_cardinality() { return partial_join_cardinality; } + bool hash_join_is_possible(bool is_allowed); } JOIN_TAB;

B8R Sergey -- Sergey Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog