Skip to content

shredded_variant: cases 41, 131, 132, 138 omit the required 'value' column but are labeled as valid cases #116

Description

@sdf-jkl

Summary

shredded_variant cases 41, 131, 132 and 138 omit the value column from a variant group, but are labeled as valid cases (plain filenames, no error_message, with expected *.variant.bin outputs). Per the spec and the discussion in apache/parquet-format#591, such files are out of spec, and the labeling has already misled implementations. They should be relabeled following the existing -INVALID convention (as suggested by @alamb in apache/parquet-format#591 (comment)).

Details

LogicalTypes.md requires:

The group must contain a field named metadata and a field named value.

and in apache/parquet-format#591 it was confirmed this is intentional and will not be relaxed: writers must always emit the value column (all-null is fine), for the top-level group and for shredded field groups alike.

The affected cases:

case test omission
41 testArrayMissingValueColumn top-level value (typed_value is a LIST)
131 testMissingValueColumn top-level value (typed_value is INT32)
132 testShreddedObjectMissingFieldValueColumn value inside the shredded object field groups
138 testShreddedObjectMissingValueColumn top-level value (typed_value is an object)

For example, case-131.parquet:

required group var (VARIANT(1)) {
  required binary metadata;
  optional int32 typed_value;   // no `value` column
}

and case-132.parquet (top level is compliant; the field groups are not):

optional group var (VARIANT(1)) {
  required binary metadata;
  optional binary value;
  optional group typed_value {
    required group a { optional int32 typed_value; }        // no `value`
    required group b { optional binary typed_value (STRING); } // no `value`
  }
}

Why it matters

This corpus is the de facto conformance suite for shredded variant readers, and the mislabeling has already propagated:

Proposed fix

Rename the four cases to case-NNN-INVALID.parquet (and their .variant.bin files accordingly) and add error_message entries in cases.json, following the existing convention used by cases 43, 84 and 125. If the community instead wants to preserve them as reader-leniency tests (readers may read out-of-spec data, per the discussion in apache/parquet-format#591), that would need a new explicit category in cases.json — the current binary taxonomy cannot express "may read".

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions