IgorBerman commented on PR #59706: URL: https://github.com/apache/spark/pull/59706#issuecomment-6381862414
@ohad-definity following finding of @sunchao wdyt about I'm adding more coverage in SchemaPruningThroughGenerateSuite? specifically as insurance against someone later extending SelectedField. P1 existed precisely because ArrayFilter/ArraySort were added as transparent path nodes (SPARK-57022) without the generator path collecting their lambda reads. If someone later adds sort_array or array_distinct to SelectedField the same way, P1 comes straight back — and only a test whose results change under wrong narrowing would catch it. That's why the data has to be crafted adversarially (order-by-first-field ≠ order-by-kept-field; duplicates differing only in the unread field), not just scan-schema assertions. The set I'd add to SchemaPruningThroughGenerateSuite (each ×V1/V2, using the existing flag-off baseline oracle): 1. posexplode(sort_array(items)) — interpreted whole-element comparator, no lambda. Pins conservative whole; data where sorting by the unread field would reorder pos pairs. 2. explode(array_distinct(items)) — whole-element equality. Duplicates differing only in the unread field, so wrong narrowing collapses rows. Representative for the union/except/intersect/remove family. 3. explode(array_union(items, items2)) — one binary set op, which also exercises the two-references → fallback guard in the impl. 4. posexplode(reverse(items)) — the opposite pin: a transparent wrapper that should keep pruning (<b> only) with pos pairing preserved. Guards against someone "fixing" the wrapper handling by making it conservative. 5. Chained outer-capture: explode(items) … explode(filter(item.parts, x -> x.inStock AND item.active)) — the new fix's subtlest path: lambda reads of the outer element flowing through elementFields to the lower generator. Prunes label, keeps active+inStock. Not covered by current regression tests, verified manually. **This is the one probably is most important one.** 6. Strict-null comparator: array_sort(items, (l, r) -> l.a - r.a) with nullable result — pins the error-parameter conservatism (whole element kept), documented with the suite's "not narrowed today" comment style. -- 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]
