mirror of
https://github.com/apache/impala.git
synced 2026-01-06 15:01:43 -05:00
The standard implementation of HashTable::Equals() did not correctly check the NULL bit when the argument row did not evaluate to NULL for a given probe expr. In the rare circumstance that this gave rise to a false positive (more on that below), two rows with different grouping values would be considered equal, and one would be excluded from the final aggregation output. HashTable::EvalRow() fills an expression value buffer with the values of either probe or build exprs evaluated for the argument row. These cached values are used to determine row equality in Equals(). In order to avoid a lot of false collisions, an 'unlikely' value is written to that buffer for NULL values, chosen to be HashUtil::FNV_SEED. So without correct NULL-bit checking in Equals(), two single-slot rows are considered to be equal if one of them has NULL for its slot, and the other has a value equal to HashUtil::FNV_SEED truncated to the size of the slot. For tinyint columns, this value is -59. As it happens, our random generator happened to create a table with one tinyint column and which contained NULL and -59 as values. In order to trigger this bug, the rows must also have been written to disk in order such that the scanners returned -59 *first*, and then NULL to the aggregation node; the bug is not symmetric and works in the opposite case. Change-Id: I17d43eaeee62b2ac01b67dd599bc4346b012a074 Reviewed-on: http://gerrit.ent.cloudera.com:8080/2130 Reviewed-by: Marcel Kornacker <marcel@cloudera.com> Tested-by: jenkins (cherry picked from commit 6e8098254280a9d5ead0b607263ca6728a3222a7) Reviewed-on: http://gerrit.ent.cloudera.com:8080/2161 Reviewed-by: Henry Robinson <henry@cloudera.com>