MDEV-40802 COUNT(DISTINCT <blob>) fails when its tmp table converts

[arcivanov]

MDEV-40802

COUNT(DISTINCT <blob>) aborts the statement with

ERROR 1169 (23000): Can't write, because of unique constraint, to table '(temporary)'

when the temporary table that collects the distinct values outgrows the
in-memory limit and is converted to an on-disk table.

Cause

Aggregator_distinct deduplicates in one of two ways. For scalar arguments it
builds a Unique tree over raw record bytes. For a blob argument it cannot:
the record holds only a pointer to the value, so a raw comparison would compare
addresses. setup() therefore skips the tree and every value goes through a
write into a temporary table carrying a unique constraint, where a duplicate
key error is the ordinary outcome and is treated as "value already seen":

if (!table->file->is_fatal_error(error, HA_CHECK_DUP))
  return FALSE;                           // duplicate, not an error

When such a write overflows the in-memory table instead,
create_internal_tmp_table_from_heap() copies the stored rows to an on-disk
table and then writes the row that overflowed, which until then was held in
record[0] alone. Whether a duplicate key error on that last write is fatal is
decided by the caller's ignore_last_dupp_key_error argument, and
Aggregator_distinct::add() passed 0 three lines below the code that ignores
the very same condition.

The argument is the same upstream, where it is unreachable: a temporary table
with a blob column was created on the on-disk engine to begin with, so the
conversion was never entered for the only tables whose pending row can be a
duplicate. Supporting blob columns in the in-memory engine made the table start
in memory and convert, which turned a dead argument live.

Fix

Pass 1, so a duplicate arriving through the conversion is discarded exactly
like one arriving through the ordinary write. endup() reads the result as
table->file->stats.records, so not storing the duplicate is what makes the
count right.

The three other call sites that pass 0 were checked and are correct: the
information schema accumulator in sql_show.cc has no unique constraint at
all, end_update() probes the key before writing, and sql_window.cc uses
Window_rowid_remapper, which replaces a row rather than appending one.

Test

heap.count_distinct_blob_convert covers one blob argument, a blob argument of
a different width, two blob arguments, a blob combined with a non-blob, a
non-blob control that stays on the Unique tree, and a grouped aggregate. Each
case asserts CREATED_TMP_DISK_TABLES > 0, so a future sizing change fails the
test rather than silently voiding its coverage. A second run at 512M is the
control.

The reproduction window is narrow, for a mechanical reason worth recording. A
write rejected as a duplicate allocates a record, fails the key check and
returns the record to the free list; hp_write_blobs() runs after the key
loop, so a duplicate never allocates blob space. With short values the record
space is what runs out, duplicates hit it too, and the overflowing row is a
duplicate. With values of a kilobyte or more the blob space runs out first, and
only a write that is not a duplicate ever reaches it, so the overflowing row is
always distinct and the failure cannot be observed at all. The test uses short
values for that reason.

Verification

• Unfixed build: the test fails with ER_DUP_UNIQUE. Fixed build: it passes.
  The fix was backed out and re-applied to confirm both directions.
• --suite=main,heap: 1432/1432 pass.
• The test also passes under --ps-protocol, --cursor-protocol and
  --view-protocol.

[gkodinov]

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

LGTM. Please stand by for the final review.

[gkodinov]

FYI: According to our development cycle we work on bugs In the following periods 15 Mar-30 Apr, 15 Jun-30 Jul, 15 Sep-30 Oct and 15 Dec-31 Jan. So, please, expect to get a review somewhere between these two dates and the goal is to have your PR merged before the second date