Skip to content

GH-53518: [Parquet] Reject unsupported compression codecs - #53522

Merged
pitrou merged 1 commit into
apache:mainfrom
divjotarora:fix-codecs
Oct 13, 2026
Merged

pitrou merged 1 commit into
apache:mainfrom
divjotarora:fix-codecs

Conversation

@divjotarora

@divjotarora divjotarora commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Rationale for this change

See #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:

  • Human
  • AI

Reviewed before submission by:

  • Human
  • AI
  • Not reviewed

Closes #53518

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #53518 has been automatically assigned in GitHub to PR creator.

@pitrou pitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, thank you @divjotarora

@pitrou
pitrou merged commit 7ee1528 into apache:main Oct 13, 2026
55 of 56 checks passed
@divjotarora
divjotarora deleted the fix-codecs branch October 13, 2026 12:54
raulcd pushed a commit that referenced this pull request Oct 29, 2026
### Rationale for this change
See #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 #53518 
* GitHub Issue: #53518

Authored-by: Divjot Arora <[email protected]>
Signed-off-by: Antoine Pitrou <[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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C++][Parquet] Parquet treats unknown compression codec as uncompressed

2 participants