You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by JingsongLee <lz...@aliyun.com.INVALID> on 2019/08/13 10:55:28 UTC

[DISCUSS] FLIP-51: Rework of the Expression Design

Hi everyone,

We would like to start a discussion thread on "FLIP-51: Rework of the
Expression Design"(Design doc: [1], FLIP: [2]), where we describe how
 to improve the new java Expressions to work with type inference and
 convert expression to the calcite RexNode. This is a follow-up plan 
for FLIP-32[3] and FLIP-37[4]. This FLIP is mostly based on FLIP-37.

This FLIP addresses several shortcomings of current:
   - New Expressions still use PlannerExpressions to type inference and
 to RexNode. Flnk-planner and blink-planner have a lot of repetitive code
 and logic.
   - Let TableApi and Cacite definitions consistent.
   - Reduce the complexity of Function development.
   - Powerful Function for user.

Key changes can be summarized as follows:
   - Improve the interface of FunctionDefinition.
   - Introduce type inference for built-in functions.
   - Introduce ExpressionConverter to convert Expression to calcite
 RexNode.
   - Remove repetitive code and logic in planners.

I also listed type inference and behavior of all built-in functions [5], to 
verify that the interface is satisfied. After introduce type inference to
table-common module, planners should have a unified function behavior.
And this gives the community also the chance to quickly discuss types
 and behavior of functions a last time before they are declared stable.

Looking forward to your feedbacks. Thank you.

[1] https://docs.google.com/document/d/1yFDyquMo_-VZ59vyhaMshpPtg7p87b9IYdAtMXv5XmM/edit?usp=sharing
[2] https://cwiki.apache.org/confluence/display/FLINK/FLIP-51%3A+Rework+of+the+Expression+Design
[3] https://cwiki.apache.org/confluence/display/FLINK/FLIP-32%3A+Restructure+flink-table+for+future+contributions
[4] https://cwiki.apache.org/confluence/display/FLINK/FLIP-37%3A+Rework+of+the+Table+API+Type+System
[5] https://docs.google.com/document/d/1fyVmdGgbO1XmIyQ1BaoG_h5BcNcF3q9UJ1Bj1euO240/edit?usp=sharing

Best,
Jingsong Lee

Re: [DISCUSS] FLIP-51: Rework of the Expression Design

Posted by Timo Walther <tw...@apache.org>.
Hi,

regarding the LegacyTypeInformation esp. for decimals. I don't have a 
clear answer yet, but I think it should not limit us. If possible it 
should travel through the type inference and we only need some special 
cases at some locations e.g. when computing the leastRestrictive. E.g. 
the logical type root is set correctly which is required for family 
checking.

I'm wondering when a decimal type with precision can occur. Usually, it 
should come from literals or defined column. But I think it might also 
be ok that the flink-planner just receives a decimal with precision and 
treats it with Java semantics.

Doing it on the planner side is the easiest option but also the one that 
could cause side-effects if the back-and-forth conversion of 
LegacyTypeConverter is not a 1:1 mapping anymore. I guess we will see 
the implications during implementation. All old Flink tests should pass 
still.

Regards,
Timo

