Comment 2 for bug 789131

Revision history for this message
Alexey Kopytov (akopytov) wrote :

The Valgrind warning is actually safe to ignore in this case. The reason is how the fixed-size record prefix is handled by the dynamic rows patch.

First fixed-size columns and key columns are copied with a single memcpy() call when copying a record from the table record buffer and vice versa for performance reasons (see hp_extract_record() and hp_process_record_data_to_chunkset()). Which means if there are keys on VARCHAR fields, all bytes are stored into the storage engine (and then read back in) even if the actual data is shorter. Which in turn means some uninitialized bytes from the table record buffer may be stored.

When such records are then read from the HEAP storage engine and written to a disk-based table (for example, when creating an internal MyISAM temporary table), those bytes will be stored on disk as well. They will never be accessed, since both the server and storage engine know the actual length of VARCHAR values. But Valgrind was unhappy about uninitialized bytes being written to disk.

One solution to this would be to remove the optimization from the HEAP engine and copy fixed-size fields one by one rather than with a single memcpy() call. Which would imply a performance overhead for every record.

Another solution, which I like more, is to initialize the table record buffer with zeroes right after allocating it, so that no uninitialized data is ever passed to a storage engine. Which only means a very minor performance overhead when opening the table.