You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@parquet.apache.org by "brkyvz (via GitHub)" <gi...@apache.org> on 2026/06/09 22:01:26 UTC
[PR] Introduce implementation for FileType [parquet-java]
brkyvz opened a new pull request, #3608:
URL: https://github.com/apache/parquet-java/pull/3608
<!--
Thanks for opening a pull request!
If you're new to Parquet-Java, information on how to contribute can be found here: https://parquet.apache.org/docs/contribution-guidelines/contributing
Please open a GitHub issue for this pull request: https://github.com/apache/parquet-java/issues/new/choose
and format pull request title as below:
GH-${GITHUB_ISSUE_ID}: ${SUMMARY}
or simply use the title below if it is a minor issue:
MINOR: ${SUMMARY}
-->
### Rationale for this change
Introduces the reference implementation for the new LogicalTypeAnnotation File as discussed in this [design document](https://docs.google.com/document/d/1AiwrstqkwkBoOZqgOkm9JGwSMcNeHyLR7EEj1CVqpZQ/edit?tab=t.0#heading=h.k8qyue4jj4rn)
### What changes are included in this PR?
Introduces a new LogicalTypeAnnotation called FILE. It enforces that a group with this type annotation have the following fields by name:
```
path STRING NOT NULL
size INT64
offset INT64
etag STRING
```
### Are these changes tested?
Yes, unit tested
### Are there any user-facing changes?
Introduces a new LogicalTypeAnnotation called FILE
<!-- Please uncomment the line below and replace ${GITHUB_ISSUE_ID} with the actual Github issue id. -->
<!-- Closes #${GITHUB_ISSUE_ID} -->
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4416247228
##########
parquet-column/src/main/java/org/apache/parquet/io/api/FileConverters.java:
##########
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.parquet.io.api;
+
+import java.util.Objects;
+import java.util.function.Consumer;
+import org.apache.parquet.schema.GroupType;
+import org.apache.parquet.schema.LogicalTypeAnnotation;
+import org.apache.parquet.schema.Type;
+
+/** Converters for assembling values represented by a Parquet {@code FILE}-annotated group. */
+public final class FileConverters {
+ private FileConverters() {}
+
+ /**
+ * Creates a converter for a possibly projected FILE group.
+ *
+ * <p>FILE values are atomic and cannot be projected by child field. The projected schema may
+ * contain fields absent from this particular file for schema-evolution purposes, but it must
+ * contain every FILE field stored in the file schema. The consumer receives {@code null} when an
+ * invalid FILE reference is encountered, as permitted by the FILE logical-type specification.
+ *
+ * @param schema schema presented to the converter
+ * @param fileSchema FILE group stored in the file
+ * @param consumer consumer for converted values
+ * @return a converter for the complete FILE value
+ * @throws IllegalArgumentException if the projected schema omits a stored FILE field
+ */
+ public static GroupConverter newFileConverter(
Review Comment:
we do allow for field-ids to be set for these fields. that's not against the spec
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r3755293927
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/FileValueWriter.java:
##########
@@ -0,0 +1,197 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.parquet.hadoop;
+
+import java.io.IOException;
+import org.apache.parquet.bytes.BytesInput;
+import org.apache.parquet.format.BlockCipher;
+import org.apache.parquet.io.api.Binary;
+
+/**
+ * Decides how a {@code FILE} value's payload is stored: inline in the value, or out of line as a
+ * self-reference within the same Parquet file.
+ *
+ * <p>Object models hand over the resolved (logical) bytes and receive back a {@link Placement}
+ * describing which fields of the {@code FILE} group to write. Callers do not choose between the two
+ * forms themselves; the choice follows the configured threshold, so the same writing code produces
+ * either form:
+ *
+ * <pre>{@code
+ * FileValueWriter.Placement placement = fileValueWriter.write(payload);
+ * if (placement.isInline()) {
+ * group.add("inline", placement.getInlineBytes());
+ * } else {
+ * group.add("offset", placement.getOffset());
+ * group.add("size", placement.getSize());
+ * }
+ * }</pre>
+ *
+ * <p>Both forms describe the same logical bytes, so {@code content_type} and {@code checksum} are
+ * written identically either way — they describe the resolved bytes, not the storage. Consumers see
+ * no difference beyond which fields are set.
+ *
+ * <p>A self-reference payload is written immediately, while the record is being written and before
+ * the row group's column chunks are flushed. It therefore lands in a contiguous run ahead of those
+ * chunks, leaving each column chunk contiguous on disk. Writing eagerly is what makes the offset
+ * knowable in time: {@code offset} and {@code size} are ordinary column values, and once a value has
+ * been handed to a column writer it is encoded into a buffered page and cannot be revised, so a
+ * placeholder could never be patched up later.
+ *
+ * @see SelfReferenceStorage
+ */
+public class FileValueWriter {
+
+ /**
+ * Where a {@code FILE} value's payload was placed, and therefore which fields of the {@code FILE}
+ * group the caller should write. Either the payload is inline, or it is a self-reference located by
+ * {@code offset} and {@code size}.
+ */
+ public static final class Placement {
+ private final Binary inlineBytes;
+ private final long offset;
+ private final long size;
+
+ private Placement(Binary inlineBytes, long offset, long size) {
+ this.inlineBytes = inlineBytes;
+ this.offset = offset;
+ this.size = size;
+ }
+
+ static Placement inline(Binary inlineBytes) {
+ return new Placement(inlineBytes, -1, -1);
+ }
+
+ static Placement selfReference(SelfReferenceStorage.StoredRange range) {
+ return new Placement(null, range.getOffset(), range.getSize());
+ }
+
+ /** Whether the payload is stored inline, i.e. whether the {@code inline} field should be set. */
+ public boolean isInline() {
+ return inlineBytes != null;
+ }
+
+ /**
+ * The bytes to write to the {@code inline} field.
+ *
+ * @throws IllegalStateException if the payload was stored as a self-reference
+ */
+ public Binary getInlineBytes() {
+ if (!isInline()) {
+ throw new IllegalStateException("Payload was stored as a self-reference, not inline");
+ }
+ return inlineBytes;
+ }
+
+ /**
+ * The value to write to the {@code offset} field.
+ *
+ * @throws IllegalStateException if the payload was stored inline
+ */
+ public long getOffset() {
+ if (isInline()) {
+ throw new IllegalStateException("Payload was stored inline; it has no offset");
+ }
+ return offset;
+ }
+
+ /**
+ * The value to write to the {@code size} field. This is the size of the stored representation
+ * after compression and encryption, not the size of the resolved bytes.
+ *
+ * @throws IllegalStateException if the payload was stored inline
+ */
+ public long getSize() {
+ if (isInline()) {
+ throw new IllegalStateException("Payload was stored inline; it has no size");
+ }
+ return size;
+ }
+ }
+
+ private final ParquetFileWriter fileWriter;
+ private final CodecFactory.BytesCompressor inlineColumnCompressor;
+ private final BlockCipher.Encryptor inlineColumnEncryptor;
+ private final int inlineColumnOrdinal;
+ private final int selfReferenceThreshold;
+
+ /**
+ * @param fileWriter the writer for the file being written; a block must be open when
+ * {@link #write} is called
+ * @param inlineColumnCompressor the compressor for the {@code inline} column chunk's codec, whose
+ * compression a self-reference inherits
+ * @param inlineColumnEncryptor the data-module encryptor of the {@code inline} column chunk, or
+ * {@code null} if that column chunk is not encrypted
+ * @param inlineColumnOrdinal the ordinal of the {@code inline} column within the schema. The
+ * schema must declare {@code inline}: it is the reference point whose compression and
+ * encryption a self-reference inherits, so a schema without it can only store payloads inline
+ * or as external references. Note that the schema builder does not require {@code inline} for
+ * groups that declare {@code uri}, so an external-reference schema may reach here; pair such a
+ * schema with a threshold of {@link Integer#MAX_VALUE} so nothing is stored out of line.
+ * @param selfReferenceThreshold payloads of at most this many bytes are stored inline; larger ones
+ * become self-references. See
+ * {@code ParquetProperties.Builder#withFileSelfReferenceThreshold(int)}.
+ */
+ public FileValueWriter(
+ ParquetFileWriter fileWriter,
+ CodecFactory.BytesCompressor inlineColumnCompressor,
+ BlockCipher.Encryptor inlineColumnEncryptor,
+ int inlineColumnOrdinal,
+ int selfReferenceThreshold) {
+ if (selfReferenceThreshold < 0) {
+ throw new IllegalArgumentException(
+ "Self-reference threshold must not be negative: " + selfReferenceThreshold);
+ }
+ if (inlineColumnOrdinal < 0) {
+ throw new IllegalArgumentException("Invalid inline column ordinal: " + inlineColumnOrdinal);
+ }
+ this.fileWriter = fileWriter;
+ this.inlineColumnCompressor = inlineColumnCompressor;
+ this.inlineColumnEncryptor = inlineColumnEncryptor;
+ this.inlineColumnOrdinal = inlineColumnOrdinal;
+ this.selfReferenceThreshold = selfReferenceThreshold;
+ }
+
+ /**
+ * Stores {@code payload} and returns which {@code FILE} group fields to write for it. Payloads at
+ * or below the configured threshold are returned for inline storage; larger ones are written to the
+ * file body immediately as self-references.
+ *
+ * <p>Must be called while a block is open on the underlying writer, and before that block's column
+ * chunks are flushed.
+ *
+ * @param payload the resolved (logical) bytes of the value
+ * @return the placement describing which fields to write
+ * @throws IOException if writing the self-reference payload fails
+ */
+ public Placement write(Binary payload) throws IOException {
Review Comment:
Also, it seems like this should also take in media type?
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4426854036
##########
parquet-column/src/main/java/org/apache/parquet/schema/LogicalTypeAnnotation.java:
##########
Review Comment:
Added [testFILEAnnotation](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-column/src/test/java/org/apache/parquet/parser/TestParquetParser.java#L70) with the URI-only text schema you suggested. It checks the parsed schema against the builder API and verifies that serializing and reparsing preserves it.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4426883613
##########
parquet-hadoop/src/test/java/org/apache/parquet/file/TestFileValueReadWrite.java:
##########
@@ -0,0 +1,288 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.parquet.io.api;
+
+import static org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.BINARY;
+import static org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.INT64;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Map;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.parquet.DirectWriterTest;
+import org.apache.parquet.conf.ParquetConfiguration;
+import org.apache.parquet.hadoop.ParquetReader;
+import org.apache.parquet.hadoop.ParquetWriter;
+import org.apache.parquet.hadoop.api.ReadSupport;
+import org.apache.parquet.hadoop.api.WriteSupport;
+import org.apache.parquet.schema.GroupType;
+import org.apache.parquet.schema.LogicalTypeAnnotation;
+import org.apache.parquet.schema.MessageType;
+import org.apache.parquet.schema.Types;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class TestFileValueReadWrite extends DirectWriterTest {
+ private static final GroupType FILE_SCHEMA = Types.optionalGroup()
+ .as(LogicalTypeAnnotation.fileType())
+ .optional(BINARY)
+ .as(LogicalTypeAnnotation.stringType())
+ .named("uri")
+ .optional(INT64)
+ .named("offset")
+ .optional(INT64)
+ .named("size")
+ .optional(BINARY)
+ .as(LogicalTypeAnnotation.stringType())
+ .named("content_type")
+ .optional(BINARY)
+ .as(LogicalTypeAnnotation.stringType())
+ .named("checksum")
+ .optional(BINARY)
+ .named("inline")
+ .named("file");
+ private static final MessageType SCHEMA = new MessageType("message", FILE_SCHEMA);
+
+ @TempDir
+ private java.nio.file.Path tempDir;
+
+ @Test
+ public void testFileValueRoundTrip() throws IOException {
+ FileValue[] expectedValues = {
+ FileValue.builder()
+ .withUri("s3://bucket/file")
+ .withOffset(10)
+ .withSize(20)
+ .withContentType("image/png")
+ .withChecksum("ETAG:abc")
+ .withInline(Binary.fromString("bytes"))
+ .build(),
+ FileValue.builder().withInline(Binary.fromString("inline only")).build(),
+ null,
+ FileValue.builder().withUri("relative/path").build(),
+ FileValue.builder().withUri("file").withOffset(0).withSize(0).build(),
+ FileValue.builder()
+ .withInline(Binary.fromConstantByteArray(new byte[0]))
+ .build()
+ };
+ Path path = new Path(tempDir.resolve("files.parquet").toUri());
+ try (ParquetWriter<FileValue> writer = new ParquetWriter<>(path, new FileWriteSupport())) {
+ for (FileValue value : expectedValues) {
+ writer.write(value);
+ }
+ }
+
+ try (ParquetReader<FileRecord> reader =
+ ParquetReader.builder(new FileReadSupport(), path).build()) {
+ for (FileValue expected : expectedValues) {
+ FileRecord record = reader.read();
+ assertThat(record).isNotNull();
+ if (expected == null) {
+ assertThat(record.value).isNull();
+ } else {
+ FileValue actual = record.value;
+ assertThat(actual).isNotNull();
+ assertThat(actual.getUri()).isEqualTo(expected.getUri());
+ assertThat(actual.getOffset()).isEqualTo(expected.getOffset());
+ assertThat(actual.getSize()).isEqualTo(expected.getSize());
+ assertThat(actual.getContentType()).isEqualTo(expected.getContentType());
+ assertThat(actual.getChecksum()).isEqualTo(expected.getChecksum());
+ assertThat(actual.getInline()).isEqualTo(expected.getInline());
+ }
+ }
+ assertThat(reader.read()).isNull();
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testInvalidStoredFileValueConvertsToNull(boolean sizeWithoutUri) throws IOException {
+ Path path = writeDirect(SCHEMA, consumer -> {
+ consumer.startMessage();
+ consumer.startField("file", 0);
+ consumer.startGroup();
+ if (sizeWithoutUri) {
+ consumer.startField("size", FILE_SCHEMA.getFieldIndex("size"));
+ consumer.addLong(2);
+ consumer.endField("size", FILE_SCHEMA.getFieldIndex("size"));
+ }
+ consumer.startField("content_type", FILE_SCHEMA.getFieldIndex("content_type"));
+ consumer.addBinary(Binary.fromString("image/png"));
+ consumer.endField("content_type", FILE_SCHEMA.getFieldIndex("content_type"));
+ if (sizeWithoutUri) {
+ consumer.startField("inline", FILE_SCHEMA.getFieldIndex("inline"));
+ consumer.addBinary(Binary.fromString("bytes"));
+ consumer.endField("inline", FILE_SCHEMA.getFieldIndex("inline"));
+ }
+ consumer.endGroup();
+ consumer.endField("file", 0);
+ consumer.endMessage();
+ });
+
+ try (ParquetReader<FileRecord> reader =
+ ParquetReader.builder(new FileReadSupport(), path).build()) {
+ assertThat(reader.read().value).isNull();
+ assertThat(reader.read()).isNull();
+ }
+ }
+
+ @Test
+ public void testFileConverterRejectsPartialProjectionDuringReaderAssembly() throws IOException {
+ Path path = writeDirect(SCHEMA, consumer -> {
+ consumer.startMessage();
+ consumer.startField("file", 0);
+ FileValueWriter.write(
+ consumer, FILE_SCHEMA, FileValue.builder().withUri("file").build());
+ consumer.endField("file", 0);
+ consumer.endMessage();
+ });
+ MessageType requestedSchema = new MessageType("message", FILE_SCHEMA.withNewFields(FILE_SCHEMA.getType("uri")));
+
+ try (ParquetReader<FileRecord> reader = ParquetReader.builder(new FileReadSupport(requestedSchema), path)
+ .build()) {
+ assertThatThrownBy(reader::read)
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("FILE group 'file' cannot project stored field 'offset'");
+ }
+ }
+
+ @Test
+ public void testFileConverterReadsReorderedSchemaWithAddedFields() throws IOException {
+ GroupType storedFile = FILE_SCHEMA.withNewFields(FILE_SCHEMA.getType("uri"));
+ Path path = writeDirect(new MessageType("message", storedFile), consumer -> {
+ consumer.startMessage();
+ consumer.startField("file", 0);
+ FileValueWriter.write(
+ consumer, storedFile, FileValue.builder().withUri("file").build());
+ consumer.endField("file", 0);
+ consumer.endMessage();
+ });
+ MessageType requestedSchema = new MessageType(
+ "message", FILE_SCHEMA.withNewFields(FILE_SCHEMA.getType("inline"), FILE_SCHEMA.getType("uri")));
+
+ try (ParquetReader<FileRecord> reader = ParquetReader.builder(new FileReadSupport(requestedSchema), path)
+ .build()) {
+ FileValue actual = reader.read().value;
+ assertThat(actual.getUri()).isEqualTo("file");
+ assertThat(actual.getInline()).isNull();
+ assertThat(reader.read()).isNull();
+ }
+ }
+
+ private static final class FileRecord {
+ private final FileValue value;
+
+ private FileRecord(FileValue value) {
+ this.value = value;
+ }
+ }
+
+ private static final class FileReadSupport extends ReadSupport<FileRecord> {
+ private final MessageType requestedSchema;
+
+ private FileReadSupport() {
+ this(null);
+ }
+
+ private FileReadSupport(MessageType requestedSchema) {
+ this.requestedSchema = requestedSchema;
+ }
+
+ @Override
+ public ReadContext init(
+ ParquetConfiguration configuration, Map<String, String> metadata, MessageType fileSchema) {
+ return new ReadContext(requestedSchema == null ? fileSchema : requestedSchema);
+ }
+
+ @Override
+ public RecordMaterializer<FileRecord> prepareForRead(
+ Configuration configuration,
+ Map<String, String> metadata,
+ MessageType fileSchema,
+ ReadContext readContext) {
+ return new FileRecordMaterializer(fileSchema, readContext.getRequestedSchema());
+ }
+ }
+
+ private static final class FileWriteSupport extends WriteSupport<FileValue> {
+ private RecordConsumer consumer;
+
+ @Override
+ public WriteContext init(Configuration configuration) {
+ return new WriteContext(SCHEMA, Collections.emptyMap());
+ }
+
+ @Override
+ public void prepareForWrite(RecordConsumer recordConsumer) {
+ consumer = recordConsumer;
+ }
+
+ @Override
+ public void write(FileValue value) {
+ consumer.startMessage();
+ if (value != null) {
+ consumer.startField("file", 0);
+ FileValueWriter.write(consumer, FILE_SCHEMA, value);
Review Comment:
Moved field validation into the [GroupType constructor](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-column/src/main/java/org/apache/parquet/schema/GroupType.java#L117). FILE groups retain an immutable copy of their validated fields, so subsequent writes and converter construction can use that schema without rescanning its field definitions. Per-record checks still validate the actual value and ensure its set fields exist in the schema. Constructor/replacement and immutability tests cover this in [TestTypeBuildersWithLogicalTypes](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-column/src/test/java/org/apache/parquet/schema/TestTypeBuildersWithLogicalTypes.java#L900).
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4426836436
##########
parquet-column/src/test/java/org/apache/parquet/schema/TestTypeBuildersWithLogicalTypes.java:
##########
Review Comment:
Added coverage in [TestFileValueWriter](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-column/src/test/java/org/apache/parquet/file/Test
FileValueWriter.java#L129) for offset without size, offset/size without a URI, and negative offset/size. [TestFileValueReadWrite](https://github.com/apache/parquet-java/blob/bab237a1
f7b57457052f520f7cd5dae84b211e16/parquet-hadoop/src/test/java/org/apache/parquet/file/TestFileValueReadWrite.java#L76) round-trips valid values, including a zero offset and zero size
, and checks that invalid stored references convert to null. The shared checks live in [FileValueValidator.validateValue](https://github.com/apache/parquet-java/blob/bab237a1f7b57457
052f520f7cd5dae84b211e16/parquet-column/src/main/java/org/apache/parquet/file/FileValueValidator.java#L35), called before writing and after assembling a value on read.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "divjotarora (via GitHub)" <gi...@apache.org>.
divjotarora commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r3393851721
##########
parquet-column/src/main/java/org/apache/parquet/schema/LogicalTypeAnnotation.java:
##########
@@ -1221,6 +1231,55 @@ public boolean equals(Object obj) {
}
}
+ /**
+ * File logical type annotation. Annotates a group (struct) that represents a reference to
+ * an external file. The group must contain the following fields by name:
+ * <ul>
+ * <li>{@code path} (required): STRING - the path/URI of the file</li>
+ * <li>{@code size} (optional): INT64 - size of the file content in bytes</li>
+ * <li>{@code offset} (optional): INT64 - byte offset within the file; if present, size must be present</li>
+ * <li>{@code etag} (optional): STRING - opaque identifier for the file version</li>
+ * </ul>
+ * No optional fields with names other than the above are permitted.
Review Comment:
```suggestion
* No fields other than the above are permitted.
```
##########
parquet-column/src/main/java/org/apache/parquet/schema/Types.java:
##########
@@ -821,12 +821,34 @@ 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) {
Review Comment:
Where do we validate:
1. If `offset` is present, `size` must be present
2. `offset` and `size` must be non-negative (note spec doesn't technically say `offset` must be >= 0, but I left a suggestion on that change to add it
##########
parquet-column/src/test/java/org/apache/parquet/schema/TestTypeBuildersWithLogicalTypes.java:
##########
Review Comment:
We should add tests for validity of size/offset fields, both their values (>= 0) and valid/invalid combinations
##########
parquet-column/src/main/java/org/apache/parquet/schema/Types.java:
##########
@@ -821,12 +821,34 @@ 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 hasPath = false;
+ for (Type field : fields) {
+ String fieldName = field.getName();
+ if (LogicalTypeAnnotation.FileLogicalTypeAnnotation.PATH_FIELD.equals(fieldName)) {
+ Preconditions.checkArgument(
+ field.getRepetition() == Type.Repetition.REQUIRED,
+ "FILE type field 'path' must be REQUIRED in group '%s'",
+ name);
+ hasPath = true;
+ } else if (!LogicalTypeAnnotation.FileLogicalTypeAnnotation.OPTIONAL_FIELD_NAMES.contains(fieldName)) {
+ throw new IllegalArgumentException(
+ "FILE type group '" + name + "' contains unrecognized field '" + fieldName
+ + "'. Valid fields are: path, size, offset, etag");
Review Comment:
nit: stringify `FileLogicalTypeAnnotation.OPTIONAL_FIELD_NAMES` and use it in the error to avoid drift if we add new fields?
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#issuecomment-6198552277
Thanks @divjotarora great point! Added checks in parquet-column/src/main/java/org/apache/parquet/io/ColumnIOFactory.java that avoid partial projections of FileType. It should be treated atomically
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r3755199268
##########
parquet-hadoop/src/test/java/org/apache/parquet/hadoop/TestFileValueWriter.java:
##########
@@ -0,0 +1,274 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.parquet.hadoop;
+
+import static org.apache.parquet.hadoop.ParquetFileWriter.Mode.CREATE;
+import static org.apache.parquet.hadoop.ParquetWriter.DEFAULT_BLOCK_SIZE;
+import static org.apache.parquet.hadoop.ParquetWriter.MAX_PADDING_SIZE_DEFAULT;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.parquet.ParquetReadOptions;
+import org.apache.parquet.bytes.BytesInput;
+import org.apache.parquet.column.ColumnDescriptor;
+import org.apache.parquet.column.Encoding;
+import org.apache.parquet.column.ParquetProperties;
+import org.apache.parquet.column.statistics.Statistics;
+import org.apache.parquet.hadoop.metadata.BlockMetaData;
+import org.apache.parquet.hadoop.metadata.ColumnChunkMetaData;
+import org.apache.parquet.hadoop.metadata.ColumnPath;
+import org.apache.parquet.hadoop.metadata.CompressionCodecName;
+import org.apache.parquet.hadoop.util.HadoopInputFile;
+import org.apache.parquet.hadoop.util.HadoopOutputFile;
+import org.apache.parquet.io.InputFile;
+import org.apache.parquet.io.api.Binary;
+import org.apache.parquet.schema.MessageType;
+import org.apache.parquet.schema.MessageTypeParser;
+import org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName;
+import org.apache.parquet.schema.Types;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+/**
+ * Tests that {@link FileValueWriter} routes a {@code FILE} payload to inline storage or to a
+ * self-reference according to the configured threshold, and that both forms describe the same logical
+ * bytes.
+ */
+public class TestFileValueWriter {
+
+ private static final MessageType SCHEMA = MessageTypeParser.parseMessageType("message m {"
+ + " optional group file (FILE) {"
+ + " optional int64 offset;"
+ + " optional int64 size;"
+ + " optional binary inline;"
+ + " }"
+ + "}");
+
+ private static final ColumnDescriptor INLINE_COLUMN = SCHEMA.getColumnDescription(new String[] {"file", "inline"});
+
+ private static final CompressionCodecName CODEC = CompressionCodecName.SNAPPY;
+
+ private static final Statistics<?> EMPTY_STATS = Statistics.getBuilderForReading(
+ Types.required(PrimitiveTypeName.BINARY).named("inline"))
+ .build();
+
+ @TempDir
+ java.nio.file.Path tempDir;
+
+ @Test
+ public void testDefaultThresholdIsPageSize() {
+ assertThat(ParquetProperties.builder().build().getFileSelfReferenceThreshold())
+ .isEqualTo(ParquetProperties.DEFAULT_PAGE_SIZE);
+ }
+
+ @Test
+ public void testThresholdMustNotBeNegative() {
+ assertThatThrownBy(() -> ParquetProperties.builder().withFileSelfReferenceThreshold(-1))
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void testPayloadAtThresholdIsInlinedAndAboveIsSelfReference() throws IOException {
+ Configuration conf = new Configuration();
+ Path path = new Path(tempDir.resolve("routing.parquet").toUri());
+ CodecFactory codecFactory = new CodecFactory(conf, DEFAULT_BLOCK_SIZE);
+
+ int threshold = 64;
+ Binary atThreshold = Binary.fromConstantByteArray(payload(threshold));
+ Binary aboveThreshold = Binary.fromConstantByteArray(payload(threshold + 1));
+
+ ParquetFileWriter writer = new ParquetFileWriter(
+ HadoopOutputFile.fromPath(path, conf), SCHEMA, CREATE, DEFAULT_BLOCK_SIZE, MAX_PADDING_SIZE_DEFAULT);
+ writer.start();
+ writer.startBlock(2);
+
+ FileValueWriter valueWriter = new FileValueWriter(
+ writer, codecFactory.getCompressor(CODEC), null, columnOrdinalOf(INLINE_COLUMN), threshold);
+
+ FileValueWriter.Placement inlined = valueWriter.write(atThreshold);
+ FileValueWriter.Placement outOfLine = valueWriter.write(aboveThreshold);
+
+ // A payload exactly at the threshold stays inline; one byte more goes out of line.
+ assertThat(inlined.isInline()).isTrue();
+ assertThat(inlined.getInlineBytes()).isEqualTo(atThreshold);
+ assertThatThrownBy(inlined::getOffset).isInstanceOf(IllegalStateException.class);
+
+ assertThat(outOfLine.isInline()).isFalse();
+ assertThat(outOfLine.getSize()).isGreaterThan(0L);
+ assertThatThrownBy(outOfLine::getInlineBytes).isInstanceOf(IllegalStateException.class);
+
+ // Write the inline column chunk so the reader has metadata carrying the inherited codec.
+ writer.startColumn(INLINE_COLUMN, 1, CODEC);
+ writer.writeDataPage(
+ 1,
+ (int) inlined.getInlineBytes().length(),
+ codecFactory
+ .getCompressor(CODEC)
+ .compress(BytesInput.from(inlined.getInlineBytes().toByteBuffer())),
+ EMPTY_STATS,
+ Encoding.BIT_PACKED,
+ Encoding.BIT_PACKED,
+ Encoding.PLAIN);
+ writer.endColumn();
+ writer.endBlock();
+ writer.end(new java.util.HashMap<>());
+
+ // The out-of-line payload resolves back to the original bytes.
+ InputFile inputFile = HadoopInputFile.fromPath(path, conf);
+ try (ParquetFileReader reader =
+ ParquetFileReader.open(inputFile, ParquetReadOptions.builder().build())) {
+ BlockMetaData block = reader.getFooter().getBlocks().get(0);
+ ColumnChunkMetaData inlineMeta = findColumn(block, INLINE_COLUMN);
+ BytesInput resolved = reader.resolveSelfReference(inlineMeta, outOfLine.getOffset(), outOfLine.getSize());
+ assertThat(resolved.toByteArray()).isEqualTo(aboveThreshold.getBytes());
+ }
+ codecFactory.release();
+ }
+
+ @Test
+ public void testZeroThresholdAlwaysUsesSelfReferences() throws IOException {
+ Configuration conf = new Configuration();
+ Path path = new Path(tempDir.resolve("always_out_of_line.parquet").toUri());
+ CodecFactory codecFactory = new CodecFactory(conf, DEFAULT_BLOCK_SIZE);
+
+ byte[][] payloads = {payload(1), payload(1000)};
+
+ ParquetFileWriter writer = new ParquetFileWriter(
+ HadoopOutputFile.fromPath(path, conf), SCHEMA, CREATE, DEFAULT_BLOCK_SIZE, MAX_PADDING_SIZE_DEFAULT);
+ writer.start();
+ writer.startBlock(payloads.length);
+
+ FileValueWriter valueWriter =
+ new FileValueWriter(writer, codecFactory.getCompressor(CODEC), null, columnOrdinalOf(INLINE_COLUMN), 0);
+
+ List<FileValueWriter.Placement> placements = new ArrayList<>();
+ for (byte[] p : payloads) {
+ placements.add(valueWriter.write(Binary.fromConstantByteArray(p)));
+ }
+ // Every payload went out of line, including the single-byte one. An empty payload would still be
+ // inlined, since its length is not greater than the threshold.
+ assertThat(placements).allMatch(p -> !p.isInline());
+
+ writer.startColumn(INLINE_COLUMN, 0, CODEC);
+ writer.writeDataPage(
+ 0,
+ 0,
+ codecFactory.getCompressor(CODEC).compress(BytesInput.empty()),
+ EMPTY_STATS,
+ Encoding.BIT_PACKED,
+ Encoding.BIT_PACKED,
+ Encoding.PLAIN);
+ writer.endColumn();
+ writer.endBlock();
+ writer.end(new java.util.HashMap<>());
+
+ InputFile inputFile = HadoopInputFile.fromPath(path, conf);
+ try (ParquetFileReader reader =
+ ParquetFileReader.open(inputFile, ParquetReadOptions.builder().build())) {
+ BlockMetaData block = reader.getFooter().getBlocks().get(0);
+ ColumnChunkMetaData inlineMeta = findColumn(block, INLINE_COLUMN);
+ for (int i = 0; i < payloads.length; i++) {
+ FileValueWriter.Placement placement = placements.get(i);
Review Comment:
again, this seems to be going through an in-memory structure.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4426857500
##########
parquet-column/src/main/java/org/apache/parquet/schema/LogicalTypeAnnotation.java:
##########
@@ -1229,6 +1240,151 @@ public boolean equals(Object obj) {
}
}
+ /**
+ * File logical type annotation. Annotates a group (struct) that represents a reference to a
+ * range of bytes, which may be stored inline in the value or in an external file. Every field is
+ * optional, both in the schema (a writer may omit any field from the group definition) and in the
+ * data (any field that is present has a field repetition type of {@code OPTIONAL}). Fields are
+ * identified by name (case sensitively), not by field order. A group need only define the fields
+ * it uses. The group may contain the following fields:
+ * <ul>
+ * <li>{@code uri} (STRING): a URI-reference (RFC 3986) that identifies an external file, for
+ * example {@code s3://bucket/file.jpg}.</li>
+ * <li>{@code offset} (INT64): start of the byte range within the external file identified by
+ * {@code uri}; if not set, treated as 0. Must not be negative and may only be set together
+ * with {@code uri}.</li>
+ * <li>{@code size} (INT64): byte length of the referenced data within the external file. May be
+ * omitted for a whole-file external reference, in which case the range runs to the end of
+ * the referenced file. May only be set together with {@code uri}, must be set whenever
+ * {@code offset} is set, and must not be negative.</li>
+ * <li>{@code content_type} (STRING): the media (MIME) type (RFC 2046) of the resolved bytes;
+ * when not set, {@code application/octet-stream} is assumed.</li>
+ * <li>{@code checksum} (STRING): a self-describing integrity token for the resolved bytes, of
+ * the form {@code <algorithm>:<digest>}.</li>
+ * <li>{@code inline} (BYTE_ARRAY): the referenced bytes stored inline in the value.</li>
+ * </ul>
+ * No fields with names other than the above are permitted, and field names must be unique. Each
+ * declared field must match its required physical and logical types. Rules involving which fields
+ * are set and their values are enforced per value rather than by the schema.
+ */
+ public static class FileLogicalTypeAnnotation extends LogicalTypeAnnotation {
+ private static final FileLogicalTypeAnnotation INSTANCE = new FileLogicalTypeAnnotation();
+
+ /** Field name holding the URI-reference of an external file. */
+ public static final String URI_FIELD = "uri";
+
+ /** Field name holding the start of the byte range. */
+ public static final String OFFSET_FIELD = "offset";
+
+ /** Field name holding the byte length of the referenced data. */
+ public static final String SIZE_FIELD = "size";
+
+ /** Field name holding the media (MIME) type of the resolved bytes. */
+ public static final String CONTENT_TYPE_FIELD = "content_type";
+
+ /** Field name holding the integrity token for the resolved bytes. */
+ public static final String CHECKSUM_FIELD = "checksum";
+
+ /** Field name holding the referenced bytes stored inline. */
+ public static final String INLINE_FIELD = "inline";
+
+ /** All recognized field names in a FILE-annotated group. All fields are optional. */
+ public static final Set<String> FIELD_NAMES =
+ Set.of(URI_FIELD, OFFSET_FIELD, SIZE_FIELD, CONTENT_TYPE_FIELD, CHECKSUM_FIELD, INLINE_FIELD);
+
+ private FileLogicalTypeAnnotation() {}
+
+ /** Validates the fields of a FILE-annotated group. */
+ public void validate(GroupType group) {
+ validateFields(group.getName(), group.getFields());
+ }
+
+ void validateFields(String groupName, List<Type> fields) {
Review Comment:
Schema validation now has a single construction-time path: the [GroupType constructor](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-co
lumn/src/main/java/org/apache/parquet/schema/GroupType.java#L117) calls `FileLogicalTypeAnnotation.validateFields`. Removed the repeated schema-validation calls from `FileValueValidator`, `Types`, and metadata serialization. [FileValueValidator.validateValue](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-column/src/main/java/org/apache/parquet/file/FileValueValidator.java#L35) retains the value-dependent locator/range checks shared by the reader and writer; its schema check now only requires the FILE annotation.
##########
parquet-column/src/main/java/org/apache/parquet/schema/LogicalTypeAnnotation.java:
##########
@@ -1229,6 +1240,151 @@ public boolean equals(Object obj) {
}
}
+ /**
+ * File logical type annotation. Annotates a group (struct) that represents a reference to a
+ * range of bytes, which may be stored inline in the value or in an external file. Every field is
+ * optional, both in the schema (a writer may omit any field from the group definition) and in the
+ * data (any field that is present has a field repetition type of {@code OPTIONAL}). Fields are
+ * identified by name (case sensitively), not by field order. A group need only define the fields
+ * it uses. The group may contain the following fields:
+ * <ul>
+ * <li>{@code uri} (STRING): a URI-reference (RFC 3986) that identifies an external file, for
+ * example {@code s3://bucket/file.jpg}.</li>
+ * <li>{@code offset} (INT64): start of the byte range within the external file identified by
+ * {@code uri}; if not set, treated as 0. Must not be negative and may only be set together
+ * with {@code uri}.</li>
+ * <li>{@code size} (INT64): byte length of the referenced data within the external file. May be
+ * omitted for a whole-file external reference, in which case the range runs to the end of
+ * the referenced file. May only be set together with {@code uri}, must be set whenever
+ * {@code offset} is set, and must not be negative.</li>
+ * <li>{@code content_type} (STRING): the media (MIME) type (RFC 2046) of the resolved bytes;
+ * when not set, {@code application/octet-stream} is assumed.</li>
+ * <li>{@code checksum} (STRING): a self-describing integrity token for the resolved bytes, of
+ * the form {@code <algorithm>:<digest>}.</li>
+ * <li>{@code inline} (BYTE_ARRAY): the referenced bytes stored inline in the value.</li>
+ * </ul>
+ * No fields with names other than the above are permitted, and field names must be unique. Each
+ * declared field must match its required physical and logical types. Rules involving which fields
+ * are set and their values are enforced per value rather than by the schema.
+ */
+ public static class FileLogicalTypeAnnotation extends LogicalTypeAnnotation {
+ private static final FileLogicalTypeAnnotation INSTANCE = new FileLogicalTypeAnnotation();
+
+ /** Field name holding the URI-reference of an external file. */
+ public static final String URI_FIELD = "uri";
+
+ /** Field name holding the start of the byte range. */
+ public static final String OFFSET_FIELD = "offset";
+
+ /** Field name holding the byte length of the referenced data. */
+ public static final String SIZE_FIELD = "size";
+
+ /** Field name holding the media (MIME) type of the resolved bytes. */
+ public static final String CONTENT_TYPE_FIELD = "content_type";
+
+ /** Field name holding the integrity token for the resolved bytes. */
+ public static final String CHECKSUM_FIELD = "checksum";
+
+ /** Field name holding the referenced bytes stored inline. */
+ public static final String INLINE_FIELD = "inline";
+
+ /** All recognized field names in a FILE-annotated group. All fields are optional. */
+ public static final Set<String> FIELD_NAMES =
+ Set.of(URI_FIELD, OFFSET_FIELD, SIZE_FIELD, CONTENT_TYPE_FIELD, CHECKSUM_FIELD, INLINE_FIELD);
+
+ private FileLogicalTypeAnnotation() {}
+
+ /** Validates the fields of a FILE-annotated group. */
+ public void validate(GroupType group) {
+ validateFields(group.getName(), group.getFields());
+ }
+
+ void validateFields(String groupName, List<Type> fields) {
Review Comment:
Schema validation now has a single construction-time path: the [GroupType constructor](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-column/src/main/java/org/apache/parquet/schema/GroupType.java#L117) calls `FileLogicalTypeAnnotation.validateFields`. Removed the repeated schema-validation calls from `FileValueValidator`, `Types`, and metadata serialization. [FileValueValidator.validateValue](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-column/src/main/java/org/apache/parquet/file/FileValueValidator.java#L35) retains the value-dependent locator/range checks shared by the reader and writer; its schema check now only requires the FILE annotation.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4426854036
##########
parquet-column/src/main/java/org/apache/parquet/schema/LogicalTypeAnnotation.java:
##########
Review Comment:
Added [testFILEAnnotation](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-column/src/test/java/org/apache/parquet/parser/TestParquetPars
er.java#L70) with the URI-only text schema you suggested. It checks the parsed schema against the builder API and verifies that serializing and reparsing preserves it.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "divjotarora (via GitHub)" <gi...@apache.org>.
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4426828606
##########
parquet-column/src/main/java/org/apache/parquet/schema/Types.java:
##########
@@ -821,12 +821,34 @@ 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 hasPath = false;
+ for (Type field : fields) {
+ String fieldName = field.getName();
+ if (LogicalTypeAnnotation.FileLogicalTypeAnnotation.PATH_FIELD.equals(fieldName)) {
+ Preconditions.checkArgument(
+ field.getRepetition() == Type.Repetition.REQUIRED,
+ "FILE type field 'path' must be REQUIRED in group '%s'",
+ name);
+ hasPath = true;
+ } else if (!LogicalTypeAnnotation.FileLogicalTypeAnnotation.OPTIONAL_FIELD_NAMES.contains(fieldName)) {
+ throw new IllegalArgumentException(
+ "FILE type group '" + name + "' contains unrecognized field '" + fieldName
+ + "'. Valid fields are: path, size, offset, etag");
Review Comment:
Addressed: the unrecognized-field error now uses `String.join(", ", FIELD_NAMES)` from `FileLogicalTypeAnnotation.FIELD_NAMES`, which is also the set used to validate names. The diag
nostic will therefore follow changes to the supported fields. See [validateFields](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-column
/src/main/java/org/apache/parquet/schema/LogicalTypeAnnotation.java#L1297).
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4426846446
##########
parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java:
##########
@@ -2184,7 +2194,15 @@ && getLogicalTypeAnnotation(schemaElement.logicalType) != null)
childBuilder.id(schemaElement.field_id);
}
- childBuilder.named(schemaElement.name);
+ try {
+ childBuilder.named(schemaElement.name);
+ } catch (IllegalArgumentException e) {
+ if (!fileLogicalType) {
+ throw e;
+ }
+ LOG.warn("Dropping malformed FILE logical type annotation on group {}", schemaElement.name, e);
+ childBuilder.as((LogicalTypeAnnotation) null).named(schemaElement.name);
+ }
Review Comment:
Removed the FILE-specific catch that dropped the annotation. Invalid fields in a FILE-annotated group now propagate the schema-validation error. FILE on a primitive still follows the
existing unsupported-annotation handling. [testMalformedFileLogicalTypeFailsOnRead and testFileLogicalTypeOnPrimitiveIsIgnored](https://github.com/apache/parquet-java/blob/bab237a1f
7b57457052f520f7cd5dae84b211e16/parquet-hadoop/src/test/java/org/apache/parquet/format/converter/TestParquetMetadataConverter.java#L2506) cover both behaviors.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "divjotarora (via GitHub)" <gi...@apache.org>.
divjotarora commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r3393851721
##########
parquet-column/src/main/java/org/apache/parquet/schema/LogicalTypeAnnotation.java:
##########
@@ -1221,6 +1231,55 @@ public boolean equals(Object obj) {
}
}
+ /**
+ * File logical type annotation. Annotates a group (struct) that represents a reference to
+ * an external file. The group must contain the following fields by name:
+ * <ul>
+ * <li>{@code path} (required): STRING - the path/URI of the file</li>
+ * <li>{@code size} (optional): INT64 - size of the file content in bytes</li>
+ * <li>{@code offset} (optional): INT64 - byte offset within the file; if present, size must be present</li>
+ * <li>{@code etag} (optional): STRING - opaque identifier for the file version</li>
+ * </ul>
+ * No optional fields with names other than the above are permitted.
Review Comment:
```suggestion
* No fields other than the above are permitted.
```
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4402344025
##########
parquet-column/src/main/java/org/apache/parquet/schema/LogicalTypeAnnotation.java:
##########
@@ -1229,6 +1240,151 @@ public boolean equals(Object obj) {
}
}
+ /**
+ * File logical type annotation. Annotates a group (struct) that represents a reference to a
+ * range of bytes, which may be stored inline in the value or in an external file. Every field is
+ * optional, both in the schema (a writer may omit any field from the group definition) and in the
+ * data (any field that is present has a field repetition type of {@code OPTIONAL}). Fields are
+ * identified by name (case sensitively), not by field order. A group need only define the fields
+ * it uses. The group may contain the following fields:
+ * <ul>
+ * <li>{@code uri} (STRING): a URI-reference (RFC 3986) that identifies an external file, for
+ * example {@code s3://bucket/file.jpg}.</li>
+ * <li>{@code offset} (INT64): start of the byte range within the external file identified by
+ * {@code uri}; if not set, treated as 0. Must not be negative and may only be set together
+ * with {@code uri}.</li>
+ * <li>{@code size} (INT64): byte length of the referenced data within the external file. May be
+ * omitted for a whole-file external reference, in which case the range runs to the end of
+ * the referenced file. May only be set together with {@code uri}, must be set whenever
+ * {@code offset} is set, and must not be negative.</li>
+ * <li>{@code content_type} (STRING): the media (MIME) type (RFC 2046) of the resolved bytes;
+ * when not set, {@code application/octet-stream} is assumed.</li>
+ * <li>{@code checksum} (STRING): a self-describing integrity token for the resolved bytes, of
+ * the form {@code <algorithm>:<digest>}.</li>
+ * <li>{@code inline} (BYTE_ARRAY): the referenced bytes stored inline in the value.</li>
+ * </ul>
+ * No fields with names other than the above are permitted, and field names must be unique. Each
+ * declared field must match its required physical and logical types. Rules involving which fields
+ * are set and their values are enforced per value rather than by the schema.
+ */
+ public static class FileLogicalTypeAnnotation extends LogicalTypeAnnotation {
+ private static final FileLogicalTypeAnnotation INSTANCE = new FileLogicalTypeAnnotation();
+
+ /** Field name holding the URI-reference of an external file. */
+ public static final String URI_FIELD = "uri";
+
+ /** Field name holding the start of the byte range. */
+ public static final String OFFSET_FIELD = "offset";
+
+ /** Field name holding the byte length of the referenced data. */
+ public static final String SIZE_FIELD = "size";
+
+ /** Field name holding the media (MIME) type of the resolved bytes. */
+ public static final String CONTENT_TYPE_FIELD = "content_type";
+
+ /** Field name holding the integrity token for the resolved bytes. */
+ public static final String CHECKSUM_FIELD = "checksum";
+
+ /** Field name holding the referenced bytes stored inline. */
+ public static final String INLINE_FIELD = "inline";
+
+ /** All recognized field names in a FILE-annotated group. All fields are optional. */
+ public static final Set<String> FIELD_NAMES =
+ Set.of(URI_FIELD, OFFSET_FIELD, SIZE_FIELD, CONTENT_TYPE_FIELD, CHECKSUM_FIELD, INLINE_FIELD);
+
+ private FileLogicalTypeAnnotation() {}
+
+ /** Validates the fields of a FILE-annotated group. */
+ public void validate(GroupType group) {
+ validateFields(group.getName(), group.getFields());
+ }
+
+ void validateFields(String groupName, List<Type> fields) {
Review Comment:
This logic feels very similar to what's happening in `FileValueValidator`. Maybe we should put it into a single place?
##########
parquet-column/src/main/java/org/apache/parquet/io/api/FileConverters.java:
##########
@@ -0,0 +1,146 @@
+/*
Review Comment:
More a meta question, should we have a `file` package to store all the `File` annotation related logic?
##########
parquet-column/src/main/java/org/apache/parquet/io/api/FileConverters.java:
##########
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.parquet.io.api;
+
+import java.util.Objects;
+import java.util.function.Consumer;
+import org.apache.parquet.schema.GroupType;
+import org.apache.parquet.schema.LogicalTypeAnnotation;
+import org.apache.parquet.schema.Type;
+
+/** Converters for assembling values represented by a Parquet {@code FILE}-annotated group. */
+public final class FileConverters {
+ private FileConverters() {}
+
+ /**
+ * Creates a converter for a possibly projected FILE group.
+ *
+ * <p>FILE values are atomic and cannot be projected by child field. The projected schema may
+ * contain fields absent from this particular file for schema-evolution purposes, but it must
+ * contain every FILE field stored in the file schema. The consumer receives {@code null} when an
+ * invalid FILE reference is encountered, as permitted by the FILE logical-type specification.
+ *
+ * @param schema schema presented to the converter
+ * @param fileSchema FILE group stored in the file
+ * @param consumer consumer for converted values
+ * @return a converter for the complete FILE value
+ * @throws IllegalArgumentException if the projected schema omits a stored FILE field
+ */
+ public static GroupConverter newFileConverter(
+ GroupType schema, GroupType fileSchema, Consumer<FileValue> consumer) {
+ Objects.requireNonNull(schema, "schema cannot be null");
+ Objects.requireNonNull(fileSchema, "fileSchema cannot be null");
+ Objects.requireNonNull(consumer, "consumer cannot be null");
+ FileValueValidator.validateSchema(schema);
+ FileValueValidator.validateSchema(fileSchema);
+ for (Type field : fileSchema.getFields()) {
+ if (!schema.containsField(field.getName())) {
+ throw new IllegalArgumentException(
+ "FILE group '" + schema.getName() + "' cannot project stored field '" + field.getName() + "'");
+ }
+ }
+ return new FileGroupConverter(schema, consumer);
+ }
+
+ private static final class FileGroupConverter extends GroupConverter {
+ private final Converter[] converters;
+ private final Consumer<FileValue> consumer;
+ private FileValue.Builder builder;
+
+ private FileGroupConverter(GroupType schema, Consumer<FileValue> consumer) {
+ this.converters = new Converter[schema.getFieldCount()];
+ this.consumer = consumer;
+ for (int index = 0; index < schema.getFieldCount(); index++) {
+ String fieldName = schema.getFieldName(index);
+ switch (fieldName) {
+ case LogicalTypeAnnotation.FileLogicalTypeAnnotation.URI_FIELD:
+ converters[index] = stringConverter(value -> builder.withUri(value));
+ break;
+ case LogicalTypeAnnotation.FileLogicalTypeAnnotation.OFFSET_FIELD:
+ converters[index] = longConverter(value -> builder.withOffset(value));
+ break;
+ case LogicalTypeAnnotation.FileLogicalTypeAnnotation.SIZE_FIELD:
+ converters[index] = longConverter(value -> builder.withSize(value));
+ break;
+ case LogicalTypeAnnotation.FileLogicalTypeAnnotation.CONTENT_TYPE_FIELD:
+ converters[index] = stringConverter(value -> builder.withContentType(value));
+ break;
+ case LogicalTypeAnnotation.FileLogicalTypeAnnotation.CHECKSUM_FIELD:
+ converters[index] = stringConverter(value -> builder.withChecksum(value));
+ break;
+ case LogicalTypeAnnotation.FileLogicalTypeAnnotation.INLINE_FIELD:
+ converters[index] = binaryConverter(value -> builder.withInline(value));
+ break;
+ default:
+ throw new IllegalArgumentException("Unrecognized FILE field: " + fieldName);
+ }
+ }
+ }
+
+ @Override
+ public Converter getConverter(int fieldIndex) {
+ return converters[fieldIndex];
+ }
+
+ @Override
+ public void start() {
+ builder = FileValue.builder();
+ }
+
+ @Override
+ public void end() {
+ FileValue value = builder.build();
+ boolean valid = true;
+ try {
+ FileValueValidator.validateValue(value);
+ } catch (IllegalArgumentException e) {
+ valid = false;
+ }
+ builder = null;
+ consumer.accept(valid ? value : null);
+ }
+ }
+
+ private static PrimitiveConverter stringConverter(Consumer<String> consumer) {
+ return new PrimitiveConverter() {
+ @Override
+ public void addBinary(Binary value) {
+ consumer.accept(value.toStringUsingUTF8());
+ }
+ };
+ }
+
+ private static PrimitiveConverter binaryConverter(Consumer<Binary> consumer) {
+ return new PrimitiveConverter() {
+ @Override
+ public void addBinary(Binary value) {
+ consumer.accept(Binary.fromConstantByteArray(value.getBytes()));
Review Comment:
This potentially avoids a copy:
```suggestion
consumer.accept(value.copy());
```
##########
parquet-column/src/main/java/org/apache/parquet/io/api/FileConverters.java:
##########
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.parquet.io.api;
+
+import java.util.Objects;
+import java.util.function.Consumer;
+import org.apache.parquet.schema.GroupType;
+import org.apache.parquet.schema.LogicalTypeAnnotation;
+import org.apache.parquet.schema.Type;
+
+/** Converters for assembling values represented by a Parquet {@code FILE}-annotated group. */
+public final class FileConverters {
+ private FileConverters() {}
+
+ /**
+ * Creates a converter for a possibly projected FILE group.
+ *
+ * <p>FILE values are atomic and cannot be projected by child field. The projected schema may
+ * contain fields absent from this particular file for schema-evolution purposes, but it must
+ * contain every FILE field stored in the file schema. The consumer receives {@code null} when an
+ * invalid FILE reference is encountered, as permitted by the FILE logical-type specification.
+ *
+ * @param schema schema presented to the converter
+ * @param fileSchema FILE group stored in the file
+ * @param consumer consumer for converted values
+ * @return a converter for the complete FILE value
+ * @throws IllegalArgumentException if the projected schema omits a stored FILE field
+ */
+ public static GroupConverter newFileConverter(
Review Comment:
We have logic here asserting the names, which is great. But it leaves out field-IDs completely. Maybe we should check that the fields inside of the FILE group don't have any IDs assigned to them
##########
parquet-hadoop/src/test/java/org/apache/parquet/io/api/TestFileValueReadWrite.java:
##########
@@ -0,0 +1,288 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.parquet.io.api;
+
+import static org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.BINARY;
+import static org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.INT64;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Map;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.parquet.DirectWriterTest;
+import org.apache.parquet.conf.ParquetConfiguration;
+import org.apache.parquet.hadoop.ParquetReader;
+import org.apache.parquet.hadoop.ParquetWriter;
+import org.apache.parquet.hadoop.api.ReadSupport;
+import org.apache.parquet.hadoop.api.WriteSupport;
+import org.apache.parquet.schema.GroupType;
+import org.apache.parquet.schema.LogicalTypeAnnotation;
+import org.apache.parquet.schema.MessageType;
+import org.apache.parquet.schema.Types;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class TestFileValueReadWrite extends DirectWriterTest {
+ private static final GroupType FILE_SCHEMA = Types.optionalGroup()
+ .as(LogicalTypeAnnotation.fileType())
+ .optional(BINARY)
+ .as(LogicalTypeAnnotation.stringType())
+ .named("uri")
+ .optional(INT64)
+ .named("offset")
+ .optional(INT64)
+ .named("size")
+ .optional(BINARY)
+ .as(LogicalTypeAnnotation.stringType())
+ .named("content_type")
+ .optional(BINARY)
+ .as(LogicalTypeAnnotation.stringType())
+ .named("checksum")
+ .optional(BINARY)
+ .named("inline")
+ .named("file");
+ private static final MessageType SCHEMA = new MessageType("message", FILE_SCHEMA);
+
+ @TempDir
+ private java.nio.file.Path tempDir;
+
+ @Test
+ public void testFileValueRoundTrip() throws IOException {
+ FileValue[] expectedValues = {
+ FileValue.builder()
+ .withUri("s3://bucket/file")
+ .withOffset(10)
+ .withSize(20)
+ .withContentType("image/png")
+ .withChecksum("ETAG:abc")
+ .withInline(Binary.fromString("bytes"))
+ .build(),
+ FileValue.builder().withInline(Binary.fromString("inline only")).build(),
+ null,
+ FileValue.builder().withUri("relative/path").build(),
+ FileValue.builder().withUri("file").withOffset(0).withSize(0).build(),
+ FileValue.builder()
+ .withInline(Binary.fromConstantByteArray(new byte[0]))
+ .build()
+ };
+ Path path = new Path(tempDir.resolve("files.parquet").toUri());
+ try (ParquetWriter<FileValue> writer = new ParquetWriter<>(path, new FileWriteSupport())) {
+ for (FileValue value : expectedValues) {
+ writer.write(value);
+ }
+ }
+
+ try (ParquetReader<FileRecord> reader =
+ ParquetReader.builder(new FileReadSupport(), path).build()) {
+ for (FileValue expected : expectedValues) {
+ FileRecord record = reader.read();
+ assertThat(record).isNotNull();
+ if (expected == null) {
+ assertThat(record.value).isNull();
+ } else {
+ FileValue actual = record.value;
+ assertThat(actual).isNotNull();
+ assertThat(actual.getUri()).isEqualTo(expected.getUri());
+ assertThat(actual.getOffset()).isEqualTo(expected.getOffset());
+ assertThat(actual.getSize()).isEqualTo(expected.getSize());
+ assertThat(actual.getContentType()).isEqualTo(expected.getContentType());
+ assertThat(actual.getChecksum()).isEqualTo(expected.getChecksum());
+ assertThat(actual.getInline()).isEqualTo(expected.getInline());
+ }
+ }
+ assertThat(reader.read()).isNull();
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testInvalidStoredFileValueConvertsToNull(boolean sizeWithoutUri) throws IOException {
+ Path path = writeDirect(SCHEMA, consumer -> {
+ consumer.startMessage();
+ consumer.startField("file", 0);
+ consumer.startGroup();
+ if (sizeWithoutUri) {
+ consumer.startField("size", FILE_SCHEMA.getFieldIndex("size"));
+ consumer.addLong(2);
+ consumer.endField("size", FILE_SCHEMA.getFieldIndex("size"));
+ }
+ consumer.startField("content_type", FILE_SCHEMA.getFieldIndex("content_type"));
+ consumer.addBinary(Binary.fromString("image/png"));
+ consumer.endField("content_type", FILE_SCHEMA.getFieldIndex("content_type"));
+ if (sizeWithoutUri) {
+ consumer.startField("inline", FILE_SCHEMA.getFieldIndex("inline"));
+ consumer.addBinary(Binary.fromString("bytes"));
+ consumer.endField("inline", FILE_SCHEMA.getFieldIndex("inline"));
+ }
+ consumer.endGroup();
+ consumer.endField("file", 0);
+ consumer.endMessage();
+ });
+
+ try (ParquetReader<FileRecord> reader =
+ ParquetReader.builder(new FileReadSupport(), path).build()) {
+ assertThat(reader.read().value).isNull();
+ assertThat(reader.read()).isNull();
+ }
+ }
+
+ @Test
+ public void testFileConverterRejectsPartialProjectionDuringReaderAssembly() throws IOException {
+ Path path = writeDirect(SCHEMA, consumer -> {
+ consumer.startMessage();
+ consumer.startField("file", 0);
+ FileValueWriter.write(
+ consumer, FILE_SCHEMA, FileValue.builder().withUri("file").build());
+ consumer.endField("file", 0);
+ consumer.endMessage();
+ });
+ MessageType requestedSchema = new MessageType("message", FILE_SCHEMA.withNewFields(FILE_SCHEMA.getType("uri")));
+
+ try (ParquetReader<FileRecord> reader = ParquetReader.builder(new FileReadSupport(requestedSchema), path)
+ .build()) {
+ assertThatThrownBy(reader::read)
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("FILE group 'file' cannot project stored field 'offset'");
+ }
+ }
+
+ @Test
+ public void testFileConverterReadsReorderedSchemaWithAddedFields() throws IOException {
+ GroupType storedFile = FILE_SCHEMA.withNewFields(FILE_SCHEMA.getType("uri"));
+ Path path = writeDirect(new MessageType("message", storedFile), consumer -> {
+ consumer.startMessage();
+ consumer.startField("file", 0);
+ FileValueWriter.write(
+ consumer, storedFile, FileValue.builder().withUri("file").build());
+ consumer.endField("file", 0);
+ consumer.endMessage();
+ });
+ MessageType requestedSchema = new MessageType(
+ "message", FILE_SCHEMA.withNewFields(FILE_SCHEMA.getType("inline"), FILE_SCHEMA.getType("uri")));
+
+ try (ParquetReader<FileRecord> reader = ParquetReader.builder(new FileReadSupport(requestedSchema), path)
+ .build()) {
+ FileValue actual = reader.read().value;
+ assertThat(actual.getUri()).isEqualTo("file");
+ assertThat(actual.getInline()).isNull();
+ assertThat(reader.read()).isNull();
+ }
+ }
+
+ private static final class FileRecord {
+ private final FileValue value;
+
+ private FileRecord(FileValue value) {
+ this.value = value;
+ }
+ }
+
+ private static final class FileReadSupport extends ReadSupport<FileRecord> {
+ private final MessageType requestedSchema;
+
+ private FileReadSupport() {
+ this(null);
+ }
+
+ private FileReadSupport(MessageType requestedSchema) {
+ this.requestedSchema = requestedSchema;
+ }
+
+ @Override
+ public ReadContext init(
+ ParquetConfiguration configuration, Map<String, String> metadata, MessageType fileSchema) {
+ return new ReadContext(requestedSchema == null ? fileSchema : requestedSchema);
+ }
+
+ @Override
+ public RecordMaterializer<FileRecord> prepareForRead(
+ Configuration configuration,
+ Map<String, String> metadata,
+ MessageType fileSchema,
+ ReadContext readContext) {
+ return new FileRecordMaterializer(fileSchema, readContext.getRequestedSchema());
+ }
+ }
+
+ private static final class FileWriteSupport extends WriteSupport<FileValue> {
+ private RecordConsumer consumer;
+
+ @Override
+ public WriteContext init(Configuration configuration) {
+ return new WriteContext(SCHEMA, Collections.emptyMap());
+ }
+
+ @Override
+ public void prepareForWrite(RecordConsumer recordConsumer) {
+ consumer = recordConsumer;
+ }
+
+ @Override
+ public void write(FileValue value) {
+ consumer.startMessage();
+ if (value != null) {
+ consumer.startField("file", 0);
+ FileValueWriter.write(consumer, FILE_SCHEMA, value);
Review Comment:
Within this method, we validate the schema each time, which is pretty costly. I don't think we need to do that for `FILE_SCHEMA` on runtime. Ideally, we want to validate this before we initiate the write, when constructing the `GroupType` with the annotation.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4426850081
##########
parquet-column/src/test/java/org/apache/parquet/file/TestFileValueWriter.java:
##########
@@ -0,0 +1,222 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.parquet.io.api;
+
+import static org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.BINARY;
+import static org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.INT64;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.ArrayDeque;
+import java.util.Arrays;
+import java.util.Deque;
+import org.apache.parquet.io.ExpectationValidatingRecordConsumer;
+import org.apache.parquet.schema.GroupType;
+import org.apache.parquet.schema.LogicalTypeAnnotation;
+import org.apache.parquet.schema.Types;
+import org.junit.jupiter.api.Test;
+
+public class TestFileValueWriter {
+ private static final GroupType ALL_FIELDS_SCHEMA = Types.optionalGroup()
+ .as(LogicalTypeAnnotation.fileType())
+ .optional(BINARY)
+ .as(LogicalTypeAnnotation.stringType())
+ .named("uri")
+ .optional(INT64)
+ .named("offset")
+ .optional(INT64)
+ .named("size")
+ .optional(BINARY)
+ .as(LogicalTypeAnnotation.stringType())
+ .named("content_type")
+ .optional(BINARY)
+ .as(LogicalTypeAnnotation.stringType())
+ .named("checksum")
+ .optional(BINARY)
+ .named("inline")
+ .named("file");
+
+ @Test
+ public void testWritesSetFieldsInSchemaOrder() {
Review Comment:
Updated [testWritesSetFieldsInSchemaOrder](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-column/src/test/java/org/apache/parquet/file/TestFileValueWriter.java#L58): the builder calls and the schema fields now each use a different order from the spec and from each other. The expected `RecordConsumer` calls check the supplied schema's field order and indexes, so writing in canonical order would fail the test.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r3755267428
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/FileValueWriter.java:
##########
@@ -0,0 +1,197 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.parquet.hadoop;
+
+import java.io.IOException;
+import org.apache.parquet.bytes.BytesInput;
+import org.apache.parquet.format.BlockCipher;
+import org.apache.parquet.io.api.Binary;
+
+/**
+ * Decides how a {@code FILE} value's payload is stored: inline in the value, or out of line as a
+ * self-reference within the same Parquet file.
+ *
+ * <p>Object models hand over the resolved (logical) bytes and receive back a {@link Placement}
+ * describing which fields of the {@code FILE} group to write. Callers do not choose between the two
+ * forms themselves; the choice follows the configured threshold, so the same writing code produces
+ * either form:
+ *
+ * <pre>{@code
+ * FileValueWriter.Placement placement = fileValueWriter.write(payload);
+ * if (placement.isInline()) {
+ * group.add("inline", placement.getInlineBytes());
+ * } else {
+ * group.add("offset", placement.getOffset());
+ * group.add("size", placement.getSize());
+ * }
+ * }</pre>
+ *
+ * <p>Both forms describe the same logical bytes, so {@code content_type} and {@code checksum} are
+ * written identically either way — they describe the resolved bytes, not the storage. Consumers see
+ * no difference beyond which fields are set.
+ *
+ * <p>A self-reference payload is written immediately, while the record is being written and before
+ * the row group's column chunks are flushed. It therefore lands in a contiguous run ahead of those
+ * chunks, leaving each column chunk contiguous on disk. Writing eagerly is what makes the offset
+ * knowable in time: {@code offset} and {@code size} are ordinary column values, and once a value has
+ * been handed to a column writer it is encoded into a buffered page and cannot be revised, so a
+ * placeholder could never be patched up later.
+ *
+ * @see SelfReferenceStorage
+ */
+public class FileValueWriter {
+
+ /**
+ * Where a {@code FILE} value's payload was placed, and therefore which fields of the {@code FILE}
+ * group the caller should write. Either the payload is inline, or it is a self-reference located by
+ * {@code offset} and {@code size}.
+ */
+ public static final class Placement {
+ private final Binary inlineBytes;
+ private final long offset;
+ private final long size;
+
+ private Placement(Binary inlineBytes, long offset, long size) {
+ this.inlineBytes = inlineBytes;
+ this.offset = offset;
+ this.size = size;
+ }
+
+ static Placement inline(Binary inlineBytes) {
+ return new Placement(inlineBytes, -1, -1);
+ }
+
+ static Placement selfReference(SelfReferenceStorage.StoredRange range) {
+ return new Placement(null, range.getOffset(), range.getSize());
+ }
+
+ /** Whether the payload is stored inline, i.e. whether the {@code inline} field should be set. */
+ public boolean isInline() {
+ return inlineBytes != null;
+ }
+
+ /**
+ * The bytes to write to the {@code inline} field.
+ *
+ * @throws IllegalStateException if the payload was stored as a self-reference
+ */
+ public Binary getInlineBytes() {
+ if (!isInline()) {
+ throw new IllegalStateException("Payload was stored as a self-reference, not inline");
+ }
+ return inlineBytes;
+ }
+
+ /**
+ * The value to write to the {@code offset} field.
+ *
+ * @throws IllegalStateException if the payload was stored inline
+ */
+ public long getOffset() {
+ if (isInline()) {
+ throw new IllegalStateException("Payload was stored inline; it has no offset");
+ }
+ return offset;
+ }
+
+ /**
+ * The value to write to the {@code size} field. This is the size of the stored representation
+ * after compression and encryption, not the size of the resolved bytes.
+ *
+ * @throws IllegalStateException if the payload was stored inline
+ */
+ public long getSize() {
+ if (isInline()) {
+ throw new IllegalStateException("Payload was stored inline; it has no size");
+ }
+ return size;
+ }
+ }
+
+ private final ParquetFileWriter fileWriter;
+ private final CodecFactory.BytesCompressor inlineColumnCompressor;
+ private final BlockCipher.Encryptor inlineColumnEncryptor;
+ private final int inlineColumnOrdinal;
+ private final int selfReferenceThreshold;
+
+ /**
+ * @param fileWriter the writer for the file being written; a block must be open when
+ * {@link #write} is called
+ * @param inlineColumnCompressor the compressor for the {@code inline} column chunk's codec, whose
+ * compression a self-reference inherits
+ * @param inlineColumnEncryptor the data-module encryptor of the {@code inline} column chunk, or
+ * {@code null} if that column chunk is not encrypted
+ * @param inlineColumnOrdinal the ordinal of the {@code inline} column within the schema. The
+ * schema must declare {@code inline}: it is the reference point whose compression and
+ * encryption a self-reference inherits, so a schema without it can only store payloads inline
+ * or as external references. Note that the schema builder does not require {@code inline} for
+ * groups that declare {@code uri}, so an external-reference schema may reach here; pair such a
+ * schema with a threshold of {@link Integer#MAX_VALUE} so nothing is stored out of line.
+ * @param selfReferenceThreshold payloads of at most this many bytes are stored inline; larger ones
+ * become self-references. See
+ * {@code ParquetProperties.Builder#withFileSelfReferenceThreshold(int)}.
+ */
+ public FileValueWriter(
+ ParquetFileWriter fileWriter,
+ CodecFactory.BytesCompressor inlineColumnCompressor,
+ BlockCipher.Encryptor inlineColumnEncryptor,
+ int inlineColumnOrdinal,
+ int selfReferenceThreshold) {
+ if (selfReferenceThreshold < 0) {
+ throw new IllegalArgumentException(
+ "Self-reference threshold must not be negative: " + selfReferenceThreshold);
+ }
+ if (inlineColumnOrdinal < 0) {
+ throw new IllegalArgumentException("Invalid inline column ordinal: " + inlineColumnOrdinal);
+ }
+ this.fileWriter = fileWriter;
+ this.inlineColumnCompressor = inlineColumnCompressor;
+ this.inlineColumnEncryptor = inlineColumnEncryptor;
+ this.inlineColumnOrdinal = inlineColumnOrdinal;
+ this.selfReferenceThreshold = selfReferenceThreshold;
+ }
+
+ /**
+ * Stores {@code payload} and returns which {@code FILE} group fields to write for it. Payloads at
+ * or below the configured threshold are returned for inline storage; larger ones are written to the
+ * file body immediately as self-references.
+ *
+ * <p>Must be called while a block is open on the underlying writer, and before that block's column
+ * chunks are flushed.
+ *
+ * @param payload the resolved (logical) bytes of the value
+ * @return the placement describing which fields to write
+ * @throws IOException if writing the self-reference payload fails
+ */
+ public Placement write(Binary payload) throws IOException {
Review Comment:
Why are we leaving this to only inline? should we have another API for external references to keep these consistent?
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r3540692773
##########
parquet-column/src/main/java/org/apache/parquet/schema/Types.java:
##########
@@ -821,12 +821,34 @@ 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) {
Review Comment:
addressed
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#issuecomment-5946173645
@divjotarora addressed your comments. Added reader/writer interfaces
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4426836436
##########
parquet-column/src/test/java/org/apache/parquet/schema/TestTypeBuildersWithLogicalTypes.java:
##########
Review Comment:
Added coverage in [TestFileValueWriter](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-column/src/test/java/org/apache/parquet/file/Test
FileValueWriter.java#L129)
for offset without size, offset/size without a URI, and negative offset/size. [TestFileValueReadWrite](https://github.com/apache/parquet-java/blob/bab237a1
f7b57457052f520f7cd5dae84b211e16/parquet-hadoop/src/test/java/org/apache/parquet/file/TestFileValueReadWrite.java#L76) round-trips valid values, including a zero offset and zero size
, and checks that invalid stored references convert to null. The shared checks live in [FileValueValidator.validateValue](https://github.com/apache/parquet-java/blob/bab237a1f7b57457
052f520f7cd5dae84b211e16/parquet-column/src/main/java/org/apache/parquet/file/FileValueValidator.java#L35), called before writing and after assembling a value on read.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "gunnarmorling (via GitHub)" <gi...@apache.org>.
gunnarmorling commented on PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#issuecomment-6219619060
Hey, I've sent PR https://github.com/apache/parquet-testing/pull/128 for adding FILE fixtures to parquet-testing, it might be interesting for testing here?
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r3755214618
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileWriter.java:
##########
@@ -609,6 +609,50 @@ public InternalFileEncryptor getEncryptor() {
return fileEncryptor;
}
+ /**
+ * Writes a {@code FILE} self-reference payload into the file body, inheriting the compression and
+ * encryption of the {@code inline} column chunk, and returns the {@code offset} and {@code size} a
+ * writer records in the self-reference's {@code offset} and {@code size} fields. See
+ * {@link SelfReferenceStorage} for the layout and the Parquet format's "FILE" logical type
+ * specification for the storage-inheritance semantics.
+ *
+ * <p>The payload is compressed as an independent compression block using {@code compressor} (the
+ * compressor for the {@code inline} column chunk's {@link CompressionCodecName}) and, when
+ * {@code pageBlockEncryptor} is non-null, encrypted as an independent module with the
+ * {@code Self-Reference} module type. The row group ordinal is that of the block currently being
+ * written.
+ *
+ * <p>Payloads are written while a block is open but before its column chunks are flushed, so they
+ * land in a contiguous run ahead of the row group's chunks. This keeps each column chunk
+ * contiguous on disk, which the read path relies on when coalescing adjacent chunks into a single
+ * range read.
+ *
+ * <p>This must be called while a block is open (after {@link #startBlock(long)} and before
+ * {@link #endBlock()}) so that the returned offset falls within the file body.
+ *
+ * @param resolvedBytes the resolved (logical) bytes of the self-reference
+ * @param compressor the compressor for the {@code inline} column chunk's codec
+ * @param pageBlockEncryptor the data-module encryptor of the {@code inline} column chunk, or
+ * {@code null} if the column chunk is not encrypted
+ * @param columnOrdinal the ordinal of the {@code inline} column the self-reference inherits from
+ * @return the offset and size of the stored representation
+ * @throws IOException if writing or compression fails
+ */
+ public SelfReferenceStorage.StoredRange writeSelfReference(
+ BytesInput resolvedBytes,
+ CodecFactory.BytesCompressor compressor,
Review Comment:
this seems like an awkward API, shold compressor and pageBlockEncryptor be abstracted in some way (so all that is needed is column Ordinal)? or should all three be grouped together?
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4426850081
##########
parquet-column/src/test/java/org/apache/parquet/file/TestFileValueWriter.java:
##########
@@ -0,0 +1,222 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.parquet.io.api;
+
+import static org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.BINARY;
+import static org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.INT64;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.ArrayDeque;
+import java.util.Arrays;
+import java.util.Deque;
+import org.apache.parquet.io.ExpectationValidatingRecordConsumer;
+import org.apache.parquet.schema.GroupType;
+import org.apache.parquet.schema.LogicalTypeAnnotation;
+import org.apache.parquet.schema.Types;
+import org.junit.jupiter.api.Test;
+
+public class TestFileValueWriter {
+ private static final GroupType ALL_FIELDS_SCHEMA = Types.optionalGroup()
+ .as(LogicalTypeAnnotation.fileType())
+ .optional(BINARY)
+ .as(LogicalTypeAnnotation.stringType())
+ .named("uri")
+ .optional(INT64)
+ .named("offset")
+ .optional(INT64)
+ .named("size")
+ .optional(BINARY)
+ .as(LogicalTypeAnnotation.stringType())
+ .named("content_type")
+ .optional(BINARY)
+ .as(LogicalTypeAnnotation.stringType())
+ .named("checksum")
+ .optional(BINARY)
+ .named("inline")
+ .named("file");
+
+ @Test
+ public void testWritesSetFieldsInSchemaOrder() {
Review Comment:
Updated [testWritesSetFieldsInSchemaOrder](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-column/src/test/java/org/apache/parquet/file/T
estFileValueWriter.java#L58): the builder calls and the schema fields now each use a different order from the spec and from each other. The expected `RecordConsumer` calls check the
supplied schema's field order and indexes, so writing in canonical order would fail the test.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4426857500
##########
parquet-column/src/main/java/org/apache/parquet/schema/LogicalTypeAnnotation.java:
##########
@@ -1229,6 +1240,151 @@ public boolean equals(Object obj) {
}
}
+ /**
+ * File logical type annotation. Annotates a group (struct) that represents a reference to a
+ * range of bytes, which may be stored inline in the value or in an external file. Every field is
+ * optional, both in the schema (a writer may omit any field from the group definition) and in the
+ * data (any field that is present has a field repetition type of {@code OPTIONAL}). Fields are
+ * identified by name (case sensitively), not by field order. A group need only define the fields
+ * it uses. The group may contain the following fields:
+ * <ul>
+ * <li>{@code uri} (STRING): a URI-reference (RFC 3986) that identifies an external file, for
+ * example {@code s3://bucket/file.jpg}.</li>
+ * <li>{@code offset} (INT64): start of the byte range within the external file identified by
+ * {@code uri}; if not set, treated as 0. Must not be negative and may only be set together
+ * with {@code uri}.</li>
+ * <li>{@code size} (INT64): byte length of the referenced data within the external file. May be
+ * omitted for a whole-file external reference, in which case the range runs to the end of
+ * the referenced file. May only be set together with {@code uri}, must be set whenever
+ * {@code offset} is set, and must not be negative.</li>
+ * <li>{@code content_type} (STRING): the media (MIME) type (RFC 2046) of the resolved bytes;
+ * when not set, {@code application/octet-stream} is assumed.</li>
+ * <li>{@code checksum} (STRING): a self-describing integrity token for the resolved bytes, of
+ * the form {@code <algorithm>:<digest>}.</li>
+ * <li>{@code inline} (BYTE_ARRAY): the referenced bytes stored inline in the value.</li>
+ * </ul>
+ * No fields with names other than the above are permitted, and field names must be unique. Each
+ * declared field must match its required physical and logical types. Rules involving which fields
+ * are set and their values are enforced per value rather than by the schema.
+ */
+ public static class FileLogicalTypeAnnotation extends LogicalTypeAnnotation {
+ private static final FileLogicalTypeAnnotation INSTANCE = new FileLogicalTypeAnnotation();
+
+ /** Field name holding the URI-reference of an external file. */
+ public static final String URI_FIELD = "uri";
+
+ /** Field name holding the start of the byte range. */
+ public static final String OFFSET_FIELD = "offset";
+
+ /** Field name holding the byte length of the referenced data. */
+ public static final String SIZE_FIELD = "size";
+
+ /** Field name holding the media (MIME) type of the resolved bytes. */
+ public static final String CONTENT_TYPE_FIELD = "content_type";
+
+ /** Field name holding the integrity token for the resolved bytes. */
+ public static final String CHECKSUM_FIELD = "checksum";
+
+ /** Field name holding the referenced bytes stored inline. */
+ public static final String INLINE_FIELD = "inline";
+
+ /** All recognized field names in a FILE-annotated group. All fields are optional. */
+ public static final Set<String> FIELD_NAMES =
+ Set.of(URI_FIELD, OFFSET_FIELD, SIZE_FIELD, CONTENT_TYPE_FIELD, CHECKSUM_FIELD, INLINE_FIELD);
+
+ private FileLogicalTypeAnnotation() {}
+
+ /** Validates the fields of a FILE-annotated group. */
+ public void validate(GroupType group) {
+ validateFields(group.getName(), group.getFields());
+ }
+
+ void validateFields(String groupName, List<Type> fields) {
Review Comment:
Schema validation now has a single construction-time path: the [GroupType constructor](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-co
lumn/src/main/java/org/apache/parquet/schema/GroupType.java#L117) calls `FileLogicalTypeAnnotation.validateFields`. Removed the repeated schema-validation calls from `FileValueValida
tor`, `Types`, and metadata serialization. [FileValueValidator.validateValue](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-column/src/
main/java/org/apache/parquet/file/FileValueValidator.java#L35) retains the value-dependent locator/range checks shared by the reader and writer; its schema check now only requires th
e FILE annotation.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4426846446
##########
parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java:
##########
@@ -2184,7 +2194,15 @@ && getLogicalTypeAnnotation(schemaElement.logicalType) != null)
childBuilder.id(schemaElement.field_id);
}
- childBuilder.named(schemaElement.name);
+ try {
+ childBuilder.named(schemaElement.name);
+ } catch (IllegalArgumentException e) {
+ if (!fileLogicalType) {
+ throw e;
+ }
+ LOG.warn("Dropping malformed FILE logical type annotation on group {}", schemaElement.name, e);
+ childBuilder.as((LogicalTypeAnnotation) null).named(schemaElement.name);
+ }
Review Comment:
Removed the FILE-specific catch that dropped the annotation. Invalid fields in a FILE-annotated group now propagate the schema-validation error. FILE on a primitive still follows the existing unsupported-annotation handling. [testMalformedFileLogicalTypeFailsOnRead and testFileLogicalTypeOnPrimitiveIsIgnored](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-hadoop/src/test/java/org/apache/parquet/format/converter/TestParquetMetadataConverter.java#L2506) cover both behaviors.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r3540695386
##########
parquet-column/src/test/java/org/apache/parquet/schema/TestTypeBuildersWithLogicalTypes.java:
##########
Review Comment:
where can we add value checks during reads and writes? Can you provide any code references please?
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#issuecomment-5938001969
CI failure: 'TestParquetMetadataConverter.testFileLogicalTypeIsIgnoredRatherThanFailing`
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r3755197052
##########
parquet-hadoop/src/test/java/org/apache/parquet/hadoop/TestFileValueWriter.java:
##########
@@ -0,0 +1,274 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.parquet.hadoop;
+
+import static org.apache.parquet.hadoop.ParquetFileWriter.Mode.CREATE;
+import static org.apache.parquet.hadoop.ParquetWriter.DEFAULT_BLOCK_SIZE;
+import static org.apache.parquet.hadoop.ParquetWriter.MAX_PADDING_SIZE_DEFAULT;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.parquet.ParquetReadOptions;
+import org.apache.parquet.bytes.BytesInput;
+import org.apache.parquet.column.ColumnDescriptor;
+import org.apache.parquet.column.Encoding;
+import org.apache.parquet.column.ParquetProperties;
+import org.apache.parquet.column.statistics.Statistics;
+import org.apache.parquet.hadoop.metadata.BlockMetaData;
+import org.apache.parquet.hadoop.metadata.ColumnChunkMetaData;
+import org.apache.parquet.hadoop.metadata.ColumnPath;
+import org.apache.parquet.hadoop.metadata.CompressionCodecName;
+import org.apache.parquet.hadoop.util.HadoopInputFile;
+import org.apache.parquet.hadoop.util.HadoopOutputFile;
+import org.apache.parquet.io.InputFile;
+import org.apache.parquet.io.api.Binary;
+import org.apache.parquet.schema.MessageType;
+import org.apache.parquet.schema.MessageTypeParser;
+import org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName;
+import org.apache.parquet.schema.Types;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+/**
+ * Tests that {@link FileValueWriter} routes a {@code FILE} payload to inline storage or to a
+ * self-reference according to the configured threshold, and that both forms describe the same logical
+ * bytes.
+ */
+public class TestFileValueWriter {
+
+ private static final MessageType SCHEMA = MessageTypeParser.parseMessageType("message m {"
+ + " optional group file (FILE) {"
+ + " optional int64 offset;"
+ + " optional int64 size;"
+ + " optional binary inline;"
+ + " }"
+ + "}");
+
+ private static final ColumnDescriptor INLINE_COLUMN = SCHEMA.getColumnDescription(new String[] {"file", "inline"});
+
+ private static final CompressionCodecName CODEC = CompressionCodecName.SNAPPY;
+
+ private static final Statistics<?> EMPTY_STATS = Statistics.getBuilderForReading(
+ Types.required(PrimitiveTypeName.BINARY).named("inline"))
+ .build();
+
+ @TempDir
+ java.nio.file.Path tempDir;
+
+ @Test
+ public void testDefaultThresholdIsPageSize() {
+ assertThat(ParquetProperties.builder().build().getFileSelfReferenceThreshold())
+ .isEqualTo(ParquetProperties.DEFAULT_PAGE_SIZE);
+ }
+
+ @Test
+ public void testThresholdMustNotBeNegative() {
+ assertThatThrownBy(() -> ParquetProperties.builder().withFileSelfReferenceThreshold(-1))
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void testPayloadAtThresholdIsInlinedAndAboveIsSelfReference() throws IOException {
+ Configuration conf = new Configuration();
+ Path path = new Path(tempDir.resolve("routing.parquet").toUri());
+ CodecFactory codecFactory = new CodecFactory(conf, DEFAULT_BLOCK_SIZE);
+
+ int threshold = 64;
+ Binary atThreshold = Binary.fromConstantByteArray(payload(threshold));
+ Binary aboveThreshold = Binary.fromConstantByteArray(payload(threshold + 1));
+
+ ParquetFileWriter writer = new ParquetFileWriter(
+ HadoopOutputFile.fromPath(path, conf), SCHEMA, CREATE, DEFAULT_BLOCK_SIZE, MAX_PADDING_SIZE_DEFAULT);
+ writer.start();
+ writer.startBlock(2);
+
+ FileValueWriter valueWriter = new FileValueWriter(
+ writer, codecFactory.getCompressor(CODEC), null, columnOrdinalOf(INLINE_COLUMN), threshold);
+
+ FileValueWriter.Placement inlined = valueWriter.write(atThreshold);
+ FileValueWriter.Placement outOfLine = valueWriter.write(aboveThreshold);
+
+ // A payload exactly at the threshold stays inline; one byte more goes out of line.
+ assertThat(inlined.isInline()).isTrue();
+ assertThat(inlined.getInlineBytes()).isEqualTo(atThreshold);
+ assertThatThrownBy(inlined::getOffset).isInstanceOf(IllegalStateException.class);
+
+ assertThat(outOfLine.isInline()).isFalse();
+ assertThat(outOfLine.getSize()).isGreaterThan(0L);
+ assertThatThrownBy(outOfLine::getInlineBytes).isInstanceOf(IllegalStateException.class);
+
+ // Write the inline column chunk so the reader has metadata carrying the inherited codec.
+ writer.startColumn(INLINE_COLUMN, 1, CODEC);
+ writer.writeDataPage(
+ 1,
+ (int) inlined.getInlineBytes().length(),
+ codecFactory
+ .getCompressor(CODEC)
+ .compress(BytesInput.from(inlined.getInlineBytes().toByteBuffer())),
+ EMPTY_STATS,
+ Encoding.BIT_PACKED,
+ Encoding.BIT_PACKED,
+ Encoding.PLAIN);
+ writer.endColumn();
+ writer.endBlock();
+ writer.end(new java.util.HashMap<>());
+
+ // The out-of-line payload resolves back to the original bytes.
+ InputFile inputFile = HadoopInputFile.fromPath(path, conf);
+ try (ParquetFileReader reader =
+ ParquetFileReader.open(inputFile, ParquetReadOptions.builder().build())) {
+ BlockMetaData block = reader.getFooter().getBlocks().get(0);
+ ColumnChunkMetaData inlineMeta = findColumn(block, INLINE_COLUMN);
+ BytesInput resolved = reader.resolveSelfReference(inlineMeta, outOfLine.getOffset(), outOfLine.getSize());
Review Comment:
this isn't really showing a round trip and we are using the value from memory, not the one persisted.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r3755265582
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/FileValueWriter.java:
##########
@@ -0,0 +1,197 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.parquet.hadoop;
+
+import java.io.IOException;
+import org.apache.parquet.bytes.BytesInput;
+import org.apache.parquet.format.BlockCipher;
+import org.apache.parquet.io.api.Binary;
+
+/**
+ * Decides how a {@code FILE} value's payload is stored: inline in the value, or out of line as a
+ * self-reference within the same Parquet file.
+ *
+ * <p>Object models hand over the resolved (logical) bytes and receive back a {@link Placement}
+ * describing which fields of the {@code FILE} group to write. Callers do not choose between the two
+ * forms themselves; the choice follows the configured threshold, so the same writing code produces
+ * either form:
+ *
+ * <pre>{@code
+ * FileValueWriter.Placement placement = fileValueWriter.write(payload);
+ * if (placement.isInline()) {
+ * group.add("inline", placement.getInlineBytes());
+ * } else {
+ * group.add("offset", placement.getOffset());
+ * group.add("size", placement.getSize());
+ * }
+ * }</pre>
+ *
+ * <p>Both forms describe the same logical bytes, so {@code content_type} and {@code checksum} are
+ * written identically either way — they describe the resolved bytes, not the storage. Consumers see
+ * no difference beyond which fields are set.
+ *
+ * <p>A self-reference payload is written immediately, while the record is being written and before
+ * the row group's column chunks are flushed. It therefore lands in a contiguous run ahead of those
+ * chunks, leaving each column chunk contiguous on disk. Writing eagerly is what makes the offset
+ * knowable in time: {@code offset} and {@code size} are ordinary column values, and once a value has
+ * been handed to a column writer it is encoded into a buffered page and cannot be revised, so a
+ * placeholder could never be patched up later.
+ *
+ * @see SelfReferenceStorage
+ */
+public class FileValueWriter {
+
+ /**
+ * Where a {@code FILE} value's payload was placed, and therefore which fields of the {@code FILE}
+ * group the caller should write. Either the payload is inline, or it is a self-reference located by
+ * {@code offset} and {@code size}.
+ */
+ public static final class Placement {
+ private final Binary inlineBytes;
+ private final long offset;
+ private final long size;
+
+ private Placement(Binary inlineBytes, long offset, long size) {
+ this.inlineBytes = inlineBytes;
+ this.offset = offset;
+ this.size = size;
+ }
+
+ static Placement inline(Binary inlineBytes) {
+ return new Placement(inlineBytes, -1, -1);
+ }
+
+ static Placement selfReference(SelfReferenceStorage.StoredRange range) {
+ return new Placement(null, range.getOffset(), range.getSize());
+ }
+
+ /** Whether the payload is stored inline, i.e. whether the {@code inline} field should be set. */
+ public boolean isInline() {
+ return inlineBytes != null;
+ }
+
+ /**
+ * The bytes to write to the {@code inline} field.
+ *
+ * @throws IllegalStateException if the payload was stored as a self-reference
+ */
+ public Binary getInlineBytes() {
+ if (!isInline()) {
+ throw new IllegalStateException("Payload was stored as a self-reference, not inline");
+ }
+ return inlineBytes;
+ }
+
+ /**
+ * The value to write to the {@code offset} field.
+ *
+ * @throws IllegalStateException if the payload was stored inline
+ */
+ public long getOffset() {
+ if (isInline()) {
+ throw new IllegalStateException("Payload was stored inline; it has no offset");
+ }
+ return offset;
+ }
+
+ /**
+ * The value to write to the {@code size} field. This is the size of the stored representation
+ * after compression and encryption, not the size of the resolved bytes.
+ *
+ * @throws IllegalStateException if the payload was stored inline
+ */
+ public long getSize() {
+ if (isInline()) {
+ throw new IllegalStateException("Payload was stored inline; it has no size");
+ }
+ return size;
+ }
+ }
+
+ private final ParquetFileWriter fileWriter;
+ private final CodecFactory.BytesCompressor inlineColumnCompressor;
+ private final BlockCipher.Encryptor inlineColumnEncryptor;
+ private final int inlineColumnOrdinal;
+ private final int selfReferenceThreshold;
+
+ /**
+ * @param fileWriter the writer for the file being written; a block must be open when
+ * {@link #write} is called
+ * @param inlineColumnCompressor the compressor for the {@code inline} column chunk's codec, whose
+ * compression a self-reference inherits
+ * @param inlineColumnEncryptor the data-module encryptor of the {@code inline} column chunk, or
+ * {@code null} if that column chunk is not encrypted
+ * @param inlineColumnOrdinal the ordinal of the {@code inline} column within the schema. The
+ * schema must declare {@code inline}: it is the reference point whose compression and
+ * encryption a self-reference inherits, so a schema without it can only store payloads inline
+ * or as external references. Note that the schema builder does not require {@code inline} for
+ * groups that declare {@code uri}, so an external-reference schema may reach here; pair such a
+ * schema with a threshold of {@link Integer#MAX_VALUE} so nothing is stored out of line.
+ * @param selfReferenceThreshold payloads of at most this many bytes are stored inline; larger ones
+ * become self-references. See
+ * {@code ParquetProperties.Builder#withFileSelfReferenceThreshold(int)}.
+ */
+ public FileValueWriter(
Review Comment:
Seems like this API might be error prone should it be package private and instead users rely on a static factory on ParquetFileWriter?
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "divjotarora (via GitHub)" <gi...@apache.org>.
divjotarora commented on PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#issuecomment-6198438838
Hey @brkyvz, how should FileConverters behave with projected FILE groups? For example, suppose a valid stored value has uri and content_type, but the reader projects only content_type. The converter never sees `uri`, builds a value without a locator, fails validation, and returns null. It therefore cannot distinguish "URI was null in the file" from "URI was not projected." Is the intended contract that FILE projections must include enough fields to form a valid locator? If so, that should probably be documented/enforced, otherwise the converter needs to support partial values.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "divjotarora (via GitHub)" <gi...@apache.org>.
divjotarora commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4387478733
##########
parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java:
##########
@@ -2184,7 +2194,15 @@ && getLogicalTypeAnnotation(schemaElement.logicalType) != null)
childBuilder.id(schemaElement.field_id);
}
- childBuilder.named(schemaElement.name);
+ try {
+ childBuilder.named(schemaElement.name);
+ } catch (IllegalArgumentException e) {
+ if (!fileLogicalType) {
+ throw e;
+ }
+ LOG.warn("Dropping malformed FILE logical type annotation on group {}", schemaElement.name, e);
+ childBuilder.as((LogicalTypeAnnotation) null).named(schemaElement.name);
+ }
Review Comment:
Why do we suppress errors here and drop the type annotation? For other logical types, we only validate that the logical/physical type combination is valid. If the combination is valid but the annotation's parameters are invalid or corrupted, we don't drop the annotation and instead raise that error. For example, if `DECIMAL` annotates a `BOOLEAN` column, that's an invalid combination and gets suppressed. But if `DECIMAL` annotates an FLBA column but declares an invalid precision, that's a valid combination with invalid parameters and throws.
IMO we should do the same here: as long as FILE annotates a struct column, we should error if the FILE schema validation fails.
##########
parquet-column/src/main/java/org/apache/parquet/schema/LogicalTypeAnnotation.java:
##########
Review Comment:
We should have a simple test case to validate parsing of a text FILE schema in `parquet-column/src/test/java/org/apache/parquet/parser/TestParquetParser.java`. The schema can be something like
```
optional group file (FILE) {
optional binary uri (STRING);
}
```
##########
parquet-column/src/test/java/org/apache/parquet/io/api/TestFileValueWriter.java:
##########
@@ -0,0 +1,222 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.parquet.io.api;
+
+import static org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.BINARY;
+import static org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.INT64;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.ArrayDeque;
+import java.util.Arrays;
+import java.util.Deque;
+import org.apache.parquet.io.ExpectationValidatingRecordConsumer;
+import org.apache.parquet.schema.GroupType;
+import org.apache.parquet.schema.LogicalTypeAnnotation;
+import org.apache.parquet.schema.Types;
+import org.junit.jupiter.api.Test;
+
+public class TestFileValueWriter {
+ private static final GroupType ALL_FIELDS_SCHEMA = Types.optionalGroup()
+ .as(LogicalTypeAnnotation.fileType())
+ .optional(BINARY)
+ .as(LogicalTypeAnnotation.stringType())
+ .named("uri")
+ .optional(INT64)
+ .named("offset")
+ .optional(INT64)
+ .named("size")
+ .optional(BINARY)
+ .as(LogicalTypeAnnotation.stringType())
+ .named("content_type")
+ .optional(BINARY)
+ .as(LogicalTypeAnnotation.stringType())
+ .named("checksum")
+ .optional(BINARY)
+ .named("inline")
+ .named("file");
+
+ @Test
+ public void testWritesSetFieldsInSchemaOrder() {
Review Comment:
[nit] This case orders the FILE fields in the same order as the spec, so a bug in the writer that causes it to use the canonical order rather than the supplied one would still pass. Can we reorder the fields on the `FileValue` builder to exercise this?
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "alkis (via GitHub)" <gi...@apache.org>.
alkis commented on PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#issuecomment-5195437021
I had a look at this against the revised self-reference semantics in apache/parquet-format#603 and ended up writing some of it, so rather than leave it as review comments I opened it as a PR against this branch: brkyvz/parquet-java#1 (delta is just two commits on top of `fileType`).
The main piece is the writer-side API you'd need anyway: `FileValueWriter.write(payload)` returns either inline bytes or an `offset`/`size` pair based on a configurable threshold (`ParquetProperties.withFileSelfReferenceThreshold`), so object models don't choose between the two forms themselves. It has to write the payload eagerly, mid-record — `offset`/`size` are ordinary column values, so once one reaches a `ColumnWriter` it's encoded into a buffered page and a placeholder can't be patched up later.
Three things worth flagging regardless of whether you take the PR:
1. **The AAD uses a per-chunk counter, but the spec defines field 6 as the file offset.** The counter isn't recoverable from the file — nothing stores it, so a reader has to walk the preceding values to rebuild it, which defeats exactly the property the offset-keyed AAD is there for ("available to a reader without counting preceding values... may therefore resolve a self-reference without decoding the pages it skips"). Switching to `offset` also drops the parameter from five signatures, since the value already carries it.
2. **`decompressUnknownSize` doesn't work for any codec, including SNAPPY.** The drain loop ends on `read() == -1`, but `NonBlockedDecompressorStream` throws `IOException("Corrupt file: Zero bytes read during decompression.")` when its block is consumed rather than returning `-1`. It was never caught because CI dies at compile. Replaced with grow-and-retry into a dynamically sized buffer, which the spec explicitly allows; that covers LZ4_RAW too.
3. **`parquet-hadoop` has two test compile errors that block the module** independent of anything I changed (they fail on `be266d31`): `TestParquetMetadataConverter` calls `assertEquals`/`assertTrue` with no JUnit import, and `TestSelfReferenceFileWrite` is missing the `ParquetReadOptions` import. Fixing those is what let me actually run things — currently 679/679 in `parquet-column` and 761/762 in `parquet-hadoop`.
One thing I deliberately did **not** change: I'd tightened schema validation to require `inline` whenever `offset` is declared, on the reasoning that `uri` is optional per value so a `uri`+`offset`+`size` schema can still emit a self-reference with no `inline` column chunk to inherit from. But your tests assert that schema is valid, so I reverted it — your call which way it should go.
Also worth noting for sequencing: this can't go green until a parquet-format release carries `FileType` (2.13.0's `LogicalType` union stops at 18 — that's the current CI failure), and #603 is still open, so the AAD detail could still move. To build locally I pinned `parquet.format.version` to an artifact holding parquet-format master's `parquet.thrift`.
Happy to fold any of it in differently, or split it up, if that's easier for you.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "brkyvz (via GitHub)" <gi...@apache.org>.
brkyvz commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4426872571
##########
parquet-column/src/main/java/org/apache/parquet/file/FileConverters.java:
##########
@@ -0,0 +1,146 @@
+/*
Review Comment:
Moved `FileValue`, `FileConverters`, `FileValueWriter`, and `FileValueValidator` into [org.apache.parquet.file](https://github.com/apache/parquet-java/blob/bab237a1f7b57457052f520f7cd5dae84b211e16/parquet-column/src/main/java/org/apache/parquet/file), with their tests in the corresponding package. The logical annotation remains alongside the other annotations in `org.apache.parquet.schema`.
--
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
Re: [PR] Introduce implementation for FileType [parquet-java]
Posted by "Fokko (via GitHub) via issues" <is...@parquet.apache.org>.
Fokko commented on code in PR #3608:
URL: https://github.com/apache/parquet-java/pull/3608#discussion_r4443903180
##########
parquet-column/src/main/java/org/apache/parquet/schema/LogicalTypeAnnotation.java:
##########
@@ -1229,6 +1240,146 @@ public boolean equals(Object obj) {
}
}
+ /**
+ * File logical type annotation. Annotates a group (struct) that represents a reference to a
+ * range of bytes, which may be stored inline in the value or in an external file. Every field is
+ * optional, both in the schema (a writer may omit any field from the group definition) and in the
+ * data (any field that is present has a field repetition type of {@code OPTIONAL}). Fields are
+ * identified by name (case sensitively), not by field order. A group need only define the fields
+ * it uses. The group may contain the following fields:
+ * <ul>
+ * <li>{@code uri} (STRING): a URI-reference (RFC 3986) that identifies an external file, for
+ * example {@code s3://bucket/file.jpg}.</li>
+ * <li>{@code offset} (INT64): start of the byte range within the external file identified by
+ * {@code uri}; if not set, treated as 0. Must not be negative and may only be set together
+ * with {@code uri}.</li>
+ * <li>{@code size} (INT64): byte length of the referenced data within the external file. May be
+ * omitted for a whole-file external reference, in which case the range runs to the end of
+ * the referenced file. May only be set together with {@code uri}, must be set whenever
+ * {@code offset} is set, and must not be negative.</li>
+ * <li>{@code content_type} (STRING): the media (MIME) type (RFC 2046) of the resolved bytes;
+ * when not set, {@code application/octet-stream} is assumed.</li>
+ * <li>{@code checksum} (STRING): a self-describing integrity token for the resolved bytes, of
+ * the form {@code <algorithm>:<digest>}.</li>
+ * <li>{@code inline} (BYTE_ARRAY): the referenced bytes stored inline in the value.</li>
+ * </ul>
+ * No fields with names other than the above are permitted, and field names must be unique. Each
+ * declared field must match its required physical and logical types. Rules involving which fields
+ * are set and their values are enforced per value rather than by the schema.
+ */
+ public static class FileLogicalTypeAnnotation extends LogicalTypeAnnotation {
+ private static final FileLogicalTypeAnnotation INSTANCE = new FileLogicalTypeAnnotation();
+
+ /** Field name holding the URI-reference of an external file. */
+ public static final String URI_FIELD = "uri";
+
+ /** Field name holding the start of the byte range. */
+ public static final String OFFSET_FIELD = "offset";
+
+ /** Field name holding the byte length of the referenced data. */
+ public static final String SIZE_FIELD = "size";
+
+ /** Field name holding the media (MIME) type of the resolved bytes. */
+ public static final String CONTENT_TYPE_FIELD = "content_type";
+
+ /** Field name holding the integrity token for the resolved bytes. */
+ public static final String CHECKSUM_FIELD = "checksum";
+
+ /** Field name holding the referenced bytes stored inline. */
+ public static final String INLINE_FIELD = "inline";
+
+ /** All recognized field names in a FILE-annotated group. All fields are optional. */
+ public static final Set<String> FIELD_NAMES =
+ Set.of(URI_FIELD, OFFSET_FIELD, SIZE_FIELD, CONTENT_TYPE_FIELD, CHECKSUM_FIELD, INLINE_FIELD);
Review Comment:
```suggestion
Arrays.stream(FileField.values()).map(FileField::fieldName).collect(Collectors.toUnmodifiableSet());
```
--
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