Am 15.08.19 um 10:43 schrieb JingsongLee:
> Hi @Timo Walther @Dawid Wysakowicz:
>
> Now, flink-planner have some legacy DataTypes:
> like: legacy decimal, legacy basic array type info...
> And If the new type inference infer a Decimal/VarChar with precision, there should will fail in TypeConversions.
>
> The better we do on DataType, the more problems we will have with TypeInformation conversion, and the new TypeInference is a lot of precision related.
> What do you think?
> 1. Should TypeConversions support all data types and flink-planner support new types?2. Or do a special conversion between flink-planner and type inference.
>
> (There are many problems with the conversion between TypeInformation and DataType, and I think we should solve them completely in 1.10.)
>
> Best,
> Jingsong Lee
>
>
> ------------------------------------------------------------------
> From:JingsongLee <lz...@aliyun.com.INVALID>
> Send Time:2019年8月15日(星期四) 10:31
> To:dev <de...@flink.apache.org>
> Subject:Re: [DISCUSS] FLIP-51: Rework of the Expression Design
>
> Hi jark:
>
> I'll add a chapter to list blink planner extended functions.
>
> Best,
>   Jingsong Lee
>
>
> ------------------------------------------------------------------
> From:Jark Wu <im...@gmail.com>
> Send Time:2019年8月15日(星期四) 05:12
> To:dev <de...@flink.apache.org>
> Subject:Re: [DISCUSS] FLIP-51: Rework of the Expression Design
>
> Thanks Jingsong for starting the discussion.
>
> The general design of the FLIP looks good to me. +1 for the FLIP. It's time
> to get rid of the old Expression!
>
> Regarding to the function behavior, shall we also include new functions
> from blink planner (e.g. LISTAGG, REGEXP, TO_DATE, etc..) ?
>
>
> Best,
> Jark
>
>
>
>
>
> On Wed, 14 Aug 2019 at 23:34, Timo Walther <tw...@apache.org> wrote:
>
>> Hi Jingsong,
>>
>> thanks for writing down this FLIP. Big +1 from my side to finally get
>> rid of PlannerExpressions and have consistent and well-defined behavior
>> for Table API and SQL updated to FLIP-37.
>>
>> We might need to discuss some of the behavior of particular functions
>> but this should not affect the actual FLIP-51.
>>
>> Regards,
>> Timo
>>
>>
>> Am 13.08.19 um 12:55 schrieb JingsongLee:
>>> Hi everyone,
>>>
>>> We would like to start a discussion thread on "FLIP-51: Rework of the
>>> Expression Design"(Design doc: [1], FLIP: [2]), where we describe how
>>>    to improve the new java Expressions to work with type inference and
>>>    convert expression to the calcite RexNode. This is a follow-up plan
>>> for FLIP-32[3] and FLIP-37[4]. This FLIP is mostly based on FLIP-37.
>>>
>>> This FLIP addresses several shortcomings of current:
>>>      - New Expressions still use PlannerExpressions to type inference and
>>>    to RexNode. Flnk-planner and blink-planner have a lot of repetitive
>> code
>>>    and logic.
>>>      - Let TableApi and Cacite definitions consistent.
>>>      - Reduce the complexity of Function development.
>>>      - Powerful Function for user.
>>>
>>> Key changes can be summarized as follows:
>>>      - Improve the interface of FunctionDefinition.
>>>      - Introduce type inference for built-in functions.
>>>      - Introduce ExpressionConverter to convert Expression to calcite
>>>    RexNode.
>>>      - Remove repetitive code and logic in planners.
>>>
>>> I also listed type inference and behavior of all built-in functions [5],
>> to
>>> verify that the interface is satisfied. After introduce type inference to
>>> table-common module, planners should have a unified function behavior.
>>> And this gives the community also the chance to quickly discuss types
>>>    and behavior of functions a last time before they are declared stable.
>>>
>>> Looking forward to your feedbacks. Thank you.
>>>
>>> [1]
>> https://docs.google.com/document/d/1yFDyquMo_-VZ59vyhaMshpPtg7p87b9IYdAtMXv5XmM/edit?usp=sharing
>>> [2]
>> https://cwiki.apache.org/confluence/display/FLINK/FLIP-51%3A+Rework+of+the+Expression+Design
>>> [3]
>> https://cwiki.apache.org/confluence/display/FLINK/FLIP-32%3A+Restructure+flink-table+for+future+contributions
>>> [4]
>> https://cwiki.apache.org/confluence/display/FLINK/FLIP-37%3A+Rework+of+the+Table+API+Type+System
>>> [5]
>> https://docs.google.com/document/d/1fyVmdGgbO1XmIyQ1BaoG_h5BcNcF3q9UJ1Bj1euO240/edit?usp=sharing
>>> Best,
>>> Jingsong Lee
>>
>>


Re: [DISCUSS] FLIP-51: Rework of the Expression Design

Posted by JingsongLee <lz...@aliyun.com.INVALID>.
Hi @Timo Walther @Dawid Wysakowicz:

Now, flink-planner have some legacy DataTypes:
like: legacy decimal, legacy basic array type info...
And If the new type inference infer a Decimal/VarChar with precision, there should will fail in TypeConversions.

The better we do on DataType, the more problems we will have with TypeInformation conversion, and the new TypeInference is a lot of precision related.
What do you think?
1. Should TypeConversions support all data types and flink-planner support new types?2. Or do a special conversion between flink-planner and type inference.

