Bug #53970 join cache unduly disabled for incremental buffers with semijoin
Submitted: 25 May 2010 11:55 Modified: 27 May 2010 14:34
Reporter: Guilhem Bichot Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Optimizer Severity:S3 (Non-critical)
Version:next-mr-opt-backporting OS:Any
Assigned to: Guilhem Bichot CPU Architecture:Any
Tags: firstmatch, join_cache_level, optimizer_switch, semijoin

[25 May 2010 11:55] Guilhem Bichot
Description:
In check_join_cache_usage() in opt-backporting/6.0-codebase there is:

  /*
    Use join cache with FirstMatch semi-join strategy only when semi-join
    contains only one table.
  */
  if (tab->is_inner_table_of_semi_join_with_first_match() &&
      !tab->is_single_inner_of_semi_join_with_first_match())
    goto no_join_cache;

created by this revision:

"Bug#45191: Incorrectly initialized semi-join led to a wrong result.
Current implementation of join cache allows combination with the FirstMatch semi-join strategy only when FirstMatch applied only to one table. But because of a typo in the FirstMatch initialization code, all tables in the semi-join except the first one was incorrectly marked as not belonging to the semi-join. This wrongly allowed join cached usage and led to a wrong result.
The setup_semijoin_dups_elimination function now correctly marks all tables of a semi-join when the FirstMatch strategy is used."

Before this fix, the limitation applied only to non-incremental buffers, as the patch shows:

@@ -10016,15 +10016,19 @@
   */
   if (tab->use_quick == 2)
     goto no_join_cache;
-
+  /*
+    Use join cache with FirstMatch semi-join strategy only when semi-join
+    contains only one table.
+  */
+  if (tab->is_inner_table_of_semi_join_with_first_match() &&
+      !tab->is_single_inner_of_semi_join_with_first_match())
+    goto no_join_cache;
   /*
     Non-linked join buffers can't guarantee one match
   */
   if (force_unlinked_cache &&
-      ((tab->is_inner_table_of_semi_join_with_first_match() &&
-        !tab->is_single_inner_of_semi_join_with_first_match()) ||
-       (tab->is_inner_table_of_outer_join() &&
-        !tab->is_single_inner_of_outer_join())))
+      (tab->is_inner_table_of_outer_join() &&
+       !tab->is_single_inner_of_outer_join()))
     goto no_join_cache;

So the patch did two things:
1) extend the limitation to incremental buffers
2) fix correct setting of first/last_sj_inner_tab so that the is_*inner_table_of_semi_join_with_first_match() functions are reliable.

And the commit comment only describes (2).
Evgeny who did the commit agrees that (1) might be reverted.
So this bug report is about investigating whether this is possible.
When reverting, there is an assertion failure in subselect_sj_jcl6.

I'm using epotemkin@mysql.com-20100524072222-57b2xevlp7waryr4

How to repeat:
code review
[25 May 2010 12:46] MySQL Verification Team
Thank you for the bug report.
[27 May 2010 14:34] Guilhem Bichot
I undid (1) and tested. subselect_sj_jcl6 crashes in the testcase for BUG#45191:
SELECT EMPNUM, EMPNAME FROM STAFF WHERE EMPNUM IN   (SELECT EMPNUM  FROM WORKS    WHERE PNUM IN      (SELECT PNUM  FROM PROJ));
The query plan is STAFF, PROJ(using-join-buffer),
WORKS(using-join-buffer,firstmatch(STAFF)). PROJ and WORKS are the two
semijoin-inner tables of the semijoin nest. It crashes in
bool JOIN_CACHE::skip_record_if_match()
{
  DBUG_ASSERT(with_match_flag && with_length); // here!
The JOIN_CACHE above, having JOIN_TAB==WORKS, holds records of
PROJ. When a match in WORKS has been found, the code decides the
current record of PROJ can be skipped, but the cache's with_match_flag
(and thus with_length) is false and so skipping is
impossible. with_match_flag is correctly false because
WORKS is not the first semi-join inner table (see
JOIN_TAB::use_match_flag()). Having a match flag is indeed needed only
for the first semi-join inner table, which holds records of the outer
table (for which we need to remember whether a match has been found
and in that case the record needs to be skipped).
I forced each JOIN_CACHE to have a match_flag. Then we see that the
logic itself, skipping a record of PROJ and going to next record of
PROJ, is wrong: it leads to duplicate rows. 
What would be needed is a jump back to STAFF.

I think that extending limitation (1) as Evgeny has done was thus
sane, and removing this limitation in a big task.
So I'm closing this bug, and creating a worklog task, WL#5410.

For my tests I use next-mr-opt-backporting, epotemkin@mysql.com-20100524072222-57b2xevlp7waryr4 .