You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@parquet.apache.org by "divjotarora (via GitHub)" <gi...@apache.org> on 2026/09/29 09:11:20 UTC
Re: [PR] Introduce implementation for FileType [parquet-java]
divjotarora commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4131471049
##########
parquet-column/src/main/java/org/apache/parquet/schema/Types.java:
##########
@@ -847,12 +847,115 @@ public THIS addFields(Type... types) {
@Override
protected GroupType build(String name) {
if (newLogicalTypeSet) {
+ if (logicalTypeAnnotation instanceof LogicalTypeAnnotation.FileLogicalTypeAnnotation) {
+ validateFileTypeFields(name, fields);
+ }
return new GroupType(repetition, name, logicalTypeAnnotation, fields, id);
} else {
return new GroupType(repetition, name, getOriginalType(), fields, id);
}
}
+ private static void validateFileTypeFields(String name, List<Type> fields) {
+ boolean hasUri = false;
+ boolean hasOffset = false;
+ boolean hasSize = false;
+ boolean hasInline = false;
+ for (Type field : fields) {
+ String fieldName = field.getName();
+ if (!LogicalTypeAnnotation.FileLogicalTypeAnnotation.FIELD_NAMES.contains(fieldName)) {
+ throw new IllegalArgumentException("FILE type group '" + name + "' contains unrecognized field '"
+ + fieldName + "'. Valid fields are: "
+ + String.join(", ", LogicalTypeAnnotation.FileLogicalTypeAnnotation.FIELD_NAMES));
+ }
+ Preconditions.checkArgument(
+ field.isPrimitive() && field.getRepetition() == Type.Repetition.OPTIONAL,
+ "FILE type field '%s' must be an optional primitive in group '%s'",
+ fieldName,
+ name);
+ validateFileTypeFieldPhysicalType(name, field.asPrimitiveType());
+ if (LogicalTypeAnnotation.FileLogicalTypeAnnotation.URI_FIELD.equals(fieldName)) {
+ hasUri = true;
+ } else if (LogicalTypeAnnotation.FileLogicalTypeAnnotation.OFFSET_FIELD.equals(fieldName)) {
+ hasOffset = true;
+ } else if (LogicalTypeAnnotation.FileLogicalTypeAnnotation.SIZE_FIELD.equals(fieldName)) {
+ hasSize = true;
+ } else if (LogicalTypeAnnotation.FileLogicalTypeAnnotation.INLINE_FIELD.equals(fieldName)) {
+ hasInline = true;
+ }
+ }
+ // A value resolves to bytes only via inline storage or an external reference, so a group must
+ // declare at least one of `inline` or `uri`. A group declaring neither — even if it declares
+ // `offset` or `size` — can never produce a resolvable value, so reject it at schema-build
+ // time.
+ Preconditions.checkArgument(
+ hasInline || hasUri,
+ "FILE type group '%s' must declare at least one of 'inline' or 'uri'; a value resolves to "
+ + "bytes only via inline storage or an external reference, so a group declaring "
+ + "neither can never produce a valid value",
+ name);
+ // `offset` locates a byte range within the external file identified by `uri`, so it is
+ // meaningless without `uri`. A group that declares `offset` but not `uri` can never produce a
+ // valid value, so reject it at schema-build time.
+ Preconditions.checkArgument(
+ !hasOffset || hasUri,
+ "FILE type group '%s' declares field 'offset' but not 'uri'; 'offset' locates a range "
+ + "within an external file and may only be set together with 'uri'",
+ name);
+ // The spec requires `size` to be set whenever `offset` is set. A group that declares
+ // `offset` but not `size` can never produce a valid value, so reject it at schema-build
+ // time.
+ Preconditions.checkArgument(
+ !hasOffset || hasSize,
+ "FILE type group '%s' declares field 'offset' but not 'size'; 'size' is required whenever 'offset' is set",
Review Comment:
Same here, if all `offset` values are `null`, it is technically not set per the spec and we can't assert on this here.
##########
parquet-column/src/main/java/org/apache/parquet/schema/Types.java:
##########
@@ -847,12 +847,115 @@ public THIS addFields(Type... types) {
@Override
protected GroupType build(String name) {
if (newLogicalTypeSet) {
+ if (logicalTypeAnnotation instanceof LogicalTypeAnnotation.FileLogicalTypeAnnotation) {
+ validateFileTypeFields(name, fields);
+ }
return new GroupType(repetition, name, logicalTypeAnnotation, fields, id);
} else {
return new GroupType(repetition, name, getOriginalType(), fields, id);
}
}
+ private static void validateFileTypeFields(String name, List<Type> fields) {
+ boolean hasUri = false;
+ boolean hasOffset = false;
+ boolean hasSize = false;
+ boolean hasInline = false;
+ for (Type field : fields) {
+ String fieldName = field.getName();
+ if (!LogicalTypeAnnotation.FileLogicalTypeAnnotation.FIELD_NAMES.contains(fieldName)) {
+ throw new IllegalArgumentException("FILE type group '" + name + "' contains unrecognized field '"
+ + fieldName + "'. Valid fields are: "
+ + String.join(", ", LogicalTypeAnnotation.FileLogicalTypeAnnotation.FIELD_NAMES));
+ }
+ Preconditions.checkArgument(
+ field.isPrimitive() && field.getRepetition() == Type.Repetition.OPTIONAL,
+ "FILE type field '%s' must be an optional primitive in group '%s'",
+ fieldName,
+ name);
+ validateFileTypeFieldPhysicalType(name, field.asPrimitiveType());
+ if (LogicalTypeAnnotation.FileLogicalTypeAnnotation.URI_FIELD.equals(fieldName)) {
+ hasUri = true;
+ } else if (LogicalTypeAnnotation.FileLogicalTypeAnnotation.OFFSET_FIELD.equals(fieldName)) {
+ hasOffset = true;
+ } else if (LogicalTypeAnnotation.FileLogicalTypeAnnotation.SIZE_FIELD.equals(fieldName)) {
+ hasSize = true;
+ } else if (LogicalTypeAnnotation.FileLogicalTypeAnnotation.INLINE_FIELD.equals(fieldName)) {
+ hasInline = true;
+ }
+ }
+ // A value resolves to bytes only via inline storage or an external reference, so a group must
+ // declare at least one of `inline` or `uri`. A group declaring neither — even if it declares
+ // `offset` or `size` — can never produce a resolvable value, so reject it at schema-build
+ // time.
+ Preconditions.checkArgument(
+ hasInline || hasUri,
+ "FILE type group '%s' must declare at least one of 'inline' or 'uri'; a value resolves to "
+ + "bytes only via inline storage or an external reference, so a group declaring "
+ + "neither can never produce a valid value",
+ name);
+ // `offset` locates a byte range within the external file identified by `uri`, so it is
+ // meaningless without `uri`. A group that declares `offset` but not `uri` can never produce a
+ // valid value, so reject it at schema-build time.
+ Preconditions.checkArgument(
+ !hasOffset || hasUri,
+ "FILE type group '%s' declares field 'offset' but not 'uri'; 'offset' locates a range "
+ + "within an external file and may only be set together with 'uri'",
+ name);
Review Comment:
I don't think we can make this assertion at the schema layer. If a FILE group contains `inline` + `offset`, it could still be correct as long as all of the values of the `offset` column are null. This is because the [spec](https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#file) says:
> For the descriptions below, a field is set when it is present in the FILE group and its value is non-null (and, for string fields, non-empty[1]). A field is not set when it is absent from the group, or is present but null or empty.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org