List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:October 26 2010 9:31am
Subject:Re: bzr commit into mysql-next-mr-bugfixing branch (roy.lyseng:3269)
Bug#45174 Bug#50019
View as plain text  
Hi Roy,

Thanks for adressing my comments.  I approve this patch.  See inline
for some minor suggestions on comments.

On 10/25/10 11:29 AM, Roy Lyseng wrote:
 > #At file:///home/rl136806/mysql/repo/mysql-work5/ based on 
revid:tor.didriksen@stripped
 >
 >   3269 Roy Lyseng	2010-10-25
 >        Bug#45174: Incorrectly applied equality propagation caused 
wrong result
 >        on a query with a materialized semi-join.
 >        Bug#50019: Wrong result for IN-query with materialization.

...

 >        For a field that is not in a materialized semijoin we can use 
any field,
 >        even those that are embedded in a materialized semijoin. This 
is because
 >        such fields are "copied back" to their original join-tab 
structures when
 >        the materialized temporary table is being read.
 >
 >        Now we have added a bew function Item_equal::get_subst_item() 
that accepts

Typo: bew

...

 >        Also fixed problems with pushdown of SJM-aware predicates during
 >        make_join_select():
 >         - Wrong predicates were sometimes generated,
 >         - make_cond_after_sjm() was called at the wrong position

I suggest "wrong position in the join sequence" or something.  First,
I thought you meant wrong location in the code.

 >         - make_cond_after_sjm() was never actually considering the 
pushed-down
 >           SJM predicates.

...

 > @@ -9350,6 +9390,22 @@ static bool pushdown_on_conditions(JOIN*
 >   }
 >
 >
 > +/**
 > +  Separates the predicates in a join condition and pushes them to the
 > +  join step where all involved tables are available in the join prefix.
 > +  ON clauses from JOIN expressions are also pushed to the most 
appropriate step.
 > +
 > +  @param join Join object where predicates are pushed.
 > +
 > +  @param cond Pointer to condition which may contain an arbitrary 
number of
 > +              predicates, combined using AND, OR and XOR items.
 > +              If NULL, equivalent to a predicate that returns TRUE 
for all
 > +              row combinations.
 > +
 > +  @retval TRUE if condition makes query always false OR an error 
occurred.

"makes query always false" sounds a bit strange.  Do you mean "makes
condition always false"?

...

 > @@ -12563,57 +12624,42 @@ Item *eliminate_item_equal(Item *cond, C
 >       {
 >         if (eq_item)
 >           eq_list.push_back(eq_item);
 > -      /*
 > -        item_field might refer to a table that is within a semi-join
 > -        materialization nest. In that case, the join order looks 
like this:
 >
 > -          outer_tbl1 outer_tbl2 SJM (inner_tbl1 inner_tbl2) outer_tbl3
 > +      /*
 > +        @c item_field may refer to a table that is within a semijoin

I withdraw my suggestion on @c here.  This is not a doxygen
comment. Sorry about that.

-- 
Øystein
Thread
bzr commit into mysql-next-mr-bugfixing branch (roy.lyseng:3269) Bug#45174Bug#50019Roy Lyseng25 Oct
  • Re: bzr commit into mysql-next-mr-bugfixing branch (roy.lyseng:3269)Bug#45174 Bug#50019Øystein Grøvlen26 Oct
    • Re: bzr commit into mysql-next-mr-bugfixing branch (roy.lyseng:3269)Bug#45174 Bug#50019Roy Lyseng26 Oct