yihua commented on code in PR #18441:
URL: https://github.com/apache/hudi/pull/18441#discussion_r3034532291
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/HiveIncrementalPuller.java:
##########
@@ -126,7 +125,29 @@ private void validateConfig(Config config) {
}
}
+ private void validateIncrementalSQL() throws IOException {
Review Comment:
🤖 The SQL file is now read twice — once here for validation and again in
`executeIncrementalSQL`. Could this method return the validated SQL string so
it can be passed into `executeIncrementalSQL` instead of re-reading? That would
also eliminate a subtle TOCTOU window where the file could change between the
two reads.
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/HiveIncrementalPuller.java:
##########
@@ -175,30 +196,18 @@ public void saveDelta() throws IOException {
}
}
- private void executeIncrementalSQL(String tempDbTable, String
tempDbTablePath, Statement stmt)
+ void executeIncrementalSQL(String tempDbTable, String tempDbTablePath,
Statement stmt)
Review Comment:
🤖 Now that validation was moved to `validateIncrementalSQL`, this method
still declares `throws FileNotFoundException` and re-reads the file. If the
validated SQL were passed in as a parameter, this method wouldn't need file
access at all — making it easier to test without a real file on disk.
--
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]
