Bug#36104667: Assertion failure in Sort_param::make_sortkey with

hypergraph_optimizer

When using the hypergraph optimizer, an assertion failure was
sometimes seen when sorting NULL-complemented rows from a left or
right outer join.

The problem was that Filesort did not check table->has_null_row()
before saving or restoring values of non-nullable columns. If the row
was NULL-complemented, this could lead to attempts to store garbage in
the sort buffer, and possibly writing garbage beyond the bounds of the
sort buffer.

This did not happen with the old optimizer, because it always streams
the rows through a temporary table before sorting rows after a join,
so Filesort wouldn't see a NULL-complemented row in this case, but
rather an ordinary row with NULL values in a temporary table.

Also, it did not happen when Filesort used packed addon fields, as
that code path already took NULL-complemented rows into account. It
only happened with non-packed addon fields.

Fixed by handling NULL-complemented rows the same way for non-packed
addon fields as for packed addon fields.

Change-Id: Ia2d2dc2f579546db3dd79c150f993f3f08ba652a

Author: kahatlen

mysql-test/r/filesort.result

@@ -216,3 +216,29 @@ a	LENGTH(b)
 1	40001
 SET sort_buffer_size=DEFAULT;
 DROP TABLE t1;
+#
+# Bug#36104667: Assertion failure in Sort_param::make_sortkey with
+#               hypergraph_optimizer
+#
+CREATE TABLE t1 (i INT);
+INSERT INTO t1 VALUES (), ();
+CREATE TABLE t2 (
+pk INT PRIMARY KEY,
+vc VARCHAR(1) NOT NULL,
+gc INT GENERATED ALWAYS AS (1));
+INSERT INTO t2 VALUES (1, 'a', DEFAULT), (2, 'b', DEFAULT);
+ANALYZE TABLE t1, t2;
+Table	Op	Msg_type	Msg_text
+test.t1	analyze	status	OK
+test.t2	analyze	status	OK
+SELECT gc FROM t2;
+gc
+1
+1
+SELECT 1
+FROM t1,
+t2 RIGHT OUTER JOIN t1 AS t3 ON t2.pk = t3.i
+WHERE (SELECT 1 FROM t1 WHERE t2.vc = 'x')
+ORDER BY t2.vc;
+1
+DROP TABLE t1, t2;

mysql-test/t/filesort.test

@@ -211,3 +211,34 @@ SELECT a, LENGTH(b) FROM t1 ORDER BY a DESC;
 SET sort_buffer_size=DEFAULT;
 
 DROP TABLE t1;
+
+--echo #
+--echo # Bug#36104667: Assertion failure in Sort_param::make_sortkey with
+--echo #               hypergraph_optimizer
+--echo #
+
+CREATE TABLE t1 (i INT);
+
+INSERT INTO t1 VALUES (), ();
+
+CREATE TABLE t2 (
+  pk INT PRIMARY KEY,
+  vc VARCHAR(1) NOT NULL,
+  gc INT GENERATED ALWAYS AS (1));
+
+INSERT INTO t2 VALUES (1, 'a', DEFAULT), (2, 'b', DEFAULT);
+
+ANALYZE TABLE t1, t2;
+
+# The next query doesn't fail unless we do this first. (It fills the
+# vc column in t2's record buffer with garbage.)
+SELECT gc FROM t2;
+
+# Used to hit an assertion failure with the hypergraph optimizer.
+SELECT 1
+FROM t1,
+     t2 RIGHT OUTER JOIN t1 AS t3 ON t2.pk = t3.i
+WHERE (SELECT 1 FROM t1 WHERE t2.vc = 'x')
+ORDER BY t2.vc;
+
+DROP TABLE t1, t2;

sql/filesort.cc

@@ -1523,7 +1523,9 @@ uint Sort_param::make_sortkey(Bounds_checked_array<uchar> dst,
         if (static_cast<size_t>(to_end - to) < addonf.max_length) {
           return UINT_MAX;
         }
-        if (addonf.null_bit && field->is_null()) {
+        if (field->table->has_null_row()) {
+          assert(field->table->is_nullable());
+        } else if (addonf.null_bit && field->is_null()) {
           nulls[addonf.null_offset] |= addonf.null_bit;
         } else {
           uchar *ptr [[maybe_unused]] =

sql/iterators/sorting_iterator.cc

@@ -556,25 +556,22 @@ inline void Filesort_info::unpack_addon_fields(
   }
 
   // Unpack the actual addon fields (if any).
-  const uchar *start_of_record = buff + addon_fields->first_addon_offset();
+  const uchar *next_record = buff + addon_fields->first_addon_offset();
   for (const Sort_addon_field &addonf : *addon_fields) {
     Field *field = addonf.field;
+    const uchar *end_of_record = next_record;
     const bool is_null =
         addonf.null_bit && (addonf.null_bit & nulls[addonf.null_offset]);
     if (is_null) {
       field->set_null();
+    } else if (!field->table->has_null_row()) {
+      field->set_notnull();
+      end_of_record = field->unpack(next_record);
     }
-    if (Packed_addon_fields) {
-      if (!is_null && !field->table->has_null_row()) {
-        field->set_notnull();
-        start_of_record = field->unpack(start_of_record);
-      }
+    if constexpr (Packed_addon_fields) {
+      next_record = end_of_record;
     } else {
-      if (!is_null) {
-        field->set_notnull();
-        field->unpack(start_of_record);
-      }
-      start_of_record += addonf.max_length;
+      next_record += addonf.max_length;
     }
   }
 }