Bug#21872184 CONDITIONAL JUMP AT JOIN_CACHE::WRITE_RECORD_DATA IN SQL_JOIN_BUFFER.CC

When a query using join buffering and one of the tables inserted into
the join buffer was accessed using "dynamic range scan" on an index
containing a virtual column, a valgrind error occurred when writing
columns to the join buffer.

Using the query from the added test case as example:

SELECT t1.c1, t2.i1
FROM t1 STRAIGHT_JOIN t3 STRAIGHT_JOIN t2
WHERE ( t3.pk IN
  (
    SELECT t4.i1
    FROM t4
    WHERE t4.c1 < 'o'
  )
)
AND t1.i1 <= t3.i2_key;

This query joins three tables in the following order:

  t1 -> t3 -> t2

The access methods used are:

 t1: table scan
 t3: dynamic range scan
 t2: join buffering

The table t3 is defined as follows:

CREATE TABLE t3 (
  pk INTEGER NOT NULL,
  i1 INTEGER,
  i2_key INTEGER GENERATED ALWAYS AS (i1 + i1) VIRTUAL NOT NULL,
  PRIMARY KEY (pk),
  KEY (i2_key)
);

The issue arises when we write the (t1, t3) rows into the
join buffer. The join buffer has been configured to include all
columns marked in the read_set for t1 and t3. For the table t3, the
read set contains (pk, i1, i2_key). Out of these, pk and i2_key are
included in the read set due to they are needed for evaluating query
conditions. i1 is included in the read set since it is needed for
computing the value for the virtual column i2_key.

This works fine when t3 is accessed using a table scan but for the
rows where we switch to use "dynamic range scan", a valgrind error is
reported. The valgrind error occurs when we want to copy the i1 field
from the record buffer into the join cache buffer. Since this is a
range scan on the i2_key index, the value for i1 has not been read
into the record buffer (it is not needed). But it is still copied into
the join buffer and the valgrind error happens when the code
checks the null value for this field.

The underlying cause for this problem, is that in order to support
reading the table t3 from the storage engine as a table scan, we add
the i1 field to the TABLE::read_set. When setting up join buffering
for the query, we analyze which fields to include in the join
buffer. This is done in
JOIN_CACHE::filter_virtual_gcol_base_cols(). For table scans, all
fields included in TABLE::read_set is copied to the join buffer. For
access methods that read an index, only the fields included in the
index is copied to the join buffer. When "dynamic range scan" is used,
we do not know at the time when setting up the join buffer whether a table
scan or an index read will be used (or which index that will be
used). So in this case, all fields in the TABLE::read_set is set up to
be copied to the join buffer.

This patch extends JOIN_CACHE::filter_virtual_gcol_base_cols() to
handle dynamic range scan. If any of the indexes that are candidates
for the dynamic range scan is covering, then the table's read set is
adjusted to only contain the columns that can be read by the
alternative covering indexes.

Author: Olav Sandstaa

mysql-test/suite/gcol/inc/gcol_select.inc

@@ -828,6 +828,8 @@ EXPLAIN SELECT * FROM t2 AS t1 WHERE b NOT IN (SELECT b FROM t1 FORCE INDEX(b));
 DROP TABLE t1;
 }
 
+DROP TABLE t2, t3;
+
 --echo #
 --echo # Bug#21317507:GC: STORED COLUMN REJECTED, BUT VIRTUAL IS ACCEPTED
 --echo #
@@ -1007,3 +1009,204 @@ DROP TABLE t0, t1;
 # Restore defaults
 set optimizer_switch= @optimizer_switch_save;
 set @@read_rnd_buffer_size= @read_rnd_buffer_size_save;
