List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:April 26 2010 5:35pm
Subject:Re: bzr commit into mysql-6.0-codebase-bugfixing branch
(roy.lyseng:3836) WL#5347
View as plain text  
Hello Roy,

Roy Lyseng a écrit, Le 26.04.2010 12:54:
> #At file:///home/rl136806/mysql/repo/mysql-6.0-work1/ based on
> revid:alfranio.correia@stripped
> 
>  3836 Roy Lyseng	2010-04-26
>       WL#5347: Refactor optimizer function at_sjmat_pos()

> @@ -7241,58 +7270,79 @@ optimize_straight_join(JOIN *join, table
>  }
>  
>  
> -/*
> -  Check if the last tables of the partial join order allow to use
> -  sj-materialization strategy for them
> +/**
> +  Check whether a semijoin materialization strategy is allowed for
> +  the current (semi)join table order.
>  
> -  SYNOPSIS
> -    at_sjmat_pos()
> -      join              
> -      remaining_tables
> -      tab                the last table's join tab
> -      idx                last table's index
> -      loose_scan    OUT  TRUE <=> use LooseScan
> +  @param join              Join object
> +  @param remaining_tables  Tables that have not yet been added to the join plan
> +  @param tab               Join tab of the table being considered
> +  @param idx               Index of table with join tab "tab"
> +
> +  @retval SJ_OPT_NONE               - Materialization not applicable
> +  @retval SJ_OPT_MATERIALIZE_LOOKUP - Materialization with lookup applicable
> +  @retval SJ_OPT_MATERIALIZE_SCAN   - Materialization with scan applicable
>  
> -  RETURN
> -    TRUE   Yes, can apply sj-materialization
> -    FALSE  No, some of the requirements are not met
> +  @details
> +  The function checks applicability of both MaterializeLookup and
> +  MaterializeScan strategies.
> +  No checking is made until "tab" is pointing to the last inner table
> +  of a semijoin nest that can be executed using materialization -
> +  for all other cases SJ_OPT_NONE is returned.
> +  If both MaterializeLookup and MaterializeScan are applicable
> +  (ie if there are no correlated outer tables),

I don't understand the equivalence ("i.e. if there are" etc)

> +  MaterializeLookup is returned.
> +  This is a purely heuristic decision.

Is this decision something which is introduced by the patch?

> +  MaterializeScan allows correlated outer tables both before and after
> +  the materialized inner tables. In this case, the materialized table must
> +  contain keys from the prefix outer tables followed by keys from the
> +  suffix outer tables, called prefix-key and suffix-key, respectively.

What do you mean by "keys" here... I think the materialized tables has a 
single index.

> +  First, a scan is set up over the prefix outer tables. Then a range scan
> +  is done over the prefix-key of the materialized table.
> +  Finally, the suffix-key from the materialized table is used for a scan
> +  over the suffix outer tables.

mmmm the suffix outer tables are not always scanned, if they have an 
index then we will do a index lookup on them (searching for a value 
which we found when scanning the materialized table).

Is the comment a description of how things work now?
I was surprised to see that materialization-scan is expected work with 
the inner table not first, so I tested it and filed BUG#53172. It may or 
not be related to BUG#52068 - semijoin materialization has a bit too 
many bugs it seems :-(

> -SJ_MATERIALIZATION_INFO *
> -at_sjmat_pos(const JOIN *join, table_map remaining_tables, const JOIN_TAB *tab,
> -             uint idx, bool *loose_scan)
> +static int
> +semijoin_order_allows_materialization(const JOIN *join,
> +                                      table_map remaining_tables,
> +                                      const JOIN_TAB *tab, uint idx)
>  {
>    /*
>     Check if 
>      1. We're in a semi-join nest that can be run with SJ-materialization
> -    2. All the tables correlated through the IN subquery are in the prefix
> +    2. All the tables from the subquery are in the prefix
>    */
> -  TABLE_LIST *emb_sj_nest= tab->emb_sj_nest;
> -  table_map suffix= remaining_tables & ~tab->table->map;
> -  if (emb_sj_nest && emb_sj_nest->sj_mat_info &&
> -      !(suffix & emb_sj_nest->sj_inner_tables))
> +  const TABLE_LIST *emb_sj_nest= tab->emb_sj_nest;
> +  if (!emb_sj_nest ||
> +      !emb_sj_nest->sj_mat_info ||
> +      (remaining_tables & emb_sj_nest->sj_inner_tables))

