From bc23cf52e6e82eb5b6755520b12d067bc0fbf1a3 Mon Sep 17 00:00:00 2001 From: Asif Mohammed Date: Fri, 31 Jul 2026 20:56:22 -0500 Subject: [PATCH 1/2] GH-3696: Cache ParsedVersion in FileMetaData to eliminate redundant parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parse the createdBy version string once during FileMetaData construction and cache the result as a transient field. This avoids redundant VersionParser.parse() calls at every downstream call site (R×C times during footer decode alone). --- .../parquet/hadoop/metadata/FileMetaData.java | 23 ++++++ .../hadoop/metadata/FileMetaDataTest.java | 77 +++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 parquet-hadoop/src/test/java/org/apache/parquet/hadoop/metadata/FileMetaDataTest.java diff --git a/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/metadata/FileMetaData.java b/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/metadata/FileMetaData.java index 4143dd805a..07257035c0 100644 --- a/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/metadata/FileMetaData.java +++ b/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/metadata/FileMetaData.java @@ -24,6 +24,8 @@ import java.io.Serializable; import java.util.Map; import java.util.Objects; +import org.apache.parquet.VersionParser; +import org.apache.parquet.VersionParser.ParsedVersion; import org.apache.parquet.crypto.InternalFileDecryptor; import org.apache.parquet.schema.MessageType; @@ -42,6 +44,7 @@ public enum EncryptionType { private final MessageType schema; private final Map keyValueMetaData; private final String createdBy; + private final transient ParsedVersion writerVersion; private final InternalFileDecryptor fileDecryptor; private final EncryptionType encryptionType; @@ -80,6 +83,7 @@ public FileMetaData( this.keyValueMetaData = unmodifiableMap(Objects.requireNonNull(keyValueMetaData, "keyValueMetaData cannot be null")); this.createdBy = createdBy; + this.writerVersion = parseVersion(createdBy); this.fileDecryptor = fileDecryptor; this.encryptionType = encryptionType; } @@ -118,4 +122,23 @@ public InternalFileDecryptor getFileDecryptor() { public EncryptionType getEncryptionType() { return encryptionType; } + + /** + * @return the parsed writer version, or {@code null} if {@code createdBy} is null, empty, or unparseable + */ + @JsonIgnore + public ParsedVersion getWriterVersion() { + return writerVersion; + } + + private static ParsedVersion parseVersion(String createdBy) { + if (createdBy == null || createdBy.isEmpty()) { + return null; + } + try { + return VersionParser.parse(createdBy); + } catch (RuntimeException | VersionParser.VersionParseException e) { + return null; + } + } } diff --git a/parquet-hadoop/src/test/java/org/apache/parquet/hadoop/metadata/FileMetaDataTest.java b/parquet-hadoop/src/test/java/org/apache/parquet/hadoop/metadata/FileMetaDataTest.java new file mode 100644 index 0000000000..7b9e2f0fd9 --- /dev/null +++ b/parquet-hadoop/src/test/java/org/apache/parquet/hadoop/metadata/FileMetaDataTest.java @@ -0,0 +1,77 @@ +/* + * 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.metadata; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Collections; +import org.apache.parquet.schema.MessageType; +import org.apache.parquet.schema.PrimitiveType; +import org.apache.parquet.schema.Type; +import org.junit.jupiter.api.Test; + +class FileMetaDataTest { + + private static final MessageType SCHEMA = new MessageType( + "test", new PrimitiveType(Type.Repetition.REQUIRED, PrimitiveType.PrimitiveTypeName.INT32, "id")); + + @Test + void validCreatedByIsParsed() { + FileMetaData meta = + new FileMetaData(SCHEMA, Collections.emptyMap(), "parquet-mr version 1.12.0 (build abc123)"); + + assertThat(meta.getWriterVersion()).isNotNull(); + assertThat(meta.getWriterVersion().application).isEqualTo("parquet-mr"); + assertThat(meta.getWriterVersion().version).isEqualTo("1.12.0"); + assertThat(meta.getWriterVersion().appBuildHash).isEqualTo("abc123"); + } + + @Test + void nullCreatedByReturnsNullWriterVersion() { + FileMetaData meta = new FileMetaData(SCHEMA, Collections.emptyMap(), null); + + assertThat(meta.getWriterVersion()).isNull(); + assertThat(meta.getCreatedBy()).isNull(); + } + + @Test + void emptyCreatedByReturnsNullWriterVersion() { + FileMetaData meta = new FileMetaData(SCHEMA, Collections.emptyMap(), ""); + + assertThat(meta.getWriterVersion()).isNull(); + } + + @Test + void unparseableCreatedByReturnsNullWriterVersion() { + FileMetaData meta = new FileMetaData(SCHEMA, Collections.emptyMap(), "no-version-here"); + + assertThat(meta.getWriterVersion()).isNull(); + } + + @Test + void versionWithoutBuildHash() { + FileMetaData meta = new FileMetaData(SCHEMA, Collections.emptyMap(), "parquet-mr version 1.8.0"); + + assertThat(meta.getWriterVersion()).isNotNull(); + assertThat(meta.getWriterVersion().application).isEqualTo("parquet-mr"); + assertThat(meta.getWriterVersion().version).isEqualTo("1.8.0"); + assertThat(meta.getWriterVersion().appBuildHash).isNull(); + } + +} From 51ca7b77a4fc49817090f4d5c40fb48c450cae25 Mon Sep 17 00:00:00 2001 From: Asif Mohammed Date: Sun, 2 Aug 2026 12:31:53 -0500 Subject: [PATCH 2/2] Address review: lazy init for writerVersion - Change writerVersion to lazy computation on first getWriterVersion() call - Fixes deserialization correctness (transient fields recompute from createdBy) - Add writerVersionParsed flag to avoid retrying on parse failure - Document contract for distinguishing missing vs. unparseable in javadoc --- .../parquet/hadoop/metadata/FileMetaData.java | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/metadata/FileMetaData.java b/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/metadata/FileMetaData.java index 07257035c0..f9baa249e3 100644 --- a/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/metadata/FileMetaData.java +++ b/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/metadata/FileMetaData.java @@ -44,7 +44,8 @@ public enum EncryptionType { private final MessageType schema; private final Map keyValueMetaData; private final String createdBy; - private final transient ParsedVersion writerVersion; + private transient volatile ParsedVersion writerVersion; + private transient volatile boolean writerVersionParsed; private final InternalFileDecryptor fileDecryptor; private final EncryptionType encryptionType; @@ -83,7 +84,6 @@ public FileMetaData( this.keyValueMetaData = unmodifiableMap(Objects.requireNonNull(keyValueMetaData, "keyValueMetaData cannot be null")); this.createdBy = createdBy; - this.writerVersion = parseVersion(createdBy); this.fileDecryptor = fileDecryptor; this.encryptionType = encryptionType; } @@ -124,21 +124,23 @@ public EncryptionType getEncryptionType() { } /** - * @return the parsed writer version, or {@code null} if {@code createdBy} is null, empty, or unparseable + * Returns the parsed writer version from the {@code createdBy} string, or {@code null} + * if {@code createdBy} is null, empty, or unparseable. The result is computed lazily + * and cached. Callers that need to distinguish missing vs. unparseable can check + * {@link #getCreatedBy()}. */ @JsonIgnore public ParsedVersion getWriterVersion() { - return writerVersion; - } - - private static ParsedVersion parseVersion(String createdBy) { - if (createdBy == null || createdBy.isEmpty()) { - return null; - } - try { - return VersionParser.parse(createdBy); - } catch (RuntimeException | VersionParser.VersionParseException e) { - return null; + if (!writerVersionParsed) { + if (createdBy != null && !createdBy.isEmpty()) { + try { + writerVersion = VersionParser.parse(createdBy); + } catch (RuntimeException | VersionParser.VersionParseException e) { + // leave writerVersion as null — parse failed + } + } + writerVersionParsed = true; } + return writerVersion; } }