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:
- Adding the
ParsedVersion field and getter to FileMetaData
- Adding
shouldIgnoreStatistics(ParsedVersion, PrimitiveTypeName) overload to CorruptStatistics
- Updating
ColumnReadStoreImpl to accept ParsedVersion directly
- Migrating existing call sites to use the cached version where
FileMetaData is accessible
Context
Discussion: #3607 (comment)
Related: #3601
Summary
VersionParser.parse(createdBy)is called from 7 distinct production sites, all parsing the same constant string fromFileMetaData.getCreatedBy(). SinceFileMetaDatais constructed once per file and already stores thecreatedBystring, it is the natural place to parse once and cache.Problem
The
created_bystring is re-parsed into aParsedVersionat every call site independently:CorruptStatistics.shouldIgnoreStatisticsviabuildColumnChunkMetaDataCorruptStatistics.shouldIgnoreStatisticsviaParquetFileReader.readAllPagesCorruptStatistics.shouldIgnoreStatisticsviaParquetRewriter.convertStatisticsCorruptStatistics.shouldIgnoreStatisticsviaEncryptedColumnChunkMetaData.decryptIfNeededCorruptDeltaByteArrays.requiresSequentialReads(String, Encoding)inParquetRecordReaderColumnReadStoreImplconstructor viaMessageColumnIO.getRecordReaderColumnReadStoreImplconstructor viaParquetRewriter.nullifyColumnColumnReadStoreImplalready parsescreatedByinto aParsedVersionand 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
ParsedVersionfield toFileMetaData:Then add
ParsedVersion-accepting overloads following the existing pattern established byCorruptDeltaByteArrays.requiresSequentialReads(ParsedVersion, Encoding):CorruptStatistics.shouldIgnoreStatistics(ParsedVersion, PrimitiveTypeName)ColumnReadStoreImplconstructor acceptingParsedVersiondirectlyWhy This Approach
transientfield + getter, no breaking changestransientCorruptDeltaByteArraysalready hasParsedVersion-based overloads used fromColumnReaderBaseParsedVersion-based APIs instead of threading a PARQUET-251-specific booleanScope
This issue covers:
ParsedVersionfield and getter toFileMetaDatashouldIgnoreStatistics(ParsedVersion, PrimitiveTypeName)overload toCorruptStatisticsColumnReadStoreImplto acceptParsedVersiondirectlyFileMetaDatais accessibleContext
Discussion: #3607 (comment)
Related: #3601