You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Chathura Widanage <ch...@gmail.com> on 2022/02/10 04:17:21 UTC
Invalid IntervalSQLType
Hi community,
I'm comparing two rel expressions generated by calcite 1.25.0 and 1.29.0
and noticed there is an invalid IntervalSQLType plugged into the query.
<=($6, 1998-09-02 00:00:00) : Calcite 1.25.0
vs
<=($6, CAST(-(1998-12-01, 7776000000:INTERVAL DAY)):TIMESTAMP(0) NOT NULL) :
Calcite 1.29.0
7776000000 is 90 days in milliseconds, but the IntervalSQLType/value
combination is invalid.
Could you please let me know whether this could be a bug and whether there
an option to prevent such transformations at all?
Regards,
Chathura
PS: This comes on queries from tpch benchmark and invalid conversion is
from tpch-01.
I'm seeing similar conversions in other queries, but they seem to be
correct, but feels this transformation is redundant.
AND(>=($1, 1993-07-01 00:00:00), <($1, 1993-10-01 00:00:00))
vs
AND(>=($1, 1993-07-01 00:00:00), <($1, CAST(+(1993-07-01, 3:INTERVAL
MONTH)):TIMESTAMP(0) NOT NULL))
Re: Invalid IntervalSQLType
Posted by Alessandro Solimando <al...@gmail.com>.
I agree with Stamatis, and as always adding a unit test and using git
bisect can help spot the exact commit that introduced the change.
HTH,
Alessandro
On Thu, 10 Feb 2022 at 12:37, Stamatis Zampetakis <za...@gmail.com> wrote:
> Hi Chathura,
>
> It is difficult to reason about correctness without having the actual SQL
> query at hand.
>
> The fact that you have milliseconds is not by itself a problem and has to
> do with the way Calcite internally represents intervals (see comment in
> [1]).
>
> Also from the examples you provided the behavior in 1.29.0 does not seem to
> be an additional transformation rather than a missing simplification
> (constant reduction). I am not sure if this is intentional or not but I
> guess you can have a look at the changes landed around
> RexSimplify/RexExecutor.
>
> Best,
> Stamatis
>
> [1]
>
> https://github.com/apache/calcite/blob/812e3e98eae518cf85cd1b6b7f055fb96784a423/core/src/main/java/org/apache/calcite/rex/RexLiteral.java#L357
>
>
> On Thu, Feb 10, 2022 at 8:02 AM Chathura Widanage <
> chathurawidanage@gmail.com> wrote:
>
> > Hi community,
> >
> > I'm comparing two rel expressions generated by calcite 1.25.0 and 1.29.0
> > and noticed there is an invalid IntervalSQLType plugged into the query.
> >
> > <=($6, 1998-09-02 00:00:00) : Calcite 1.25.0
> > vs
> > <=($6, CAST(-(1998-12-01, 7776000000:INTERVAL DAY)):TIMESTAMP(0) NOT
> NULL)
> > :
> > Calcite 1.29.0
> >
> > 7776000000 is 90 days in milliseconds, but the IntervalSQLType/value
> > combination is invalid.
> >
> > Could you please let me know whether this could be a bug and whether
> there
> > an option to prevent such transformations at all?
> >
> > Regards,
> > Chathura
> >
> > PS: This comes on queries from tpch benchmark and invalid conversion is
> > from tpch-01.
> >
> > I'm seeing similar conversions in other queries, but they seem to be
> > correct, but feels this transformation is redundant.
> >
> > AND(>=($1, 1993-07-01 00:00:00), <($1, 1993-10-01 00:00:00))
> > vs
> > AND(>=($1, 1993-07-01 00:00:00), <($1, CAST(+(1993-07-01, 3:INTERVAL
> > MONTH)):TIMESTAMP(0) NOT NULL))
> >
>
Re: Invalid IntervalSQLType
Posted by Chathura Widanage <ch...@gmail.com>.
Thanks, Julian. Now it makes sense.
Regards,
Chathura
On Fri, Feb 11, 2022 at 2:02 AM Julian Hyde <jh...@gmail.com> wrote:
> There are two kinds of intervals (in Calcite and in standard SQL).
> Days-hours-minutes-seconds-millseconds intervals, and year-month intervals.
> The former are represented internally in milliseconds. The latter are
> represented in months.
>
> This is one area where Postgres does things differently from the SQL
> standard.
>
>
> > On Feb 10, 2022, at 3:48 AM, Chathura Widanage <
> chathurawidanage@gmail.com> wrote:
> >
> > Thanks, Stamatis. Below is the original SQL query.
> >
> > select l_returnflag, l_linestatus, sum(l_quantity) as sum_qty,
> > sum(l_extendedprice) as sum_base_price,
> sum(l_extendedprice*(1-l_discount))
> > as sum_disc_price, sum(l_extendedprice*(1-l_discount)*(1+l_tax)) as
> > sum_charge, avg(l_quantity) as avg_qty, avg(l_extendedprice) as
> avg_price,
> > avg(l_discount) as avg_disc, count(*) as count_order from lineitem where
> > l_shipdate <= date '1998-12-01' - interval '90' day group by
> l_returnflag,
> > l_linestatus order by l_returnflag, l_linestatus
> >
> > Based on the shared line of code, even a month should be represented in
> > milis right? But when the below query is transformed it shows months in
> > months.
> >
> > with revenue (suplier_no, total_revenue) as ( select l_suppkey,
> > sum(l_extendedprice * (1-l_discount)) from lineitem where l_shipdate >=
> > date '1996-01-01' and l_shipdate < date '1996-01-01' + interval '3' month
> > group by l_suppkey ) select s_suppkey, s_name, s_address, s_phone,
> > total_revenue from supplier, revenue where s_suppkey = suplier_no and
> > total_revenue = ( select max(total_revenue) from revenue ) order by
> > s_suppkey
> >
> > AND(>=($1, 1993-07-01 00:00:00), <($1, CAST(+(1993-07-01, 3:INTERVAL
> > MONTH)):TIMESTAMP(0) NOT NULL))
> >
> > Thanks for the pointer Stamatis, I'll see whether there is something to
> do
> > with the RexSimplify/RexExecutor.
> >
> > Regards,
> > Chathura
> >
> >
> > On Thu, Feb 10, 2022 at 5:07 PM Stamatis Zampetakis <za...@gmail.com>
> > wrote:
> >
> >> Hi Chathura,
> >>
> >> It is difficult to reason about correctness without having the actual
> SQL
> >> query at hand.
> >>
> >> The fact that you have milliseconds is not by itself a problem and has
> to
> >> do with the way Calcite internally represents intervals (see comment in
> >> [1]).
> >>
> >> Also from the examples you provided the behavior in 1.29.0 does not
> seem to
> >> be an additional transformation rather than a missing simplification
> >> (constant reduction). I am not sure if this is intentional or not but I
> >> guess you can have a look at the changes landed around
> >> RexSimplify/RexExecutor.
> >>
> >> Best,
> >> Stamatis
> >>
> >> [1]
> >>
> >>
> https://github.com/apache/calcite/blob/812e3e98eae518cf85cd1b6b7f055fb96784a423/core/src/main/java/org/apache/calcite/rex/RexLiteral.java#L357
> >>
> >>
> >> On Thu, Feb 10, 2022 at 8:02 AM Chathura Widanage <
> >> chathurawidanage@gmail.com> wrote:
> >>
> >>> Hi community,
> >>>
> >>> I'm comparing two rel expressions generated by calcite 1.25.0 and
> 1.29.0
> >>> and noticed there is an invalid IntervalSQLType plugged into the query.
> >>>
> >>> <=($6, 1998-09-02 00:00:00) : Calcite 1.25.0
> >>> vs
> >>> <=($6, CAST(-(1998-12-01, 7776000000:INTERVAL DAY)):TIMESTAMP(0) NOT
> >> NULL)
> >>> :
> >>> Calcite 1.29.0
> >>>
> >>> 7776000000 is 90 days in milliseconds, but the IntervalSQLType/value
> >>> combination is invalid.
> >>>
> >>> Could you please let me know whether this could be a bug and whether
> >> there
> >>> an option to prevent such transformations at all?
> >>>
> >>> Regards,
> >>> Chathura
> >>>
> >>> PS: This comes on queries from tpch benchmark and invalid conversion is
> >>> from tpch-01.
> >>>
> >>> I'm seeing similar conversions in other queries, but they seem to be
> >>> correct, but feels this transformation is redundant.
> >>>
> >>> AND(>=($1, 1993-07-01 00:00:00), <($1, 1993-10-01 00:00:00))
> >>> vs
> >>> AND(>=($1, 1993-07-01 00:00:00), <($1, CAST(+(1993-07-01, 3:INTERVAL
> >>> MONTH)):TIMESTAMP(0) NOT NULL))
> >>>
> >>
>
>
Re: Invalid IntervalSQLType
Posted by Julian Hyde <jh...@gmail.com>.
There are two kinds of intervals (in Calcite and in standard SQL). Days-hours-minutes-seconds-millseconds intervals, and year-month intervals. The former are represented internally in milliseconds. The latter are represented in months.
This is one area where Postgres does things differently from the SQL standard.
> On Feb 10, 2022, at 3:48 AM, Chathura Widanage <ch...@gmail.com> wrote:
>
> Thanks, Stamatis. Below is the original SQL query.
>
> select l_returnflag, l_linestatus, sum(l_quantity) as sum_qty,
> sum(l_extendedprice) as sum_base_price, sum(l_extendedprice*(1-l_discount))
> as sum_disc_price, sum(l_extendedprice*(1-l_discount)*(1+l_tax)) as
> sum_charge, avg(l_quantity) as avg_qty, avg(l_extendedprice) as avg_price,
> avg(l_discount) as avg_disc, count(*) as count_order from lineitem where
> l_shipdate <= date '1998-12-01' - interval '90' day group by l_returnflag,
> l_linestatus order by l_returnflag, l_linestatus
>
> Based on the shared line of code, even a month should be represented in
> milis right? But when the below query is transformed it shows months in
> months.
>
> with revenue (suplier_no, total_revenue) as ( select l_suppkey,
> sum(l_extendedprice * (1-l_discount)) from lineitem where l_shipdate >=
> date '1996-01-01' and l_shipdate < date '1996-01-01' + interval '3' month
> group by l_suppkey ) select s_suppkey, s_name, s_address, s_phone,
> total_revenue from supplier, revenue where s_suppkey = suplier_no and
> total_revenue = ( select max(total_revenue) from revenue ) order by
> s_suppkey
>
> AND(>=($1, 1993-07-01 00:00:00), <($1, CAST(+(1993-07-01, 3:INTERVAL
> MONTH)):TIMESTAMP(0) NOT NULL))
>
> Thanks for the pointer Stamatis, I'll see whether there is something to do
> with the RexSimplify/RexExecutor.
>
> Regards,
> Chathura
>
>
> On Thu, Feb 10, 2022 at 5:07 PM Stamatis Zampetakis <za...@gmail.com>
> wrote:
>
>> Hi Chathura,
>>
>> It is difficult to reason about correctness without having the actual SQL
>> query at hand.
>>
>> The fact that you have milliseconds is not by itself a problem and has to
>> do with the way Calcite internally represents intervals (see comment in
>> [1]).
>>
>> Also from the examples you provided the behavior in 1.29.0 does not seem to
>> be an additional transformation rather than a missing simplification
>> (constant reduction). I am not sure if this is intentional or not but I
>> guess you can have a look at the changes landed around
>> RexSimplify/RexExecutor.
>>
>> Best,
>> Stamatis
>>
>> [1]
>>
>> https://github.com/apache/calcite/blob/812e3e98eae518cf85cd1b6b7f055fb96784a423/core/src/main/java/org/apache/calcite/rex/RexLiteral.java#L357
>>
>>
>> On Thu, Feb 10, 2022 at 8:02 AM Chathura Widanage <
>> chathurawidanage@gmail.com> wrote:
>>
>>> Hi community,
>>>
>>> I'm comparing two rel expressions generated by calcite 1.25.0 and 1.29.0
>>> and noticed there is an invalid IntervalSQLType plugged into the query.
>>>
>>> <=($6, 1998-09-02 00:00:00) : Calcite 1.25.0
>>> vs
>>> <=($6, CAST(-(1998-12-01, 7776000000:INTERVAL DAY)):TIMESTAMP(0) NOT
>> NULL)
>>> :
>>> Calcite 1.29.0
>>>
>>> 7776000000 is 90 days in milliseconds, but the IntervalSQLType/value
>>> combination is invalid.
>>>
>>> Could you please let me know whether this could be a bug and whether
>> there
>>> an option to prevent such transformations at all?
>>>
>>> Regards,
>>> Chathura
>>>
>>> PS: This comes on queries from tpch benchmark and invalid conversion is
>>> from tpch-01.
>>>
>>> I'm seeing similar conversions in other queries, but they seem to be
>>> correct, but feels this transformation is redundant.
>>>
>>> AND(>=($1, 1993-07-01 00:00:00), <($1, 1993-10-01 00:00:00))
>>> vs
>>> AND(>=($1, 1993-07-01 00:00:00), <($1, CAST(+(1993-07-01, 3:INTERVAL
>>> MONTH)):TIMESTAMP(0) NOT NULL))
>>>
>>
Re: Invalid IntervalSQLType
Posted by Chathura Widanage <ch...@gmail.com>.
Thanks, Stamatis. Below is the original SQL query.
select l_returnflag, l_linestatus, sum(l_quantity) as sum_qty,
sum(l_extendedprice) as sum_base_price, sum(l_extendedprice*(1-l_discount))
as sum_disc_price, sum(l_extendedprice*(1-l_discount)*(1+l_tax)) as
sum_charge, avg(l_quantity) as avg_qty, avg(l_extendedprice) as avg_price,
avg(l_discount) as avg_disc, count(*) as count_order from lineitem where
l_shipdate <= date '1998-12-01' - interval '90' day group by l_returnflag,
l_linestatus order by l_returnflag, l_linestatus
Based on the shared line of code, even a month should be represented in
milis right? But when the below query is transformed it shows months in
months.
with revenue (suplier_no, total_revenue) as ( select l_suppkey,
sum(l_extendedprice * (1-l_discount)) from lineitem where l_shipdate >=
date '1996-01-01' and l_shipdate < date '1996-01-01' + interval '3' month
group by l_suppkey ) select s_suppkey, s_name, s_address, s_phone,
total_revenue from supplier, revenue where s_suppkey = suplier_no and
total_revenue = ( select max(total_revenue) from revenue ) order by
s_suppkey
AND(>=($1, 1993-07-01 00:00:00), <($1, CAST(+(1993-07-01, 3:INTERVAL
MONTH)):TIMESTAMP(0) NOT NULL))
Thanks for the pointer Stamatis, I'll see whether there is something to do
with the RexSimplify/RexExecutor.
Regards,
Chathura
On Thu, Feb 10, 2022 at 5:07 PM Stamatis Zampetakis <za...@gmail.com>
wrote:
> Hi Chathura,
>
> It is difficult to reason about correctness without having the actual SQL
> query at hand.
>
> The fact that you have milliseconds is not by itself a problem and has to
> do with the way Calcite internally represents intervals (see comment in
> [1]).
>
> Also from the examples you provided the behavior in 1.29.0 does not seem to
> be an additional transformation rather than a missing simplification
> (constant reduction). I am not sure if this is intentional or not but I
> guess you can have a look at the changes landed around
> RexSimplify/RexExecutor.
>
> Best,
> Stamatis
>
> [1]
>
> https://github.com/apache/calcite/blob/812e3e98eae518cf85cd1b6b7f055fb96784a423/core/src/main/java/org/apache/calcite/rex/RexLiteral.java#L357
>
>
> On Thu, Feb 10, 2022 at 8:02 AM Chathura Widanage <
> chathurawidanage@gmail.com> wrote:
>
> > Hi community,
> >
> > I'm comparing two rel expressions generated by calcite 1.25.0 and 1.29.0
> > and noticed there is an invalid IntervalSQLType plugged into the query.
> >
> > <=($6, 1998-09-02 00:00:00) : Calcite 1.25.0
> > vs
> > <=($6, CAST(-(1998-12-01, 7776000000:INTERVAL DAY)):TIMESTAMP(0) NOT
> NULL)
> > :
> > Calcite 1.29.0
> >
> > 7776000000 is 90 days in milliseconds, but the IntervalSQLType/value
> > combination is invalid.
> >
> > Could you please let me know whether this could be a bug and whether
> there
> > an option to prevent such transformations at all?
> >
> > Regards,
> > Chathura
> >
> > PS: This comes on queries from tpch benchmark and invalid conversion is
> > from tpch-01.
> >
> > I'm seeing similar conversions in other queries, but they seem to be
> > correct, but feels this transformation is redundant.
> >
> > AND(>=($1, 1993-07-01 00:00:00), <($1, 1993-10-01 00:00:00))
> > vs
> > AND(>=($1, 1993-07-01 00:00:00), <($1, CAST(+(1993-07-01, 3:INTERVAL
> > MONTH)):TIMESTAMP(0) NOT NULL))
> >
>
Re: Invalid IntervalSQLType
Posted by Stamatis Zampetakis <za...@gmail.com>.
Hi Chathura,
It is difficult to reason about correctness without having the actual SQL
query at hand.
The fact that you have milliseconds is not by itself a problem and has to
do with the way Calcite internally represents intervals (see comment in
[1]).
Also from the examples you provided the behavior in 1.29.0 does not seem to
be an additional transformation rather than a missing simplification
(constant reduction). I am not sure if this is intentional or not but I
guess you can have a look at the changes landed around
RexSimplify/RexExecutor.
Best,
Stamatis
[1]
https://github.com/apache/calcite/blob/812e3e98eae518cf85cd1b6b7f055fb96784a423/core/src/main/java/org/apache/calcite/rex/RexLiteral.java#L357
On Thu, Feb 10, 2022 at 8:02 AM Chathura Widanage <
chathurawidanage@gmail.com> wrote:
> Hi community,
>
> I'm comparing two rel expressions generated by calcite 1.25.0 and 1.29.0
> and noticed there is an invalid IntervalSQLType plugged into the query.
>
> <=($6, 1998-09-02 00:00:00) : Calcite 1.25.0
> vs
> <=($6, CAST(-(1998-12-01, 7776000000:INTERVAL DAY)):TIMESTAMP(0) NOT NULL)
> :
> Calcite 1.29.0
>
> 7776000000 is 90 days in milliseconds, but the IntervalSQLType/value
> combination is invalid.
>
> Could you please let me know whether this could be a bug and whether there
> an option to prevent such transformations at all?
>
> Regards,
> Chathura
>
> PS: This comes on queries from tpch benchmark and invalid conversion is
> from tpch-01.
>
> I'm seeing similar conversions in other queries, but they seem to be
> correct, but feels this transformation is redundant.
>
> AND(>=($1, 1993-07-01 00:00:00), <($1, 1993-10-01 00:00:00))
> vs
> AND(>=($1, 1993-07-01 00:00:00), <($1, CAST(+(1993-07-01, 3:INTERVAL
> MONTH)):TIMESTAMP(0) NOT NULL))
>