Skip to content

Conversation

ebyhr
Copy link
Contributor

@ebyhr ebyhr commented Oct 13, 2025

Follow-up of #13941

Closes #13498

@ebyhr ebyhr force-pushed the ebi/parquet-variant-type-logical-annotation branch from 35cfcdd to 1fe88c4 Compare October 13, 2025 02:05
@ebyhr ebyhr force-pushed the ebi/parquet-variant-type-logical-annotation branch from 1fe88c4 to 2c74c3c Compare October 13, 2025 02:33
@huaxingao
Copy link
Contributor

cc @aihuaxu

Copy link
Contributor

@aihuaxu aihuaxu left a comment

Choose a reason for hiding this comment

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

LGTM.

Comment on lines +258 to +262
if (column.type() == Types.VariantType.get()) {
int fieldIndex = schema.getFieldIndex(column.name());
assertThat(schema.getFields().get(fieldIndex).getLogicalTypeAnnotation())
.isEqualTo(LogicalTypeAnnotation.variantType(Variant.VARIANT_SPEC_VERSION));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:
can we club this read with the read below ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe no? The subsequent Parquet reader doesn't expose logical type annotations.

I assume we intentionally use Parquet.read instead of the low-level ParquetFileReader because it's the primary API intended for third-party developers.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Parquet][Variant] The Parquet writer should set the VARIANT logicalType on the produced parquet files.

4 participants