blinding-pixels commented on PR #23627: URL: https://github.com/apache/datafusion/pull/23627#issuecomment-5013006018
I see how my wording was confusing. The matrix uses a neutral synthetic operator, so it's a shared-contract test, not a universal operator contract test. It proves that once an operator enables Range satisfaction, the shared optimizer handles all 24 matrix combinations correctly. It does not prove that production aggregates, joins, and windows enable Range satisfaction or reach that shared logic correctly. `KeyPartitioningRequirementExec` enables Range satisfaction itself, so removing an opt-in or breaking an operator-specific path would leave the matrix green. The mutations show this: * Test 7 — aggregate preservation path * Test 13 — partitioned-join guard * Test 14 — join preservation path * Tests 6 and 8 — same aggregate RoundRobin path, so one is redundant To make the SLT file leaner, I could keep the neutral matrix for the shared contract and add a second Rust table using real aggregate and join plans. That would replace the optimizer-plan coverage of all the proposed SLT cases and allow them to be removed. The existing window SLTs would still protect the window operators. This would add roughly 80–100 lines to the existing Rust test file. I'm new to this codebase, so I'll defer to you on whether the added Rust coverage is worthwhile or you'd rather just drop the SLT cases. -- 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]
