danny0405 commented on pull request #2238:
URL: https://github.com/apache/calcite/pull/2238#issuecomment-722216992
> > RexProgramTest already does that.
>
> It has a very limited set of checks. Do you have an exhaustive check? I do
not see that, so you can't claim `RexProgramTest`
danny0405 commented on pull request #2238:
URL: https://github.com/apache/calcite/pull/2238#issuecomment-722199402
> 3457 is very related because it was the reason to disable fuzzer testing
which is a significant test case for RexSimplify logic.
>
> For instance, you've just pushed
danny0405 commented on pull request #2238:
URL: https://github.com/apache/calcite/pull/2238#issuecomment-722188731
> @danny0405 , before you commit more fixes to `RexSimplify` could I kindly
ask you to review the regression which was introduced a year ago
danny0405 commented on pull request #2238:
URL: https://github.com/apache/calcite/pull/2238#issuecomment-721613490
> I believe the logic with `needToFix` makes no sense. Sarg replacement
should be on per-sarg basis rather than "all or nothing"
I'm not sure, because the Sarg
danny0405 commented on pull request #2238:
URL: https://github.com/apache/calcite/pull/2238#issuecomment-721595262
@vlsi Do you have other comments ? I'm planning to merge in the following 24
hours.
This is an automated
danny0405 commented on pull request #2238:
URL: https://github.com/apache/calcite/pull/2238#issuecomment-720289249
> We should merge two sargs if they apply to the same argument
Totally agree, and actually this patch makes more sargs merged.
