Skip to content

Commit 61ef672

Browse files
authored
GH-49392: [C++][Compute] Fix fixed-width gather byte offset overflow in list filtering (#49602)
## Rationale for this change Issue #49392 reports a user-visible corruption when filtering a table that contains a `list<double>` column: slicing the last row returns the expected values, while filtering the same row returns values from a different child span. The corruption only appears once the selected child-value range is large enough, which points to an overflow in the fixed-width gather path used when list filtering materializes the selected child values. ## What changes are included in this PR? This patch moves fixed-width gather byte-offset scaling onto an explicit `int64_t` helper before the `memcpy` and `memset` address calculations. That fixes the underlying 32-bit byte-offset overflow when a `uint32` gather index is multiplied by the fixed value width. In the source issue's last-row case, the selected child span starts at `999998000`; for `double` values, scaling that index by 8 bytes wrapped in 32-bit arithmetic and redirected the gather to the wrong child span. Keeping the byte-offset arithmetic in 64 bits makes the fixed-width gather path, the child `Take()` call used under list filtering, and the final filtered `Table` all address the correct bytes. To validate that this is the same bug reported in the issue, I also used a local near-e2e C++ reproduction that keeps the issue's logical shape (`N=500000`, `ARRAY_LEN=2000`, an `id` column, and a `numbers: list<double>` column), filters the last row, and seeds both the true target child span and the pre-fix wrapped span with distinct sentinels. In that setup, `Slice()` returns the expected row, a replay of the pre-fix gather arithmetic returns the wrapped sentinel span instead, and the fixed child `Take()` and table `Filter()` results both match the sliced row. That ties the user-visible issue and this root-cause fix back to the same overflow boundary. ## Are these changes tested? Yes. The patch adds a targeted unit test that checks fixed-width gather byte offsets are computed with 64-bit arithmetic. This is intentionally smaller than an end-to-end filter regression: the original user-visible failure only shows up at very large logical offsets, while the new unit test isolates the exact overflow boundary directly without constructing a huge table or depending on a PyArrow-level reproduction. That makes it smaller, more stable, and more appropriate for regular C++ CI, while the near-e2e local reproduction was used separately to confirm that this root-cause regression test and the reported filtering corruption are exercising the same bug. ## Are there any user-facing changes? Yes. Filtering tables with large list columns backed by fixed-width child values no longer risks returning data from a wrapped byte offset. * GitHub Issue: #49392 Authored-by: Rossi Sun <zanmato1984@gmail.com> Signed-off-by: Rossi Sun <zanmato1984@gmail.com>
1 parent 4e22f4d commit 61ef672

1 file changed

Lines changed: 10 additions & 4 deletions

File tree

cpp/src/arrow/compute/kernels/gather_internal.h

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@
3535

3636
namespace arrow::internal {
3737

38+
template <typename IndexType>
39+
constexpr int64_t FixedWidthByteOffset(IndexType index, int64_t value_width) {
40+
return static_cast<int64_t>(index) * value_width;
41+
}
42+
3843
// CRTP [1] base class for Gather that provides a gathering loop in terms of
3944
// Write*() methods that must be implemented by the derived class.
4045
//
@@ -185,8 +190,8 @@ class Gather : public GatherBaseCRTP<Gather<kValueWidthInBits, IndexCType, kWith
185190
memcpy(out_ + position * scaled_factor, src_ + idx_[position] * scaled_factor,
186191
scaled_factor);
187192
} else {
188-
memcpy(out_ + position * kValueWidth, src_ + idx_[position] * kValueWidth,
189-
kValueWidth);
193+
memcpy(out_ + FixedWidthByteOffset(position, kValueWidth),
194+
src_ + FixedWidthByteOffset(idx_[position], kValueWidth), kValueWidth);
190195
}
191196
}
192197

@@ -195,7 +200,7 @@ class Gather : public GatherBaseCRTP<Gather<kValueWidthInBits, IndexCType, kWith
195200
const int64_t scaled_factor = kValueWidth * factor_;
196201
memset(out_ + position * scaled_factor, 0, scaled_factor);
197202
} else {
198-
memset(out_ + position * kValueWidth, 0, kValueWidth);
203+
memset(out_ + FixedWidthByteOffset(position, kValueWidth), 0, kValueWidth);
199204
}
200205
}
201206

@@ -204,7 +209,8 @@ class Gather : public GatherBaseCRTP<Gather<kValueWidthInBits, IndexCType, kWith
204209
const int64_t scaled_factor = kValueWidth * factor_;
205210
memset(out_ + position * scaled_factor, 0, length * scaled_factor);
206211
} else {
207-
memset(out_ + position * kValueWidth, 0, length * kValueWidth);
212+
memset(out_ + FixedWidthByteOffset(position, kValueWidth), 0,
213+
FixedWidthByteOffset(length, kValueWidth));
208214
}
209215
}
210216

0 commit comments

Comments
 (0)