Hi!
Please find review comments below. Ping me on irc if there are any
questions.
On Tue, Jan 13, 2009 at 01:23:08PM +0000, Sergey Glukhov wrote:
> #At file:///home/gluh/MySQL/mysql-6.0-opt/ based on
> revid:sergefp@stripped
>
> 2707 Sergey Glukhov 2009-01-13
> WL#4424 Full index condition pushdown with batched key access join(3rd ver)
> added index condition pushdown for BKA[_UNIQUE] join
...
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc 2008-12-24 20:41:33 +0000
> +++ b/sql/sql_select.cc 2009-01-13 13:22:47 +0000
> @@ -8235,12 +8235,14 @@ Item *make_cond_remainder(Item *cond, bo
> in tab->select_cond
> keyno Index for which extract and push the condition
> other_tbls_ok TRUE <=> Fields of other non-const tables are allowed
> + options Join's options (checking for SELECT_DESCRIBE)
>
> DESCRIPTION
> Try to extract and push the index condition down to table handler
> */
>
> -static void push_index_cond(JOIN_TAB *tab, uint keyno, bool other_tbls_ok)
> +static void push_index_cond(JOIN_TAB *tab, uint keyno, bool other_tbls_ok,
> + ulonglong options)
Please remove the options parameter, "(options & SELECT_DESCRIBE)" check can be
substituted with checking "tab->cache".
> {
> DBUG_ENTER("push_index_cond");
> Item *idx_cond;
> @@ -8258,9 +8260,33 @@ static void push_index_cond(JOIN_TAB *ta
>
> if (idx_cond)
> {
> + Item *idx_remainder_cond= 0;
> tab->pre_idx_push_select_cond= tab->select_cond;
> - Item *idx_remainder_cond=
> - tab->table->file->idx_cond_push(keyno, idx_cond);
> +
> + /*
> + For BKA cache we store condition to special BKA cache field
> + because evaluation of the condition requires additional operations
> + before the evaluation. This condition is used in
> + JOIN_CACHE_BKA[_UNIQUE]::skip_index_tuple() functions.
> + */
> + if (tab->use_join_cache &&
> + /*
> + if cache is used then the value is TRUE only
> + for BKA[_UNIQUE] cache (see check_join_cache_usage func).
> + In this case other_tbls_ok is an equivalent of
> + cache->is_key_access().
> + */
> + other_tbls_ok &&
> + (idx_cond->used_tables() &
> + ~(tab->table->map | tab->join->const_table_map)))
> + {
> + if ((options & SELECT_DESCRIBE))
> + tab->cache_idx_cond= idx_cond;
> + else
> + tab->cache_idx_cond= tab->cache->pushed_idx_cond= idx_cond;
> + }
> + else
> + idx_remainder_cond= tab->table->file->idx_cond_push(keyno,
> idx_cond);
>
> /*
> Disable eq_ref's "lookup cache" if we've pushed down an index
> @@ -8422,10 +8448,12 @@ void revise_cache_usage(JOIN_TAB *join_t
>
> SYNOPSIS
> check_join_cache_usage()
> - tab joined table to check join buffer usage for
> - join join for which the check is performed
> - options options of the join
> - no_jbuf_after don't use join buffering after table with this number
> + tab joined table to check join buffer usage for
> + join join for which the check is performed
> + options options of the join
> + no_jbuf_after don't use join buffering after table with this number
> + icp_other_tables_ok returns TRUE if condition pushdown supports
> + other tables presence
Please s/returns/OUT/ so ti looks like other comments.
>
> DESCRIPTION
> The function finds out whether the table 'tab' can be joined using a join
> @@ -8476,14 +8504,15 @@ void revise_cache_usage(JOIN_TAB *join_t
> results in that no join buffer is used to join the table.
>
> RETURN
> - TRUE if a join buffer can be employed to join the table 'tab'
> - FALSE otherwise
> +
> + cache level if cache is used, otherwise returns 0
I don't see why it was needed to change return value but since it is a
logical return value (and we might need it later on), let it be.
> */
>
> static
> -bool check_join_cache_usage(JOIN_TAB *tab,
> +uint check_join_cache_usage(JOIN_TAB *tab,
> JOIN *join, ulonglong options,
> - uint no_jbuf_after)
> + uint no_jbuf_after,
> + bool *icp_other_tables_ok)
> {
> uint flags;
> COST_VECT cost;
> @@ -8493,11 +8522,12 @@ bool check_join_cache_usage(JOIN_TAB *ta
> uint cache_level= join->thd->variables.join_cache_level;
> bool force_unlinked_cache= test(cache_level & 1);
> uint i= tab-join->join_tab;
> -
> +
> + *icp_other_tables_ok= TRUE;
> if (cache_level == 0)
> - return FALSE;
> + return 0;
> if (i == join->const_tables)
> - return FALSE;
> + return 0;
> if (options & SELECT_NO_JOIN_CACHE)
> goto no_join_cache;
> if (tab->use_quick == 2)
...
> === modified file 'sql/sql_select.h'
> --- a/sql/sql_select.h 2008-12-14 11:36:15 +0000
> +++ b/sql/sql_select.h 2009-01-13 13:22:47 +0000
> @@ -216,6 +216,10 @@ typedef struct st_join_table {
> TABLE_REF ref;
> bool use_join_cache;
> JOIN_CACHE *cache;
> + /*
> + Index condition for BKA access join
> + */
> + Item *cache_idx_cond;
> SQL_SELECT *cache_select;
> JOIN *join;
> /* SemiJoinDuplicateElimination variables: */
> @@ -666,6 +670,9 @@ public:
> JOIN_CACHE *prev_cache;
> /* Pointer to the next join cache if there is any */
> JOIN_CACHE *next_cache;
> + /* Pointer to the condition, used in BKA only*/
> + Item *pushed_idx_cond;
> +
Can we remove this and make BKA refer to tab->cache_idx_cond instead? If
yes, please do (if not, explain why we can't have this).
>
> /* Shall initialize the join cache structure */
> virtual int init()=0;
BR
Sergey
--
Sergey Petrunia, Lead Software Engineer
MySQL AB, www.mysql.com
Office: N/A
Blog: http://s.petrunia.net/blog