(There are many problems with the conversion between TypeInformation and DataType, and I think we should solve them completely in 1.10.)

Best,
Jingsong Lee


------------------------------------------------------------------
From:JingsongLee <lz...@aliyun.com.INVALID>
Send Time:2019年8月15日(星期四) 10:31
To:dev <de...@flink.apache.org>
Subject:Re: [DISCUSS] FLIP-51: Rework of the Expression Design

Hi jark:

I'll add a chapter to list blink planner extended functions.

Best,
 Jingsong Lee


------------------------------------------------------------------
From:Jark Wu <im...@gmail.com>
Send Time:2019年8月15日(星期四) 05:12
To:dev <de...@flink.apache.org>
Subject:Re: [DISCUSS] FLIP-51: Rework of the Expression Design

Thanks Jingsong for starting the discussion.

The general design of the FLIP looks good to me. +1 for the FLIP. It's time
to get rid of the old Expression!

Regarding to the function behavior, shall we also include new functions
from blink planner (e.g. LISTAGG, REGEXP, TO_DATE, etc..) ?


Best,
Jark





On Wed, 14 Aug 2019 at 23:34, Timo Walther <tw...@apache.org> wrote:

> Hi Jingsong,
>
> thanks for writing down this FLIP. Big +1 from my side to finally get
> rid of PlannerExpressions and have consistent and well-defined behavior
> for Table API and SQL updated to FLIP-37.
>
> We might need to discuss some of the behavior of particular functions
> but this should not affect the actual FLIP-51.
>
> Regards,
> Timo
>
>
> Am 13.08.19 um 12:55 schrieb JingsongLee:
> > Hi everyone,
> >
> > We would like to start a discussion thread on "FLIP-51: Rework of the
> > Expression Design"(Design doc: [1], FLIP: [2]), where we describe how
> >   to improve the new java Expressions to work with type inference and
> >   convert expression to the calcite RexNode. This is a follow-up plan
> > for FLIP-32[3] and FLIP-37[4]. This FLIP is mostly based on FLIP-37.
> >
> > This FLIP addresses several shortcomings of current:
> >     - New Expressions still use PlannerExpressions to type inference and
> >   to RexNode. Flnk-planner and blink-planner have a lot of repetitive
> code
> >   and logic.
> >     - Let TableApi and Cacite definitions consistent.
> >     - Reduce the complexity of Function development.
> >     - Powerful Function for user.
> >
> > Key changes can be summarized as follows:
> >     - Improve the interface of FunctionDefinition.
> >     - Introduce type inference for built-in functions.
> >     - Introduce ExpressionConverter to convert Expression to calcite
> >   RexNode.
> >     - Remove repetitive code and logic in planners.
> >
> > I also listed type inference and behavior of all built-in functions [5],
> to
> > verify that the interface is satisfied. After introduce type inference to
> > table-common module, planners should have a unified function behavior.
> > And this gives the community also the chance to quickly discuss types
> >   and behavior of functions a last time before they are declared stable.
> >
> > Looking forward to your feedbacks. Thank you.
> >
> > [1]
> https://docs.google.com/document/d/1yFDyquMo_-VZ59vyhaMshpPtg7p87b9IYdAtMXv5XmM/edit?usp=sharing
> > [2]
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-51%3A+Rework+of+the+Expression+Design
> > [3]
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-32%3A+Restructure+flink-table+for+future+contributions
> > [4]
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-37%3A+Rework+of+the+Table+API+Type+System
> > [5]
> https://docs.google.com/document/d/1fyVmdGgbO1XmIyQ1BaoG_h5BcNcF3q9UJ1Bj1euO240/edit?usp=sharing
> >
> > Best,
> > Jingsong Lee
>
>
>

Re: [DISCUSS] FLIP-51: Rework of the Expression Design

Posted by JingsongLee <lz...@aliyun.com.INVALID>.
Hi jark:

I'll add a chapter to list blink planner extended functions.

Best,
 Jingsong Lee


------------------------------------------------------------------
From:Jark Wu <im...@gmail.com>
Send Time:2019年8月15日(星期四) 05:12
To:dev <de...@flink.apache.org>
Subject:Re: [DISCUSS] FLIP-51: Rework of the Expression Design

Thanks Jingsong for starting the discussion.

