Replace Apache Thrift by cudf APIs for Parquet footer reading/writing by ttnghia · Pull Request #4851 · NVIDIA/cudf-spark-jni

ttnghia · 2026-07-20T20:35:19Z

Rewrite NativeParquetJni to use cudf public APIs for Parquet footer reading/writing instead of using Apache Thrift. This effectively drops libthrift and libparquet build dependency completely.

Depends on:

This PR and its dependencies have been tested and validated against cudf-spark plugin integration tests (Spark 3.5.5):

┌────────────────────────────────────┬────────────────────────┐
│                Test                │         Result         │
├────────────────────────────────────┼────────────────────────┤
│ parquet_test.py                    │ ✅ 3240 / 0 / 479-skip │
├────────────────────────────────────┼────────────────────────┤
│ parquet_testing_test.py            │ ✅ 88 / 0 / 8-skip     │
├────────────────────────────────────┼────────────────────────┤
│ parquet_test.py — field_id subset  │ ✅ 17 / 0 / 1-skip     │
├────────────────────────────────────┼────────────────────────┤
│ delta_lake_test.py — native_footer │ ✅ 24 / 24 PASSED      │
└────────────────────────────────────┴────────────────────────┘

cudf-spark-jni linked `libparquet` and `libthrift` solely so `NativeParquetJni` could deserialize, prune, and re-serialize a Parquet footer via the Arrow `parquet::format` types and the Thrift `TCompactProtocol`; the bundled Thrift is a standing build fragility through its archive-mirror source fetch. Rewrite `NativeParquetJni` on the new public cudf facade `cudf::io::read_parquet_footer_bytes` and `write_parquet_footer_bytes` (a host-only Thrift-compact codec, no libthrift), calling the reader in `throw_if_type_mismatch::NO` mode so a field whose wire type cudf does not model is skipped rather than rejected, matching the forward compatibility of the Arrow Thrift reader it replaces. Port the `__isset` presence checks to `std::optional` and the `meta_data`-absent rows to a `data_page_offset == 0` heuristic; drop the `PARQUET_LIB` and `THRIFT_LIB` `find_library` and link entries from the JNI `CMakeLists.txt` while keeping `ARROW_LIB` for the whole-archived Arrow-IPC symbols. The cudfjni build needs no Arrow-Parquet flag: the facade commit makes cudf-java default `CUDF_ENABLE_ARROW_PARQUET` OFF, so `libparquet` and the bundled Apache Thrift fall away on their own. Expand `ParquetFooterTest` with coverage for the rewritten footer path: nested-schema column pruning, the `parquet-mr` re-serialization round-trip, the `meta_data`-absent (PARQUET-2078) fallback, the `std::optional`-ported fields (absent `converted_type`, `file_offset`, `total_compressed_size`, and `column_orders`), and the trailing-length frame the spark-rapids caller appends after the footer. --------- Signed-off-by: Nghia Truong <nghiat@nvidia.com>

Copilot

Pull request overview

This PR rewrites the Parquet footer JNI path to use cuDF’s public Parquet metadata read/write APIs instead of Apache Thrift/parquet-mr C++ plumbing, removing the libthrift/libparquet static-link dependencies from the native build while preserving footer filtering and column-pruning behavior.

Changes:

Switch NativeParquetJni footer parsing/serialization to cudf::io::{read,write}_parquet_footer_bytes and update pruning logic to cuDF’s Parquet metadata structs.
Remove libparquet/libthrift discovery, include paths, and linkage from the native CMake build.
Expand ParquetFooterTest coverage to exercise nested-schema pruning, PARQUET-2078 metadata absence handling, column_orders reindexing, corrupt footer exceptions, and tolerance to trailing footer-length bytes.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/test/java/com/nvidia/spark/rapids/jni/ParquetFooterTest.java	Adds targeted regression tests for nested pruning, footer round-trip via parquet-mr, and defensive parsing behaviors needed by the new cuDF-based implementation.
src/main/cpp/src/NativeParquetJni.cpp	Replaces Thrift-based footer decode/encode with cuDF APIs and adapts row-group filtering + column pruning to cuDF metadata types.
src/main/cpp/CMakeLists.txt	Drops parquet/thrift static library dependencies and related include paths to match the new implementation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

 #include <cwctype>
 #include <limits>
