Support for string column to Parquet Variant infrastructure

[abigalekim]

Description

Adds the new API function cudf::io::parquet::experimental::encode_strings_to_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 JSON string => Parquet Variant features.

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]

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: 2ea5e2b7-bd1c-4ab4-88af-09886055e1a4

📥 Commits

Reviewing files that changed from the base of the PR and between 1e198a7 and 6128792.

📒 Files selected for processing (1)

• cpp/src/io/parquet/experimental/variant_encode.cu

🚧 Files skipped from review as they are similar to previous changes (1)

• cpp/src/io/parquet/experimental/variant_encode.cu

---

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Added support for encoding flat JSON object strings into Parquet VARIANT struct columns.
  • Supports nulls, booleans, strings, 64-bit integers, and floating-point values.
  • Preserves input nulls and supports multiple fields, rows, missing fields, and empty inputs.
  • Validates duplicate or invalid field names and limits requests to 255 fields.

• Tests

  • Added comprehensive coverage for scalar values, UTF-8 strings, nulls, sliced inputs, missing fields, empty inputs, field limits, and output structure.

Walkthrough

Changes

The PR adds encode_strings_to_variant, a CUDA implementation that converts selected scalar fields from flat JSON object strings into Parquet VARIANT struct columns. It adds build integration and tests for scalar encoding, null handling, field selection, and output structure.

String-to-VARIANT encoding

Layer / File(s)	Summary
API and encoder entry points cpp/include/cudf/io/experimental/variant.hpp, cpp/src/io/parquet/experimental/variant_encode.cu, cpp/CMakeLists.txt	Adds the public API, validates and sorts field names, extracts JSON fields, and registers the CUDA source.
GPU scalar encoding and column assembly cpp/src/io/parquet/experimental/variant_encode.cu	Encodes scalar values, computes value offsets, builds metadata and value lists, preserves validity masks, and assembles the VARIANT struct column.
Encoding behavior validation cpp/tests/io/experimental/variant_encode_test.cpp, cpp/tests/CMakeLists.txt	Tests scalar types, field selection, multiple rows, missing fields, null rows, sliced inputs, empty inputs, output structure, and the 255-field limit.

Estimated code review effort: 4 (Complex) | ~60 minutes

Mergeability Score: 🔵 Low · up to 61287

The change may incur increasing host-launch overhead because metadata is copied once per row, which can reduce performance on large inputs. The PR is otherwise mergeable with explicit owner awareness and follow-up on scaling behavior.

Suggested reviewers: kingcrimsontianyu, lamarrr

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 identifies the main change: adding string-column support to Parquet Variant infrastructure.
Description check	✅ Passed	The description directly explains the new API, supported scalar values, implementation scope, 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: 16

🧹 Nitpick comments (1)

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

588-591: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Copy the input null mask once and reuse it.

cudf::detail::copy_bitmask runs three times on the same input_null_mask: at line 590 for the value column, at line 606 for the struct column, and at line 454 inside make_constant_metadata_column. Each call allocates a device buffer and launches a copy. Two of the three are avoidable.

Copy the mask once before line 588 and construct the additional copies from that buffer, or pass the already-copied buffer into make_constant_metadata_column.

Also applies to: 604-607

🤖 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 588 - 591,
Update the surrounding encoding flow to copy input_null_mask only once, then
reuse that device buffer for the value column, struct column, and
make_constant_metadata_column instead of invoking cudf::detail::copy_bitmask
separately at each site. Adjust make_constant_metadata_column’s inputs as needed
to accept and use the existing copied mask while preserving current null-mask
behavior.

