4ktLuffy commented on issue #5328:
URL:
https://github.com/apache/datafusion-comet/issues/5328#issuecomment-5256575980
To sharpen the distinction above, since it is the obvious first question:
**what this suite adds is population coverage, not a new instance.**
`CometCodegenSuite`'s `(#5218)` test guards *that* instance — `add_months`
over `('notadate', NULL)` with ANSI on — and it guards it well. But the
mechanism it protects against is general: `canShortCircuitNulls` gates on
`isNullIntolerant(expr) && allNullIntolerant(expr)` plus the ordinal count, so
the same short-circuit reasoning applies to **any** multi-input
`NullIntolerant` tree. `CometCodegenSourceSuite`'s two guards already
demonstrate that generality by using different expressions for the same shape
(`Add(Cast(..), ..)` and `MakeTimestamp(Cast(..), ..)`).
What is missing is mechanical: nothing sweeps error-class parity under
forced fallback for the other 292 registered expressions. Each existing guard
was written because someone already knew where to look. The invariant here does
not need to know — it compares outcomes for whatever is in the corpus, and
reports a named witness when they diverge.
So the honest accounting is: the replay validated the harness, `#5218`'s own
tests own that instance, and the contribution is the sweep that would cover the
rest of the population as the corpus grows. If maintainers would rather see the
corpus widened substantially before this is worth merging, I am happy to do
that in this PR — widening is mechanical, and the bind gate is what keeps the
coverage number honest as it does.
--
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]
