Skip to content

feat(variant): support variant data type#406

Open
SteNicholas wants to merge 1 commit into
alibaba:mainfrom
SteNicholas:PAIMON-91
Open

feat(variant): support variant data type#406
SteNicholas wants to merge 1 commit into
alibaba:mainfrom
SteNicholas:PAIMON-91

Conversation

@SteNicholas

Copy link
Copy Markdown
Contributor

Purpose

Linked issue: close #91

Introduce native support for the variant data type for storing and efficiently querying semi-structured data (e.g. JSON), following the parquet-format VariantEncoding.md / VariantShredding.md specifications:

  • Variant binary codec, JSON parsing/rendering, and the public paimon::Variant API with path access (e.g. $.a[0]) to scalar targets and, via VariantGetArrow, nested STRUCT / LIST / MAP / VARIANT targets.
  • VARIANT in the type system: parsed from / serialized to "VARIANT", represented in Arrow as struct<value: binary, metadata: binary> marked with paimon.extension.type=paimon.type.variant; field-id assignment and schema validation treat variant structs as leaves.
  • Parquet read/write with an unannotated group holding REQUIRED BINARY value (field id 0) / metadata (field id 1); ORC/Avro report NotImplemented for VARIANT columns.
  • Variant shredding on a generic shredding write-plan framework shared with MAP shared-shredding: configured via variant.shreddingSchema, or inferred per file from sampled rows when variant.inferShreddingSchema is enabled (tuned by the variant.shredding.* options).
  • Read-side generic shredding read plans: shredded files are detected structurally and reassembled back into variants transparently; a variant column can also be read as a variant-access projection built with VariantAccessBuilder, extracting specific paths while pruning the scan to metadata and the requested typed_value sub-columns.

Tests

  • UT: generic_variant_test.cpp (golden-byte compatibility, malformed/corrupted input), variant_test.cpp (public API), variant_get_test.cpp (path access, scalar and nested targets), variant_type_utils_test.cpp, variant_shredding_test.cpp (shred/reassemble round trips), infer_variant_shredding_schema_test.cpp, variant_shredding_write_plan_factory_test.cpp, infer_shredding_file_writer_test.cpp, plus VARIANT cases in data_type_test.cpp, data_type_json_parser_test.cpp, arrow_schema_validator_test.cpp, table_schema_test.cpp, orc_format_writer_test.cpp and avro_schema_converter_test.cpp.
  • Format IT: variant_parquet_test.cpp (physical layout, unshredded/shredded round trips, variant-access reads with typed_value pruning).
  • IT: test/inte/variant_table_inte_test.cpp (append and primary-key tables, variant-access projection read, ORC rejection).

API and Format

  • New public API: include/paimon/data/variant.h (paimon::Variant, VariantCastArgs, VariantAccessBuilder), FieldType::VARIANT in include/paimon/defs.h, and the new variant.* table options.
  • The storage format follows the parquet-format VariantEncoding.md / VariantShredding.md specifications; the parquet physical layout is byte-compatible with Java Paimon. No changes to existing formats or protocols.

Documentation

docs/source/user_guide/data_types.rst documents the VARIANT type, its Arrow representation, shredding configuration and variant-access reads.

Generative AI tooling

Generated-by: Claude Code (Claude Fable 5)

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.

Adds end-to-end support for the VARIANT data type across schema/type parsing, Parquet read/write (including shredding + inference), and variant-access projections, with explicit ORC/Avro “NotImplemented” behavior and extensive tests.

Changes:

  • Introduces VARIANT as an Arrow extension-backed leaf type (struct<value: binary, metadata: binary> with metadata marker) with parsing/serialization and field-id handling.
  • Adds generic shredding infrastructure (write-plan factories, inference buffering writer, file-reader reassembly) and integrates it into writers/readers.
  • Adds IT/UT coverage for VARIANT, shredding inference, and format support/rejection (Parquet supported; ORC/Avro rejected).