The general design of the FLIP looks good to me. +1 for the FLIP. It's time
to get rid of the old Expression!

Regarding to the function behavior, shall we also include new functions
from blink planner (e.g. LISTAGG, REGEXP, TO_DATE, etc..) ?


Best,
Jark





On Wed, 14 Aug 2019 at 23:34, Timo Walther <tw...@apache.org> wrote:

> Hi Jingsong,
>
> thanks for writing down this FLIP. Big +1 from my side to finally get
> rid of PlannerExpressions and have consistent and well-defined behavior
> for Table API and SQL updated to FLIP-37.
>
> We might need to discuss some of the behavior of particular functions
> but this should not affect the actual FLIP-51.
>
> Regards,
> Timo
>
>
> Am 13.08.19 um 12:55 schrieb JingsongLee:
> > Hi everyone,
> >
> > We would like to start a discussion thread on "FLIP-51: Rework of the
> > Expression Design"(Design doc: [1], FLIP: [2]), where we describe how
> >   to improve the new java Expressions to work with type inference and
> >   convert expression to the calcite RexNode. This is a follow-up plan
> > for FLIP-32[3] and FLIP-37[4]. This FLIP is mostly based on FLIP-37.
> >
> > This FLIP addresses several shortcomings of current:
> >     - New Expressions still use PlannerExpressions to type inference and
> >   to RexNode. Flnk-planner and blink-planner have a lot of repetitive
> code
> >   and logic.
> >     - Let TableApi and Cacite definitions consistent.
> >     - Reduce the complexity of Function development.
> >     - Powerful Function for user.
> >
> > Key changes can be summarized as follows:
> >     - Improve the interface of FunctionDefinition.
> >     - Introduce type inference for built-in functions.
> >     - Introduce ExpressionConverter to convert Expression to calcite
> >   RexNode.
> >     - Remove repetitive code and logic in planners.
> >
> > I also listed type inference and behavior of all built-in functions [5],
> to
> > verify that the interface is satisfied. After introduce type inference to
> > table-common module, planners should have a unified function behavior.
> > And this gives the community also the chance to quickly discuss types
> >   and behavior of functions a last time before they are declared stable.
> >
> > Looking forward to your feedbacks. Thank you.
> >
> > [1]
> https://docs.google.com/document/d/1yFDyquMo_-VZ59vyhaMshpPtg7p87b9IYdAtMXv5XmM/edit?usp=sharing
> > [2]
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-51%3A+Rework+of+the+Expression+Design
> > [3]
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-32%3A+Restructure+flink-table+for+future+contributions
> > [4]
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-37%3A+Rework+of+the+Table+API+Type+System
> > [5]
> https://docs.google.com/document/d/1fyVmdGgbO1XmIyQ1BaoG_h5BcNcF3q9UJ1Bj1euO240/edit?usp=sharing
> >
> > Best,
> > Jingsong Lee
>
>
>

Re: [DISCUSS] FLIP-51: Rework of the Expression Design

Posted by Jark Wu <im...@gmail.com>.
Thanks Jingsong for starting the discussion.

The general design of the FLIP looks good to me. +1 for the FLIP. It's time
to get rid of the old Expression!

Regarding to the function behavior, shall we also include new functions
from blink planner (e.g. LISTAGG, REGEXP, TO_DATE, etc..) ?


Best,
Jark





On Wed, 14 Aug 2019 at 23:34, Timo Walther <tw...@apache.org> wrote:

