From 03ac80578ff365bcdad68c5a316f020fcaa42742 Mon Sep 17 00:00:00 2001 From: JonSnow1807 Date: Wed, 21 Oct 2026 11:24:34 -0500 Subject: [PATCH] GH-54906: [C++][Compute] Fix ascii_upper/lower/swapcase when the first offset is not 0 StringDataTransform reused the input offsets buffer whenever the array offset was 0, but it writes the transformed data to a new buffer starting at byte 0. When the first value offset is not 0 the reused offsets then pointed past the end of the output data and the result failed validation. Only reuse the offsets buffer when the first offset is 0 and rebase the offsets otherwise, as is already done for sliced inputs. --- .../compute/kernels/scalar_string_ascii.cc | 5 ++-- .../compute/kernels/scalar_string_test.cc | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_string_ascii.cc b/cpp/src/arrow/compute/kernels/scalar_string_ascii.cc index 8e7b626837fe..adda9f376bea 100644 --- a/cpp/src/arrow/compute/kernels/scalar_string_ascii.cc +++ b/cpp/src/arrow/compute/kernels/scalar_string_ascii.cc @@ -452,8 +452,9 @@ Status StringDataTransform(KernelContext* ctx, const ExecSpan& batch, const auto offsets = input.GetValues(1); int64_t offset_nbytes = (input.length + 1) * sizeof(offset_type); - if (input.offset == 0) { - // We can reuse offsets from input if the input owns it + if (input.offset == 0 && (input.length == 0 || offsets[0] == 0)) { + // We can reuse offsets from input if the input owns it. This is only valid + // when the first offset is 0, since the output data buffer starts at 0. if (input.buffers[1].owner != nullptr) { out_arr->buffers[1] = input.GetBuffer(1); } else { diff --git a/cpp/src/arrow/compute/kernels/scalar_string_test.cc b/cpp/src/arrow/compute/kernels/scalar_string_test.cc index 74650c1d76c5..82cd04366314 100644 --- a/cpp/src/arrow/compute/kernels/scalar_string_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_string_test.cc @@ -1176,6 +1176,29 @@ TYPED_TEST(TestStringKernels, AsciiSwapCase) { "[\"HeLLo, wOrLD!\", \"$. a35?\"]"); } +TYPED_TEST(TestStringKernels, AsciiCaseTransformNonZeroFirstOffset) { + // GH-54906: the input is valid even though its first value offset is not 0, + // so the kernels must not reuse its offsets buffer as is. + auto validity = + ArrayFromJSON(boolean(), "[true, false, true, true]")->data()->buffers[1]; + auto offsets = + ArrayFromJSON(this->offset_type(), "[5, 8, 8, 8, 10]")->data()->buffers[1]; + auto data = Buffer::FromString("xxxxxabCDe"); + auto input = + MakeArray(ArrayData::Make(this->type(), /*length=*/4, {validity, offsets, data})); + ASSERT_OK(input->ValidateFull()); + AssertArraysEqual(*ArrayFromJSON(this->type(), R"(["abC", null, "", "De"])"), *input); + + this->CheckUnary("ascii_upper", input, this->type(), R"(["ABC", null, "", "DE"])"); + this->CheckUnary("ascii_lower", input, this->type(), R"(["abc", null, "", "de"])"); + this->CheckUnary("ascii_swapcase", input, this->type(), R"(["ABc", null, "", "dE"])"); + + // Slices with a non-zero array offset + this->CheckUnary("ascii_upper", input->Slice(1), this->type(), R"([null, "", "DE"])"); + this->CheckUnary("ascii_lower", input->Slice(2, 1), this->type(), R"([""])"); + this->CheckUnary("ascii_swapcase", input->Slice(3), this->type(), R"(["dE"])"); +} + TYPED_TEST(TestStringKernels, AsciiCapitalize) { this->CheckUnary("ascii_capitalize", "[]", this->type(), "[]"); this->CheckUnary("ascii_capitalize",