Reviewed changes

Copilot reviewed 96 out of 96 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/inte/variant_table_inte_test.cpp New integration tests for VARIANT tables (append/PK/projection/ORC rejection).
test/inte/CMakeLists.txt Registers new VARIANT integration test target.
src/paimon/testing/utils/variant_test_data.h Adds helper to build Arrow batches containing VARIANT encodings.
src/paimon/testing/utils/test_helper.h Adds ReadResult helper for raw Arrow results (needed for VARIANT checks).
src/paimon/format/parquet/CMakeLists.txt Adds Parquet VARIANT IT to build.
src/paimon/format/orc/orc_format_writer_test.cpp Adds ORC “VARIANT not supported” test.
src/paimon/format/orc/orc_format_writer.cpp Rejects VARIANT schema for ORC writer creation.
src/paimon/format/avro/avro_schema_converter_test.cpp Adds Avro “VARIANT not supported” test.
src/paimon/format/avro/avro_schema_converter.cpp Rejects VARIANT schema for Avro schema conversion.
src/paimon/core/utils/nested_projection_utils.cpp Exempts variant-access projections from nested subfield projection logic.
src/paimon/core/schema/table_schema_test.cpp Tests mapping of variant-marked struct to FieldType::VARIANT.
src/paimon/core/schema/table_schema.cpp Treats variant structs as leaves for field-id assignment; fixes field type conversion.
src/paimon/core/schema/schema_validation.cpp Disallows MAP shared-shredding when VARIANT is present.
src/paimon/core/schema/arrow_schema_validator_test.cpp Adds schema validation tests for variant and variant-access projections.
src/paimon/core/schema/arrow_schema_validator.cpp Validates variant shape as leaf; allows variant-access projections as leaf.
src/paimon/core/postpone/postpone_bucket_writer.cpp Switches shredding writer selection to generic plan factory selection.
src/paimon/core/operation/internal_read_context.cpp Aligns variant-access read types against VARIANT table fields.
src/paimon/core/operation/append_only_file_store_write.cpp Uses generic shredding plan factory selection for compaction writers.
src/paimon/core/operation/abstract_split_read.h Declares hook to apply VARIANT shredding reader reassembly/extraction.
src/paimon/core/operation/abstract_split_read.cpp Wraps readers with ShreddingFileReader based on VARIANT read plans.
src/paimon/core/mergetree/merge_tree_writer.cpp Uses generic shredding plan factory selection for append writers.
src/paimon/core/mergetree/compact/merge_tree_compact_rewriter.cpp Uses generic shredding plan factory selection for compact writers.
src/paimon/core/io/single_file_writer.h Makes key writer methods virtual for wrapper/derived writer support.
src/paimon/core/io/shredding_key_value_data_file_writer_factory.h Generalizes shredding writer factory to accept ShreddingWritePlanFactory.
src/paimon/core/io/shredding_key_value_data_file_writer_factory.cpp Adds inferred-plan wrapper writer and generic metadata finalization.
src/paimon/core/io/shredding_append_data_file_writer_factory.h Generalizes append shredding writer factory to accept ShreddingWritePlanFactory.
src/paimon/core/io/shredding_append_data_file_writer_factory.cpp Adds inferred-plan wrapper writer for append writers + generic finalizer hook.
src/paimon/core/io/rolling_file_writer.h Moves metrics read after close to support lazy inner writer creation.
src/paimon/core/io/infer_shredding_file_writer_test.cpp Adds tests for buffered inference + replay behavior.
src/paimon/core/io/infer_shredding_file_writer.h Adds file-writer wrapper that buffers rows to infer shredding plan per file.
src/paimon/core/core_options.h Adds VARIANT shredding option accessors.
src/paimon/core/core_options.cpp Implements VARIANT shredding option resolution/defaults.
src/paimon/core/append/append_only_writer.cpp Uses generic shredding plan factory selection for append-only writes.
src/paimon/common/utils/field_type_utils.h Adds field-aware conversion to distinguish VARIANT extension type.
src/paimon/common/types/data_type_test.cpp Adds VARIANT stringification / JSON serialization tests.
src/paimon/common/types/data_type_json_parser_test.cpp Adds parsing tests for "VARIANT" and "VARIANT NOT NULL".
src/paimon/common/types/data_type_json_parser.cpp Adds VARIANT keyword parsing and applies metadata marker at field level.
src/paimon/common/types/data_type.cpp Treats variant-marked struct as scalar type + renders it as VARIANT.
src/paimon/common/defs.cpp Adds VARIANT shredding option keys.
src/paimon/common/data/variant/variant_type_utils_test.cpp Tests VARIANT field construction/detection/shape validation and type conversion.
src/paimon/common/data/variant/variant_type_utils.h Declares VARIANT Arrow/metadata utilities.
src/paimon/common/data/variant/variant_type_utils.cpp Implements VARIANT Arrow/metadata utilities and schema scanning.
src/paimon/common/data/variant/variant_test.cpp Tests public paimon::Variant API behavior.
src/paimon/common/data/variant/variant_shredding_writer.h Declares writer to shred VARIANT column values to physical schema.
src/paimon/common/data/variant/variant_shredding_write_plan_factory_test.cpp Tests configured and inferred VARIANT shredding plan factory behavior.
src/paimon/common/data/variant/variant_shredding_write_plan_factory.h Declares VARIANT shredding plan factory.
src/paimon/common/data/variant/variant_shredding_write_plan_factory.cpp Implements configured/inferred shredding plan selection and converter creation.
src/paimon/common/data/variant/variant_shredding_write_plan.h Declares physical write plan mapping logical->shredded schema.
src/paimon/common/data/variant/variant_shredding_write_plan.cpp Implements plan creation and configured-schema parsing.
src/paimon/common/data/variant/variant_shredding_utils.h Declares schema conversion utilities for shredding/reassembly.
src/paimon/common/data/variant/variant_shredding_read_plan_factory.h Declares per-column read plan factory for VARIANT reassembly/projection.
src/paimon/common/data/variant/variant_shredding_batch_converter.h Declares logical->physical batch converter for VARIANT shredding.
src/paimon/common/data/variant/variant_shredding_batch_converter.cpp Implements batch conversion and per-row shredding.
src/paimon/common/data/variant/variant_schema.h Declares shredding schema model (typed/value/metadata indices).
src/paimon/common/data/variant/variant_reassembler.h Declares shredded->unshredded reassembly logic.
src/paimon/common/data/variant/variant_reassembler.cpp Implements reconstruction algorithm per VariantShredding spec.
src/paimon/common/data/variant/variant_path_segment.h Declares JSONPath-like segment parser for variant access.
src/paimon/common/data/variant/variant_path_segment.cpp Implements path parsing for object keys and array indices.
src/paimon/common/data/variant/variant_json_utils.h Declares JSON rendering utilities matching Java behavior.
src/paimon/common/data/variant/variant_json_utils.cpp Implements Java-compatible float/date/timestamp formatting and zone resolution.
src/paimon/common/data/variant/variant_get.h Declares variant_get executor interface (extract + cast).
src/paimon/common/data/variant/variant_defs.h Adds VARIANT encoding + Arrow extension constants.
src/paimon/common/data/variant/variant_builder.h Declares builder for variant value/metadata encoding.
src/paimon/common/data/variant/variant_binary_util.h Declares utilities for variant binary decoding/validation.
src/paimon/common/data/variant/variant_access_utils.h Declares projection metadata parsing + shredded-field pruning.
src/paimon/common/data/variant/variant_access_utils.cpp Implements projection description parsing and typed_value subtree clipping.
src/paimon/common/data/variant/variant.cpp Implements public paimon::Variant and VariantAccessBuilder APIs.
src/paimon/common/data/variant/infer_variant_shredding_schema_test.cpp Tests inferred shredding type behavior and limits.
src/paimon/common/data/variant/infer_variant_shredding_schema.h Declares per-column inferred shredding type algorithm.
src/paimon/common/data/variant/generic_variant.h Declares core variant value/metadata container and accessors.
src/paimon/common/data/shredding/shredding_write_plan_factory.h Adds generic shredding write-plan factory abstraction.
src/paimon/common/data/shredding/shredding_write_plan_factories.h Declares selection logic across shredding strategies.
src/paimon/common/data/shredding/shredding_write_plan_factories.cpp Implements selection logic (MAP shared-shredding vs VARIANT shredding).
src/paimon/common/data/shredding/shredding_read_plan.h Declares per-column read plan abstraction for shredded files.
src/paimon/common/data/shredding/shredding_file_reader.h Declares reader wrapper to push down physical schema + assemble logical arrays.
src/paimon/common/data/shredding/shredding_file_reader.cpp Implements schema pushdown + batch assembly for shredded reads.
src/paimon/common/data/shredding/map_shared_shredding_write_plan_factory.h Adds MAP shared-shredding factory under common write-plan abstraction.
src/paimon/common/data/shredding/map_shared_shredding_write_plan_factory.cpp Implements MAP shared-shredding plan factory + metadata finalizer.
src/paimon/common/data/shredding/map_shared_shredding_batch_converter.h Makes MAP batch converter implement the common converter interface.
src/paimon/CMakeLists.txt Wires new VARIANT/shredding sources and tests into the build.
include/paimon/defs.h Adds FieldType::VARIANT and VARIANT shredding options.
include/paimon/data/variant.h Adds public VARIANT API (Variant, VariantCastArgs, VariantAccessBuilder).
docs/source/user_guide/data_types.rst Documents VARIANT type, Arrow layout, shredding options, and access projections.
Comments suppressed due to low confidence (4)

