asifsmohammed commented on code in PR #3700:
URL: https://github.com/apache/parquet-java/pull/3700#discussion_r3723306669
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/metadata/FileMetaData.java:
##########
@@ -42,6 +44,8 @@ public enum EncryptionType {
private final MessageType schema;
private final Map<String, String> keyValueMetaData;
private final String createdBy;
+ private transient volatile ParsedVersion writerVersion;
+ private transient volatile boolean writerVersionParsed;
Review Comment:
I have a couple of callouts on this approach, happy to get your input on
this @wgtmac
The `shouldIgnoreStatistics(ParsedVersion, PrimitiveTypeName)` overload
achieves full behavioral parity
with the String-based overload for all real-world inputs. There are two
theoretical edge cases where
the log output differs (return value is always identical):
1. **Empty version field** (e.g., `"parquet-mr version (build abc)"`): The
log message appends
`ParsedVersion.toString()` instead of the raw `createdBy` string, since
the ParsedVersion overload
doesn't have access to the original string.
2. **Non-empty but invalid semver** (e.g., `"parquet-mr version xyz (build
abc)"`): The old code threw
`SemanticVersionParseException` caught by the outer catch block,
logging via `warnParseErrorOnce`
with a stack trace. The new code logs via `warnOnce` without a stack
trace, since `ParsedVersion`'s
constructor already caught and discarded the exception internally.
Neither case occurs in practice as no known parquet writer produces such
strings. For truly unparseable
`createdBy` strings (where `VersionParser.parse` itself fails),
`fromParquetMetadata` falls back to
the String-based path via the `useWriterVersion` flag, preserving exact
logging parity including the
stack trace.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