The old code would use ~tab->table->map here...?

> +    return SJ_OPT_NONE;
> +
> +  /* 
> +    Walk back and check if all immediately preceding tables are from
> +    this semi-join.
> +  */
> +  const uint n_tables= my_count_bits(emb_sj_nest->sj_inner_tables);
> +  for (uint i= 1; i < n_tables ; i++)
>    {
> -    /* 
> -      Walk back and check if all immediately preceding tables are from
> -      this semi-join.
> -    */
> -    uint n_tables= my_count_bits(tab->emb_sj_nest->sj_inner_tables);
> -    for (uint i= 1; i < n_tables ; i++)
> -    {
> -      if (join->positions[idx - i].table->emb_sj_nest != tab->emb_sj_nest)
> -        return NULL;
> -    }
> -    *loose_scan= test(remaining_tables & ~tab->table->map &
> -                             (emb_sj_nest->sj_inner_tables |
> -                              emb_sj_nest->nested_join->sj_depends_on));
> -    if (*loose_scan && !emb_sj_nest->sj_subq_pred->sjm_scan_allowed)
> -      return NULL;
> -    else
> -      return emb_sj_nest->sj_mat_info;
> +    if (join->positions[idx - i].table->emb_sj_nest != emb_sj_nest)
> +      return SJ_OPT_NONE;
>    }
> -  return NULL;
> -}
>  
> +  /*
> +    Must use MaterializeScan strategy if there are outer correlated tables
> +    among the remaining tables, otherwise use MaterializeLookup.
> +  */
> +  if (remaining_tables & emb_sj_nest->nested_join->sj_depends_on)
> +  {
> +    if (!emb_sj_nest->sj_subq_pred->sjm_scan_allowed)

There seems to be code changes too (I don't see ~tab->table->map, 
emb_sj_nest->sj_inner_tables). They may well be fine, but I'd really 
need an explanation, please. I don't fully understand what all those 
bitmaps are about and why they are not needed in fact...

> +      return SJ_OPT_NONE;
> +    return SJ_OPT_MATERIALIZE_SCAN;
> +  }
> +  return SJ_OPT_MATERIALIZE_LOOKUP;
> +}
Thread
bzr commit into mysql-6.0-codebase-bugfixing branch (roy.lyseng:3836) WL#5347Roy Lyseng26 Apr
  • Re: bzr commit into mysql-6.0-codebase-bugfixing branch(roy.lyseng:3836) WL#5347Guilhem Bichot26 Apr
    • Re: bzr commit into mysql-6.0-codebase-bugfixing branch(roy.lyseng:3836) WL#5347Roy Lyseng26 Apr
      • Re: bzr commit into mysql-6.0-codebase-bugfixing branch(roy.lyseng:3836) WL#5347Guilhem Bichot26 Apr
        • Re: bzr commit into mysql-6.0-codebase-bugfixing branch(roy.lyseng:3836) WL#5347Roy Lyseng28 Apr
      • Re: bzr commit into mysql-6.0-codebase-bugfixing branch(roy.lyseng:3836) WL#5347Øystein Grøvlen27 Apr
        • Re: bzr commit into mysql-6.0-codebase-bugfixing branch(roy.lyseng:3836) WL#5347Roy Lyseng27 Apr
  • Re: bzr commit into mysql-6.0-codebase-bugfixing branch(roy.lyseng:3836) WL#5347Øystein Grøvlen27 Apr
    • Re: bzr commit into mysql-6.0-codebase-bugfixing branch(roy.lyseng:3836) WL#5347Roy Lyseng28 Apr