MDEV-40803 Recursive CTE loses a row when its increment table converts

[arcivanov]

https://jira.mariadb.org/browse/MDEV-40803

A recursive CTE whose increment table outgrows the in-memory temporary table
returns a wrong result. The row that overflows the increment table is replaced
by an all-NULL record, so that row and every row the recursion would have
derived from it are missing from the answer. No error and no warning is raised.

The defect

select_union_recursive::send_data() builds each new row in the record buffer
of the union result table and writes it into the increment table. When
that write overflows, it asks for the increment table to be converted:

    if ((err= incr_table->file->ha_write_tmp_row(table->record[0])))
    {
      bool is_duplicate;
      rc= create_internal_tmp_table_from_heap(thd, incr_table, ...);
    }

create_internal_tmp_table_from_heap() copies every stored row into the new
table and then appends the row that did not fit, taking it from the record[0]
of the table it was given:

  /* copy row that filled HEAP table */
  if (unlikely((write_err=new_table.file->ha_write_tmp_row(table->record[0]))))

There, table is the parameter, so this reads incr_table->record[0]. Nothing
ever fills that buffer: TABLE::insert_all_rows_into_tmp_table() reads into the
destination's record[0], and the recursive scan reads the recursive result
table. It still holds the empty record the table was opened with.

The increment table is created with no grouping and no distinct flag, so it
carries no key and the substituted row is not rejected. On the next iteration
that row's columns are NULL, the recursion predicate evaluates to NULL, and it
produces no descendants.

The fix

Copy the row into the increment table's own record buffer before asking for the
conversion, so this caller satisfies the same precondition every other caller of
create_internal_tmp_table_from_heap() already satisfies. All 13 other call
sites were audited; each writes the pending row from the record[0] of the very
table it converts, which is why the defect is confined to this one.

Only incr_table->s->reclength bytes are copied. When the union result table
uses a unique constraint, its record carries a trailing
MARIA_UNIQUE_HASH_LENGTH hash field, appended after the real columns, that the
increment table's record does not have. This is the case the DBUG_ASSERT above
the call already describes, and it was observed live at a conversion with
incr_reclength=2409 result_reclength=2413.

Testing

Seven shapes in main.cte_recursive, each run twice, once with the increment
table small enough to stay in memory and once forced to convert, with the two
runs required to agree: overflow during the anchor, overflow during a recursive
step, UNION distinct, UNION distinct with a key too long for an on-disk
table, a prepared statement re-executed across a conversion, mutually recursive
CTEs, and INSERT ... SELECT.

Every shape was verified to fail before the fix and pass after it, with the
.result file unchanged between the two runs.

Two properties are needed to expose the defect, and neither is obvious:

1. tmp_memory_table_size=0 cannot be used to force the conversion. It makes
   Create_tmp_table::choose_engine() build the table on disk from the start,
   so the conversion never happens. This is why the existing tests in
   cte_recursive.test that set it never caught this.
2. The lost row has to be at a level of the recursion that still has
   descendants. The in-memory table is only checked for fullness at block
   boundaries, so a row width that pushes the conversion into the last level
   hides the defect entirely.

main.cte_recursive and main.union re-recorded purely additively, no existing
result changed. Full main and heap suites pass, 1434/1434.

The other two commits

They are independent of the fix and can be dropped without affecting it.

Cover converting a result table whose index has been disabled adds the only
reachable branch of create_internal_tmp_table_from_heap() that no test in the
main suite took, measured with gcov over every test that manipulates the
temporary table size limits.

Drop the view left behind by cte_update_delete.test removes a view the test
creates and never drops, which makes MTR's check of the testcase fail both for
that test and for whichever test runs next on the same worker.

Versions

Reproduced on main. No lower bound was established; older release series were
not tested, so the version fields should be set from a check against them.

[gkodinov]

Thank you for your contribution. This is a preliminary review.

According to https://mariadb.org/get-involved/getting-started-for-developers/#choosing-the-correct-branch a bug fix needs to target the lowest affected version.
Can you please re-target the fix to 10.11?