Repository navigation
GH-52161: [C++][Parquet] Add support for string/binary view to content defined chunking writer - #52162
Merged
Conversation
|
|
HuaHuaY
approved these changes
Oct 8, 2026
Signed-off-by: Robert Kruszewski <[email protected]>
raulcd
pushed a commit
that referenced
this pull request
Oct 29, 2026
…t 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 <[email protected]> Signed-off-by: Zehua Zou <[email protected]>
lriggs
added a commit
to dremio/arrow
that referenced
this pull request
Nov 12, 2026
* apacheGH-51751: [CI][Packaging] Fix formula path for homebrew-cpp job (apache#51756) ### Rationale for this change Fix apache#51751 homebrew-core's formulae got letter-folders in 2023 but our macro from commit 5ad5ddc kept copying to top-level `Formula/` hence homebrew-cpp job tested homebrew-core's formulae instead of both apache-arrow.rb and apache-arrow-glib.rb in `dev/tasks/homebrew-formulae/` ### What changes are included in this PR? Copy to `Formula/a/` and change to `formula_opt_prefix("llvm")` per audit check ### 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 - [x] AI - [ ] Not reviewed * GitHub Issue: apache#51751 Authored-by: Tadeja Kadunc <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]> * apacheGH-51817: [Dev][Release] Add support for RC curl in `download_rc_binaries.py` (apache#51818) ### Rationale for this change `curl --version` uses the `${MAJOR}.${MINOR}.${PATCH}-rc${RC}` format for RC curl and Debian GNU/Linux sid uses RC curl. ### What changes are included in this PR? Accept the `-rc${RC}` part and ignore it. ### 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: apache#51817 Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Raúl Cumplido <[email protected]> * apacheGH-54489: [CI][Integration] Install jpype1 from conda-forge in conda-integration image (apache#54490) ### Rationale for this change The `conda-integration` image doesn't pin Python, so it now gets Python 3.15. `ci/scripts/integration_arrow.sh` installs jpype1 with pip, and PyPI has no cp315 wheel for jpype1 yet. pip falls back to building from source, which fails because the image doesn't include Apache Ant. See apache#54489 for details. ### What changes are included in this PR? Install `jpype1` from conda-forge in `conda-integration.dockerfile`, as `conda-python-jpype.dockerfile` already does. conda-forge has builds for Python 3.15 on both linux-64 and linux-aarch64. `integration_arrow.sh` is unchanged: its `pip install jpype1` finds the conda package already installed and does nothing. It still installs jpype1 when the script runs outside this image. ### Are these changes tested? Yes. The Integration job in apache/arrow-dotnet builds this image from scratch and fails with the error above. I ran that job on a fork with the Arrow checkout pointed at this branch, and it got past the jpype1 installation that used to fail. When I opened this PR the run was still in progress: https://github.com/CurtHagenlocher/arrow-dotnet/actions/runs/39150917107 ### 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:** - [ ] Human - [x] AI **Reviewed before submission by:** - [ ] Human - [ ] AI - [ ] Not reviewed 🤖 Generated with [Claude Code](https://claude.com/claude-code) * GitHub Issue: apache#54489 Authored-by: Curt Hagenlocher <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]> * apacheGH-54595: [Packaging][CI] Fix regex on update_deb_package_names to cover the 3 on libarrow_s3 (apache#54608) ### Rationale for this change Linux packaging jobs for Deb packages are currently failing due to wrong libarrow_s32600 still being referred to. ### What changes are included in this PR? Update regex to cover numbers on libarrowxxx package names. Update the wrong entries at control.in. ### Are these changes tested? I have tested the regex locally with: ``` $ sed -i.bak -E -e "s/(lib(arrow|gandiva|parquet)[-a-z0-9]*)2700/\\12800/g" dev/tasks/linux-packages/apache-arrow/debian/control.in ``` And validated the diff contains libarrow s3 updated: ```diff @@ -122,13 +122,13 @@ Pre-Depends: ${misc:Pre-Depends} Depends: ${misc:Depends}, ${shlibs:Depends}, - libarrow-acero2700 (= ${binary:Version}), - libparquet2700 (= ${binary:Version}) + libarrow-acero2800 (= ${binary:Version}), + libparquet2800 (= ${binary:Version}) Description: Apache Arrow is a data processing library for analysis . This package provides C++ library files for Dataset module. -Package: libarrow-s3-2700 +Package: libarrow-s3-2800 Section: libs Architecture: any Multi-Arch: same ``` ### 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: apache#54595 Authored-by: Raúl Cumplido <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]> * apacheGH-55113: [CI][R] Install tzdata-legacy if available so tests are able to find required timezones (apache#55114) ### Rationale for this change GH runner upgrade from Ubuntu 24.04 to 26.04 made some R tests fail due to missing timezone. (Rhub) ### What changes are included in this PR? Install `tzdata-legacy` package if available to have the required timezone. ### 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 **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: apache#55113 Authored-by: Raúl Cumplido <[email protected]> Signed-off-by: Raúl Cumplido <[email protected]> * apacheGH-54928: [Packaging][CI] Change TMPDIR for reproducible builds to avoid out of space (apache#54943) ### TBD ### Rationale for this change Several Linux Packaging jobs are currently failing with out of space on the verify reproducibility step. This is due to `/tmp` size. Ubuntu runner image changed from Ubuntu 24.04 to Ubuntu 26.04 where the `/tmp` folder is mounted. In the past the `/tmp` folder was on the root disk which had 145 GB of disk space. ``` Filesystem Size Used Avail Use% Mounted on /dev/root 145G 60G 85G 42% / tmpfs 7.9G 84K 7.9G 1% /dev/shm tmpfs 3.2G 988K 3.2G 1% /run tmpfs 5.0M 0 5.0M 0% /run/lock efivarfs 128M 32K 128M 1% /sys/firmware/efi/efivars /dev/sda16 881M 85M 735M 11% /boot /dev/sda15 105M 6.2M 99M 6% /boot/efi tmpfs 1.6G 12K 1.6G 1% /run/user/1001 ``` With the new runner the `/tmp` folder is on `tmpfs` with 7.8GB. ``` 2026-10-23T02:23:08.8104328Z Filesystem Size Used Avail Use% Mounted on 2026-10-23T02:23:08.8104761Z /dev/root 145G 58G 88G 40% / 2026-10-23T02:23:08.8105145Z tmpfs 7.9G 0 7.9G 0% /dev/shm 2026-10-23T02:23:08.8105751Z tmpfs 3.2G 1000K 3.2G 1% /run 2026-10-23T02:23:08.8106428Z efivarfs 128M 32K 128M 1% /sys/firmware/efi/efivars 2026-10-23T02:23:08.8107172Z tmpfs 7.9G 38M 7.8G 1% /tmp 2026-10-23T02:23:08.8108065Z none 1.0M 0 1.0M 0% /run/credentials/systemd-journald.service 2026-10-23T02:23:08.8109058Z none 1.0M 0 1.0M 0% /run/credentials/systemd-resolved.service 2026-10-23T02:23:08.8109866Z /dev/sda13 989M 81M 842M 9% /boot 2026-10-23T02:23:08.8110512Z /dev/sda15 105M 6.3M 99M 7% /boot/efi 2026-10-23T02:23:08.8111184Z none 1.0M 0 1.0M 0% /run/credentials/systemd-networkd.service 2026-10-23T02:23:08.8112063Z tmpfs 1.6G 8.0K 1.6G 1% /run/user/1001 2026-10-23T02:23:08.8112526Z none 1.0M 0 1.0M 0% /run/credentials/getty@ tty1.service 2026-10-23T02:23:08.8113053Z none 1.0M 0 1.0M 0% /run/credentials/serial-getty@ ttyS0.service ``` ### What changes are included in this PR? Use `runner.temp` (`RUNNER_TEMP`) to get a temporary directory on the runner instead of using `/tmp`. See https://docs.github.com/en/actions/reference/workflows-and-actions/contexts and https://docs.github.com/en/actions/reference/workflows-and-actions/variables ### 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 - [x] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: apache#54928 Lead-authored-by: Raúl Cumplido <[email protected]> Co-authored-by: Raúl Cumplido <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]> * apacheGH-55112: [Packaging][Debian] Add missing opentelemetry-cpp dependency (apache#55249) ### Rationale for this change `debuild` doesn't add OpenTelemetry dependencies to `libarrow.so.XXX` automatically because OpenTelemetry package on Debian doesn't have SO version. ### What changes are included in this PR? Add `opentelemetry-cpp` dependency explicitly. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. ### 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: apache#55112 Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Raúl Cumplido <[email protected]> * apacheGH-55267: [C++][R] Arrow 26.0.0 fails to build on CRAN macOS builders: bundled simdjson C++20 concepts don't compile with the macOS 11 SDK (apache#55273) ### Rationale for this change CRAN failures on 26.0.0 release due to regex not catching incompatible MacOSX11.sdk version ### What changes are included in this PR? Make regex more permissive ### Are these changes tested? No ### Are there any user-facing changes? No **PR code and description written by:** - [x] Human - [x] AI **Reviewed before submission by:** - [x] Human - [x] AI - [ ] Not reviewed * GitHub Issue: apache#55267 Authored-by: Nic Crane <[email protected]> Signed-off-by: Nic Crane <[email protected]> * apacheGH-55165: [C++][Packaging][R] Use Brotli 1.2.1 for bundled, vcpkg and R Windows builds (apache#55167) There is a security problem in bundled Brotli (1.0.9), pinned vcpkg Brotli (1.2.0) and rtools' Brotli (1.0.9). We should update them. * Bump the bundled Brotli to 1.2.1 from 1.0.9. * Bump vcpkg to 2026.10.24 (`61d7eaeb09`) from 2026.05.27 (`9b965a1168`). * Use the bundled Brotli instead of the rtools one for R Windows binaries. Yes. Yes. **PR code and description written by:** - [x] Human - [x] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed * GitHub Issue: apache#55165 Lead-authored-by: Raúl Cumplido <[email protected]> Co-authored-by: Curt Hagenlocher <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]> * apacheGH-51690: [C++] Don't define __SSE2__ globally with MSVC (apache#53952) ### Rationale for this change `simdjson >= 5` uses GCC builtins such as `__builtin_ctz` when `__SSE2__` is defined. So we can't build Apache Arrow C++ with `simdjson >= 5` and MSVC because we define `__SSE2__` globally. ### What changes are included in this PR? We don't need to define `__SSE2__` because xsimd detects SSE2 by `_M_X64` with MSVC. We still need to define `__SSE4_1__`/`__SSE4_2__` for xsimd because MSVC doesn't define them even with `/arch:SSE4.2`. But we define them only where SSE4.2 is enabled. So they are no longer defined with `ARROW_SIMD_LEVEL=NONE`. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. ### 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 - [ ] AI - [ ] Not reviewed * GitHub Issue: apache#51690 Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]> * apacheGH-54595: [Packaging] Revert the unnecessary commit for apache#54595 * apacheGH-51354: [C++][Parquet] Update bundled Apache Thrift to 0.24.0 (apache#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: apache#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 <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]> * apacheGH-51684: [C++][Parquet] Keep the CDC chunking state across row groups (apache#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 apache#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 apache#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: apache#51684 * apacheGH-51018: [C++] Fix indices_nonzero segfault on chunkless ChunkedArray (apache#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: apache#51018 Authored-by: waterWang <[email protected]> Signed-off-by: Rok Mihevc <[email protected]> * apacheGH-53518: [Parquet] Reject unsupported compression codecs (apache#53522) ### Rationale for this change See apache#53518 for rationale. Treating unknown compression codecs as `UNCOMPRESSED` is wrong and at best causes the read to fail late during decoding and at worst yields incorrect results if the compressed stream is actually a valid decodable set of bytes (very unlikely). ### What changes are included in this PR? This PR causes unsupported compression codecs to fail a read eagerly during Thrift parsing. ### Are these changes tested? Yes, new test case added to `metadata_test.cc` ### 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 - [ ] AI - [ ] Not reviewed Closes apache#53518 * GitHub Issue: apache#53518 Authored-by: Divjot Arora <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]> * apacheGH-50544: [C++][Gandiva] Fix out-of-bounds read in round with most negative precision (apache#50545) ### Rationale for this change `round_int32_int32` and `round_int64_int32` negate the caller-supplied precision into `abs_precision` before range-checking it, so the check `abs_precision > 9` (and `> 18` for the int64 overload) never fires for `precision == INT32_MIN`, where the negation leaves the value negative. That value reaches `get_power_of_10`, which indexes a 19-element static table behind `DCHECK_GE`/`DCHECK_LE` only. The DCHECKs compile out in release builds, so `round(int_col, -2147483648)` reads far outside the table and either faults or returns arbitrary memory as the rounding multiplier. Both overloads are registered in `function_registry_arithmetic.cc`, so the precision comes straight from user SQL. `truncate_int64_int32` in the same file already compares the signed scale directly (`out_scale < -38`) and is unaffected; the two `round` overloads are the only sites that negate first. ### What changes are included in this PR? Check the bound against the signed `precision` before negating, at both sites. Every precision that was in range before still takes the same path, and the negation now only runs on values known to be within 9 (or 18) of zero. ### Are these changes tested? Yes. Added two cases to `TestExtendedMathOps.TestRound` covering `std::numeric_limits<int32_t>::min()` for both overloads, matching the existing `truncate_int64_int32` case in the same file. On the unpatched tree the debug build aborts with `extended_math_ops.cc:372: Check failed: (exp) >= (0)`; with the fix the full `gandiva-precompiled-test` suite passes (134 tests). ### Are there any user-facing changes? No. **This PR contains a "Critical Fix".** It fixes an out-of-bounds read reachable from a user-supplied `round` precision argument. * GitHub Issue: apache#50544 Authored-by: abdul rawoof <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]> * apacheGH-51669: [C++][Parquet] Preserve cipher in plaintext footer metadata (apache#54884) ### Rationale for this change Closes apache#51669. CTR files with plaintext footers incorrectly declare GCM, causing encrypted-column reads to fail. ### What changes are included in this PR? * Serialize the configured file encryption algorithm directly. * Select AES-GCM explicitly for plaintext footer signing and verification. * Add generated GCM/CTR, footer and AAD cases, including an on-disk GCM tag check independent of reader selection. Original report by [dotnwat](https://github.com/dotnwat) in apache#51669. The follow-up incorporates [pitrou's review](apache#54884 (comment)) and [adamreeve's clarification](apache#54884 (comment)). ### Are these changes tested? * Encryption: 45 passed; metadata: 50 passed; writer: 296 passed, with 2 existing disabled tests. * Released parquet-java 1.18.1 interoperability: 64 read outcomes and 8 raw metadata inspections matched their controls. * clang-format 18 and `git diff --check` pass. ### Are there any user-facing changes? New CTR/plaintext-footer files declare the correct algorithm. No public API changes. **This PR contains a "Critical Fix"**: it corrects invalid encryption algorithm metadata emitted by the writer. ### Was AI used for this PR? AI was used to generate the implementation, regression tests, and PR description. **PR code and description written by:** * [ ] Human * [x] AI **Reviewed before submission by:** * [x] Human * [x] AI * [ ] Not reviewed * GitHub Issue: apache#51669 Authored-by: Yanzhi Xin <[email protected]> Signed-off-by: Adam Reeve <[email protected]> * apacheGH-52161: [C++][Parquet] Add support for string/binary view to content defined chunking writer (apache#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: apache#52161 Authored-by: Robert Kruszewski <[email protected]> Signed-off-by: Zehua Zou <[email protected]> * apacheGH-51684: [C++][Parquet][FOLLOWUP] Speed up content-defined chunking with a local gear hash (apache#51692) ### Rationale for this change apache#51685 keeps one content-defined chunker per column for the whole file, so the file writer now creates the chunkers together. The chunker's hash loop stores its rolling hash state to memory for every hashed byte, because the values it reads through byte pointers may alias that state. That slows down the chunking, and with the chunkers next to each other, the threads writing neighbouring columns also contend for their cache lines. This PR is stacked on apache#51685, only the last commit is new. ### What changes are included in this PR? - The rolling hash and its chunking state move into a `GearHash` value. `GetChunks()` rolls a local copy of it and stores it back once, so the compiler keeps the state in registers. - `BM_WriteContentDefinedChunking` benchmarks writing 16 int32 columns of 1M rows with content-defined chunking, with and without threads. ### Are these changes tested? The existing CDC tests cover the chunking, the chunk boundaries don't change. Medians of `BM_WriteContentDefinedChunking` on an Apple M4 Max: | | main | apache#51685 | this PR | |---|---|---|---| | single-threaded | 49.6 ms | 49.6 ms | 36.9 ms | | multi-threaded | 10.5 ms | 15.4 ms | 9.1 ms | ### Are there any user-facing changes? No, writing with content-defined chunking gets faster. ### 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 * GitHub Issue: apache#51684 * apacheGH-54936: [C++] Ensure reading an invalid map array from IPC doesn't crash (apache#54939) ### Rationale for this change When loading a MapArray from IPC, we call [`MapArray::ValidateChildData`](https://github.com/apache/arrow/blob/b9479757dd5c1914c2b007f11f6679c4fc080c05/cpp/src/arrow/ipc/reader.cc#L456-L459) to avoid a later crash in `MapArray::SetData`. Unfortunately `MapArray::ValidateChildData` calls `GetNullCount()` which is unsafe if the advertised length is too large for the actual validity buffer size. Found by OSS-Fuzz in https://issues.oss-fuzz.com/issues/556994537 ### What changes are included in this PR? Check that the `null_count` of the relevant fields is equal to zero, so that `GetNullCount` does not actually try to read buffer memory. ### Are these changes tested? Yes, by additional fuzz regression test. ### Are there any user-facing changes? Just a bugfix. **This PR contains a "Critical Fix".** An invalid IPC file would lead to reading memory out of bounds for the purpose of counting validity bits. It is not obvious how this could be exploited by an attacker except for causing a denial of service. ### 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: apache#54936 Authored-by: Antoine Pitrou <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]> * apacheGH-55316: [CI][FlightRPC][ODBC] Apply ports.patch in Windows build (apache#55317) ### Rationale for this change The ODBC Windows build uses a composite action that bootstraps vcpkg in a way that doesn't pick up ports.patch (or the pinned vcpkg version). This set up a scenario where apache#55167 broke the build and the ODBC upload step failed during the 26.0.0.1 release process. As an aside, this didn't get caught by CI because the ODBC Windows job only runs on a daily schedule or at release time and apache#55167 hadn't been on `main` for long enough. This is something to keep an eye out and possibly improve in the future. ### What changes are included in this PR? Replace the way the job bootstraps vcpkg so it uses the normal `ci/scripts/install.sh` and directly overlays the port patches. ### Are these changes tested? Yes, in 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 - [X] AI - [ ] Not reviewed * GitHub Issue: apache#55316 Authored-by: Bryce Mecum <[email protected]> Signed-off-by: Raúl Cumplido <[email protected]> * MINOR: [Release] Update CHANGELOG.md for 26.0.1 * MINOR: [Release] Update .deb/.rpm changelogs for 26.0.1 * MINOR: [Release] Update versions for 26.0.1 --------- Signed-off-by: Sutou Kouhei <[email protected]> Signed-off-by: Raúl Cumplido <[email protected]> Signed-off-by: Raúl Cumplido <[email protected]> Signed-off-by: Nic Crane <[email protected]> Signed-off-by: Rok Mihevc <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]> Signed-off-by: Adam Reeve <[email protected]> Signed-off-by: Zehua Zou <[email protected]> Co-authored-by: tadeja <[email protected]> Co-authored-by: Sutou Kouhei <[email protected]> Co-authored-by: Curt Hagenlocher <[email protected]> Co-authored-by: Raúl Cumplido <[email protected]> Co-authored-by: Raúl Cumplido <[email protected]> Co-authored-by: Nic Crane <[email protected]> Co-authored-by: Hanayoshi-8744 <[email protected]> Co-authored-by: Krisztián Szűcs <[email protected]> Co-authored-by: water <[email protected]> Co-authored-by: Divjot Arora <[email protected]> Co-authored-by: Abdul Rawoof Khan <[email protected]> Co-authored-by: autoCAT <[email protected]> Co-authored-by: Robert Kruszewski <[email protected]> Co-authored-by: Antoine Pitrou <[email protected]> Co-authored-by: Bryce Mecum <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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, please disclose below whether and how AI was used in this PR.
PR code and description written by:
Reviewed before submission by: