JAIPilot campaign evaluation: apache/parquet-java #3700 - #1
Open
skrcode wants to merge 5 commits into
Open
Conversation
…dant parsing 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).
- 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
…zed init - Replace writerVersion + writerVersionParsed with single WriterVersionResult - Null field means not-yet-initialized, MISSING for null/empty createdBy - Double-checked locking with synchronized for thread-safe one-time init - Rethrow cached VersionParseException so callers preserve existing fallback - Use Strings.isNullOrEmpty for consistency with CorruptStatistics
Add shouldIgnoreStatistics(ParsedVersion, PrimitiveTypeName) overload to CorruptStatistics that uses the pre-parsed and cached SemanticVersion from ParsedVersion, eliminating redundant VersionParser.parse and SemanticVersion.parse calls in the R×C hot path. Refactor ParquetMetadataConverter.fromParquetMetadata to construct the hadoop FileMetaData before the row-group loop and extract the cached ParsedVersion once via getWriterVersion(). The loop now uses the ParsedVersion-based buildColumnChunkMetaData overload, avoiding per-column re-parsing. Falls back to the String-based path when getWriterVersion() throws VersionParseException to preserve exact logging parity.
- Add createdBy parameter to shouldIgnoreStatistics(ParsedVersion, ...) to resolve null ambiguity (distinct 3-param signature) and restore exact log parity by using the raw string in warnings. - Re-parse SemanticVersion in the !hasSemanticVersion() branch to recreate the exception for warnParseErrorOnce with stack trace. - Move shouldIgnoreStatistics evaluation inside the V1 legacy min/max branch so V2 stats never trigger the one-shot warning. - Restore original comments in both CorruptStatistics and ParquetMetadataConverter. - Add regression test for V2 stats with corrupt writer version.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Exact cloud-only evaluation mirror of apache#3700
Exact upstream head: 8ac10c9
JAIPilot must return at most one small, behavior-preserving, independently provable companion draft or leave this PR unchanged. No repository build is being run on a developer laptop.