Skip to content

fix(java): restore parquet writer factory compatibility - #24055

Open
victorwon2001 wants to merge 1 commit into
NVIDIA:mainfrom
victorwon2001:fix/issue-23972
Open

fix(java): restore parquet writer factory compatibility#24055
victorwon2001 wants to merge 1 commit into
NVIDIA:mainfrom
victorwon2001:fix/issue-23972

Conversation

@victorwon2001

Copy link
Copy Markdown

Description

Restore the three Table.writeParquetChunked overloads to their established TableWriter return type, restoring the JVM descriptors used by binaries compiled before #23912. Add matching writeParquetChunkedWithFooter overloads returning ParquetTableWriter, route the legacy factories through them, and move the footer-specific tests to the new API while adding return-type regression coverage.

Binaries compiled against the short-lived #23912 ParquetTableWriter return descriptors are not simultaneously binary-compatible with the restored legacy descriptors and must be recompiled; callers that need the concrete writer should migrate to writeParquetChunkedWithFooter.

Closes #23972

Java main/test compilation, JVM descriptor inspection, pre-commit, and diff validation passed. The focused native-backed Maven tests could not run because the environment has no nvcc or configured libcudf native build/install.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@victorwon2001
victorwon2001 requested a review from a team as a code owner September 9, 2026 06:25
@victorwon2001
victorwon2001 requested a review from pxLi September 9, 2026 06:25
@copy-pr-bot

copy-pr-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the Java Affects Java cuDF API. label Sep 9, 2026
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 232c5e23-91af-413e-adf6-f2fa69a956d9

📥 Commits

Reviewing files that changed from the base of the PR and between 5339497 and 6b82792.

📒 Files selected for processing (2)
  • java/src/main/java/ai/rapids/cudf/Table.java
  • java/src/test/java/ai/rapids/cudf/TableTest.java

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Summary

Summary by CodeRabbit

  • New Features

    • Added footer-aware Parquet chunked writer options for file and buffer-based output.
    • Added support for configuring footer-aware writers with either a specified or default memory allocator.
  • API Updates

    • Standard Parquet chunked writer factories now return the general table-writer type.
    • Existing footer-writing workflows can use dedicated factories that expose the specialized Parquet writer interface.

Walkthrough

Changes

The Parquet chunked writer factories now preserve TableWriter return types. New footer-aware overloads return ParquetTableWriter. Tests cover file output, callback output, allocator handling, post-close behavior, and reflective return types.

Parquet writer factory API

Layer / File(s) Summary
Factory method contracts and delegation
java/src/main/java/ai/rapids/cudf/Table.java
Standard factories return TableWriter and delegate to footer-aware factories. New overloads return ParquetTableWriter for file and callback output.
Footer behavior and return-type validation
java/src/test/java/ai/rapids/cudf/TableTest.java
Footer tests use the new factories. Reflection tests verify return types for all standard and footer-aware overloads.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 6b827

This restores the established Parquet writer API descriptors for existing binaries while providing dedicated footer-aware factories. The compatibility and footer-return contracts are covered, with no current merge-blocking risk identified.

Suggested reviewers: abellina

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the Java Parquet writer factory compatibility fix, which matches the primary changes.
Description check ✅ Passed The description accurately explains the restored return types, new footer-aware factories, compatibility impact, tests, and validation results.
Linked Issues check ✅ Passed The changes satisfy the requirements in [#23972] by restoring all three Table.writeParquetChunked methods to return TableWriter, preserving JVM descriptors, adding ParquetTableWriter footer-aware fact…
Out of Scope Changes check ✅ Passed The production and test changes remain within the linked issue scope. No unrelated code changes are identified.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Java Affects Java cuDF API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Java: narrowing Table.writeParquetChunked to ParquetTableWriter breaks binary compatibility and downstream test mocks

1 participant