fix(cudf): Handle sliced decimal aggregation state - #18608
Conversation
✅ Deploy Preview for meta-velox canceled.
|
CI Failure Analysis
🔴 Expression Fuzzer with Presto SOT — FUZZER Failure View logsFuzzer: Presto Expression Fuzzer with Presto as Source of Truth Error: Velox and Presto reference DB results don't match. The fuzzer detected a result mismatch — 1 extra row and 1 missing row out of 100 total rows. The differing value is in a BIGINT field: Velox produced The fuzzer aborted (core dumped) after the assertion failure. Correlation with PR changes:
Known issues:
Reproduce locally: ./velox_expression_fuzzer_test \
--seed 755394232 \
--enable_variadic_signatures \
--velox_fuzzer_enable_complex_types \
--lazy_vector_generation_ratio 0.2 \
--common_dictionary_wraps_generation_ratio=0.3 \
--velox_fuzzer_enable_column_reuse \
--velox_fuzzer_enable_expression_reuse \
--enable_dereference \
--duration_sec 300 \
--minloglevel=0 \
--special_forms="cast,coalesce,if" \
--velox_fuzzer_max_level_of_nesting=1 \
--presto_url=http://127.0.0.1:8080Note: Requires a running Presto server for SOT comparison. Recommended fix:
|
Selective Build Plan
Selective build plan |
| assert( | ||
| offsets[idx + 1] - offsets[idx] == | ||
| offsets[inputIdx + 1] - offsets[inputIdx] == | ||
| static_cast<OffsetT>(detail::kDecimalSumStateSize)); |
There was a problem hiding this comment.
Broader question: Does this assert actually propagate error to host?
| requires OffsetStorageType<OffsetT> | ||
| void operator()() const { | ||
| auto const n = static_cast<size_t>(numRows); | ||
| auto const inputSize = static_cast<size_t>(rowOffset) + n; |
There was a problem hiding this comment.
Nit: Should we add a defensive check here to verify that inputSize < offsetsView.size() to guard against malformed inputs?
| EXPECT_EQ(outSum[2], static_cast<__int128_t>(40)); | ||
| EXPECT_EQ(outCount[2], 4); | ||
| } | ||
|
|
There was a problem hiding this comment.
Nit: Can we also add a test/modify the existing test so it it accepts a sliced view of an arrow compacted column? We can then verify if the parent offsets are correctly applied to both the offsets child and the null mask.
Respect sliced string-column offsets when validating and unpacking decimal aggregation state. This keeps payload bounds, string offsets, and validity mask indices aligned with the sliced view.
Add a regression test for deserializing a sliced decimal aggregation state with a non-zero parent offset and a null row.