Danny>We all vote -1 and what's the problem there you still want to revert ?
Danny, I see only you mention "-1", however, I see no technical reasons, so
I assume the veto has no weight.
I see the general feeling is like "oh, we want to keep the SEARCH feature
in the subsequent release" (which is my feeling as well!),
however, I see nobody commits to fixing SEARCH issues.
Note: the codebase has issues that are caused by SEARCH, and it has issues
that existed **before** SEARCH.
It makes it harder to fix SEARCH since it requires to fix all the bugs at
the same time.
Now I step in, I fix RexSimplify old issues, I log bugs for further
discussion (like CALCITE-4397).
It is really sad to hear "you always push the things that rejected by most
of the fellows".
Danny>If the RexProgramFuzzyTest can not run, just fix it
Let me please repeat: the issue is that SEARCH produces invalid results,
and Sarg produces NPEs.
Sample input: case_(eq(vInt(1), vInt(1)), falseLiteral, lt(vInt(1),
literal(1)))
Error is result mismatch: when applied to {?0.int1=NULL}, CASE(=(?0.int1,
?0.int1), false, <(?0.int1, 1)) yielded NULL, and false yielded false
Danny>it is the parallelism test that cause the Sarg state unstable.
The above sample can be run in isolation, and it does result in wrong
results.
It might be a nice idea to double-check if SEARCH implementation has a
shared mutable state.
---
Of course, the fuzzer might uncover more issues if it generates Sarg
expressions as well (it should not be hard to implement by the way),
however, I would stress that the current Sarg produces wrong results even
for regular (no SEARCH) inputs which impacts all Calcite users.
Believe me, I want to get SEARCH working as well, however, it might take
significant efforts, so I believe it would be more efficient
if we revert the feature, fix tests, add workarounds to make tests pass
again, and then try re-adding SEARCH.
As you probably know, CALCITE-3457 introduced a regression that is present
for more than a year now.
Note that the regression was identified right after the merge, and nobody
cared for the whole year.
That allowed bad code to creep unnoticed, and now we have several
RexSimplify issues that are not really related to SEARCH.
Note: my "revert search" pull request fixes CALCITE-3457 regression, so I
strongly disagree with your wording of "reverting the code brainless".
I truly believe the revert moves Calcite code forward, and it enables us to
re-integrate SEARCH in a better and nicer way.
Vladimir
