[jira] [Commented] (CALCITE-3433) EQUALS operator between date/timestamp types returns false if the type is nullable
[
https://issues.apache.org/jira/browse/CALCITE-3433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16967263#comment-16967263
]
Feng Zhu commented on CALCITE-3433:
---
Hi [~julianhyde], I'm sorry I can't figure out such test cases to trigger the
cases you mention.
I think RexToLixTranslator is the final phase, and handles the comparasion
after optimizations (i.e., _{{(x1 < x2) or (x1 = x2) and (y1 < y2)}}_),
Do I get your point?
> EQUALS operator between date/timestamp types returns false if the type is
> nullable
> --
>
> Key: CALCITE-3433
> URL: https://issues.apache.org/jira/browse/CALCITE-3433
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.17.0, 1.18.0, 1.19.0, 1.20.0, 1.21.0
>Reporter: jiezouSH
>Assignee: Feng Zhu
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.22.0
>
> Time Spent: 2h 20m
> Remaining Estimate: 0h
>
> sql
> {code:sql}
> select time0 = time1
> from (select
> timestamp'2000-12-30 21:07:32' as time0,
> timestamp'2000-12-30 21:07:32' as time1
> union all
> select
> cast(null as timestamp) as time0,
> cast(null as timestamp) as time1)
> {code}
> answer is false
> but
> sql
> {code:sql}
> select time0 = time1
> from
> (select
> timestamp'2000-12-30 21:07:32' as time0,
> timestamp'2000-12-30 21:07:32'as time1)
> {code}
> answer is true
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3433) EQUALS operator between date/timestamp types returns false if the type is nullable
[
https://issues.apache.org/jira/browse/CALCITE-3433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16967000#comment-16967000
]
Julian Hyde commented on CALCITE-3433:
--
This is an improvement but it doesn't seem fully right. I can't help wondering
how we get away with it for other comparison operators (e.g. {{<}}). The
problem seems more to do with our use of {{eq}} than the fact that the logical
operator is {{EQUALS}} or {{NOT_EQUALS}}.
For example, tuple comparison {{t1 < t2}} will do something like {{(x1 < x2) or
(x1 = x2) and (y1 < y2)}} and may run afoul of {{eq}}.
> EQUALS operator between date/timestamp types returns false if the type is
> nullable
> --
>
> Key: CALCITE-3433
> URL: https://issues.apache.org/jira/browse/CALCITE-3433
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.17.0, 1.18.0, 1.19.0, 1.20.0, 1.21.0
>Reporter: jiezouSH
>Assignee: Feng Zhu
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.22.0
>
> Time Spent: 2h 20m
> Remaining Estimate: 0h
>
> sql
> {code:sql}
> select time0 = time1
> from (select
> timestamp'2000-12-30 21:07:32' as time0,
> timestamp'2000-12-30 21:07:32' as time1
> union all
> select
> cast(null as timestamp) as time0,
> cast(null as timestamp) as time1)
> {code}
> answer is false
> but
> sql
> {code:sql}
> select time0 = time1
> from
> (select
> timestamp'2000-12-30 21:07:32' as time0,
> timestamp'2000-12-30 21:07:32'as time1)
> {code}
> answer is true
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3433) EQUALS operator between date/timestamp types returns false if the type is nullable
[ https://issues.apache.org/jira/browse/CALCITE-3433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16961611#comment-16961611 ] Feng Zhu commented on CALCITE-3433: --- The PR only targets on "inp0_ == inp1_", making it as "Objects.equals(inp0_, inp1_)". It does not remove this null check logic. > EQUALS operator between date/timestamp types returns false if the type is > nullable > -- > > Key: CALCITE-3433 > URL: https://issues.apache.org/jira/browse/CALCITE-3433 > Project: Calcite > Issue Type: Bug > Components: core >Affects Versions: 1.17.0, 1.18.0, 1.19.0, 1.20.0, 1.21.0 >Reporter: jiezouSH >Assignee: Feng Zhu >Priority: Major > Labels: pull-request-available > Time Spent: 40m > Remaining Estimate: 0h > > sql > select time0=time1 from (select timestamp'2000-12-30 21:07:32'as > time0,timestamp'2000-12-30 21:07:32'as time1 union all select cast(null as > timestamp) as time0,cast(null as timestamp) as time1) calcs > answer is false > but > sql > select time0=time1 from (select timestamp'2000-12-30 21:07:32'as > time0,timestamp'2000-12-30 21:07:32'as time1) calcs > answer is true > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3433) EQUALS operator between date/timestamp types returns false if the type is nullable
[ https://issues.apache.org/jira/browse/CALCITE-3433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16961605#comment-16961605 ] Danny Chen commented on CALCITE-3433: - > Binary operators like Equals/NotEquals/Add are implemented in > BinaryImplementor, before which null check is a general phase. Could you explain why with your fix, this null check logic is removed ? > EQUALS operator between date/timestamp types returns false if the type is > nullable > -- > > Key: CALCITE-3433 > URL: https://issues.apache.org/jira/browse/CALCITE-3433 > Project: Calcite > Issue Type: Bug > Components: core >Affects Versions: 1.17.0, 1.18.0, 1.19.0, 1.20.0, 1.21.0 >Reporter: jiezouSH >Assignee: Feng Zhu >Priority: Major > Labels: pull-request-available > Time Spent: 0.5h > Remaining Estimate: 0h > > sql > select time0=time1 from (select timestamp'2000-12-30 21:07:32'as > time0,timestamp'2000-12-30 21:07:32'as time1 union all select cast(null as > timestamp) as time0,cast(null as timestamp) as time1) calcs > answer is false > but > sql > select time0=time1 from (select timestamp'2000-12-30 21:07:32'as > time0,timestamp'2000-12-30 21:07:32'as time1) calcs > answer is true > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3433) EQUALS operator between date/timestamp types returns false if the type is nullable
[
https://issues.apache.org/jira/browse/CALCITE-3433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16960990#comment-16960990
]
Feng Zhu commented on CALCITE-3433:
---
Hey [~danny0405] :
Sure, the code "_inp0_ == null || inp1_ == null_" can be optimized with some
efforts.
Binary operators like _Equals/NotEquals/Add_ are implemented in
_BinaryImplementor_, before which null check is a general phase.
But for my personal opinion, it requires work on the whole codegen framework,
not only for the problem in this case.
As discussed and illustrated in previous JIRAs (e.g., CALCITE-3224), I inclined
to refactor current codegen framework to guarantee
*{color:#FF}correctness{color}* first.
After that, we can then focus on optimizing redundant and unnecessary code
patterns.
How about your idea?
> EQUALS operator between date/timestamp types returns false if the type is
> nullable
> --
>
> Key: CALCITE-3433
> URL: https://issues.apache.org/jira/browse/CALCITE-3433
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.17.0, 1.18.0, 1.19.0, 1.20.0, 1.21.0
>Reporter: jiezouSH
>Assignee: Feng Zhu
>Priority: Major
> Labels: pull-request-available
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> sql
> select time0=time1 from (select timestamp'2000-12-30 21:07:32'as
> time0,timestamp'2000-12-30 21:07:32'as time1 union all select cast(null as
> timestamp) as time0,cast(null as timestamp) as time1) calcs
> answer is false
> but
> sql
> select time0=time1 from (select timestamp'2000-12-30 21:07:32'as
> time0,timestamp'2000-12-30 21:07:32'as time1) calcs
> answer is true
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3433) EQUALS operator between date/timestamp types returns false if the type is nullable
[
https://issues.apache.org/jira/browse/CALCITE-3433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16960957#comment-16960957
]
Danny Chen commented on CALCITE-3433:
-
Thanks [~donnyzone], can we gen code with out
{code:java}
inp0_ == null || inp1_ == null ? (Boolean) null
{code}
It seems that for EQUALS and NOT EQUALS, this "null check" is redundant and
unnecessary.
> EQUALS operator between date/timestamp types returns false if the type is
> nullable
> --
>
> Key: CALCITE-3433
> URL: https://issues.apache.org/jira/browse/CALCITE-3433
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.17.0, 1.18.0, 1.19.0, 1.20.0, 1.21.0
>Reporter: jiezouSH
>Assignee: Feng Zhu
>Priority: Major
> Labels: pull-request-available
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> sql
> select time0=time1 from (select timestamp'2000-12-30 21:07:32'as
> time0,timestamp'2000-12-30 21:07:32'as time1 union all select cast(null as
> timestamp) as time0,cast(null as timestamp) as time1) calcs
> answer is false
> but
> sql
> select time0=time1 from (select timestamp'2000-12-30 21:07:32'as
> time0,timestamp'2000-12-30 21:07:32'as time1) calcs
> answer is true
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3433) EQUALS operator between date/timestamp types returns false if the type is nullable
[
https://issues.apache.org/jira/browse/CALCITE-3433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16960846#comment-16960846
]
Feng Zhu commented on CALCITE-3433:
---
The bug is caused by wrong codegen for BinaryExpression. Even the expressions
are box types, it still adopts "==" to compare them.
The code for "union" query is demonstated as below, while for the other correct
query, the comparison is simplified as "true" before codegen phase.
{code:java}
public Object current() {
final Object[] current = (Object[]) inputEnumerator.current();
final Long inp0_ = (Long) current[0];
final Long inp1_ = (Long) current[1];
return inp0_ == null || inp1_ == null ? (Boolean) null :
Boolean.valueOf(inp0_ == inp1_);
}
{code}
I will take a look and fix this issue
> EQUALS operator between date/timestamp types returns false if the type is
> nullable
> --
>
> Key: CALCITE-3433
> URL: https://issues.apache.org/jira/browse/CALCITE-3433
> Project: Calcite
> Issue Type: Bug
> Components: core
>Affects Versions: 1.17.0, 1.18.0, 1.19.0, 1.20.0, 1.21.0
>Reporter: jiezouSH
>Priority: Major
>
> sql
> select time0=time1 from (select timestamp'2000-12-30 21:07:32'as
> time0,timestamp'2000-12-30 21:07:32'as time1 union all select cast(null as
> timestamp) as time0,cast(null as timestamp) as time1) calcs
> answer is false
> but
> sql
> select time0=time1 from (select timestamp'2000-12-30 21:07:32'as
> time0,timestamp'2000-12-30 21:07:32'as time1) calcs
> answer is true
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3433) EQUALS operator between date/timestamp types returns false if the type is nullable
[ https://issues.apache.org/jira/browse/CALCITE-3433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956327#comment-16956327 ] Julian Hyde commented on CALCITE-3433: -- The query's answer is wrong. Yes, this is a bug. I was speculating as to the cause of the bug. > EQUALS operator between date/timestamp types returns false if the type is > nullable > -- > > Key: CALCITE-3433 > URL: https://issues.apache.org/jira/browse/CALCITE-3433 > Project: Calcite > Issue Type: Bug > Components: core >Affects Versions: 1.17.0, 1.18.0, 1.19.0, 1.20.0, 1.21.0 >Reporter: jiezouSH >Priority: Major > > sql > select time0=time1 from (select timestamp'2000-12-30 21:07:32'as > time0,timestamp'2000-12-30 21:07:32'as time1 union all select cast(null as > timestamp) as time0,cast(null as timestamp) as time1) calcs > answer is false > but > sql > select time0=time1 from (select timestamp'2000-12-30 21:07:32'as > time0,timestamp'2000-12-30 21:07:32'as time1) calcs > answer is true > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3433) EQUALS operator between date/timestamp types returns false if the type is nullable
[ https://issues.apache.org/jira/browse/CALCITE-3433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16955813#comment-16955813 ] jiezouSH commented on CALCITE-3433: --- [~julianhyde] Yes, I have found that TIMESTAMP is being stored as a Long, and Date is being stored as a Integer and == is used to compare Long/Integer. But the query's answer is really confusing. > EQUALS operator between date/timestamp types returns false if the type is > nullable > -- > > Key: CALCITE-3433 > URL: https://issues.apache.org/jira/browse/CALCITE-3433 > Project: Calcite > Issue Type: Bug > Components: core >Affects Versions: 1.17.0, 1.18.0, 1.19.0, 1.20.0, 1.21.0 >Reporter: jiezouSH >Priority: Major > > sql > select time0=time1 from (select timestamp'2000-12-30 21:07:32'as > time0,timestamp'2000-12-30 21:07:32'as time1 union all select cast(null as > timestamp) as time0,cast(null as timestamp) as time1) calcs > answer is false > but > sql > select time0=time1 from (select timestamp'2000-12-30 21:07:32'as > time0,timestamp'2000-12-30 21:07:32'as time1) calcs > answer is true > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3433) EQUALS operator between date/timestamp types returns false if the type is nullable
[ https://issues.apache.org/jira/browse/CALCITE-3433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16955806#comment-16955806 ] Julian Hyde commented on CALCITE-3433: -- I wonder if the TIMESTAMP value is being stored as a Long (rather than long) because it is nullable, and we are using == to compare rather than Object.equals. > EQUALS operator between date/timestamp types returns false if the type is > nullable > -- > > Key: CALCITE-3433 > URL: https://issues.apache.org/jira/browse/CALCITE-3433 > Project: Calcite > Issue Type: Bug > Components: core >Affects Versions: 1.17.0, 1.18.0, 1.19.0, 1.20.0, 1.21.0 >Reporter: jiezouSH >Priority: Major > > sql > select time0=time1 from (select timestamp'2000-12-30 21:07:32'as > time0,timestamp'2000-12-30 21:07:32'as time1 union all select cast(null as > timestamp) as time0,cast(null as timestamp) as time1) calcs > answer is false > but > sql > select time0=time1 from (select timestamp'2000-12-30 21:07:32'as > time0,timestamp'2000-12-30 21:07:32'as time1) calcs > answer is true > -- This message was sent by Atlassian Jira (v8.3.4#803005)
