Re: Search/Sarg: untested feature merged to the default branch

[Haisheng Yuan]

I have the same feeling with Stamatis.

I completely understand that Vladimir is trying to keep it working correctly. 
However, I know some downstream project already updated Calcite version and 
changed their code to adapt to the Sarg/SEARCH operator. Reverting it and in 
case we failed to fix the regressions you mentioned, if we release the new 
version without getting it back, there will be another major breaking change 
again. If that is the case, I would rather push for the fixes instead of revert 
before we can release next version.

Thanks,
Haisheng Yuan

On 2020/11/09 08:32:20, Vladimir Sitnikov  wrote: 
> Danny>-1 for the revert, we should fix the issues we encountered instead of
> Danny>reverting the code brainless for a whole release.
> 
> Hi Danny, did you just veto the code change? (see
> https://www.apache.org/foundation/voting.html#Veto )
> I am afraid I fail to find a technical justification behind the veto.
> 
> Danny>So, logger the fail cases with issues and fix them, that is the way
> to go.
> 
> Thanks for the suggestion. I would tentatively decline it as it's going to
> be difficult to find time to do this.
> 
> Vladimir
> 

Re: ProjectReduceExpressionsRule vs matchNullability: should RelRule be allowed to change field nullability?

[Julian Hyde]

Probably RelRule should not change nullability. But we have done so in
the past, and I suspect we continue to do so today. It would be
interesting to find a list of rules that do this.

Deducing that a field is never null is very useful. With that
knowledge, further optimizations become possible. So, we must continue
to do it. But we also have to abide by the contract that we don't
change row type (except for field names).

How to square that circle? I think we should keep the columns' type as
nullable, but generate predicates (see RelMdPredicates,
RelOptPredicateList) that indicate that the field is never null.

We already use 'not null' predicates during the simplify process. If
we are simplifying 'x is null or x > 5', by the time we process the 'x
> 5' factor we will have deduced that x is not null. Predicates are
good, and we're making ever more use of them these days.

Similar issues could occur with other type constraints. Imagine simplifying

  select 'abc' from t1 where false
  union all
  select 'xy' from t2

We could tighten the type from CHAR(3) to CHAR(2) when we notice that
no rows will come from t1. But should we?

Julian

On Sun, Nov 8, 2020 at 10:46 AM Vladimir Sitnikov
 wrote:
>
> Hi,
>
> Can anybody clarify if RelRule should be allowed to change RelNode field
> nullability?
>
> For instance, RelOptRulesTest#testReduceNullableToNotNull has the following
> SQL:
> ...empno + *case* when 'a' = 'a' then 1 else *null end* as newcol...
>
> Of course, the initial type is nullable int (since the last CASE branch is
> null), however,
> you might see that the branch is never taken.
>
> A year or so ago the test ensured that the simplified output was +($0,
> CAST(1):INTEGER,
> however CALCITE-2852 RexNode simplification does not traverse unknown
> functions (see [1])
> 
> altered the behavior, so the actual test output became NEWCOL=[+($0, 1)]
> which means the rule was allowed
> to modify row type.
>
> It is strange if a rule could replace a RelNode with a different type, so I
> would expect all transformations to be type-preserving.
> Should those transformations be allowed?
> Should the planner deny such transformations?
> Should the planner add "cast to nullable" in case the original field was
> nullable?
>
> [1]:
> https://github.com/apache/calcite/pull/1053/files#diff-bc6d90f2392477eb02e40d8bc26ad2d8b0d6bd6dd2714ea0c2b94b3043ff9a8eR6952
>
> Vladimir

Re: Search/Sarg: untested feature merged to the default branch

[Vladimir Sitnikov]

Danny>-1 for the revert, we should fix the issues we encountered instead of
Danny>reverting the code brainless for a whole release.

Hi Danny, did you just veto the code change? (see
https://www.apache.org/foundation/voting.html#Veto )
I am afraid I fail to find a technical justification behind the veto.

Danny>So, logger the fail cases with issues and fix them, that is the way
to go.

Thanks for the suggestion. I would tentatively decline it as it's going to
be difficult to find time to do this.

Vladimir