-#include <sstream>
 #include <stdexcept>


  JNI_TRY
  {
-    auto meta    = std::make_unique<parquet::format::FileMetaData>();
    uint32_t len = static_cast<uint32_t>(buffer_length);


greptile-apps · 2026-07-20T20:49:07Z

Greptile Summary

This PR rewrites NativeParquetJni to use cudf public APIs (read_parquet_footer_bytes / write_parquet_footer_bytes) instead of Apache Thrift directly, eliminating the libthrift and libparquet build dependencies from CMakeLists.txt. Each __isset guard from the old Thrift struct is replaced with an equivalent value-based check (!= REPEATED, != UNDEFINED, .has_value(), != 0) and accompanied by an inline comment explaining the semantic equivalence — addressing all points raised in the previous review round.

Build simplification: find_library calls and include-path entries for Parquet/Thrift are fully removed; the link-libraries list shrinks accordingly.
Semantic equivalents: SchemaElement.type != pq::Type::UNDEFINED replaces __isset.type, std::optional::has_value() replaces __isset for converted_type/column_orders/total_compressed_size/file_offset, and != 0 sentinel checks replace __isset for offset fields — all with explanatory comments.
New tests: Ten new ParquetFooterTest cases cover nested-schema pruning, round-trip serialisation through parquet-mr, absent column metadata (PARQUET-2078), absent file_offset fallback, total_compressed_size absent summation, column_orders pruning, corrupt-footer exception safety, and trailing-bytes tolerance.

Confidence Score: 5/5

Safe to merge — the rewrite is a clean mechanical substitution of Thrift internals with cudf public APIs; integration tests passed against Spark 3.5.5 and the new unit-test suite covers all the edge-case paths introduced by the migration.

Every __isset guard has a clear value-based equivalent with an explanatory comment, the build change is purely subtractive, and the ten new tests exercise the specific paths (absent metadata, fallback offsets, schema pruning) that are most sensitive to the API swap. No logic regressions were found.

Files Needing Attention: No files require special attention. The one suggestion (widening buf_size to std::size_t) is cosmetic hardening on a path that cannot realistically be triggered.

Important Files Changed

Filename	Overview
src/main/cpp/CMakeLists.txt	Removes libparquet and libthrift from find_library, include paths, and link targets — straightforward dependency cleanup with no functional logic change.
src/main/cpp/src/NativeParquetJni.cpp	Replaces all Thrift deserialization and serialization with cudf public APIs; equivalent-semantics comments document each __isset-to-value-check substitution; adds negative buffer_length guard and a reserve() in filter_columns.
src/test/java/com/nvidia/spark/rapids/jni/ParquetFooterTest.java	Adds 10 new tests covering nested schema pruning, round-trip serialization, corrupt-footer exception safety, absent metadata (PARQUET-2078), absent file_offset fallback, total_compressed_size fallback, column_orders pruning, and trailing-bytes tolerance.

Sequence Diagram

sequenceDiagram
    participant Java as Java ParquetFooter
    participant JNI as NativeParquetJni C++
    participant cudf as cudf::io

    Java->>JNI: readAndFilter(buffer, partOffset, partLength, schema)
    JNI->>cudf: read_parquet_footer_bytes(span, throw_if_type_mismatch::NO)
    cudf-->>JNI: FileMetaData
    JNI->>JNI: column_pruner::filter_schema builds schema_map and chunk_map
    JNI->>JNI: filter_groups applies midpoint test per row group
    Note right of JNI: Absent meta_data falls back to RowGroup.file_offset
    JNI->>JNI: filter_columns reindexes column chunks
    JNI-->>Java: handle parquet_footer_with_row_group_offsets

    Java->>JNI: serializeThriftFile(handle)
    JNI->>cudf: write_parquet_footer_bytes(FileMetaData)
    cudf-->>JNI: vector of uint8_t
    JNI->>JNI: Wrap with PAR1 magic + LE footer length + PAR1
    JNI-->>Java: HostMemoryBuffer

_{Reviews (2): Last reviewed commit: "[Fix] Guard footer buffer length and doc..." | Re-trigger Greptile}

igorpeshansky · 2026-07-21T04:56:04Z

+    List<SchemaElement> schema = new ArrayList<>();
+    schema.add(root);
+    schema.add(leaf("id", FieldRepetitionType.OPTIONAL));
+    schema.add(group("names", 1, FieldRepetitionType.OPTIONAL, ConvertedType.LIST));
+    schema.add(group("list", 1, FieldRepetitionType.REPEATED, null));
+    schema.add(leaf("element", FieldRepetitionType.OPTIONAL));
+    schema.add(group("props", 1, FieldRepetitionType.OPTIONAL, ConvertedType.MAP));
+    schema.add(group("key_value", 2, FieldRepetitionType.REPEATED, null));
+    schema.add(leaf("key", FieldRepetitionType.REQUIRED));
+    schema.add(leaf("value", FieldRepetitionType.OPTIONAL));
+    schema.add(group("nested", 2, FieldRepetitionType.OPTIONAL, null));
+    schema.add(leaf("x", FieldRepetitionType.OPTIONAL));
+    schema.add(leaf("y", FieldRepetitionType.OPTIONAL));


Suggested change

List<SchemaElement> schema = new ArrayList<>();

schema.add(root);

schema.add(leaf("id", FieldRepetitionType.OPTIONAL));

schema.add(group("names", 1, FieldRepetitionType.OPTIONAL, ConvertedType.LIST));

schema.add(group("list", 1, FieldRepetitionType.REPEATED, null));

schema.add(leaf("element", FieldRepetitionType.OPTIONAL));

schema.add(group("props", 1, FieldRepetitionType.OPTIONAL, ConvertedType.MAP));

schema.add(group("key_value", 2, FieldRepetitionType.REPEATED, null));

schema.add(leaf("key", FieldRepetitionType.REQUIRED));

schema.add(leaf("value", FieldRepetitionType.OPTIONAL));

schema.add(group("nested", 2, FieldRepetitionType.OPTIONAL, null));

schema.add(leaf("x", FieldRepetitionType.OPTIONAL));

schema.add(leaf("y", FieldRepetitionType.OPTIONAL));

List<SchemaElement> schema = Arrays.asList(

root,

leaf("id", FieldRepetitionType.OPTIONAL),

group("names", 1, FieldRepetitionType.OPTIONAL, ConvertedType.LIST),

group("list", 1, FieldRepetitionType.REPEATED, null),

leaf("element", FieldRepetitionType.OPTIONAL),

group("props", 1, FieldRepetitionType.OPTIONAL, ConvertedType.MAP),

group("key_value", 2, FieldRepetitionType.REPEATED, null),

leaf("key", FieldRepetitionType.REQUIRED),

leaf("value", FieldRepetitionType.OPTIONAL),

group("nested", 2, FieldRepetitionType.OPTIONAL, null),

leaf("x", FieldRepetitionType.OPTIONAL),

leaf("y", FieldRepetitionType.OPTIONAL));

igorpeshansky · 2026-07-21T05:16:34Z

+    // Parse leniently (throw_if_type_mismatch::NO): skip a known field whose wire type does not
+    // match cudf's schema (Thrift forward-compat) rather than rejecting real files that use it.


Is this tested? Can it be?

Yes it can be tested, but in cudf PR instead.

igorpeshansky · 2026-07-21T05:17:12Z

-    rapids::jni::deserialize_parquet_footer(reinterpret_cast<uint8_t*>(buffer), len, meta.get());
+    // Parse leniently (throw_if_type_mismatch::NO): skip a known field whose wire type does not
+    // match cudf's schema (Thrift forward-compat) rather than rejecting real files that use it.
+    auto meta = std::make_unique<rapids::jni::pq::FileMetaData>(cudf::io::read_parquet_footer_bytes(


I assume this depends on rapidsai/cudf#23345 and rapidsai/cudf#23346 — don't forget to pick those up into the submodule once they're merged.

Sure, this PR explicitly depends on them.

Reject a negative `buffer_length` and stop truncating it to `uint32_t`: cast the `jlong` to `std::size_t` (lossless for the `host_span` the footer facade consumes) and throw `std::invalid_argument` on a negative length, caught by the surrounding `JNI_TRY`/`JNI_CATCH` and surfaced to Java. Document the zero-default assumptions the Thrift-to-cudf port relies on: an absent `repetition_type` defaults to `REQUIRED` (so the bare `!= REPEATED` check matches the old `__isset` guard), an absent `total_compressed_size` reads back as 0 via `value_or(0)` (matching the legacy Thrift zero-init), and `dictionary_page_offset == 0` means unset because offset 0 holds the `PAR1` magic. Include `<cstring>` directly for `std::memcpy`, and accumulate `num_to_skip` with `+=`. --------- Signed-off-by: Nghia Truong <nghiat@nvidia.com>

igorpeshansky

LGTM modulo a minor test cleanup.

ttnghia self-assigned this Jul 20, 2026

ttnghia added build dependencies Pull requests that update a dependency file tech debt labels Jul 20, 2026

ttnghia marked this pull request as ready for review July 20, 2026 20:42

Copilot AI review requested due to automatic review settings July 20, 2026 20:42

Copilot started reviewing on behalf of ttnghia July 20, 2026 20:43 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

greptile-apps Bot reviewed Jul 20, 2026

View reviewed changes

Comment thread src/main/cpp/src/NativeParquetJni.cpp

Comment thread src/main/cpp/src/NativeParquetJni.cpp

Comment thread src/main/cpp/src/NativeParquetJni.cpp

igorpeshansky reviewed Jul 21, 2026

View reviewed changes

ttnghia mentioned this pull request Jul 22, 2026

Add public Parquet footer reader/writer APIs rapidsai/cudf#23345

Open

igorpeshansky approved these changes Jul 29, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replace Apache Thrift by cudf APIs for Parquet footer reading/writing - #4851

Replace Apache Thrift by cudf APIs for Parquet footer reading/writing#4851
ttnghia wants to merge 2 commits into
NVIDIA:mainfrom
ttnghia:thrift-drop/jni

ttnghia commented Jul 20, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

greptile-apps Bot commented Jul 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

igorpeshansky Jul 21, 2026

Uh oh!

igorpeshansky Jul 21, 2026

Uh oh!

ttnghia Jul 29, 2026

Uh oh!

igorpeshansky Jul 21, 2026

Uh oh!

ttnghia Jul 28, 2026

Uh oh!

igorpeshansky left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		// Parse leniently (throw_if_type_mismatch::NO): skip a known field whose wire type does not
		// match cudf's schema (Thrift forward-compat) rather than rejecting real files that use it.

Uh oh!

Conversation

ttnghia commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

greptile-apps Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

igorpeshansky Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

igorpeshansky Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

ttnghia Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

igorpeshansky Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

ttnghia Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

igorpeshansky left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ttnghia commented Jul 20, 2026 •

edited

Loading

greptile-apps Bot commented Jul 20, 2026 •

edited

Loading