peter-toth commented on PR #58895: URL: https://github.com/apache/spark/pull/58895#issuecomment-5820024251
Thank you for the second round. It was as thorough as the first, and the individual replies carry the details. The comment that mattered most was the design question on treating the extracted filter as an optional hint. That is what the PR does now, and it went one step further than the suggestion: the conjunct is not moved out of the plan at all, it is copied. It stays in the post-scan `Filter`, so the reader may give a file or a row group up freely, and `UnsupportedFileReadException`, its case in the shared classifier and the hard failure on a page-index-less file are all gone. Two things settled that direction. The bloom is already built by the time the scan runs, so having the `Filter` use it regardless costs nothing extra and means it is never built for nothing. And whoever turns this conf on is doing it because the filter prunes a lot, so evaluating it a second time on the rows that survive is a small price rather than a regression. The description is rewritten around that, with a Design decisions section. It also now carries a measurement of splicing against re-reading the key columns, which answers your earlier question with numbers: re-reading transfers up to 49% more bytes than splicing, and more than the feature being off, because phase 1 has already read the key column once. Everything is in c255f88. -- 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]
