Skip to content

Commit ec3e63d

Browse files
Dag Wanvikdahlerlend
authored andcommitted
Bug#36075756 ASAN crash on MaterializeIterator<Profiler>::load_HF_row_into_hash_map() [2/2]
While the first patch [1/2] solves this issue by allocating on the session's main MEM_ROOT for (one of) the last rows in a chunk when re-reading it into the hash table again, the reason why we run out of space on re-reading the rows is another matter. Why do we run out of space even though the rows are read back into the hash table in another order then they were entered the first time? It turns out there a conscious "error" in CalculateColumnStorageSize to improve performance. When we are computing the size of a NULL blob, we will return the wrong (large) size (the size of previous blob row read). This causes us to run out of space. Looking at the caller, ComputeRowSizeUpperBound, we see this comment: // even though we only store non-null columns, we count up the size // of all columns unconditionally. this means that null columns may // very well be counted here, but the only effect is that we end up // reserving a bit too much space in the buffer for holding the row // data. that is more welcome than having to call field::is_null() // for every column in every row. maybe, but surely not at the cost of reading garbage? This patch changes CalculateColumnStorageSize to test for null for blobs, so that the estimate returned is sensible. This alone removes the bug even without patch [1/2] for this bug. In fact it would work also without the fix for Bug#35686098 - no use of overflow_mem_root is required in the repro. Still, we consider patch [1/2] to be useful in its own right, as a fallback to an overflow mem_root (i.e. the normal session mem_root) is now possible iff different fragmentation should lead to space overflow, so we elect to keep [1/2] as well. This patch [2/2] reduces the need for overflow_mem_root. Change-Id: Ia4d0e370fe172a021db8cda86fbc333cbea2e98c
1 parent 69ac9e4 commit ec3e63d

File tree

1 file changed

+3
-1
lines changed

1 file changed

+3
-1
lines changed

sql/pack_rows.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,9 @@ static size_t CalculateColumnStorageSize(const Column &column) {
188188
// does not include the size of the length variable for blob types, so we
189189
// have to add that ourselves.
190190
const Field_blob *field_blob = down_cast<const Field_blob *>(column.field);
191-
return field_blob->data_length() + field_blob->pack_length_no_ptr();
191+
return field_blob->is_null()
192+
? 0
193+
: field_blob->data_length() + field_blob->pack_length_no_ptr();
192194
}
193195

194196
return column.field->max_data_length();

0 commit comments

Comments
 (0)