src/paimon/core/schema/schema_validation.cpp:1

  • The VARIANT-in-schema check is executed inside the per-field loop, resulting in O(n²) behavior for schemas with many fields. Move the for (const auto& field : schema.Fields()) VARIANT check outside the outer iteration (or compute a single bool has_variant once) so it runs only once per validation call.
    src/paimon/common/data/variant/variant_json_utils.cpp:1
  • std::to_chars / std::from_chars results are not checked. If to_chars_result.ec indicates failure, or if repr.find('e') returns npos, or if from_chars fails/doesn’t consume the exponent, this can produce invalid parsing and potentially out-of-bounds access (e.g., repr[0], repr[exp_start]). Add explicit checks for to_chars_result.ec, e_pos != npos, and from_chars success/consumption; fall back to a safe formatting path if parsing fails.
    src/paimon/testing/utils/variant_test_data.h:1
  • BuildVariantBatch accepts a paimon::MemoryPool but uses arrow::default_memory_pool() for the Arrow builders, which is inconsistent with the function contract and makes it harder to track memory usage in tests. Prefer constructing builders with the Arrow pool derived from the provided pool (e.g., via the existing arrow/pool adapter used elsewhere), or accept an arrow::MemoryPool* explicitly to avoid ambiguity.
    src/paimon/common/data/variant/variant_path_segment.cpp:1
  • Public API docs (VariantGet*, VariantAccessBuilder) describe paths as starting with $, but this parser accepts arbitrary prefixes before the first $ (e.g. \"foo$.a\"). This mismatch can lead to surprising behavior for API consumers. Consider either (a) enforcing root == 0 and rejecting prefixed strings, or (b) updating the public docs to explicitly state that prefixes before $ are ignored (and add a focused test to lock in the chosen behavior).

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

Comment on lines +67 to +70
Result<BatchReader::ReadBatch> ShreddingFileReader::NextBatch() {
return Status::Invalid("paimon inner reader ShreddingFileReader should use NextBatchWithBitmap");
}

Add the VARIANT data type for storing and efficiently querying
semi-structured data (e.g. JSON), following the parquet-format
VariantEncoding.md / VariantShredding.md specifications.

- Type system: VARIANT is parsed from / serialized to "VARIANT" and is
  represented in Arrow as struct<value: binary, metadata: binary>
  marked with paimon.extension.type=paimon.type.variant; field-id
  assignment and schema validation treat variant structs as leaves.
- Public API: paimon::Variant builds variants from JSON, renders them
  back to JSON, and extracts values by path (e.g. $.a[0]) cast to
  scalar targets or, via VariantGetArrow, to nested STRUCT / LIST /
  MAP / VARIANT targets.
- Parquet: a variant column is written as an unannotated group of
  REQUIRED BINARY value (field id 0) and metadata (field id 1);
  ORC/Avro report NotImplemented for VARIANT columns.
- Shredding writes: a generic write-plan framework shared with MAP
  shared-shredding rewrites variant columns into typed parquet
  sub-columns, either from the configured variant.shreddingSchema or
  inferred per file from sampled rows when variant.inferShreddingSchema
  is enabled (tuned by the variant.shredding.* options).
- Shredding reads: shredded files are detected structurally and
  reassembled back into variants transparently; a variant column can
  also be read as a variant-access projection that extracts specific
  paths while pruning the scan to metadata and the requested
  typed_value sub-columns.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
/// A file batch reader wrapper that pushes per-column physical (shredded, possibly pruned)
/// subtrees down to the inner reader and assembles the physical batches back into the logical
/// columns as planned by `ShreddingColumnReadPlan`s.
class ShreddingFileReader : public FileBatchReader {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please add ShreddingFileReader in read chain in class RawFileSplitRead, class MergeFileSplitRead and class DataEvolutionSplitRead

Image

case arrow::Type::type::LARGE_BINARY: {
// TODO(xinyu): change binary to large binary?
if (BlobUtils::IsBlobMetadata(metadata_)) {
if (type->id() == arrow::Type::type::LARGE_BINARY &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Already in case arrow::Type::type::LARGE_BINARY, why need type->id() == arrow::Type::type::LARGE_BINARY

if (extension_name.ok() && *extension_name == VariantDefs::kExtensionTypeValue) {
return FieldType::VARIANT;
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

may use VariantTypeUtils::IsVariantField

return distinct;
}

std::optional<std::string> CoreOptions::GetVariantShreddingSchema() const {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please add ut for CoreOptions

virtual MetadataFinalizer CreateMetadataFinalizer(
const std::shared_ptr<ShreddingBatchConverter>& converter) const {
return nullptr;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we avoid adding a default implementation here if possible, and keep this function as const = 0 pure virtual instead?

result->is_object = true;
result->fields.reserve(size);
for (int32_t i = 0; i < size; ++i) {
PAIMON_ASSIGN_OR_RAISE(auto field, variant.GetFieldAtIndex(i));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we avoid PAIMON_ASSIGN_OR_RAISE(auto, ...) in production code if possible, and use a concrete type here instead?

if (inferred != nullptr) {
ASSERT_EQ(inferred->id(), arrow::Type::STRUCT);
ASSERT_LT(inferred->num_fields(), 5);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This test seems a bit ambiguous at the moment, since multiple scenarios could make it pass. Could we tighten it up with more specific checks?

arrow::ImportField(target_type));
// Validate the path eagerly so mistakes fail at build time, not at read time.
PAIMON_RETURN_NOT_OK(VariantPathSegment::Parse(path).status());
// Keep the target field's own metadata (e.g. the variant extension marker of a

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PAIMON_RETURN_NOT_OK(VariantPathSegment::Parse(path));

namespace {

Result<std::string> GetDescription(const std::shared_ptr<arrow::Field>& field) {
if (field->metadata() == nullptr) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It seems that this function may not need to return a Result.

case VariantSchema::ScalarKind::kTimestampNtz:
return builder->AppendTimestampNtz(
static_cast<const arrow::TimestampArray&>(typed_array).Value(row));
default:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I’m not entirely sure whether you’ll validate the precision of the Arrow array, since the value in an Arrow array represents the numeric value under the corresponding precision. For example, for 1s, the value would be 1000 if the precision is ms, and 10^9 if the precision is ns.

return VariantBinaryUtil::MalformedVariant();
}
std::shared_ptr<MemoryPool> paimon_pool = GetDefaultPool();
auto output_type = VariantTypeUtils::UnshreddedStructType();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

GetDefaultPool() may cause allocations outside the managed pool. It’s recommended to keep allocations within the passed-in pool, e.g. by passing two pools if necessary.

}
PAIMON_RETURN_NOT_OK_FROM_ARROW(
value_builder->Append(cursor.GetBinary(schema->variant_idx)));
PAIMON_RETURN_NOT_OK_FROM_ARROW(metadata_builder->Append(metadata));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I’d like to confirm whether unshredded really needs to copy the data. My understanding is that the physical storage is just metadata and value, so it might be possible to return them directly, with at most a row-by-row check for MalformedVariant. If this could improve read performance, could you add a TODO to revisit it later? Given the complexity of the current PR, I don’t think it needs to be implemented in this PR.

const std::shared_ptr<arrow::Schema>& logical_schema,
const std::map<std::string, std::shared_ptr<arrow::DataType>>& column_shredding_types) {
auto plan = std::shared_ptr<VariantShreddingWritePlan>(new VariantShreddingWritePlan());
plan->logical_schema_ = logical_schema;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let’s avoid using a default constructor followed by assigning values in Create if possible. Could we instead have Create call a constructor that takes the required parameters?

static void AppendEscapedJson(std::string_view str, std::string* out);

/// Formats a double like `java.lang.Double.toString`, e.g. `1.0`, `-0.001`, `1.0E7`.
static std::string JavaDoubleToString(double value);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This function appears to be intended to mimic the behavior of Java’s Double.toString. Could you add unit tests for this function, focusing on whether special values such as Double.MIN_VALUE, Double.MAX_VALUE, -Infinity, -0.0, +0.0, +Infinity, and NaN are consistent with Java?

/// Writes the least significant `num_bytes` bytes in `value` into
/// `bytes[pos, pos + num_bytes)` in little endian.
static void WriteLong(uint8_t* bytes, int32_t pos, int64_t value, int32_t num_bytes);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please place the output parameters at the end of the function parameter list.

}

Status VariantBinaryUtil::MalformedVariant() {
return Status::Invalid("MALFORMED_VARIANT");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we include an error message parameter? Many places currently just return MalformedVariant, which may make debugging harder later.

const std::shared_ptr<arrow::Field>& target_field,
arrow::ArrayBuilder* builder, const VariantCastArgs& cast_args,
const std::shared_ptr<MemoryPool>& pool);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please put all output parameters at the end of the function parameter list.

static Result<std::shared_ptr<arrow::Array>> GetAsArrow(
const std::shared_ptr<GenericVariant>& variant, const std::string& path,
const std::shared_ptr<arrow::Field>& target_field, const VariantCastArgs& cast_args,
const std::shared_ptr<MemoryPool>& pool, arrow::MemoryPool* arrow_pool);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could pool and arrow_pool use a consistent type, either both raw pointers or both smart pointers? The current usage looks a bit inconsistent.

cast_args_.fail_on_error = true;
ASSERT_OK_AND_ASSIGN(array, GetAsArrow("$.nullField", target));
ASSERT_TRUE(array->IsNull(0));
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  1. Could we add a test where one STRUCT child fails to cast with fail_on_error=false, verifying that the child is null while the parent remains non-null?

  2. Could we also add a test for casting a Variant null to a Variant target, to clarify the expected null semantics?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Introduce native support for variant data type

4 participants