> Hi Jingsong,
>
> thanks for writing down this FLIP. Big +1 from my side to finally get
> rid of PlannerExpressions and have consistent and well-defined behavior
> for Table API and SQL updated to FLIP-37.
>
> We might need to discuss some of the behavior of particular functions
> but this should not affect the actual FLIP-51.
>
> Regards,
> Timo
>
>
> Am 13.08.19 um 12:55 schrieb JingsongLee:
> > Hi everyone,
> >
> > We would like to start a discussion thread on "FLIP-51: Rework of the
> > Expression Design"(Design doc: [1], FLIP: [2]), where we describe how
> >   to improve the new java Expressions to work with type inference and
> >   convert expression to the calcite RexNode. This is a follow-up plan
> > for FLIP-32[3] and FLIP-37[4]. This FLIP is mostly based on FLIP-37.
> >
> > This FLIP addresses several shortcomings of current:
> >     - New Expressions still use PlannerExpressions to type inference and
> >   to RexNode. Flnk-planner and blink-planner have a lot of repetitive
> code
> >   and logic.
> >     - Let TableApi and Cacite definitions consistent.
> >     - Reduce the complexity of Function development.
> >     - Powerful Function for user.
> >
> > Key changes can be summarized as follows:
> >     - Improve the interface of FunctionDefinition.
> >     - Introduce type inference for built-in functions.
> >     - Introduce ExpressionConverter to convert Expression to calcite
> >   RexNode.
> >     - Remove repetitive code and logic in planners.
> >
> > I also listed type inference and behavior of all built-in functions [5],
> to
> > verify that the interface is satisfied. After introduce type inference to
> > table-common module, planners should have a unified function behavior.
> > And this gives the community also the chance to quickly discuss types
> >   and behavior of functions a last time before they are declared stable.
> >
> > Looking forward to your feedbacks. Thank you.
> >
> > [1]
> https://docs.google.com/document/d/1yFDyquMo_-VZ59vyhaMshpPtg7p87b9IYdAtMXv5XmM/edit?usp=sharing
> > [2]
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-51%3A+Rework+of+the+Expression+Design
> > [3]
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-32%3A+Restructure+flink-table+for+future+contributions
> > [4]
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-37%3A+Rework+of+the+Table+API+Type+System
> > [5]
> https://docs.google.com/document/d/1fyVmdGgbO1XmIyQ1BaoG_h5BcNcF3q9UJ1Bj1euO240/edit?usp=sharing
> >
> > Best,
> > Jingsong Lee
>
>
>

Re: [DISCUSS] FLIP-51: Rework of the Expression Design

Posted by Timo Walther <tw...@apache.org>.
Hi Jingsong,

thanks for writing down this FLIP. Big +1 from my side to finally get 
rid of PlannerExpressions and have consistent and well-defined behavior 
for Table API and SQL updated to FLIP-37.

We might need to discuss some of the behavior of particular functions 
but this should not affect the actual FLIP-51.

Regards,
Timo


Am 13.08.19 um 12:55 schrieb JingsongLee:
> Hi everyone,
>
> We would like to start a discussion thread on "FLIP-51: Rework of the
> Expression Design"(Design doc: [1], FLIP: [2]), where we describe how
>   to improve the new java Expressions to work with type inference and
>   convert expression to the calcite RexNode. This is a follow-up plan
> for FLIP-32[3] and FLIP-37[4]. This FLIP is mostly based on FLIP-37.
>
> This FLIP addresses several shortcomings of current:
>     - New Expressions still use PlannerExpressions to type inference and
>   to RexNode. Flnk-planner and blink-planner have a lot of repetitive code
>   and logic.
>     - Let TableApi and Cacite definitions consistent.
>     - Reduce the complexity of Function development.
>     - Powerful Function for user.
>
> Key changes can be summarized as follows:
>     - Improve the interface of FunctionDefinition.
>     - Introduce type inference for built-in functions.
>     - Introduce ExpressionConverter to convert Expression to calcite
>   RexNode.
>     - Remove repetitive code and logic in planners.
>
> I also listed type inference and behavior of all built-in functions [5], to
> verify that the interface is satisfied. After introduce type inference to
> table-common module, planners should have a unified function behavior.
> And this gives the community also the chance to quickly discuss types
>   and behavior of functions a last time before they are declared stable.
>
> Looking forward to your feedbacks. Thank you.
>
> [1] https://docs.google.com/document/d/1yFDyquMo_-VZ59vyhaMshpPtg7p87b9IYdAtMXv5XmM/edit?usp=sharing
> [2] https://cwiki.apache.org/confluence/display/FLINK/FLIP-51%3A+Rework+of+the+Expression+Design
> [3] https://cwiki.apache.org/confluence/display/FLINK/FLIP-32%3A+Restructure+flink-table+for+future+contributions
> [4] https://cwiki.apache.org/confluence/display/FLINK/FLIP-37%3A+Rework+of+the+Table+API+Type+System
> [5] https://docs.google.com/document/d/1fyVmdGgbO1XmIyQ1BaoG_h5BcNcF3q9UJ1Bj1euO240/edit?usp=sharing
>
> Best,
> Jingsong Lee