🤖 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`:
- Around line 115-137: Update the Doxygen block for the VARIANT encoding
function to document that column_names may contain at most 255 field names and
that exceeding this limit throws std::invalid_argument via the existing
validation. Add the constraint and `@throws` documentation without changing the
implementation.

In `@cpp/src/io/parquet/experimental/variant_encode.cu`:
- Around line 204-221: Decode JSON escape sequences in the quoted-string
handling shared by encoded_field_size and write_field_value before determining
length or writing VARIANT bytes, including \n, \t, \\, escaped quotes, and
\uXXXX to UTF-8. Ensure both functions use the same decoded length and content;
alternatively, explicitly reject backslash-containing string values in both
paths.
- Around line 230-234: Update the integer encoding path around try_parse_int64
so failed parses fall back to FLOAT64 encoding of the original raw value instead
of silently storing INT64 zero. Ensure the parser accepts and correctly
represents INT64_MIN by using unsigned-magnitude or negative-result
accumulation, while preserving INT64 encoding for successfully parsed values.
- Around line 167-181: Require quoted-string inputs to have at least two bytes
before entering the string-handling branch in both encoded_field_size and
write_field_value, changing the existing raw.size_bytes() > 0 guard
consistently. Preserve normal handling for valid quoted strings while preventing
str_len from becoming negative for a lone quote.
- Around line 262-275: Update the value-size accumulation and inclusive scan
producing value_offsets/total_value_bytes to use int64_t, including the relevant
temporary and output types. Before the allocation in the write-values path,
validate total_value_bytes is within size_type’s maximum using
std::numeric_limits<size_type>::max(), and reject or report overflow instead of
allocating an undersized buffer; preserve the existing allocation and write flow
for valid totals.
- Line 504: Route temporary allocations in
cpp/src/io/parquet/experimental/variant_encode.cu at lines 504, 516, 531, 538,
and 426 through cudf::get_current_device_resource_ref() rather than mr,
including d_sorted_to_original, get_json_object results, d_views, value_sizes,
and d_blob; at line 553, use rmm::exec_policy_nosync(stream) without mr. Keep mr
only for returned allocations value_offsets and value_child_data.
- Around line 457-459: Update the make_lists_column call in the surrounding
encoding function to pass the existing stream and memory-resource arguments,
then remove the preceding stream.synchronize() call. Preserve the existing
columns, row count, null count, and null-mask ownership while ensuring
construction stays on the producing stream and resource.
- Around line 490-502: After sorting in the field-name preparation flow,
validate adjacent entries in sort_indices or sorted_names and reject any
duplicate column_names before building metadata or writing values. Return or
propagate the existing validation error mechanism, preserving normal processing
for unique names.
- Around line 479-488: Update the num_rows == 0 branch to pass the caller’s
stream and mr through every make_empty_column, make_lists_column, and
make_structs_column invocation. Ensure all empty metadata/value and returned
struct allocations use the supplied stream and memory resource.
- Around line 127-150: Update exponent handling in parse_float64 to avoid signed
overflow and unbounded per-digit or per-power loops: parse the exponent with
saturation, clamp it to the supported double exponent range, and directly return
or produce 0.0/infinity when the exponent is outside that range. Replace
repeated factor multiplication with a bounded power-of-ten approach or lookup
that preserves correct overflow and underflow behavior, including negative
exponents.
- Around line 546-563: Update the value-offset initialization around
value_offsets and the inclusive_scan to avoid copying from the block-scoped
zero; initialize the first device element with cudaMemsetAsync on the stream
before scanning. Preserve the existing inclusive scan and offset layout while
ensuring the asynchronous operation uses device-owned storage.
- Around line 513-517: Update the JSONPath construction in the variant
extraction loop to handle column names containing `.` or `[` without
interpreting those characters as path syntax. Prefer escaping or quoting each
`column_names[i]` according to the documented JSONPath grammar; if the API
cannot safely represent them, validate and reject such names explicitly before
calling `cudf::get_json_object`.
- Line 435: Add the direct cudf/detail/utilities/cuda_memcpy.hpp include to the
translation unit containing the variant encoding logic, so the
cudf::detail::memcpy_async call is declared without relying on transitive
includes.

In `@cpp/tests/io/experimental/variant_encode_test.cpp`:
- Around line 98-115: Strengthen the scalar conversion assertions in
cpp/tests/io/experimental/variant_encode_test.cpp at lines 98-115 by comparing
extracted values in SingleRowFloat and SingleRowFloatExponent against expected
FLOAT64 columns containing 3.14 and 150.0, respectively, while retaining size
and validity checks. At lines 156-164, compare the extracted result with an
INT64 column containing one null row so JSON-null validity is explicitly
verified.
- Around line 24-40: Extend cpp/tests/io/experimental/variant_encode_test.cpp at
lines 24-40 by adding a helper or test path that constructs and passes a sliced
cudf::strings_column_view to encode_strings_to_variant. Update lines 135-153 to
include non-ASCII UTF-8 strings and cases on both sides of the short/long string
encoding boundary. Expand the tests at lines 231-243 with enough rows to
exercise encoding and extraction across multiple CUDA blocks.
- Around line 6-18: Update the includes in the variant encode test to add
cudf_test/cudf_gtest.hpp, the direct header defining cudf::strings_column_view,
and the standard headers defining std::unique_ptr and int64_t. Keep the existing
includes and avoid relying on transitive dependencies.

---

Nitpick comments:
In `@cpp/src/io/parquet/experimental/variant_encode.cu`:
- Around line 588-591: Update the surrounding encoding flow to copy
input_null_mask only once, then reuse that device buffer for the value column,
struct column, and make_constant_metadata_column instead of invoking
cudf::detail::copy_bitmask separately at each site. Adjust
make_constant_metadata_column’s inputs as needed to accept and use the existing
copied mask while preserving current null-mask behavior.

🪄 Autofix

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: b1d660af-01aa-4ca7-b9bb-ff7f64d8c3f1

📥 Commits

Reviewing files that changed from the base of the PR and between baea696 and c9ec06c.

📒 Files selected for processing (5)

• 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

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

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

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

Include <cuda/std/limits> directly.

Line 149 uses cuda::std::numeric_limits<double>::infinity(). The file includes <cuda/std/cstring> and <cuda/std/optional> but not <cuda/std/limits>. The declaration is available only through a transitive include today.

🔧 Proposed fix

 `#include` <cuda/std/cstring>
+#include <cuda/std/limits>
 `#include` <cuda/std/optional>

🤖 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 - 33, Add
the direct <cuda/std/limits> include to the include list in variant_encode.cu,
alongside the other cuda/std headers, so the numeric_limits use in the encoding
implementation is declared explicitly rather than relying on transitive
includes.

---

441-458: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Replace the per-row copies with one device pass.

total_bytes contains only non-null rows, so filling dst[idx] = d_blob[idx % m] preserves the packed layout. Use rmm::exec_policy_nosync(stream) and include <thrust/for_each.h> directly. CUDF_CUDA_TRY is valid because cudf::detail::memcpy_async returns cudaError_t.

🤖 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 441 - 458,
Replace the per-row memcpy loop in the total_bytes branch with a single
device-side thrust pass over total_bytes that assigns each destination byte from
d_blob using idx % m. Use rmm::exec_policy_nosync(stream), include
thrust/for_each.h directly, and retain the existing host-to-device blob copy and
packed non-null layout.

🤖 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`:
- Around line 132-138: Update the Doxygen contract for the variant encoding API
near the parameter documentation to include std::invalid_argument for duplicate
column_names entries and names containing '.' or '['. Preserve the existing
255-name rejection rule and reference the column_names parameter explicitly.

In `@cpp/src/io/parquet/experimental/variant_encode.cu`:
- Around line 565-566: Normalize the input null mask at entry in the variant
encoding flow using input.offset(), producing an offset-free owned mask for the
logical rows. Update compute_value_sizes_kernel, write_values_kernel, and
make_constant_metadata_column to use this normalized mask, and reuse it for the
child/struct mask outputs instead of copying from input.null_mask() with offset
zero. Ensure the constant metadata copy uses the normalized mask and
bitmask_allocation_size_bytes(num_rows), preserving correct null alignment for
sliced inputs.

In `@cpp/tests/CMakeLists.txt`:
- Line 364: Add a unit benchmark for encode_strings_to_variant alongside the
existing ConfigureTest registration in cpp/tests/CMakeLists.txt, covering
representative JSON rows and string sizes for the GPU encoder. Register it
through the project’s established benchmark configuration so it provides a
performance-regression baseline.

In `@cpp/tests/io/experimental/variant_encode_test.cpp`:
- Around line 408-416: Extend EncodeStringsToVariantTest with boundary-count
coverage for field names: add a successful encode test using 255 distinct names,
and add an assertion that encoding 256 distinct names throws
std::invalid_argument. Reuse the existing encode helper and test conventions
while keeping the current NoColumnNames test unchanged.

---

Nitpick comments:
In `@cpp/src/io/parquet/experimental/variant_encode.cu`:
- Around line 30-33: Add the direct <cuda/std/limits> include to the include
list in variant_encode.cu, alongside the other cuda/std headers, so the
numeric_limits use in the encoding implementation is declared explicitly rather
than relying on transitive includes.
- Around line 441-458: Replace the per-row memcpy loop in the total_bytes branch
with a single device-side thrust pass over total_bytes that assigns each
destination byte from d_blob using idx % m. Use rmm::exec_policy_nosync(stream),
include thrust/for_each.h directly, and retain the existing host-to-device blob
copy and packed non-null layout.

🪄 Autofix

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: 4d8f85ec-c4ab-479f-aeeb-39e5b9208a38

📥 Commits

Reviewing files that changed from the base of the PR and between 9ed1bdc and a9a2255.

📒 Files selected for processing (5)

• 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

[abigalekim]

/ok to test 6128792