Wrong result with enabled index condition pushdown and disabled subquery materialization.

Bug #992942 reported by Igor Babaev
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
MariaDB
In Progress
High
Sergey Petrunia

Bug Description

The following sequence of commands gives a wrong query result in MariaDB 5.3/5.5:

CREATE TABLE t1 (i1 INTEGER, i2 INTEGER, KEY k1 (i1));
INSERT INTO t1 VALUES (9,0), (7,1), (1,9), (7,101), (2,1), (9,102);
CREATE TABLE t2 (pk INTEGER, i2 INTEGER, PRIMARY KEY (pk));
INSERT INTO t2 VALUES (2,1), (3,2), (5,3), (6,4), (7,6), (9,4);
CREATE TABLE t3 (i1 INTEGER, i2 INTEGER);
INSERT INTO t3 VALUES (1,0), (1,1), (1,101), (1,102);
SET SESSION optimizer_switch="index_condition_pushdown=on";
SET SESSION optimizer_switch="materialization=off";
SELECT * FROM t3
   WHERE (i1, i2) IN
                ( SELECT COUNT(DISTINCT t2.i2), t1.i2 FROM t1 JOIN t2 ON t1.i1 = t2.pk
                      WHERE t2.pk BETWEEN 7 AND 9 GROUP BY t1.i2 );

MariaDB [test]> SELECT * FROM t3 WHERE (i1, i2) IN ( SELECT COUNT(DISTINCT t2.i2), t1.i2 FROM t1 JOIN t2 ON t1.i1 = t2.pk WHERE t2.pk BETWEEN 7 AND 9 GROUP BY t1.i2 );
+------+------+
| i1 | i2 |
+------+------+
| 1 | 0 |
| 1 | 101 |
| 1 | 102 |
+------+------+

The correct answer for the query is returned with these settings:
SET SESSION optimizer_switch="index_condition_pushdown=off";
SET SESSION optimizer_switch="materialization=off";

MariaDB [test]> SELECT * FROM t3 WHERE (i1, i2) IN ( SELECT COUNT(DISTINCT t2.i2), t1.i2 FROM t1 JOIN t2 ON t1.i1 = t2.pk WHERE t2.pk BETWEEN 7 AND 9 GROUP BY t1.i2 );
+------+------+
| i1 | i2 |
+------+------+
| 1 | 0 |
| 1 | 1 |
| 1 | 101 |
| 1 | 102 |
+------+------+

And with these settings also we have the correct answer
SET SESSION optimizer_switch="index_condition_pushdown=on";
SET SESSION optimizer_switch="materialization=on";

MariaDB [test]> SELECT * FROM t3 WHERE (i1, i2) IN ( SELECT COUNT(DISTINCT t2.i2), t1.i2 FROM t1 JOIN t2 ON t1.i1 = t2.pk WHERE t2.pk BETWEEN 7 AND 9 GROUP BY t1.i2 );
+------+------+
| i1 | i2 |
+------+------+
| 1 | 0 |
| 1 | 1 |
| 1 | 101 |
| 1 | 102 |
+------+------+

(See also bug #12667154 in mysql-5.6.5)

Changed in maria:
status: New → Confirmed
importance: Undecided → High
assignee: nobody → Sergey Petrunia (sergefp)
milestone: none → 5.3
Elena Stepanova (elenst)
tags: added: optimizer wrong-result
Changed in maria:
status: Confirmed → In Progress
Changed in maria:
status: In Progress → Fix Committed
status: Fix Committed → In Progress
Revision history for this message
Sergey Petrunia (sergefp) wrote :

The problem is actually inherited from 5.2. There, one can also observe that
- the first invocation of find_all_keys() is done with quick_select!=NULL, and the quick select is used to read records
- the second invocation of find_all_keys() is done with quick_select==NULL, and full scan is used.

this is clearly not what was intended. However, the effect is only performance, the query produces correct result. MariaDB 5.3/ MySQL 5.6 add IndexConditionPushdown, which is not cleaned up correctly and causes wrong query result.

Revision history for this message
Sergey Petrunia (sergefp) wrote :

This problem also causes MDEV-320 for SHOW EXPLAIN.

Revision history for this message
Sergey Petrunia (sergefp) wrote :

I'm currently considering a solution where create_sort_index() would save/restore the first JOIN_TAB. It looks like this would work, except for Index Condition Pushdown:

Let's assume there is a subquery that uses filesort() on its first table.
filesort() gets its data from quick select, which uses range access, together with IndexConditionPushdown.

This subquery will execute as follows:
> filesort
     read matching records with quick select + ICP
< filesort
> do_select
  read table with rr_from_pointers(), using handler->rnd_pos() calls.
< do_select

and suppose the subquery is correlated, so we'll need to execute it multiple times.

The problem here is as follows:
 - after we've left filesort(), we need to re-initialize the table handler for making rnd_pos() calls.
 - when starting a subsequent execution, the table needs to be re-initialized for quick select with ICP.

Re-initializing for quick select is easy: we can remember the index #, and whether index_only should be ON. (Need to check also whether table->read_map should be/is re-initialized).

There is a problem with re-initialization for ICP. Pushed index condition is not saved anywhere. There is no way to remember what was pushed and re-push it later.

Revision history for this message
Sergey Petrunia (sergefp) wrote :

It's possible to fix, there is table->file->pushed_idx_cond. pushed_idx_cond was initially intended to be an internal member, however it is already used from the SQL layer.

Revision history for this message
Sergey Petrunia (sergefp) wrote :

... There are problems with re-initialization of MRR scans, though.

Besides that, we also need to take into account that
- if we've had a quick select that was created by the range optimizer, it should be kept and re-used.
- BUT, if the quick select was created by the get_quick_select_for_ref() call in create_sort_index(), it should be destroyed and re-created again if necessary. This is because 'ref' may contain out-of-subquery references, and so will change for each execution.

Revision history for this message
Sergey Petrunia (sergefp) wrote :

FYI: have a candidate fix, it is being tested together with MDEV-325

Revision history for this message
Sergey Petrunia (sergefp) wrote :

The fix survived some rounds of testing, but then we've figured it has introduced https://mariadb.atlassian.net/browse/MDEV-416

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.