qlong commented on PR #58050: URL: https://github.com/apache/spark/pull/58050#issuecomment-5331928355
Thanks @viirya for picking this up — the soundness handling (or(leaf, isNotNull(residual)...), the negation guard, exact-case variant-key matching) fixes the holes that made the original #54598 unsound. The reason I didn't push #54598 further was I couldn't demonstrate measurable improvement. I tested your PR using https://github.com/cloudera-labs/variant-conformance-benchmark, the results (TPC-DS SF=5 flat variant + GHA 1-day event-payload, shreddedPredicatePushdown ON vs OFF, same build) does not show meaningful difference for three reasons for those two workloads: 1. Filtered fields aren't always shredded. 2. Predicates are mostly join keys, not literals. 3. One row group per file at test scale. I do think the theorical lift of rowgroup skipping is very high, but it requires a few things line up: 1. the filtered field is shreddred 2. literal predicate 3. **data sorted** on that field 4. **large number of rowgroup** in a file 3 & 4 would require some delicated setup or tuning. have you run a benchmark showing measurable improvement on realistic data? That would significantly strengthen the case for carrying this soundness-critical code. -- 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]
