peter-toth commented on PR #58895: URL: https://github.com/apache/spark/pull/58895#issuecomment-5797805610
Thank you for the review, @dongjoon-hyun -- it was unusually thorough, and two of the comments changed the design rather than the code. Every comment is addressed, in one commit on top of the reviewed one (b9981cda93b) so the delta stays readable. Replies are inline. The larger ones: - The survivor buffer is bounded now, in the cheaper form you suggested: phase 1 counts the bytes it has buffered and gives that row group up past an internal cap, after which phase 2 reads every projected column of the surviving rows. - `FileFormat.supportsStorageFilter` replaces the planner's exact-class check, so `FileSourceStrategy` no longer imports the parquet package. - The offset-index check is gone. Phase 2's read is wrapped instead and parquet's own `MissingOffsetIndexException` rethrown with guidance, so a file without a page index still scans as long as the filter never prunes inside a row group. - Every loud failure on this path now throws a new `UnsupportedFileReadException` that `ignoreCorruptFiles` does not swallow. Chasing your offset-index comment turned that up: `shouldIgnoreCorruptFileException` matches any `RuntimeException`, so before this every failure the feature depends on could have been read as a corrupt file and silently dropped the rest of a healthy one's rows. - Your design question also produced a planner gate. An all-keys projection is no longer pushed at all: the reader has to read a key column to evaluate the filter on it, so such a scan reads the same columns for the same rows as a plain one and pushing can only add cost. The description now carries a **Design decisions** section answering the design question up front, and a measured table on a real workload's file at the end of the test section. -- 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]
