lucasfang opened a new pull request, #306: URL: https://github.com/apache/paimon-cpp/pull/306
### Purpose Linked issue: close #xxx `NullFalseLeafBinaryFunction::Test` evaluates a comparison predicate (`=`, `<>`, `<`, `<=`, `>`, `>=`) by materializing the whole column into `Literal` objects, one heap allocation and one value hash per row, and comparing each row against the literal one at a time. That is `O(rows)` allocations plus `O(rows)` scalar comparisons per batch, even though a comparison needs only one vectorized pass over the column. This is the sibling of the `IN` / `NOT IN` optimization that already probes a batch with `arrow::compute::is_in`; the comparison functions are the other `LeafFunction` family still on the row-by-row path. This PR builds the literal into a one-element scalar once per batch and compares the whole column against it with the matching `arrow::compute` kernel (`equal`, `not_equal`, `less`, `less_equal`, `greater`, `greater_equal`), which turns evaluation into `O(1)` setup plus an `O(rows)` vectorized compare with no per-row `Literal`. Every `LeafFunction` is a shared stateless singleton, so the scalar cannot be cached on the function and is built per batch. Changes: - `NullFalseLeafBinaryFunction::Test(array, literals, pool)` moves out of the header into a new `null_false_leaf_binary_function.cpp`. The scalar is built with the existing `LiteralConverter::ConvertLiteralsToArray`, the same call `IN` writes its value set with. - The kernel path is taken only where a kernel agrees with `Literal::CompareTo`: `BOOLEAN`, `TINYINT`, `SMALLINT`, `INT`, `BIGINT`, `DATE`, `STRING`, `BINARY`, and `DECIMAL` / `TIMESTAMP` whose column carries the scale / the unit of the literal and no time zone. `FLOAT` and `DOUBLE` keep the row-by-row path because `FieldsComparator::CompareFloatingPoint` orders `-0.0 < +0.0` and makes every NaN equal to every NaN, where an IEEE-754 kernel says `-0.0 == +0.0` and that no NaN compares to anything. - The kernel path is gated on the layouts the row-by-row path already accepts, so which columns evaluate and which report an error stays exactly the same: the string pattern functions (`STARTS_WITH`, `ENDS_WITH`, `CONTAINS`, `LIKE`), a decimal column of another scale, a timestamp column of another unit or with a time zone, a literal whose field type disagrees with the column, and the layouts `ConvertLiteralsFromArray` rejects all keep the row-by-row path and its error reporting. - One behavior correction falls out of decoding the dictionary in the kernel: a dictionary row that points at a null dictionary value is now false for every comparison, where the row-by-row path read the empty value the slot holds and an empty literal used to match it. ### Tests New unit tests in `src/paimon/common/predicate/null_false_leaf_binary_function_test.cpp`, 21 cases, each asserting the per-row result over a batch: - Kernel path per field type: `TestInt` and `TestString` across all six comparisons, `TestTinyIntSmallIntAndBigInt` with the int64 boundaries, `TestBoolean`, `TestDate`, `TestBinary` with embedded zero bytes, `TestDecimal`, `TestTimestamp` - Fallback parity with the row-by-row path: `TestDecimalOffTheKernelPath` for another scale and the scale pair whose cast would fail, `TestTimestampOffTheKernelPath` for a coarser unit, a finer unit that would overflow int64 and a zoned column, `TestFloatAndDoubleStayOffTheKernelPath` for `-0.0`, `+0.0` and NaN - Dictionary columns: `TestDictionaryString`, `TestLargeStringDictionary`, `TestDictionaryWithNullValue` - Null, empty and error parity: `TestNullLiteralIsFalseForEveryRow`, `TestEmptyLiteralsReportTheError`, `TestMismatchedLiteralTypeReportsTheError`, `TestLayoutsLiteralConversionRejects` - Non-comparison functions and edge layouts: `TestStringPatternFunctionsKeepTheRowByRowPath`, `TestSlicedArray`, `TestEmptyArray` Verified with the full `paimon-common-test` suite: 1572 tests from 180 test suites, all passed. ### API and Format No. No header under `include/` is touched, and neither the storage format nor the protocol changes. `NullFalseLeafBinaryFunction::Test` keeps its signature; only its definition moves from the header into a new `.cpp`. ### Documentation No. This is a performance optimization plus the dictionary null-value correction above, with no user-visible API or configuration change. ### Generative AI tooling Generated-by: Qoder -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
