Skip to content

Cache ParsedVersion in FileMetaData to eliminate redundant created_by parsing #3696

Description

@asif-moh

Summary

VersionParser.parse(createdBy) is called from 7 distinct production sites, all parsing the same constant string from FileMetaData.getCreatedBy(). Since FileMetaData is constructed once per file and already stores the createdBy string, it is the natural place to parse once and cache.

Problem

The created_by string is re-parsed into a ParsedVersion at every call site independently:

# Call site Frequency per file When
1 CorruptStatistics.shouldIgnoreStatistics via buildColumnChunkMetaData R × C Footer decode
2 CorruptStatistics.shouldIgnoreStatistics via ParquetFileReader.readAllPages Pages per column chunk Page read
3 CorruptStatistics.shouldIgnoreStatistics via ParquetRewriter.convertStatistics Pages per rewritten chunk Rewrite
4 CorruptStatistics.shouldIgnoreStatistics via EncryptedColumnChunkMetaData.decryptIfNeeded 1 per encrypted column Lazy decrypt
5 CorruptDeltaByteArrays.requiresSequentialReads(String, Encoding) in ParquetRecordReader 1 per reader init Reader init
6 ColumnReadStoreImpl constructor via MessageColumnIO.getRecordReader R (once per row group read) Row group materialization
7 ColumnReadStoreImpl constructor via ParquetRewriter.nullifyColumn 1 per nullified column Rewrite with nullification

ColumnReadStoreImpl already parses createdBy into a ParsedVersion and stores it as a field — but does so R times (once per row group) because nobody upstream caches the parsed result.

Proposed Change

Add a cached ParsedVersion field to FileMetaData:

// In FileMetaData:
private final transient ParsedVersion writerVersion;

public FileMetaData(...) {
    ...
    this.createdBy = createdBy;
    this.writerVersion = parseVersion(createdBy);
    ...
}

public ParsedVersion getWriterVersion() {
    return writerVersion;
}

private static ParsedVersion parseVersion(String createdBy) {
    if (Strings.isNullOrEmpty(createdBy)) return null;
    try {
        return VersionParser.parse(createdBy);
    } catch (RuntimeException | VersionParseException e) {
        return null;
    }
}

Then add ParsedVersion-accepting overloads following the existing pattern established by CorruptDeltaByteArrays.requiresSequentialReads(ParsedVersion, Encoding):

  • CorruptStatistics.shouldIgnoreStatistics(ParsedVersion, PrimitiveTypeName)
  • ColumnReadStoreImpl constructor accepting ParsedVersion directly

Why This Approach

Scope

This issue covers:

  1. Adding the ParsedVersion field and getter to FileMetaData
  2. Adding shouldIgnoreStatistics(ParsedVersion, PrimitiveTypeName) overload to CorruptStatistics
  3. Updating ColumnReadStoreImpl to accept ParsedVersion directly
  4. Migrating existing call sites to use the cached version where FileMetaData is accessible

Context

Discussion: #3607 (comment)
Related: #3601

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions