mirror of
https://github.com/apache/arrow.git
synced 2026-10-29 00:56:31 +08:00
apache-arrow-27.0.0.dev
7599
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Raúl Cumplido
|
e304e13b68 | MINOR: [Release] Update versions for 27.0.0-SNAPSHOT | ||
|
water
|
c4a0850ebf |
GH-51018: [C++] Fix indices_nonzero segfault on chunkless ChunkedArray (#51021)
### Rationale for this change `pyarrow.compute.indices_nonzero()` segfaults (SIGSEGV) on a `ChunkedArray` with zero chunks. A zero-length `Array` and a `ChunkedArray` holding one empty chunk both work; only the chunkless case crashes. ### Are these changes tested? Yes, by new test case. ### Are there any user-facing changes? Only a bugfix. * GitHub Issue: #51018 Authored-by: waterWang <waterWang@users.noreply.github.com> Signed-off-by: Rok Mihevc <rok@mihevc.org> |
||
|
|
875cd2ed9b |
GH-48701: [C++][Parquet] Add ALPpd encoding (#48345)
Co-authored-by: Dhirhan Kanesalingam <[dhirhan17@ gmail.com](mailto:dhirhan17@ gmail.com)> With help from : @ emkornfield, @ wgtmac ### Rationale for this change [Adaptive Lossless Floating-Point (ALP)](https://docs.google.com/document/d/1PlyUSfqCqPVwNt8XA-CfRqsbc0NKRG0Kk1FigEm3JOg/edit?tab=t.0#heading=h.5xf60mx6q7xk) is designed for floating-point data that commonly represents decimal values. For these workloads, ALP can provide better compression and faster decoding than general-purpose compression or existing Parquet encodings. This PR adds ALP support for Parquet `FLOAT` and `DOUBLE` columns in the Arrow C++ implementation. ### Specification - [Original ALP specification](https://docs.google.com/document/d/1xz2cudDpN2Y1ImFcTXh15s-3fPtD_aWt/edit) - [Parquet `AlpEncoding.md`](https://github.com/apache/parquet-format/blob/main/AlpEncoding.md) (merged through apache/parquet-format#539; currently in Preview) ### What changes are included in this PR? This PR adds: - The core ALP compression and decompression implementation. - Sampling logic for selecting encoding parameters. - Page metadata serialization, validation, and decoding. - Parquet encoders and decoders for `FLOAT` and `DOUBLE`. - Incremental decoding of ALP pages one vector at a time. - CMake and Meson build integration. - C++ documentation describing how to use the encoding. ALP is opt-in on the write path. It is used only when the writer explicitly selects `Encoding::ALP` and dictionary encoding is disabled. Arrow does not currently select ALP automatically based on the input data. ### Are these changes tested? Yes. Test coverage includes: - Interoperability with the ALP conformance file in `parquet-testing`. - `FLOAT` and `DOUBLE` round trips. - Decimal-like, constant, random, and exception-heavy inputs. - Empty and all-null pages. - Configurable vector sizes. - Truncated or malformed page metadata. - Invalid offsets, element counts, bit widths, and exception positions. - Batched and incremental decoding. ### Are there any user-facing changes? Yes. Arrow C++ can read Parquet pages encoded with ALP. Writers can explicitly select ALP for supported floating-point columns. ALP requires reader support and is not selected automatically, so existing writer behavior remains unchanged unless users opt in. * GitHub Issue: #48701 Lead-authored-by: Prateek Gaur <prateek.gaur@snowflake.com> Co-authored-by: Gang Wu <ustcwg@gmail.com> Signed-off-by: Gang Wu <ustcwg@gmail.com> |
||
|
Krisztián Szűcs
|
9b34c4a6a8 |
GH-51684: [C++][Parquet] Keep the CDC chunking state across row groups (#51685)
### Rationale for this change The content-defined chunker was owned by the column writer, which is recreated for every row group, so the chunking state was reset at each row group boundary. See #51684. ### What changes are included in this PR? The file writer creates one chunker per leaf column and passes it to the column writers of every row group through a private `ColumnWriter::Make()` overload. ### Are these changes tested? Yes. `IndependentOfRowGroupBoundaries` writes the same table into one and into six row groups and expects the same page boundaries apart from the row group ends. The first commit adds it alone, failing without the fix. ### Are there any user-facing changes? - Files with multiple row groups get different page boundaries, since the chunking continues across row groups. - `ColumnWriter::Make()` is unchanged, but column writers created through it no longer chunk, so content-defined chunking requires `ParquetFileWriter`. - Multi-threaded writes with content-defined chunking are about 1.7× slower until #51692 (9.9 ms vs 5.8 ms writing 12 int32 columns of 1M rows on an M4 Max) ### Deduplication results A 1 GiB file of 128Mi random int64 values (PLAIN, snappy, default CDC options) compared with a copy that has 10 rows inserted at 20%. The ratio is the share of bytes left after deduplicating both files with the Xet chunker; 50% means the edited copy adds almost nothing. | Row groups | pyarrow 25.0.1 | This PR | arrow-rs 58.4 | |---|---|---|---| | 8 × 16Mi rows | 50.2% | 50.1% | 50.1% | | 32 × 4Mi rows | 50.6% | 50.4% | 50.5% | | 128 × 1Mi rows | 52.4% | 51.6% | 51.6% | | 512 × 256Ki rows | 59.8% | 56.4% | 56.5% | | 2048 × 64Ki rows | 84.8% | 74.2% | 74.2% | | Without CDC | 86.3% | 86.3% | 86.3% | The page boundaries now match arrow-rs, which already keeps the chunker state across row groups. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. Claude Code wrote the code, tests and description under human direction. **PR code and description written by:** - [ ] Human - [x] AI **Reviewed before submission by:** - [x] Human - [x] AI - [ ] Not reviewed 🤖 Generated with [Claude Code](https://claude.com/claude-code) * GitHub Issue: #51684 |
||
|
Robert Kruszewski
|
84472e46c9 |
GH-52161: [C++][Parquet] Add support for string/binary view to content defined chunking writer (#52162)
### Rationale for this change View types are supported in parquet writer but they error when using content defined chunking ### What changes are included in this PR? Teach the internal chunker how to calculate the size for view types ### Are these changes tested? Added tests ### Are there any user-facing changes? No ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [x] Human - [ ] AI **Reviewed before submission by:** - [x] Human - [x] AI - [ ] Not reviewed * GitHub Issue: #52161 Authored-by: Robert Kruszewski <github@robertk.io> Signed-off-by: Zehua Zou <zehuazou2000@gmail.com> |
||
|
FadyYosry77
|
5ef3c52003 |
GH-51231: [C++][Acero] Keep input nullability information for direct field projections (#51691)
### Rationale for this change ProjectNode creates output column definitions with the requested output name and the type of the bound expression for that column. Nullability information is available from the input schema but is not copied during output column creation; thus, output columns are nullable by default. As a result, projecting a nonnullable column from the input schema produces a nullable column in the output schema, even when only that column is projected. An identity projection may therefore produce a schema incompatible with its input. GH-51231 restricts this change to top-level input fields. ### What changes does this PR contain? Nullability is now exposed through `Expression::nullable()`. Binding captures the input field's nullability for direct top-level field references, and ProjectNode consumes that accessor to determine the nullability of the output field. The captured nullability is recomputed when rebinding and preserved when converting named references to field paths. The projected field name and type of the bound expression remain unchanged. Calls, literals, nested field references, and other expressions whose nullability has not been inferred remain conservatively nullable. This pull request does not include full function-aware nullability inference. A regression test covers: - direct nonnullable-nullable field correspondence - reordered projections - renaming a required field - a calculated control with nonnullable input and nullable output as before - already-bound indexed field projections Expression-level regression tests cover direct fields, the conservative fallback, rebinding, and nullability preservation when converting named references to field paths. ### Are these changes tested? Yes. The regression test checks the schemas created. It failed with the old implementation and passed with the new implementation. Local validation included: - targeted GH-51231 regression: 1/1 passed - expression-level nullability regressions: 5/5 passed - full arrow-compute-expression-test: 54/54 passed - full arrow-acero-plan-test: 63/63 passed - registered expression and plan CTests: 2/2 passed - clang-format v18.1.8 passed - cpplint 1.6.1 passed - output of git diff --check and git diff --cached --check was clean The targeted regression analysis was conducted with: ```bash ./cpp/build-gh51231/debug/arrow-acero-plan-test \ --gtest_filter=ExecPlanExecution.ProjectPreservesDirectFieldNullability ``` The expression and Acero plan test suites were evaluated with: ```bash PYTHON=python3 ctest --test-dir cpp/build-gh51231 \ --output-on-failure \ -j2 \ -R '^(arrow-compute-expression-test|arrow-acero-plan-test)$' ``` Substrait integration was not built locally. The regression test is defined at the native Acero ProjectNode level. ### Are there any visible user impacts? Yes. Top-level field projections copy the nullable attribute from input fields to output fields. This change only alters the projected schema. Projected values remain the same. The experimental Expression API gains the conservative `nullable()` accessor, and bound parameters store nullability. ### Was AI involved in this PR? Yes. AI tools were used to assist with the investigation, implementation and test drafting, review, and preparation of this description. I reproduced the issue locally and reviewed the patch, verifying that the test case failed without the patch and passed with the patch, and ran the listed C++ tests and style checks. **PR code and description by:** - [x] Human - [x] AI **Reviewed before submission by:** - [x] Human - [x] AI - [ ] Not reviewed * GitHub Issue: #51231 Authored-by: Fady Youssery <fadyyosry2005@gmail.com> Signed-off-by: Rossi Sun <zanmato1984@gmail.com> |
||
|
Hanayoshi-8744
|
c7944b35f3 |
GH-51354: [C++][Parquet] Update bundled Apache Thrift to 0.24.0 (#51559)
### Rationale for this change The bundled Apache Thrift dependency (0.22.0) is affected by CVE-2026-55969, an integer overflow vulnerability in `TTransport::checkReadBytesAvailable()` that could bypass message size checks when reading maliciously crafted Thrift-encoded data. Arrow's Parquet module relies on Thrift's compact protocol to deserialize Parquet file metadata, so this is relevant to Arrow. ### What changes are included in this PR? - Bump the bundled Apache Thrift version from 0.22.0 to 0.24.0 in `cpp/thirdparty/versions.txt` (including the SHA256 checksum). - Remove the Clang-only `thrift-3187.patch` and its application logic in `ThirdpartyToolchain.cmake`. This patch pre-applied Thrift's upstream fix for THRIFT-3268 (a compiler warning), which has since been merged into Thrift itself and is already included in 0.24.0. Keeping the patch would make `git apply`/`patch` fail during the bundled build with Clang. ### Are these changes tested? - Verified that `thrift-0.24.0.tar.gz` downloads correctly and its SHA256 checksum matches the value published at downloads.apache.org. - Confirmed the CVE fix is present in Thrift 0.24.0 by diffing `TProtocol.h`, `TCompactProtocol.h`, `TBinaryProtocol.h`, and `TTransport.h` against 0.22.0. - Confirmed `thrift-3187.patch` no longer applies cleanly against 0.24.0 sources (already fixed upstream), which is why it was removed rather than kept as a no-op. - Built Arrow C++ locally on macOS (arm64, AppleClang) with `-DARROW_PARQUET=ON -DARROW_BUILD_TESTS=ON -DThrift_SOURCE=BUNDLED`, on top of the latest `main` (after rebasing), and ran the following Parquet tests. All of them passed with no failures (some tests were skipped, e.g. those requiring Snappy, which was not enabled in my build): `parquet-internals-test`, `parquet-file-deserialize-test`, `parquet-schema-test`, `parquet-reader-test`, `parquet-writer-test`, `parquet-arrow-reader-writer-test`, `parquet-arrow-metadata-test`, `parquet-arrow-index-test`, and `parquet-arrow-internals-test`. - Not tested locally: the removed patch was only applied for `CMAKE_CXX_COMPILER_ID STREQUAL "Clang"`, which my AppleClang build does not hit, so building the bundled Thrift with Clang on Linux relies on CI. I also did not test encryption, Windows/MSVC, or the Python/R packaging builds locally. ### Are there any user-facing changes? No. * GitHub Issue: #51354 --- Disclosure: this change was prepared with the assistance of an AI coding tool (Claude Code). I reviewed the diff, verified the CVE fix and checksum myself, and ran the test suite locally before opening this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Authored-by: Hanayoshi-8744 <201203709+Hanayoshi-8744@users.noreply.github.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com> |
||
|
Animesh Kumar
|
6c82e246e2 |
GH-51780: [C++][IPC] Remove the mislabeled null check on schema fields in GetSchema (#51811)
### Rationale for this change In `GetSchema`, the loop over the schema's fields checked each `Field` for null with the label `"DictionaryEncoding.indexType"`. That label belongs to a different check, so if this one ever failed, the error would point at the wrong field. As the `XXX` comment next to it said, the check also can't fail: for a vector of tables, `fields()->Get(i)` returns the element's address plus its stored offset, never null. `Schema.fields` itself is already checked for null just above. ### What changes are included in this PR? The check and its comment are removed, which is the first option in the issue. Child fields in `FieldFromFlatbuffer` are already read through `children->Get(i)` without a null check, so top-level fields now match. ### Are these changes tested? No new test, since the removed check couldn't fail. The `arrow-ipc-*` tests pass locally. ### Are there any user-facing changes? No. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [x] Human - [ ] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #51780 Authored-by: Animesh Kumar <kanimesh187@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
赵家贝
|
bffbbe7428 |
GH-51673: [C++][Parquet] Skip missing min/max in metadata printer (#51705)
### Rationale for this change Parquet column statistics can contain a null count without min/max values, for example when every value in a column chunk is null. `ParquetFilePrinter::DebugPrint` currently formats min and max whenever any statistics are present. For a DECIMAL stored as `FIXED_LEN_BYTE_ARRAY`, formatting the missing max passes an empty byte array to `Decimal256::FromBigEndian` and makes `parquet-reader --only-metadata` fail. Closes #51673. ### What changes are included in this PR? - Print min and max only when their respective encoded statistics are present. Keep the existing output for columns with both values. - Add a self-contained reader test that writes an all-null DECIMAL column, verifies that null-count statistics exist without min/max, and checks that the text printer omits both values. ### Are these changes tested? - `parquet-reader-test --gtest_filter=TestDebugPrintWithMemoryFile.AllNullDecimalStatistics:TestAllTypesPlain.DebugPrintWorks` - `parquet-reader --only-metadata` against the Open Prices `prices.parquet` file referenced in the issue: all 313 row groups print successfully. ### Are there any user-facing changes? `parquet-reader --only-metadata` now prints valid files with all-null DECIMAL column chunks without an error. Missing min/max values are omitted instead of being formatted. ### Was AI used for this PR? In accordance with the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [ ] Human - [x] AI **Reviewed before submission by:** - [x] Human - [x] AI - [ ] Not reviewed AI assisted with the code, test, and PR description. The issue was reproduced on the public Parquet file and investigated at the throw site with LLDB. * GitHub Issue: #51673 Lead-authored-by: 赵家贝 <970058803@qq.com> Co-authored-by: 赵家贝 <41840745+Sunny-Island@users.noreply.github.com> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
Antoine Pitrou
|
c74c9576f7 |
GH-51757: [C++] Limit max JSON nesting depth in JSON parser (#51758)
### Rationale for this change Our JSON parser is inherently recursive, and a crafted JSON document with huge nesting can send it into a stack overflow. Given that real-world JSON files will not have thousands levels of nesting, we limit the JSON nesting to an arbitrary value of 300. ### Are these changes tested? Yes, by new tests. ### Are there any user-facing changes? **This PR contains a "Critical Fix".** A crafted JSON document with a large nesting depth could trigger a denial of service through stack overflow. ### Was AI used for this PR? **PR code and description written by:** - [x] Human - [ ] AI **Reviewed before submission by:** - [x] Human (self-reviewed) - [ ] AI - [ ] Not reviewed * GitHub Issue: #51757 Authored-by: Antoine Pitrou <antoine@python.org> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
Aaditya Srinivasan
|
bbbc06f8a8 |
GH-51037: [C++] Replace RapidJSON with simdjson in JSON parser (#51038)
### Rationale for this change This PR continues the simdjson migration by replacing the RapidJSON-based parsing implementation used by the JSON parser. The existing parser uses RapidJSON's SAX/handler interface to parse JSON values and populate Arrow builders. This change replaces that implementation with simdjson's ondemand API while retaining the existing builder and type-inference logic. ### Changes * Replace the RapidJSON parser and handler interface with simdjson's ondemand API. * Parse JSON documents using `simdjson::ondemand::parser::iterate_many`. * Use `ResolveSimdjsonResult()` consistently when handling simdjson results. * Preserve support for nested objects and arrays. * Preserve explicit-schema and inferred-field behavior. * Preserve unexpected-field handling for `Error`, `Ignore`, and `InferType`. * Continue storing numeric values as raw JSON tokens. * Trim trailing whitespace from numeric raw tokens to preserve existing behavior. * Preserve JSON parse error propagation through `Status::Invalid`. * Remove the parser's RapidJSON-specific dependencies. ### Are there any user-facing changes? No ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [X] Human - [X] AI **Reviewed before submission by:** - [X] Human - [X] AI - [ ] Not reviewed Fixes: #51037 * GitHub Issue: #51037 Authored-by: Aaditya Srinivasan <aadityasri03@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
Dominique Belhachemi
|
36f655f44c |
GH-51476: [C++][Parquet] Vectorise FindMinMax explicitly (#51478)
### Rationale for this change FindMinMaxAvx2 in parquet/level_comparison_avx2.cc was not vectorised. ### What changes are included in this PR? A single xsimd kernel `FindMinMaxSimd<Arch>` in `level_comparison_simd_kernel_internal.h`, plus support for multiple architectures. ### Are these changes tested? Yes, with a nice speedup. | dispatch level | time | |---|---| | NONE | 513 ns | | SSE4_2 | 54.9 ns (9.3x) | | AVX2 | 28.7 ns (17.9x) | ### Are there any user-facing changes? No ### Was AI used for this PR? **PR code and description written by:** - [x] Human - [x] AI **Reviewed before submission by:** - [x] Human - [x] AI - [ ] Not reviewed * GitHub Issue: #51476 Authored-by: Dominique Belhachemi <db@domibel.de> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
fenfeng9
|
6b1ad60ab5 |
GH-50658: [C++][Dev] Add RunEndEncoded support to gdb_arrow.py (#50768)
### Rationale for this change The Arrow GDB pretty-printers do not support run-end encoded data types. ### What changes are included in this PR? Add GDB pretty-printer support and tests for `run-end encoded` types, scalars, and arrays. ### Are these changes tested? Yes. ### Are there any user-facing changes? No. * GitHub Issue: #50658 Authored-by: fenfeng9 <fenfeng9@qq.com> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
Lorenz Hübschle
|
bb1aba270e |
GH-51308: [C++] Initialize gzip codec state lazily (#51483)
### Rationale for this change `GZipCodec::Init()` initializes compressor state and then immediately initializes decompressor state, which tears the compressor state down again. Compressor state is fairly expensive to initialize (the `deflateInit2` call allocates ~256KiB), so this is rather wasteful. A caller that wants to compress has to do it twice (because decompressor initialization throws it away), and a caller that only wants to decompress still has to do the compressor setup. Whatever the caller wants to do with the GZipCodec, this is pure waste. ### What changes are included in this PR? `GZipCodec::Init()` now validates the compression level and window size without allocating codec state. Since all the compression and decompression methods have to check the mode anyway before doing any work, this is safe. ### Are these changes tested? Largely relying on existing test coverage; the interesting part is that `InitCompressor()` validated the compression level internally, so this check has to be extracted. Split the unit test that covered this into a few separate cases to make it clearer. ### Are there any user-facing changes? No. ### Was AI used for this PR? Yes, both to find the issue and to fix it (Codex). **PR code and description written by:** - [X] Human (PR desc) - [X] AI (code) **Reviewed before submission by:** - [X] Human - [ ] AI - [ ] Not reviewed # * GitHub Issue: #51308 Authored-by: Lorenz Hübschle <lorenz@firebolt.io> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
Antoine Pitrou
|
ab7a6af3ee |
GH-51649: [C++][Parquet] Tell encoding fuzzer to skip inputs with invalid parameters (#51650)
### Rationale for this change `LLVMFuzzerTestOneInput` allows two return values: 0 and -1. While 0 allows for the input to be (potentially) saved in the corpus, -1 tells the fuzzer not to save it. In the words of the [fine documentation](https://llvm.org/docs/LibFuzzer.html#rejecting-unwanted-inputs): > It may be desirable to reject some inputs, i.e. to not add them to the corpus. > For example, when fuzzing an API consisting of parsing and other logic, one may want > to allow only those inputs into the corpus that parse successfully. > If the fuzz target returns -1 on a given input, libFuzzer will not add that > input to the corpus, regardless of what coverage it triggers. ### What changes are included in this PR? 1. Add a `FuzzStatus` type that allows telling the fuzzing engine whether an input should be skipped (i.e. not saved in the corpus). 2. Skip inputs where the parameter header contains invalid values in the Parquet encoding fuzzer ### Are these changes tested? Manually, by running the fuzzer from the command line and watching logs with `ARROW_FUZZING_VERBOSITY` enabled. Without a seed corpus and trying to generate 100k inputs (`./build/out/arrow/parquet-encoding-fuzz -seed=42 -runs=100000`), I get: * before: ``` #100000 DONE cov: 1377 ft: 2265 corp: 46/3357b lim: 778 exec/s: 7142 rss: 201Mb Done 100000 runs in 14 second(s) ``` * after: ``` #100000 DONE cov: 2051 ft: 3723 corp: 41/5725b lim: 832 exec/s: 50000 rss: 259Mb Done 100000 runs in 2 second(s) ``` It seems that this PR allows the fuzzer to proceed much faster when mutation generates such inputs. Despite running faster, it also reaches more coverage (see [doc](https://llvm.org/docs/LibFuzzer.html#output) about the `cov:` and `ft:` values). (*caveat:* these measurements is with a fixed random seed, and without a seed corpus) (of course, ideally the fuzzer would not produce such inputs at all, but ensuring that is [more involved](https://llvm.org/docs/LibFuzzer.html#user-supplied-mutators)) ### Are there any user-facing changes? No. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [x] Human - [ ] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #51649 Authored-by: Antoine Pitrou <antoine@python.org> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
Hiroyuki Sato
|
e85e181188 |
GH-51670: [C++][Dev] Fix shellcheck errors in the cpp/examples/minimal_build directory (#51671)
### Rationale for this change This is the sub issue #44748. * SC2046: Quote this to prevent word splitting * SC2086: Double quote to prevent globbing and word splitting * SC2223: This default assignment may cause DoS due to globbing. Quote it ``` In build_arrow.sh line 23: mkdir -p $ARROW_BUILD_DIR ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: mkdir -p "$ARROW_BUILD_DIR" In build_arrow.sh line 24: pushd $ARROW_BUILD_DIR ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: pushd "$ARROW_BUILD_DIR" In build_arrow.sh line 30: $ARROW_CMAKE_OPTIONS ^------------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "$ARROW_CMAKE_OPTIONS" In build_arrow.sh line 32: make -j$NPROC ^----^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: make -j"$NPROC" In build_example.sh line 21: mkdir -p $EXAMPLE_BUILD_DIR ^----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: mkdir -p "$EXAMPLE_BUILD_DIR" In build_example.sh line 22: pushd $EXAMPLE_BUILD_DIR ^----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: pushd "$EXAMPLE_BUILD_DIR" In run_static.sh line 21: : ${ARROW_DIR:=/arrow} ^------------------^ SC2223 (info): This default assignment may cause DoS due to globbing. Quote it. In run_static.sh line 22: : ${EXAMPLE_DIR:=/io} ^-----------------^ SC2223 (info): This default assignment may cause DoS due to globbing. Quote it. In run_static.sh line 23: : ${ARROW_BUILD_DIR:=/build/arrow} ^-- SC2223 (info): This default assignment may cause DoS due to globbing. Quote it. In run_static.sh line 24: : ${EXAMPLE_BUILD_DIR:=/build/example} ^-- SC2223 (info): This default assignment may cause DoS due to globbing. Quote it. In run_static.sh line 26: : ${ARROW_DEPENDENCY_SOURCE:=BUNDLED} ^-- SC2223 (info): This default assignment may cause DoS due to globbing. Quote it. In run_static.sh line 34: mkdir -p $ARROW_BUILD_DIR ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: mkdir -p "$ARROW_BUILD_DIR" In run_static.sh line 35: pushd $ARROW_BUILD_DIR ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: pushd "$ARROW_BUILD_DIR" In run_static.sh line 39: cmake $ARROW_DIR/cpp \ ^--------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: cmake "$ARROW_DIR"/cpp \ In run_static.sh line 43: -DARROW_DEPENDENCY_SOURCE=${ARROW_DEPENDENCY_SOURCE} \ ^------------------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: -DARROW_DEPENDENCY_SOURCE="${ARROW_DEPENDENCY_SOURCE}" \ In run_static.sh line 46: $ARROW_CMAKE_OPTIONS ^------------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "$ARROW_CMAKE_OPTIONS" In run_static.sh line 48: make -j$NPROC ^----^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: make -j"$NPROC" In run_static.sh line 60: rm -rf $EXAMPLE_BUILD_DIR ^----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: rm -rf "$EXAMPLE_BUILD_DIR" In run_static.sh line 61: mkdir -p $EXAMPLE_BUILD_DIR ^----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: mkdir -p "$EXAMPLE_BUILD_DIR" In run_static.sh line 62: pushd $EXAMPLE_BUILD_DIR ^----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: pushd "$EXAMPLE_BUILD_DIR" In run_static.sh line 64: cmake $EXAMPLE_DIR -DARROW_LINK_SHARED=OFF ^----------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: cmake "$EXAMPLE_DIR" -DARROW_LINK_SHARED=OFF In run_static.sh line 76: pushd $EXAMPLE_DIR ^----------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: pushd "$EXAMPLE_DIR" In run_static.sh line 78: $EXAMPLE_BUILD_DIR/arrow-example ^----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "$EXAMPLE_BUILD_DIR"/arrow-example In run_static.sh line 87: rm -rf $EXAMPLE_BUILD_DIR ^----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: rm -rf "$EXAMPLE_BUILD_DIR" In run_static.sh line 88: mkdir -p $EXAMPLE_BUILD_DIR ^----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: mkdir -p "$EXAMPLE_BUILD_DIR" In run_static.sh line 90: -o $EXAMPLE_BUILD_DIR/arrow-example \ ^----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: -o "$EXAMPLE_BUILD_DIR"/arrow-example \ In run_static.sh line 91: $EXAMPLE_DIR/example.cc \ ^----------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "$EXAMPLE_DIR"/example.cc \ In run_static.sh line 92: $(PKG_CONFIG_PATH=$ARROW_BUILD_DIR/lib/pkgconfig \ ^-- SC2046 (warning): Quote this to prevent word splitting. In run_static.sh line 104: pushd $EXAMPLE_DIR ^----------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: pushd "$EXAMPLE_DIR" In run_static.sh line 106: $EXAMPLE_BUILD_DIR/arrow-example ^----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "$EXAMPLE_BUILD_DIR"/arrow-example For more information: https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt... https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ... https://www.shellcheck.net/wiki/SC2223 -- This default assignment may cause... ``` ### What changes are included in this PR? * SC2046: Quote command substitutions * SC2086: Quote variables * SC2223: Quote default assignments ### Are these changes tested? Yes. ### Are there any user-facing changes? No. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [x] Human - [ ] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #51670 Authored-by: Hiroyuki Sato <hiroysato@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com> |
||
|
Raúl Cumplido
|
676dd43c5f |
GH-51687: [C++] Skipt tests that require threads if ARROW_ENABLE_THREADING=OFF (#51688)
### Rationale for this change The following PR introduced some tests that require threeading but those are not skipped when ARROW_ENABLE_THREADING=OFF failing on jobs like emscripten. - https://github.com/apache/arrow/pull/51498 ### What changes are included in this PR? Skip the tests if ARROW_ENABLE_THREADING=OFF ### Are these changes tested? Yes via archer with the failing job. ### Are there any user-facing changes? No ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [x] Human - [ ] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #51687 Authored-by: Raúl Cumplido <raulcumplido@gmail.com> Signed-off-by: Rossi Sun <zanmato1984@gmail.com> |
||
|
Hiroyuki Sato
|
142829f4fe |
GH-51682: [CI][C++] Add liburiparser-dev to system dependency image (#51683)
### Rationale for this change Changes around UriParser, including #51007, caused example-cpp-minimal-build-static-system-dependency to fail to build. ### What changes are included in this PR? This change adds liburiparser-dev to the system dependency image. ### Are these changes tested? Yes. ### Are there any user-facing changes? No. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [x] Human - [ ] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #51682 Authored-by: Hiroyuki Sato <hiroysato@gmail.com> Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com> |
||
|
tadeja
|
ba6c5fecdd |
GH-51638: [CI][C++][Parquet] Update expected JSON output in parquet-reader-test for simdjson 5.0 (#51661)
### Rationale for this change See #51638 - `fractured_json` formatting changes with the new simdjson version 5.0 (whitespace, line breaks, key order in tables), cause `parquet-reader-test` to fail on four of its tests `TestJSONWithLocalFile.JSONOutput...` ### What changes are included in this PR? Check `simdjson::SIMDJSON_VERSION_MAJOR`, and for `>= 5` use the new expected formatting (JSON content hasn't changed). Keep existing expected output formatting for simdjson 4.x ### Are these changes tested? Yes, CI runs pass. ### Are there any user-facing changes? No, only test changes. ### Was AI used for this PR? **PR code and description written by:** - [x] Human - [x] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #51638 Authored-by: Tadeja Kadunc <tadeja.kadunc@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com> |
||
|
khan
|
ccfcdd24fc |
GH-31076: [C++] Reject overflowing STL allocation sizes (#51477)
### Rationale for this change The STL allocator computes `n * sizeof(T)` without checking for overflow. On a 64-bit platform, requesting `2^61` uint64_t elements wraps the byte count to zero. Requesting one more element wraps it to eight. Both allocations incorrectly succeed on upstream commit `5448aa7a916e73116ccc009454e3c19572dd5aad`, returning less storage than requested. ### What changes are included in this PR? Check the element count before multiplication against both the size_t limit and the int64_t limit used by MemoryPool. Reject oversized requests with the existing BadAlloc exception, which derives from std::bad_alloc. Add one regression test covering wraparound to zero and eight bytes. The test frees any unexpectedly successful allocation before reporting the missing exception. ### Are these changes tested? Yes, by additional unit test. ### Are there any user-facing changes? Just a bugfix. ### Was AI used for this PR? **PR code and description written by** - [ ] Human - [x] AI **Reviewed before submission by** - [x] Human - [x] AI - [ ] Not reviewed OpenAI Codex researched the issue and related PRs, wrote the reproducer, regression test, fix, and this description, ran the listed checks, and reviewed the diff with another AI agent. Grok was used for preliminary X research. No external implementation was copied into the patch. The human author reviewed the complete patch, discussed the overflow checks and regression test, and takes responsibility for the change. * GitHub Issue: #31076 Authored-by: Aikhan Jumashukurov <117706338+aikhanj@users.noreply.github.com> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
|
5183e21386 |
GH-46901: [C++][Compute] Add remainder and modulo kernels (#48914)
### Rationale for this change Arrow is currently missing remainder and modulo kernels. ### What changes are included in this PR? Add the kernels `remainder`, `remainder_checked`, `modulo`, and `modulo_checked`, following the terminology in the [divmod proposal](https://lists.apache.org/thread/h3t6nz1ys2k2hnbrjvwyoxkf70cps8sh): * `remainder` uses truncated (C/C++) semantics — the result has the sign of the dividend, e.g. `remainder(-7, 3) == -1`. * `modulo` uses floored (Python/R) semantics — the result has the sign of the divisor, e.g. `modulo(-7, 3) == 2`. Both are supported for integer, floating-point and decimal inputs, and are exposed as `compute::Remainder` / `compute::Modulo` in `api_scalar.h`. Decimal arguments are promoted to a common scale like `add`, but the result type is resolved by a dedicated resolver rather than reusing `ResolveDecimalAdditionOrSubtractionOutput`: a remainder is always smaller in magnitude than the divisor, so no extra digit is needed for a carry, and the result is `precision = max(p1, p2)`, `scale = s1`. This keeps maximum-precision inputs (`decimal128(38, 0)`, `decimal256(76, 0)`) from overflowing the decimal precision range. Left as follow-ups: combined divmod #27909, floor division #39386, and duration support. ### Are these changes tested? Yes. New tests cover truncated vs. floored semantics across signed/unsigned integer, floating-point and decimal types, division by zero, `INT_MIN % -1` overflow, and decimal result-type resolution (same type in/out, mixed precisions and scales, Decimal128/Decimal256 promotion, decimal/integer promotion, and maximum-precision decimals). ### Are there any user-facing changes? Yes — four new compute functions (`remainder`, `remainder_checked`, `modulo`, `modulo_checked`), the corresponding `compute::Remainder` / `compute::Modulo` C++ APIs, and a new `BasicDecimal256 operator%`. These are additions only; no existing API is changed or removed. * GitHub Issue: #46901 * xref: https://github.com/apache/arrow/pull/46902, * xref: https://github.com/apache/arrow/pull/28497 Lead-authored-by: Fangchen Li <fangchen.li@outlook.com> Co-authored-by: Rok Mihevc <54589+rok@users.noreply.github.com> Co-authored-by: tadeja <864005+tadeja@users.noreply.github.com> Signed-off-by: Rok Mihevc <rok@mihevc.org> |
||
|
tadeja
|
e6adb62b3d |
MINOR: [Docs][C++] Fix Sphinx building html warning (#51672)
### Rationale for this change Fix Sphinx building [html] [warning](https://github.com/apache/arrow/actions/runs/36743843708/job/109984877241#step:7:5190) ``` /build/python/docs/source/cpp/api/filesystem.rst:81: WARNING: doxygenfunction: Unable to resolve function "arrow::fs::InitializeS3" with arguments "(const S3GlobalOptions&)". Candidate function could not be parsed. Parsing error is ... ARROW_S3_EXPORT Status InitializeS3 (const S3GlobalOptions &options) ``` ### What changes are included in this PR? Add new macro `ARROW_S3_EXPORT` to the PREDEFINED list in `Doxyfile` ### Are these changes tested? Yes. ### Are there any user-facing changes? No. ### Was AI used for this PR? **PR code and description written by:** - [X] Human - [ ] AI **Reviewed before submission by:** - [X] Human - [ ] AI - [ ] Not reviewed Authored-by: Tadeja Kadunc <tadeja.kadunc@gmail.com> Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com> |
||
|
tadeja
|
275872bf9b |
GH-51651: [C++][CI] Fix static linking for system Abseil and bundled GCS (#51653)
### Rationale for this change Static Arrow linking can fail for bundled GCS and system Abseil. ### What changes are included in this PR? Export Abseil dependencies via CMake target and pkg-config metadata. ### Are these changes tested? To be additionally verified by CI. ### Are there any user-facing changes? This improves downstream static linking. ### Was AI used for this PR? **PR code and description written by:** - [X] Human - [X] AI **Reviewed before submission by:** - [X] Human - [X] AI - [ ] Not reviewed * GitHub Issue: #51651 Authored-by: Tadeja Kadunc <tadeja.kadunc@gmail.com> Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com> |
||
|
Divjot AroraandIsaac
|
01cb4315cc |
GH-50910: [Parquet] Tolerate unrecognized logical/physical type combinations when reading (#50909)
### Rationale for this change See https://github.com/apache/parquet-format/issues/607 for rationale. ### What changes are included in this PR? This PR gracefully handles unrecognized logical/physical type combinations by dropping the logical type during the read and dropping any associated statistics for the relevant columns. Note that unrecognized logical types are already handled gracefully and no changes were required. ### Are these changes tested? Yes, via unit tests and an e2e test that reads a real file that contains an invalid type combination. ### Are there any user-facing changes? No * GitHub Issue: #50910 Lead-authored-by: Divjot Arora <div.arora@databricks.com> Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
Narendra
|
1998ed3556 |
GH-51007: [C++] Make uriparser an external dependency (#51244)
### Rationale for this change The vendored uriparser in `cpp/src/arrow/vendored/uriparser/` is 0.9.3, which is old and misses upstream security fixes. Instead of refreshing the vendored copy, this makes uriparser an external dependency like our other third-party dependencies. ### What changes are included in this PR? - Add `uriparser` to `ARROW_THIRDPARTY_DEPENDENCIES` - Add `build_uriparser()` that builds uriparser 1.0.2 with `FetchContent` for `uriparser_SOURCE=BUNDLED` (static; docs, tests, tools and `wchar_t` support are disabled). The built library is bundled into `arrow_bundled_dependencies`. - Require uriparser 0.9.6 or later for `uriparser_SOURCE=SYSTEM`. All uriparser APIs used by Arrow are available in 0.9.6, so supported platforms such as Ubuntu 22.04 can use their distribution package. - Add `cmake_modules/FinduriparserAlt.cmake`. It tries uriparser's CMake package configuration first and falls back to pkg-config (`liburiparser.pc`), because Debian and Ubuntu's `liburiparser-dev` provides only the pkg-config file. - Add `uriparser::uriparser` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS` for `uriparser_SOURCE=SYSTEM`, so that static library users link uriparser. - meson: Use the system uriparser 0.9.6 or later if available. Otherwise, build uriparser 1.0.2 via `subprojects/uriparser.wrap` with the CMake subproject support. - Remove `cpp/src/arrow/vendored/uriparser/` and its section in `LICENSE.txt`. - Packaging: Install uriparser in our Docker images, conda environment, `Brewfile`, vcpkg manifests, MSYS2 setup and Linux packages build environments. - Packaging: Pass `-Duriparser_SOURCE` in `ci/scripts/cpp_build.sh`. - Packaging: Add uriparser to the dependency list in `docs/source/developers/cpp/building.rst`. ***NOTICE*** : The floor is 0.9.6 rather than 1.0.2, so supported platforms such as Ubuntu 22.04 can use their distro package and CI does not have to build uriparser from source. ### Are these changes tested? All existing tests pass. ### Are there any user-facing changes? Now Arrow requires uriparser or builds it . No user facing changes as such because no API changes . **This PR contains a "Critical Fix".** It has security fixes that has gone in uriparser. **AI Usage**: - Used for testing <-> building loop. * GitHub Issue: #51007 Authored-by: Narendra <naskastark11@gmail.com> Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com> |
||
|
Abdul Rawoof Khan
|
45801eb417 |
GH-50440: [C++][Gandiva] Fix out-of-bounds read in set_error_for_date (#50441)
### Rationale for this change `set_error_for_date` formats the invalid-date/timestamp message with `snprintf(error, size, "%s%s", msg, input)`, but `input` is a Gandiva string passed as pointer plus `length` and is not NUL-terminated, so the `%s` conversion scans past the `length` bytes looking for a terminator and over-reads the values buffer (past its end for the last value). It is reached from `castDATE_utf8`/`castTIMESTAMP_utf8` on any invalid `CAST(str AS DATE/TIMESTAMP)` input. Switching to the bounded `%.*s` form with the explicit length keeps the read inside the string, matching every other error formatter in the precompiled sources. ### What changes are included in this PR? Use `%.*s` with `length` in `set_error_for_date`, which covers all of its call sites in both `castDATE_utf8` and `castTIMESTAMP_utf8`. ### Are these changes tested? Yes. Added `TestTime.TestCastDateInvalidUnterminated`, which feeds an invalid date held in an exactly-sized heap buffer with no trailing NUL so the over-read trips AddressSanitizer, and checks the error string is bounded to the input length. ### Are there any user-facing changes? No. **This PR contains a "Critical Fix".** It fixes a heap out-of-bounds read reachable from user-supplied CAST-to-date/timestamp input. * GitHub Issue: #50440 Authored-by: abdul rawoof <abdulr@bugqore.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com> |
||
|
alytantawyy
|
f36c238fd0 |
GH-50148: [C++] Add Content-Encoding support to S3 filesystem metadata (#50167)
### Rationale for this change The S3 filesystem metadata handling supported headers such as `Content-Type`, `Content-Language`, `Cache-Control`, and `Expires`, but omitted `Content-Encoding`. As a result, `Content-Encoding` was not propagated when writing S3 object metadata, and it was also not returned when reading metadata back. ### What changes are included in this PR? - add `Content-Encoding` to S3 object metadata extraction - add `Content-Encoding` to the S3 metadata setter whitelist - reuse the existing string metadata setter for `Content-Encoding` - extend the S3 metadata round-trip test to cover explicit and default `Content-Encoding` metadata ### Are these changes tested? Yes. ```text ./cpp/build-s3-system/debug/arrow-s3fs-test --gtest_filter=TestS3FS.OpenOutputStreamMetadata ``` * GitHub Issue: #50148 Authored-by: alytantawyy <alytantawy@usf.edu> Signed-off-by: Sutou Kouhei <kou@clear-code.com> |
||
|
|
fcac3dfd8e |
GH-48072: [C++][Acero] fix a bug of materialize boolean (#48073)
### Rationale for this change Use the correct offset when accessing boolean values from the buffer. ### What changes are included in this PR? In `unmaterialized_table_internal.h`, use the correct offset when accessing boolean values from the buffer. Also, add a unit test to regress this bug. ### Are these changes tested? Yes ### Are there any user-facing changes? No **This PR contains a "Critical Fix".** Yes, fix a bug that caused incorrect data * GitHub Issue: #48072 Lead-authored-by: jsonwu <jsonwu@tencent.com> Co-authored-by: Nic Crane <thisisnic@gmail.com> Co-authored-by: Rossi Sun <zanmato1984@gmail.com> Signed-off-by: Nic Crane <thisisnic@gmail.com> |
||
|
tadeja
|
382ffd94f9 |
GH-51329: [C++][CI] Test static linking with S3 and fix Azure/GCS dependencies on static builds (#51280)
### Rationale for this change Fix #51329 Additional downstream static-linking test, after #50195 moved S3 into the separate ArrowS3 library. ### What changes are included in this PR? Test downstream static linking of `ArrowS3::arrow_s3_static` via minimal_build example and `cpp_test.sh`, Fix dependencies for static Azure and GCS consumers: append `libcurl` for bundled GCS, append `libcurl` and `LibXml2` for bundled Azure(NOT WIN32), append links for system Azure SDK and pkg-config --static arrow to include `libcurl` and `libxml-2.0` for bundled GCS/Azure ### Are these changes tested? Verified by CI ### Are there any user-facing changes? Downstream consumers now have required static dependencies for Azure/GCS * GitHub Issue: #51329 Authored-by: Tadeja Kadunc <tadeja.kadunc@gmail.com> Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com> |
||
|
Hiroyuki SatoandSutou Kouhei
|
b8a45d226b |
GH-51634: [C++][CI] Apply a google-cloud-cpp patch for OpenSSL 4.x compatibility (#51635)
### Rationale for this change The current version of google-cloud-cpp used by Arrow does not build with OpenSSL 4.x ### What changes are included in this PR? This change applies a patch to google-cloud-cpp as a workaround for OpenSSL 4.x compatibility. ### Are these changes tested? Yes. ### Are there any user-facing changes? No. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [x] Human - [ ] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #51634 Lead-authored-by: Hiroyuki Sato <hiroysato@gmail.com> Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Signed-off-by: Sutou Kouhei <kou@clear-code.com> |
||
|
Raúl Cumplido
|
f2df1b047e |
GH-51632: [C++][CI] Link OpenTelemetry libs for parquet-arrow-reader-writer-tracing-test depending on System vs Bundled OpenTelemetry (#51633)
### Rationale for this change After merging https://github.com/apache/arrow/pull/51606 some debian jobs that use System opentelemetry were failing with missing symbols. ### What changes are included in this PR? Change what we link depending on bundled vs system OpenTelemetry. ### Are these changes tested? Yes all the CI jobs that the previous issue and this new issue had failing have been exercised and are successful ### Are there any user-facing changes? No ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [x] Human - [ ] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #51632 Authored-by: Raúl Cumplido <raulcumplido@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com> |
||
|
Dominique Belhachemi
|
5592d99193 |
GH-51492: [C++] Do not left shift a bpacking lane by its full width (#51493)
### Rationale for this change When a packed value starts on a byte boundary its high part contributes nothing, but LargeKernelPlan::Build still asks for a left shift, and on 8 bit lanes that shift is the whole lane width. Some backends saturate and give zero, others use the low bits of the shift amount and return the lane unchanged, so its bits survive the mask. So affected values decode incorrectly with no error reported, that can lead to silent data corruption. ### What changes are included in this PR? Point the high swizzle back at the low byte instead and set the shift to zero. So the plan never asks for the shift at all. This is a compile-time change to the kernel plan. The emitted kernel is unchanged. ### Are these changes tested? Yes I also added a new assert that fails to compile on main. ### Are there any user-facing changes? No ### Was AI used for this PR? **PR code and description written by:** - [x] Human - [x] AI **Reviewed before submission by:** - [x] Human - [x] AI - [ ] Not reviewed * GitHub Issue: #51492 Authored-by: Dominique Belhachemi <db@domibel.de> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
Alexander Taepper
|
331b0f5774 |
GH-51471: [C++] Disable simdjson threading in chunker (#51473)
### Rationale for this change Resolves #51471: When invoking `simdjson`'s `parse_many`, it uses threading to perform its stage 1 parsing on the next batch when while parsing the current batch: https://github.com/simdjson/simdjson/blob/master/doc/parse_many.md?plain=1#L96-L104 In most cases, we do not need to parse any batches after the first one to find delimiters (in fact, we currently would even error, when the first batch does not contain the whole first document). By disabling the parser's threading we can expect performance improvements ### What changes are included in this PR? This disables threading in the parser by setting `parser_.threaded = false;` ### Are these changes tested? Yes before (`origin/main` `bb83012743`): ``` ChunkJSONPrettyPrintedMultipleBlocks 187462 ns 158760 ns 4415 block_size=27.344k bytes_per_second=1.28328Gi/s json_size=218.757k ``` after (`taepper:GH-51471` `fb56ccb2a1`): ``` ChunkJSONPrettyPrintedMultipleBlocks 98726 ns 98721 ns 7144 block_size=27.344k bytes_per_second=2.06374Gi/s json_size=218.757k ``` ### Are there any user-facing changes? No ### Was AI used for this PR? **PR code and description written by:** - [x] Human - [ ] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #51471 Authored-by: Alexander Taepper <alexander.taepper@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
Bryce Mecum
|
8fa322c3e2 |
GH-49538: [C++][FlightRPC][ODBC] Use static linkage in Windows FlightSQL ODBC driver (#49585)
### Rationale for this change The FlightSQL ODBC driver was previously using dynamic linkage for the Arrow deps it relies on. This worked but prevented a challenge for code signing because (I think) we'd have to sign each DLL and signing costs money. We can use static linkage with a dynamically linked MSVC runtime so we only have one DLL to sign. Ref: https://github.com/apache/arrow/issues/49404 ### What changes are included in this PR? Updates to CMake and CI for the build to allow static linkage with dynamic MSVC runtime (`x64-windows-static-md`). ### Are these changes tested? Yes. ### Are there any user-facing changes? No. * GitHub Issue: #49538 Authored-by: Bryce Mecum <petridish@gmail.com> Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com> |
||
|
Alenka FrimandCopilot Autofix powered by AI
|
1369f7fa69 |
GH-45747: [C++] Remove deprecated ObjectType and FileStatistics, refactor hdfs code (#45998)
### Rationale for this change `ObjectType` and `FileStatistics` in io/hdfs.h have been deprecated for a while and can be removed. ### What changes are included in this PR? `ObjectType` and `FileStatistics` structs are removed and instead FileSystem API in `arrow::fs` is used. Together with this change, the hdfs connected code is moved from `cpp/src/arrow/io` to `cpp/src/arrow/filesystem` merging `FileSystem` and `HadoopFileSystem` classes from `arrow::io` into the public `HadoopFileSystem` class. ### Are these changes tested? Existing tests should pass. ### Are there any user-facing changes? Deprecated structs are removed and all hdfs related code is now a part of the filesystem module. * GitHub Issue: #45747 Also closes: https://github.com/apache/arrow/issues/22457 (not sure about `io/interfaces.h`?) Lead-authored-by: AlenkaF <frim.alenka@gmail.com> Co-authored-by: Alenka Frim <AlenkaF@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com> |
||
|
Jonas Dedden
|
4e61c97e3a |
GH-51011: [C++][CI] Poll for GCS testbench readiness instead of one 10s attempt (#51012)
### Rationale for this change `arrow-gcsfs-test` sometimes fails on CI with `Could not start GCS emulator 'storage-testbench' (failed to listen)`. The testbench does listen, it is just slow to answer: it runs under Werkzeug's reloader, which binds the socket in the parent and serves from a child that re-imports grpcio, protobuf and flask first. `GcsTestbench` passes its 10s startup budget to `LimitedTimeRetryPolicy` as well, so the readiness loop only makes one attempt. And since stderr is discarded, the CI log can't tell a slow start from a crash. ### What changes are included in this PR? In `gcsfs_test.cc`: * 5s per attempt, 60s overall, so the loop actually polls * keep the testbench's stderr * error message says `(did not become ready)` instead of `(failed to listen)` `util::Process::IgnoreStderr` is now unused; I can remove it if preferred. ### Are these changes tested? By `arrow-gcsfs-test` itself. I couldn't reproduce the CI flake on demand, so this widens the margin and makes the next failure diagnosable. ### Are there any user-facing changes? No. * GitHub Issue: #51011 Authored-by: Jonas Dedden <mail@jonas-dedden.de> Signed-off-by: Sutou Kouhei <kou@clear-code.com> |
||
|
Hiroyuki Sato
|
d0f318d1da |
GH-51485: [C++] Fix SC2086 errors in cpp/examples/tutorial_examples directory (#51486)
### Rationale for this change This is the sub issue #44748. ``` In build_arrow.sh line 23: mkdir -p $ARROW_BUILD_DIR ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: mkdir -p "$ARROW_BUILD_DIR" In build_arrow.sh line 24: pushd $ARROW_BUILD_DIR ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: pushd "$ARROW_BUILD_DIR" In build_arrow.sh line 33: $ARROW_CMAKE_OPTIONS ^------------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "$ARROW_CMAKE_OPTIONS" In build_arrow.sh line 35: make -j$NPROC ^----^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: make -j"$NPROC" In build_example.sh line 21: mkdir -p $EXAMPLE_BUILD_DIR ^----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: mkdir -p "$EXAMPLE_BUILD_DIR" In build_example.sh line 22: pushd $EXAMPLE_BUILD_DIR ^----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: pushd "$EXAMPLE_BUILD_DIR" In build_example.sh line 24: cmake /io $EXAMPLE_CMAKE_OPTIONS ^--------------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: cmake /io "$EXAMPLE_CMAKE_OPTIONS" For more information: https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ... ``` ### What changes are included in this PR? * SC2086: Quote variables ### Are these changes tested? Yes. ### Are there any user-facing changes? No. ### Was AI used for this PR? **PR code and description written by:** - [x] Human - [ ] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #51485 Authored-by: Hiroyuki Sato <hiroysato@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com> |
||
|
Raúl Cumplido
|
7bc31d7d0a |
GH-51604: [C++][Parquet] Only link opentelemetry-cpp::sdk on arrow_reader_writer_tracing_test.cc to avoid double freeing (#51606)
### Rationale for this change ASAN and TSAN jobs were failing due to double freeing opentelemetry globals. ### What changes are included in this PR? Avoid linking all OpenTelemetry libs and only link `opentelemetry-cpp::sdk` to the test to avoid double free as OpenTelemetry is already linked in libarrow. ) opentelemetry-cpp::sdk adds the required include directories and compile definitions. ### Are these changes tested? Yes on CI ### Are there any user-facing changes? No ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [x] Human - [x] AI Code was written by a human but I've used Claude Opus 5.5 in order to analyze the problem and brainstorm solutions. **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #51604 Authored-by: Raúl Cumplido <raulcumplido@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com> |
||
|
Kita RenjiandPanchawat
|
065db5f6e6 |
GH-51495: [C++] Fix race in MergedGenerator that could drop an error and end the stream early (#51498)
### Rationale for this change `MergedGenerator` could turn an error from a subscription into a normal end-of-stream. When an inner or outer subscription failed while no caller was waiting, the callback set `broken = true` under the mutex but only stored the error in `final_error` after releasing it. A call to `operator()` in that window saw `broken` with an OK `final_error` and returned `IterationEnd`, and the error was never delivered. In the dataset scanner this shows up as `ToTable()` returning a truncated table without raising when a fragment cannot be opened (#51495). ### What changes are included in this PR? - The first error is recorded in the same locked section that sets `broken`, via `SetFinalErrorUnlocked` (replacing `MarkFinalError`), together with the caller that will receive it, if one is already waiting. Otherwise the error goes to the next caller that asks, so a concurrent pull that sees `broken` also sees the error. - `all_finished` is removed, and the remaining futures are no longer completed through callbacks on it. `Future` callbacks have no guaranteed order (`future.h`): one added while a future is being marked finished may run immediately. With every terminal item chained on `all_finished`, a pull made during that dispatch could complete before an earlier future: the one receiving the error (pointed out in review), or an earlier `IterationEnd`. - Terminal pulls made once the generator is broken or exhausted are queued in `waiting_jobs`. - `MarkFinishedAndPurge` completes the remaining futures itself, in the order they were handed out: the future receiving the error first, then the waiting callers and terminal items, which get `IterationEnd`. Pulls that arrive meanwhile are queued behind them, and the queue is drained until it stays empty. - A caller that asks from the callbacks of the last pending future still gets an already-finished `IterationEnd`, as before. - A private, test-only static hook, `MergedGenerator<T>::error_signaled_hook_for_testing`, that runs right after the generator enters its error state and the mutex is released. It is empty by default and only reachable from the `MergedGeneratorErrorHookTest` fixture, which is a friend. Without it I couldn't find a way to test this deterministically, since no user code runs inside the window. - Tests, for both the inner and outer error paths: - `MergedGeneratorErrorHookTest.{Inner,Outer}ErrorNotLostToConcurrentPull`: nobody waiting; a pull from inside the hook must raise. - `MergedGeneratorErrorHookTest.{Inner,Outer}ErrorToWaiterNotOvertakenByLaterPull`: a caller already waiting must get the error before a later pull gets end-of-stream. - `MergedGeneratorErrorHookTest.{Inner,Outer}ErrorToWaiterNotOvertakenDuringCompletion` and `ClaimedErrorNotOvertakenDuringCompletion`: the later pull is made while the generator is completing its futures, with the earlier future's completion held up (via `TryAddCallback`, as in the review). - `MergedGeneratorTest.PullFromLastFutureCallbackCompletesAtOnce`. - `MergedGeneratorStressTest.TerminalNeverOvertakesEarlierFutures`: inner and outer failures on the thread pool; checks that no terminal item completes while an earlier future is pending and that exactly one error is delivered. ### Are these changes tested? Yes. `arrow-async-utility-test`. Latest commit (macOS arm64): all 138 tests pass. The `MergedGenerator` tests pass 300/300 with `--gtest_repeat=300` (Debug) and 30/30 under TSAN with no reports. The three `*DuringCompletion` tests fail every time on the previous commit, and the stress test found the `IterationEnd` ordering issue before the fix. Original commit (Linux): - With this PR: all 131 tests pass. The two new tests also passed 1000/1000 with `--gtest_repeat=1000`. - With only the `final_error` change reverted (hook kept): both new tests fail every time (2000/2000 over `--gtest_repeat=1000`) with `Expected '_fut.status()' to fail with Invalid, but got OK`. The other 129 tests still pass. I also checked the fix end to end outside the test suite. I built `libarrow_dataset` from the 25.0.1 tag with the same header change and swapped it into the 25.0.1 wheel. The Python reproducer from the issue (going through a `PyFileSystem` wrapper, which is what made it reproducible for me) went from 57/4500 truncated tables to 0 in about 11.6k runs. ### Are there any user-facing changes? A scan that hits a fragment error now always raises. Before, it could sometimes return a partial result. **This PR contains a "Critical Fix".** It fixes a bug where a dataset scan could return incorrect (truncated) results with no error. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [ ] Human - [x] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed 🤖 Generated with [Claude Code](https://claude.com/claude-code) * GitHub Issue: #51495 Lead-authored-by: Panchawat <k.pattara@outlook.com> Co-authored-by: Panchawat <whitepattpon@gmail.com> Signed-off-by: Rossi Sun <zanmato1984@gmail.com> |
||
|
Divjot Arora
|
464ae94d81 |
GH-50915: [FORMAT] Allow TIMESTAMP logical type to annotate FIXED_LEN_BYTE_ARRAY(12) (#50916)
### Rationale for this change See https://github.com/apache/parquet-format/issues/600 for rationale. ### What changes are included in this PR? This PR adds support for using `TimestampType` to annotate `FIXED_LEN_BYTE_ARRAY(12)` values. It also adds functionality to convert FLBA(12) values to Arrow INT64 timestamps with the following flags/logic to handle overflow: 1. The conversion is guarded behind the `convert_flba_timestamps` property (default true). If false, conversion fails regardless of value. 2. If the FLBA(12) value overflows max int64 or underflows min int64, the `flba_timestamp_clamp_on_overflow` property (default false) is consulted. If false, conversion fails. If true, the value is clamped to min/max int64. ### Are these changes tested? Yes, via unit tests and an e2e test that reads the file added in parquet-testing (https://github.com/apache/parquet-testing/pull/123). ### Are there any user-facing changes? No * GitHub Issue: #50915 Authored-by: Divjot Arora <div.arora@databricks.com> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
Jeremy Schoemaker
|
c6620b47de |
GH-51127: [C++] Fix is_null nan_is_null for dictionary-encoded float arrays (#51000)
### Rationale for this change `is_null(..., nan_is_null=true)` did not detect NaN values stored in a dictionary's values, so NaN entries in dictionary-encoded float/double/half-float arrays were reported as valid instead of null. Null dictionary values must also keep being reported as null with both default options and `nan_is_null=true`. ### What changes are included in this PR? Adds NaN and null detection to `is_null` for dictionary-encoded floating-point arrays. For dictionaries of floating types, the output bitmap is built by calling `is_null` on the dictionary values (which also catches NaN when `nan_is_null=true`), mapping that through the indices with `take`, and OR-ing it into the existing null-index bitmap. `take` runs without bounds checking, matching how other compute kernels treat dictionary indices as already valid. Every other `is_null` path is unchanged. ### Are these changes tested? Yes. A single parameterized test in `scalar_validity_test.cc` covers float16, float32, and float64 dictionaries: NaN values, null dictionary entries, null indices, default options, and empty arrays. ### Are there any user-facing changes? Yes. `is_null(..., nan_is_null=true)` now reports NaN entries in dictionary-encoded floating-point arrays as null. Null dictionary values were already reported as null and remain so. * GitHub Issue: #51127 Authored-by: Jeremy Schoemaker <jeremy@shoemoney.com> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
Alexander Taepper
|
65974f5316 |
GH-51470: [C++] Delimit JSON documents without parsing them (#51472)
### Rationale for this change Resolves #51470: `simdjson` has a multi-stage parser that first finds structural elements in the entire buffer (including document boundaries). Only the second stage, which is lazy, actually need to parse the documents. Similarily `parse_many` already computes all document boundaries and we can iterate its output to consume the documents without parsing any documents: https://github.com/simdjson/simdjson/blob/6913ee1a17a57d23a2e9f458d8e04e96cd43ecab/doc/parse_many.md?plain=1#L85-L88 ### What changes are included in this PR? This delimits JSON documents without fully parsing them ### Are these changes tested? Yes. before: ``` ChunkJSONPrettyPrinted 528656 ns 528396 ns 1340 bytes_per_second=394.823Mi/s json_size=218.757k ChunkJSONPrettyPrintedMultipleBlocks 664758 ns 625186 ns 1143 block_size=27.344k bytes_per_second=333.697Mi/s json_size=218.757k ``` after: ``` ChunkJSONPrettyPrinted 91815 ns 91811 ns 7358 bytes_per_second=2.21906Gi/s json_size=218.757k ChunkJSONPrettyPrintedMultipleBlocks 197358 ns 167654 ns 3719 block_size=27.344k bytes_per_second=1.2152Gi/s json_size=218.757k ``` ### Are there any user-facing changes? No. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [X] Human - [ ] (AI assisted in code drafting for the initial version) **Reviewed before submission by:** - [X] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #51470 Authored-by: Alexander Taepper <alexander.taepper@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
Stefan Wang
|
626407c899 |
GH-51224: [C++] Keep the correct nulls when winsorizing a sliced array (#51251)
### Rationale for this change Winsorizing a sliced array can put nulls in the wrong positions. Slices containing only nulls or NaNs can also return values from outside the slice. Closes https://github.com/apache/arrow/issues/51224. ### What changes are included in this PR? Use the bitmap helper to match validity bits to the output values. Preserve the input offset when returning the original buffers. ### Are these changes tested? I ran the native C++ tests and PyArrow on macOS arm64 with Apple clang 21. The new regression covers null-only, NaN-only and mixed null/NaN slices, with chunked inputs as controls. With PyArrow loading the native library being tested, save this as repro-winsorize.py: ```python import pyarrow as pa import pyarrow.compute as pc parent = pa.array([1.0, 2.0, None, 4.0, None, 6.0, 7.0, 8.0]) result = pc.winsorize(parent.slice(2, 5), lower_limit=0.0, upper_limit=1.0) print("mixed slice:", result.to_pylist(), flush=True) parent = pa.array([1.0, 2.0, None, None, None, 3.0]) nulls = pc.winsorize(parent.slice(2, 3), lower_limit=0.0, upper_limit=1.0) print("all-null slice:", nulls.to_pylist(), flush=True) assert result.to_pylist() == [None, 4.0, None, 6.0, 7.0] assert nulls.to_pylist() == [None, None, None] ``` <details><summary>Raw logs</summary> Before: ```console $ python repro-winsorize.py mixed slice: [0.0, 4.0, None, 6.0, None] all-null slice: [1.0, 2.0, None] AssertionError ``` After: ```console $ python repro-winsorize.py mixed slice: [None, 4.0, None, 6.0, 7.0] all-null slice: [None, None, None] exit_code=0 $ build/release/arrow-compute-vector-test [ PASSED ] 1157 tests. $ build/release/arrow-compute-aggregate-test --gtest_filter='*Quantile*:*TDigest*' [ PASSED ] 28 tests. ``` </details> ### Are there any user-facing changes? Sliced inputs retain the correct values and null positions. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [ ] Human - [x] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #51224 Authored-by: 1fanwang <1fannnw@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
Jonas Dedden
|
c75bc9aa9f |
GH-50988: [C++][Python] Substrait: add mappings for starts_with, ends_with and match_substring (#50989)
### Rationale for this change
`starts_with`, `ends_with` and `match_substring` have no Substrait mapping, so any expression using them fails to serialize:
```python
import pyarrow as pa
import pyarrow.compute as pc
from pyarrow.substrait import serialize_expressions
schema = pa.schema([pa.field("cat", pa.string())])
serialize_expressions([pc.starts_with(pc.field("cat"), "al")], ["f"], schema)
# ArrowNotImplementedError: No conversion function exists to convert the
# Arrow function starts_with to a Substrait call
```
Comparisons, `isin` and arithmetic serialize fine, so this is a per-function gap. It matters for engines that ingest a PyArrow filter through Substrait, where an unmappable function becomes a hard failure rather than a fallback.
See #50988.
### What changes are included in this PR?
Map the three kernels onto `starts_with`, `ends_with` and `contains` from Substrait's `functions_string.yaml`, in both directions.
The signatures do not line up. Substrait passes the pattern as a second argument, while the Arrow kernels are unary and carry it in `MatchSubstringOptions`. So:
* encoding lifts `MatchSubstringOptions::pattern` out into a literal argument
* decoding requires that argument to be a non-null string literal, and returns `NotImplemented` otherwise
Substrait's `case_sensitivity` option maps onto `ignore_case`. `CASE_INSENSITIVE_ASCII` has no Arrow equivalent and returns `NotImplemented`.
Both caveats are documented in `docs/source/cpp/acero/substrait.rst`.
### Are these changes tested?
Yes.
`Substrait.StringMatchExpressionSerialization` round-trips all three functions with and without `ignore_case`. `Substrait.StringMatchExpressionDeserialization` deserializes hand-written Substrait JSON to cover the default (no option) case and the two rejection paths. `test_serializing_string_match_expressions` round-trips each function through `pyarrow.substrait`.
### Are there any user-facing changes?
Yes. `pc.starts_with`, `pc.ends_with` and `pc.match_substring` can now be serialized to Substrait and consumed back, and Acero can consume plans using Substrait's `starts_with`, `ends_with` and `contains`. No existing behaviour changes: these previously raised.
* GitHub Issue: #50988
Authored-by: Jonas Dedden <mail@jonas-dedden.de>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
|
||
|
Jáchym Barvínek
|
43d04c2f6f |
GH-51496: [C++] Fix 32-bit block offset overflows in SwissTable (#51497)
### Rationale for this change `SwissTable::early_filter_imp_avx2_x8` computes each block's byte offset with `_mm256_mullo_epi32`, so modulo 2^32. With 32-bit group ids a block takes 40 bytes, and every block with id >= 107,374,183 (ceil(2^32 / 40)) gets a wrapped offset. The kernel then reads the status bytes from the wrong place, reports no match (or the wrong slot), and `find()` misses the key. There is no error. A hash join whose build side has more than ~403M distinct keys (`log_blocks` >= 27) loses about 20%, 60% and 80% of its matches at `log_blocks` 27, 28 and 29. The kernel is only dispatched on CPUs with AVX2 and efficient BMI2 (`HasEfficientBmi2()`, that is Intel), so the same join is correct on AMD, on ARM and with `ARROW_USER_SIMD_LEVEL=NONE`. See GH-XXXXX for the full analysis and a pyarrow reproducer. While checking the other block offset computations I found the same overflow in scalar code. When `grow_double()` reinserts an overflow entry and the entry's new home block is full, it probes the next blocks at `blocks_new->mutable_data() + block_id_new * block_size_after`, a 32-bit multiply. GH-45506 fixed the line just above but not this one. Once a single table grows to 2^27 blocks, such an entry is written to an unrelated location and can't be found. This happens on any CPU. A multi-threaded hash join builds small per-partition tables and merges them into a table that is sized up front, so the join numbers below come from the early filter bug alone. A single table that grows past ~403M keys can hit this one, for example in a group-by or in a join built as one partition. ### What changes are included in this PR? - `key_map_internal_avx2.cc`: compute `voffset_A`/`voffset_B` in 64 bits with `_mm256_mul_epu32` on the even and odd 32-bit block ids. This is the same even/odd split as before, now without wraparound, and matches what `extract_group_ids_avx2` does since GH-44513. - `key_map_internal.cc`: use `mutable_block_data()`, which computes the offset in 64 bits, for the probe in `grow_double()`. - New `key_map_test.cc` (added to `arrow-compute-row-test` in CMake and Meson) with two large-memory tests: - `SwissTable.EarlyFilterOver4GB`: a table with 2^27 blocks and no hash array (5.4 GB). It inserts keys into the blocks on both sides of the 4GB boundary and checks `early_filter` for every supported hardware flag set (scalar, and AVX2 where available). - `SwissTable.GrowOver4GB` (about 14.5 GB peak): it grows a table from 2^26 to 2^27 blocks with an entry that has to move past a full block beyond 4GB, then checks `early_filter` and `find` for all keys. It triggers the growth through `num_inserted()` and the 75% fill threshold, and asserts that the table grew. ### Are these changes tested? Yes. Both new tests are `LARGE_MEMORY_TEST`s, so they only run with `ARROW_LARGE_MEMORY_TESTS=ON`. The AVX2 part of `EarlyFilterOver4GB` only exercises the bug on Intel CPUs with AVX2 and BMI2. The results below are from Google Cloud `n1-highmem-32` (Intel Xeon @ 2.00GHz, AVX2, BMI2 and AVX-512), Ubuntu 24.04, gcc 13.3, with `ARROW_RUNTIME_SIMD_LEVEL=MAX`, in Release and in Debug with `BUILD_WARNING_LEVEL=CHECKIN`: - `main` with only the new tests added: - `EarlyFilterOver4GB` fails at `key_map_test.cc:78` with `local_slots[i]` 7, expected 0, at `block_id = 107374183`, `hardware_flags = 32` (AVX2). It passes with `ARROW_USER_SIMD_LEVEL=NONE`. - `GrowOver4GB` fails at `key_map_test.cc:151` (key 8 not found, `hardware_flags = 0`), with any SIMD level. - The early filter fix alone: `EarlyFilterOver4GB` passes, and `GrowOver4GB` still fails at `:151`. - This PR: both tests pass, also with `ARROW_USER_SIMD_LEVEL=NONE` (about 10 s and 16 s in Release). - `arrow-compute-row-test`: 90 tests pass (the 88 existing ones and the 2 new ones). - `arrow-acero-hash-join-node-test`: 36 pass and 1 is skipped (`BuildSideLargeRowIds`, which is skipped in its body), the same as on `main`. - No new compiler warnings in the Debug `-Werror` build. Before the fix, the pyarrow reproducer from the issue gave these results on the same machine type with pyarrow 25.0.1 (an inner join where all 2,000,000 probe keys have a match): | build rows | rows returned | with `ARROW_USER_SIMD_LEVEL=NONE` | |---:|---:|---:| | 390,000,000 | 2,000,000 (100.00%) | 2,000,000 | | 450,000,000 | 1,603,163 (80.16%) | 2,000,000 | | 900,000,000 | 814,637 (40.73%) | 2,000,000 | I did not rerun the pyarrow reproducer on a patched build. The C++ tests above cover the same code path. ### Are there any user-facing changes? No API changes. Hash joins with more than ~403M distinct build keys on Intel AVX2 CPUs, and single hash tables that grow past ~403M keys, now return correct results. **This PR contains a "Critical Fix".** Both bugs silently produce incorrect data. The AVX2 early filter makes hash joins with more than ~403M distinct build-side keys drop 20% to 80% of the matching rows on Intel CPUs with AVX2 and BMI2. The `grow_double()` overflow can misplace entries, and so lose keys, when a hash table grows to 2^27 blocks or more (past ~403M keys), on any CPU. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [ ] Human - [x] AI **Reviewed before submission by:** - [x] Human - [x] AI - [ ] Not reviewed Claude Code (Claude Opus) did the root-cause analysis and wrote the fix, the tests and this description, under my direction. I found the bug in a production pipeline. A separate AI pass reviewed the diff and rebuilt and reran the tests on an Intel machine. * GitHub Issue: #51496 Authored-by: Jachym.Barvinek <jachym.barvinek@alza.cz> Signed-off-by: Rossi Sun <zanmato1984@gmail.com> |
||
|
Edmond AbrahamandEnrico Minack
|
a99c14ed5e |
GH-51370: [C++][Parquet] Fix tracing column attributes (#51401)
### Rationale for this change Fixes #51370. The `parquet::arrow::read_column` span used a reader-vector position as a Parquet leaf-column index. This reported the wrong column attributes for reordered or subset reads and did not represent nested top-level fields correctly. ### What changes are included in this PR? * Map selected Parquet leaf indices back to their top-level Arrow field indices before recording tracing attributes. * Report the top-level field name and index. Preserve the physical type for primitive fields and leave it empty for nested fields, which do not have a single physical type. * Apply the same mapping to direct row-group column reads. * Add an OpenTelemetry-only regression test target covering flat, nested, reordered, full-schema, and direct reads. ### Are these changes tested? Yes. * `parquet-arrow-reader-writer-tracing-test`: 5 tests passed. * `parquet-arrow-reader-writer-test`: 823 tests passed and 9 configuration-dependent tests were skipped. * C++ clang-format and cpplint checks passed for the changed C++ files. ### Are there any user-facing changes? OpenTelemetry spans now report the correct top-level Arrow field metadata. There are no public API changes. ### Was AI used for this PR? In accordance with the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), AI was used to implement, test, rebase, and prepare this change. The proposed behavior was discussed on #51370 before submission. **PR code and description written by:** - [x] Human - [x] AI **Reviewed before submission by:** - [x] Human - [x] AI - [ ] Not reviewed * GitHub Issue: #51370 Lead-authored-by: Edmond Abraham <54119952+gitedmond@users.noreply.github.com> Co-authored-by: Enrico Minack <github@enrico.minack.dev> Signed-off-by: Adam Reeve <adreeve@gmail.com> |
||
|
Anurag Tryambak RautandGang Wu
|
51955ecd2c |
GH-51097: Fix Parquet null counts for fixed-width leaves (#51357)
### Rationale for this change Parquet null-count statistics can be incorrect for fixed-width leaf columns nested below a repeated ancestor, such as `list<struct<...>>`. When a list is null or empty, its descendant leaf does not produce a value in the leaf values buffer. `DefLevelsToBitmap` intentionally excludes these repeated-ancestor nulls from its `null_count`. `MaybeCalculateValidityBits` was using that value directly for the Parquet column statistics, causing the null count to be undercounted. For example, a `list<struct<string, int32>>` column can report an incorrect `null_count` for the `int32` leaf even though the encoded definition levels correctly represent the null and empty list entries. ### What changes are included in this PR? * Update `MaybeCalculateValidityBits` to calculate the total null count from `batch_size - out_values_to_write`. * This includes nulls represented by repeated ancestors, such as null or empty lists, while preserving the existing validity bitmap behavior. * Add a regression test covering a fixed-width `int32` leaf under `list<struct<...>>`. ### Are these changes tested? Yes. Added a regression test that verifies the `int32` leaf reports: * `null_count = 3` * `num_values = 2` The focused regression test passes: `StatisticsTest.FixedWidthLeafUnderListStructNullCount` The existing `parquet-writer-test` also passes. ### Are there any user-facing changes? Yes. This fixes incorrect Parquet column statistics for affected nested fixed-width columns. The change does not alter the encoded data or public APIs. ### This PR contains a "Critical Fix". This fixes a bug that produces incorrect Parquet statistics. The underlying data remains correct, but the reported `null_count` for affected fixed-width leaf columns can be incorrect. ### Was AI used for this PR? **PR code and description written by:** * [x] Human * [ ] AI **Reviewed before submission by:** * [x] Human * [ ] AI * [ ] Not reviewed * GitHub Issue: #51097 Lead-authored-by: AnuragRaut08 <anuragtraut2003@gmail.com> Co-authored-by: Gang Wu <ustcwg@gmail.com> Signed-off-by: Gang Wu <ustcwg@gmail.com> |
||
|
Raúl Cumplido
|
5448aa7a91 |
GH-51467: [CI][C++] Remove unused ranges include which breaks R build (#51469)
### Rationale for this change After: - https://github.com/apache/arrow/pull/49679 The nightly CI job for [test-r-macos-as-cran](https://github.com/ursacomputing/crossbow/actions/runs/35804144934/job/107001010915) failed due to ranges file not found: ``` [ 95%] Building CXX object src/arrow/CMakeFiles/arrow_compute_objlib.dir/compute/kernels/vector_search_sorted.cc.o /Users/runner/work/crossbow/crossbow/arrow/cpp/src/arrow/compute/kernels/vector_search_sorted.cc:23:10: fatal error: 'ranges' file not found 23 | #include <ranges> | ^~~~~~~~ 1 error generated. ``` ### What changes are included in this PR? Remove `#include <ranges>` as is never used on the file. ### Are these changes tested? Yes via CI ### Are there any user-facing changes? No ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [x] Human - [ ] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: #51467 Authored-by: Raúl Cumplido <raulcumplido@gmail.com> Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com> |
||
|
Hadrian Reppas
|
f4265ecb0f |
GH-49889: [C++][Compute] Handle logical nulls in validity kernels (#50269)
### Rationale for this change I was looking into #49889 and updated the handling of logical nulls in the `is_valid`, `is_null`, and `true_unless_null` kernels. It turns out there is some prior work here that I didn't see before I starting implementing my fix (#35058, #35036, #37642). For some reason, that work has stalled. ### What changes are included in this PR? This PR updates the `IsValidExec`, `IsNullExec`, and `TrueUnlessNullExec` kernels so that they call out to a new function `SetLogicalNullBits`. The `SetLogicalNullBits` function matches the logic in `ArraySpan::ComputeLogicalNullCount()`. The logic is similar enough that they may be some opportunities for code deduplication in `dict_util.cc` and `ree_util.cc`. ### Are these changes tested? Yes ### Are there any user-facing changes? No * GitHub Issue: #49889 Authored-by: Hadrian <hadrian@uchicago.edu> Signed-off-by: Antoine Pitrou <antoine@python.org> |
||
|
Abdul Azeem Makarim
|
0efbdec7a1 |
GH-51223: [C++] Fix copying sliced boolean arrays (#51240)
### Rationale for this change Boolean values are bit-packed. When copying values from a sliced boolean array, the copy path did not include the array's slice offset. This caused `fill_null_forward`, `fill_null_backward`, and `replace_with_mask` to read values from earlier positions in the parent array. ### What changes are included in this PR? - Apply `ArraySpan::offset` to the bit index when copying boolean values from an `ArraySpan`. - Add regression coverage for sliced boolean inputs in `replace_with_mask`, `fill_null_forward`, and `fill_null_backward`. ### Are these changes tested? Yes. I ran `arrow-compute-vector-test` locally. ### Are there any user-facing changes? Only a bugfix. ### This PR contains a "Critical Fix". This fixes a bug that caused compute operations on sliced boolean arrays with a non-zero offset to produce incorrect values. * GitHub Issue: #51223 Authored-by: Abdul Azeem Makarim <114302821+A-makarim@users.noreply.github.com> Signed-off-by: Antoine Pitrou <antoine@python.org> |