+
+--echo #
+--echo # Bug#21872184 CONDITIONAL JUMP AT JOIN_CACHE::WRITE_RECORD_DATA IN 
+--echo #              SQL_JOIN_BUFFER.CC
+--echo #
+
+--echo #
+--echo # Test 1: Dynamic range scan with one covering index
+--echo #
+
+# This is the original test case which produces the valgrind error when
+# inserting data into the join buffer. The test failure only occurs with
+# InnoDB since it is only InnoDB that currently supports indexes on
+# virtual columns and is the only storage engine that includes the
+# primary key in each secondary key. 
+
+CREATE TABLE t1 (
+  i1 INTEGER NOT NULL,
+  c1 VARCHAR(1) NOT NULL
+);
+
+INSERT INTO t1
+VALUES (10, 'c'), (10, 'i'), (2, 't'), (4, 'g');
+
+CREATE TABLE t2 (
+  i1 INTEGER NOT NULL,
+  c1 VARCHAR(1) NOT NULL
+);
+
+INSERT INTO t2
+VALUES (2, 'k'), (9, 'k'), (7, 'o'), (5, 'n'), (7, 'e');
+
+CREATE TABLE t3 (
+  pk INTEGER NOT NULL,
+  i1 INTEGER,
+  i2_key INTEGER GENERATED ALWAYS AS (i1 + i1) VIRTUAL NOT NULL,
+  PRIMARY KEY (pk)
+);
+
+if ($support_virtual_index)
+{
+--echo # Add a covering index. The reason for this index being covering is that
+--echo # secondary indexes in InnoDB include the primary key.
+ALTER TABLE t3 ADD INDEX v_idx (i2_key);
+}
+
+INSERT INTO t3 (pk, i1)
+VALUES (1, 1), (2, 48), (3, 228), (4, 3), (5, 5),
+       (6, 39), (7, 6), (8, 8), (9, 3);
+
+CREATE TABLE t4 (
+  i1 INTEGER NOT NULL,
+  c1 VARCHAR(1) NOT NULL
+);
+
+INSERT INTO t4 
+VALUES (1, 'j'), (2, 'c'), (0, 'a');
+
+# Hint is added to avoid materialization of the subquery
+let query=
+SELECT /*+ NO_SEMIJOIN(@subq1) */ t1.c1, t2.i1
+FROM t1 STRAIGHT_JOIN t3 STRAIGHT_JOIN t2 
+WHERE ( t3.pk IN 
+  (
+    SELECT /*+ QB_NAME(subq1) */ t4.i1
+    FROM t4
+    WHERE t4.c1 < 'o'
+  )
+) 
+AND t1.i1 <= t3.i2_key;
+
+eval EXPLAIN $query;
+eval $query;
+
+--echo #
+--echo # Test 2: Two alternative covering indexes for the range scan
+--echo #
+
+# Adding second covering index
+if ($support_virtual_index)
+{
+ALTER TABLE t3 ADD INDEX v_idx2 (i2_key, i1);
+}
+
+# Hint is added to avoid materialization of the subquery
+let query=
+SELECT /*+ NO_SEMIJOIN(@subq1) */ t1.c1, t2.i1
+FROM t1 STRAIGHT_JOIN t3 STRAIGHT_JOIN t2 
+WHERE ( t3.pk IN 
+  (
+    SELECT /*+ QB_NAME(subq1) */ t4.i1
+    FROM t4
+    WHERE t4.c1 < 'o'
+  )
+) 
+AND t1.i1 <= t3.i2_key;
+
+eval EXPLAIN $query;
+eval $query;
+
+--echo #
+--echo # Test 3: One covering index including the base column for the virtual
+--echo #         column
+--echo #
+
+if ($support_virtual_index)
+{
+--echo # Drop the index with only the virtual column
+ALTER TABLE t3 DROP INDEX v_idx;
+}
+
+# Hint is added to avoid materialization of the subquery
+let query=
+SELECT /*+ NO_SEMIJOIN(@subq1) */ t1.c1, t2.i1
+FROM t1 STRAIGHT_JOIN t3 STRAIGHT_JOIN t2 
+WHERE ( t3.pk IN 
+  (
+    SELECT /*+ QB_NAME(subq1) */ t4.i1
+    FROM t4
+    WHERE t4.c1 < 'o'
+  )
+) 
+AND t1.i1 <= t3.i2_key;
+
+eval EXPLAIN $query;
+eval $query;
+
+--echo #
+--echo # Test 4: One non-covering index
+--echo #
+
+if ($support_virtual_index)
+{
+--echo # Drop the index on two columns, add index on just one virtual column
+ALTER TABLE t3 DROP INDEX v_idx2;
+ALTER TABLE t3 ADD INDEX v_idx (i2_key);
+}
+
+--echo # Add more data to the table so that it will run the dynamic range scan
+--echo # as both table scan and range scan (the purpose of this is to make the
+--echo # table scan more expensive).
+INSERT INTO t3 (pk, i1)
+VALUES (10,1), (11,1), (12,1), (13,1), (14,1),(15,1), (16,1),(17,1), (18,1),
+       (19,1), (20,1), (21,1), (22,1), (23,1), (24,1),(25,1),(26,1),(27,1), 
+       (28,1), (29,1);
+
+--echo # Change the query to read an extra column (t3.i1) making the index
+--echo # non-covering.
+# Hint is added to avoid materialization of the subquery
+let query=
+SELECT /*+ NO_SEMIJOIN(@subq1) */ t1.c1, t2.i1, t3.i1
+FROM t1 STRAIGHT_JOIN t3 STRAIGHT_JOIN t2 
+WHERE ( t3.pk IN 
+  (
+    SELECT /*+ QB_NAME(subq1) */ t4.i1
+    FROM t4
+    WHERE t4.c1 < 'o'
+  )
+) 
+AND t1.i1 <= t3.i2_key;
+
+eval EXPLAIN $query;
+eval $query;
+
+--echo #
+--echo # Test 5: Test where the added primary key to secondary indexes is
+--echo #         used after it has been included in the join buffer
+--echo #
+
+# This test is only relevant for storage engines that add the primary key
+# to all secondary keys (e.g. InnoDB). For these engines, the fields in the
+# primary key might be included when deciding that a secondary index is
+# covering for the query. This is the case for most of the secondary indexes
+# on t3 in this test. But in the above queries, the subquery is non-dependent
+# and the "t3.pk IN .." will be evaluated after rows for t3 are read. At this
+# time t3.pk is in the record buffer. t3.pk is not used after it has been
+# inserted into the join buffer. To test that t3.pk is actually correctly
+# included in the join buffer we change the subquery to be dependent and
+# only evaluated after the join has been done.
+# The purpose of this test is to ensure that we correctly handle and
+# include primary key fields that are added to a covering secondary index.
+
+# The difference between this query and the query in test 1 is that
+# an extra query condition is added to the subquery.
+# Hint is added to avoid materialization of the subquery
+let query=
+SELECT /*+ NO_SEMIJOIN(@subq1) */ t1.c1, t2.i1
+FROM t1 STRAIGHT_JOIN t3 STRAIGHT_JOIN t2 
+WHERE ( t3.pk IN 
+  (
+    SELECT /*+ QB_NAME(subq1) */ t4.i1
+    FROM t4
+    WHERE t4.c1 < 'o' and t4.i1 < (t2.i1 + 1)
+  )
+) 
+AND t1.i1 <= t3.i2_key;
+
+eval EXPLAIN $query;
+eval $query;
+
+DROP TABLE t1, t2, t3, t4;