Table to Parquet Variant Encoding

[abigalekim]

Description

Adds the new API function cudf::io::parquet::experimental::encode_variant mentioned in #23251. Currently this code only supports scalar, non-nested variant values. This PR is mainly to enable the infrastructure to support the rest of the Parquet Variant JSON converter support.

encode_variant(table, column_names, stream, mr) converts a flat cuDF table (output from cudf::read_json) into a Parquet Variant object column. Each row contains a self contained variant object. Supported column types are INT8/16/32/64, FLOAT32/64, STRING, and EMPTY (all-null).

Boolean support (#23276) is now merged. There is support for Boolean types in this PR.

Checklist

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

[copy-pr-bot]

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.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Summary by CodeRabbit

• New Features
  • Added experimental GPU encoding for Parquet VARIANT from string columns and tables, producing per-row VARIANT structs with field names and preserved nullability.
  • Added support for encoding VARIANT numeric fields with precise float32/float64 round-trip behavior.
• Bug Fixes
  • Improved input validation for VARIANT encoding by rejecting duplicate column names (including duplicates that appear after internal sorting).
• Tests
  • Added/expanded test coverage for encoding and extraction, including duplicate-name rejection, float32/float64 round-trips, null mask preservation, and mixed-type per-field extraction.

Walkthrough

Adds public VARIANT encoding APIs, duplicate column-name validation, expanded string, numeric, floating-point, null, and round-trip tests, and CUDA build registrations for VARIANT encoding and split filtered-join implementations.

Changes

VARIANT encoding

Layer / File(s)	Summary
Public encoding API cpp/include/cudf/io/experimental/variant.hpp	Declares APIs for encoding string columns and tables into VARIANT columns with documented defaults and exception conditions.
Encoding validation and coverage cpp/src/io/parquet/experimental/variant_encode.cu, cpp/tests/CMakeLists.txt, cpp/tests/io/experimental/variant_encode_test.cpp	Rejects duplicate column names after sorting and registers tests covering encoding payloads, validation, nulls, numeric types, strings, and round-trip extraction.
Floating-point extraction coverage cpp/tests/io/experimental/variant_extract_test.cpp	Adds FLOAT32/FLOAT64 cast-matrix checks and encoding round trips, including exact-width decoding and validity preservation.
VARIANT CUDA build registration cpp/CMakeLists.txt	Adds the VARIANT encoding CUDA source to the cudf target.

Filtered join build refactor

Layer / File(s)	Summary
Filtered join source registration cpp/CMakeLists.txt	Replaces the single filtered-join CUDA source with separate flat, nested, nested-query, and primitive implementation sources.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• rapidsai/cudf#23036: Updates related VARIANT extraction tests and coverage.
• rapidsai/cudf#23320: Updates the filtered-join CUDA source registration.

Suggested labels: tests

Suggested reviewers: vuule, davidwendt, qbacpey

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the PR's main change: adding infrastructure for Parquet Variant to JSON support.
Description check	✅ Passed	The description directly explains the new APIs, supported types, intended infrastructure, and test coverage.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 7

🧹 Nitpick comments (4)

cpp/src/io/parquet/experimental/variant_encode.cu (2)

268-285: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

1 KiB per-thread local array will spill heavily; the sizes can be streamed instead.

uint32_t field_sizes[256] is 1 KiB of local memory per thread (256 KiB per block) and it recomputes what pass 1 already computed. Since the offsets are a simple prefix sum over the fields, write the offset table in a first loop over fields and the values in a second, without materializing the array.

♻️ Suggested restructuring

-    // Compute per-field sizes (stack array, safe for N < 256)
-    uint32_t field_sizes[256];
-    for (int i = 0; i < N; i++) {
-      field_sizes[i] = static_cast<uint32_t>(field_encoded_size(tbl.column(sort_order[i]), row));
-    }
-
-    // Write (N+1) field offsets, 4 bytes each (LE)
-    uint32_t running = 0;
-    for (int i = 0; i <= N; i++) {
-      cuda::std::memcpy(p, &running, 4);
-      p += 4;
-      if (i < N) { running += field_sizes[i]; }
-    }
+    // Write (N+1) field offsets, 4 bytes each (LE)
+    uint32_t running = 0;
+    for (int i = 0; i <= N; i++) {
+      cuda::std::memcpy(p, &running, 4);
+      p += 4;
+      if (i < N) {
+        running += static_cast<uint32_t>(field_encoded_size(tbl.column(sort_order[i]), row));
+      }
+    }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/io/parquet/experimental/variant_encode.cu` around lines 268 - 285,
Remove the per-thread field_sizes array and its size-computation pass in the
variant encoding flow. In the offset-writing loop, compute each field’s encoded
size directly via field_encoded_size(tbl.column(sort_order[i]), row) and
accumulate running; keep the existing second loop for writing field values in
sorted order.

Source: Coding guidelines

---

30-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing direct includes for used standard symbols. std::numeric_limits (Lines 407, 515), std::pair (Lines 295, 348) and std::unique_ptr are used without <limits>, <utility>, <memory>.

As per coding guidelines: "include headers directly for every used symbol without unused or incorrectly styled includes."

📦 Proposed include additions

 `#include` <algorithm>
 `#include` <cstdint>
+#include <limits>
+#include <memory>
 `#include` <numeric>
 `#include` <string>
+#include <utility>
 `#include` <vector>

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/io/parquet/experimental/variant_encode.cu` around lines 30 - 34, Add
direct standard-library includes for every used symbol in this translation unit:
include the headers declaring std::numeric_limits, std::pair, and
std::unique_ptr alongside the existing includes. Keep the existing include set
and style, adding only the required <limits>, <utility>, and <memory> headers.

Source: Coding guidelines

cpp/tests/io/experimental/variant_encode_test.cpp (2)

158-162: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

meta_child is unused. Either drop it or add a test that asserts the metadata child bytes directly (which would also cover the offset_size > 1 metadata path in build_metadata_blob).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/tests/io/experimental/variant_encode_test.cpp` around lines 158 - 162,
Remove the unused meta_child helper, or add a test using it to assert metadata
child bytes produced by build_metadata_blob, including the offset_size > 1 path.
Prefer removal unless direct metadata-byte coverage is needed.

---

176-286: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Coverage gaps: sliced inputs, multi-block sizes, and non-ASCII UTF-8.

All string cases are single-block (≤3 rows) and ASCII, and no test exercises a sliced input column — which is where the column_device_view offset handling and copy_bitmask path in encode_strings_to_variant are most likely to break. Adding a >256-row case (block_size is 256) plus a multi-byte UTF-8 string near the 63/64-byte short/long boundary would cover the boundary logic in write_field_value.

As per coding guidelines: "Tests must cover empty inputs, nulls, sliced columns, boundary and multi-block sizes, and non-ASCII UTF-8 for string tests". Want me to draft these cases?

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/tests/io/experimental/variant_encode_test.cpp` around lines 176 - 286,
Expand EncodeStringsToVariantTest to cover the requested gaps: add a sliced
strings column and verify values/nulls, add more than 256 rows to exercise
multi-block encoding, and add non-ASCII UTF-8 inputs around the 63/64-byte
short/long boundary with expected round-trip or encoded results. Preserve the
existing empty, null, type-error, and current encoding tests.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/include/cudf/io/experimental/variant.hpp`:
- Line 130: Update the throws documentation for the variant API around
strings_column_view to name the input parameter and specify cudf::logic_error,
matching the exception surfaced during strings_column_view construction; remove
the stale std::invalid_argument and strings references.
- Around line 137-162: Update the encode_variant documentation to list FLOAT32
and FLOAT64 among the supported column types, and document that exceeding the 2
GiB encoded-output limit throws std::overflow_error rather than
std::invalid_argument. Keep the existing std::invalid_argument conditions
limited to unsupported types, too many columns, and mismatched column_names
size.

In `@cpp/src/io/parquet/experimental/variant_encode.cu`:
- Line 397: Update the temporary device allocations d_val_sizes,
d_meta_template, and d_sort_order to use cudf::get_current_device_resource_ref()
instead of the caller-provided mr; keep mr only for allocations returned from
the function.
- Around line 378-381: Update the make_empty_list lambda and the corresponding
empty-list assembly sites to pass the active stream and memory resource to every
make_empty_column and make_lists_column call. Apply the same stream/MR
propagation at all four referenced locations, preserving the existing column
types and structure while eliminating default-stream and default-resource usage.
- Around line 348-362: Update make_constant_list_buffers and
object_encode_write_kernel to perform row-count, metadata-size, and offset
arithmetic in a sufficiently wide integer type before multiplication, preventing
size_type overflow during allocation and device writes. Ensure the offsets
representation and sequence remain valid for large products, or explicitly
reject inputs that cannot fit the existing INT32 offsets.

In `@cpp/tests/io/experimental/variant_encode_test.cpp`:
- Around line 6-18: Add the required cudf_test/cudf_gtest.hpp include to
variant_encode_test.cpp alongside the existing cudf_test headers, without
changing the test implementation.

In `@cpp/tests/io/experimental/variant_extract_test.cpp`:
- Around line 964-987: The test named FloatEncodeMatchesApacheBytes does not
verify encoded bytes, only round-trips values through extract_variant_field.
Either add a direct comparison of the encoded value-child bytes against the
Apache fixture using the existing build_object_value pattern, or rename the test
and comments to describe round-trip validation instead.

---

Nitpick comments:
In `@cpp/src/io/parquet/experimental/variant_encode.cu`:
- Around line 268-285: Remove the per-thread field_sizes array and its
size-computation pass in the variant encoding flow. In the offset-writing loop,
compute each field’s encoded size directly via
field_encoded_size(tbl.column(sort_order[i]), row) and accumulate running; keep
the existing second loop for writing field values in sorted order.
- Around line 30-34: Add direct standard-library includes for every used symbol
in this translation unit: include the headers declaring std::numeric_limits,
std::pair, and std::unique_ptr alongside the existing includes. Keep the
existing include set and style, adding only the required <limits>, <utility>,
and <memory> headers.

In `@cpp/tests/io/experimental/variant_encode_test.cpp`:
- Around line 158-162: Remove the unused meta_child helper, or add a test using
it to assert metadata child bytes produced by build_metadata_blob, including the
offset_size > 1 path. Prefer removal unless direct metadata-byte coverage is
needed.
- Around line 176-286: Expand EncodeStringsToVariantTest to cover the requested
gaps: add a sliced strings column and verify values/nulls, add more than 256
rows to exercise multi-block encoding, and add non-ASCII UTF-8 inputs around the
63/64-byte short/long boundary with expected round-trip or encoded results.
Preserve the existing empty, null, type-error, and current encoding tests.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b598d0f0-c250-42d9-9dc1-842f541f6dd6

📥 Commits

Reviewing files that changed from the base of the PR and between 16f187b and 12e0193.

📒 Files selected for processing (6)

• cpp/CMakeLists.txt
• cpp/include/cudf/io/experimental/variant.hpp
• cpp/src/io/parquet/experimental/variant_encode.cu
• cpp/tests/CMakeLists.txt
• cpp/tests/io/experimental/variant_encode_test.cpp
• cpp/tests/io/experimental/variant_extract_test.cpp

[abigalekim]

After discussion with @vuule, we have decided to split the implementation of the table => Variant API function and the JSON => variant API function. I will link the PR to the JSON => Variant API function here when it is completed.

[abigalekim]

The JSON string column to Variant column PR is here: #23614

[abigalekim]

/ok to test 49619b6