You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by Kurt Young <yk...@gmail.com> on 2021/03/09 07:14:51 UTC
Re: [DISCUSS] FLIP-162 follow-up discussion
Hi Leonard,
Thanks for this careful consideration. Given the fallback option will
eventually change the behavior twice, which means
potentially break user's job twice, I would also +1 to not introduce it.
Best,
Kurt
On Fri, Mar 5, 2021 at 3:00 PM Leonard Xu <xb...@gmail.com> wrote:
> Hi, all
>
> As the FLIP-162 discussed, we agreed current time functions’ behavior is
> incorrect and plan to introduce the option *t**able.exec.fallback-legacy-time-function
> *to enable user fallback to incorrect behavior.
>
> (1) The option is convenient for users who want to upgrade to 1.13 but
> don't want to change their sql job, user need to config the option value, *this
> is the first time users influenced by these wrong functions.*
>
> (2) But we didn’t consider that the option will be deleted after one or
> two major versions, users have to change their sql job again at that time
> point, *this the second time** users influenced by these wrong functions.*
>
> (3) Besides, maintaining two sets of functions is prone to bugs.
>
> I’ve discussed with some community developers offline, they tend to solve
> these functions at once i.e. Correct the wrong functions directly and do
> not introduce this option.
>
> Considering that we will delete the configuration eventually, comparing
> hurting users twice and bothering them for a long time, I would rather hurt
> users once.
> *Thus I also +1* that we should directly correct these wrong functions
> and remove the wrong functions at the same time.
>
>
> If we can make a consensus in this thread, I think we can remove this
> option support in FLIP-162.
> How do you think?
>
> Best,
> Leonard
>
>
>
>
>
Re: [DISCUSS] FLIP-162 follow-up discussion
Posted by Jark Wu <im...@gmail.com>.
Thanks Leonard,
I'm also +1 with not introducing this fallback option.
It's error-prone to mix the implementation of wrong behavior and correct
behavior.
And it's better to educate users the right way in one version instead of
spanning multiple versions.
Best,
Jark
On Tue, 9 Mar 2021 at 15:15, Kurt Young <yk...@gmail.com> wrote:
> Hi Leonard,
>
> Thanks for this careful consideration. Given the fallback option will
> eventually change the behavior twice, which means
> potentially break user's job twice, I would also +1 to not introduce it.
>
> Best,
> Kurt
>
> On Fri, Mar 5, 2021 at 3:00 PM Leonard Xu <xb...@gmail.com> wrote:
>
>> Hi, all
>>
>> As the FLIP-162 discussed, we agreed current time functions’ behavior is
>> incorrect and plan to introduce the option *t**able.exec.fallback-legacy-time-function
>> *to enable user fallback to incorrect behavior.
>>
>> (1) The option is convenient for users who want to upgrade to 1.13 but
>> don't want to change their sql job, user need to config the option value, *this
>> is the first time users influenced by these wrong functions.*
>>
>> (2) But we didn’t consider that the option will be deleted after one or
>> two major versions, users have to change their sql job again at that time
>> point, *this the second time** users influenced by these wrong
>> functions.*
>>
>> (3) Besides, maintaining two sets of functions is prone to bugs.
>>
>> I’ve discussed with some community developers offline, they tend to solve
>> these functions at once i.e. Correct the wrong functions directly and do
>> not introduce this option.
>>
>> Considering that we will delete the configuration eventually, comparing
>> hurting users twice and bothering them for a long time, I would rather hurt
>> users once.
>> *Thus I also +1* that we should directly correct these wrong functions
>> and remove the wrong functions at the same time.
>>
>>
>> If we can make a consensus in this thread, I think we can remove this
>> option support in FLIP-162.
>> How do you think?
>>
>> Best,
>> Leonard
>>
>>
>>
>>
>>
Re: [DISCUSS] FLIP-162 follow-up discussion
Posted by Leonard Xu <xb...@gmail.com>.
Thank you all for your positive feedbacks.
Considering that you’re binding votes for this FLIP and all of us agreed to remove this option,
Thus I update this section of FLIP-162[1], I’ll start work on it next .
Best,
Leonard
[1] https://cwiki.apache.org/confluence/display/FLINK/FLIP-162%3A+Consistent+Flink+SQL+time+function+behavior <https://cwiki.apache.org/confluence/display/FLINK/FLIP-162:+Consistent+Flink+SQL+time+function+behavior>
> 在 2021年3月9日，16:34，Jingsong Li <ji...@gmail.com> 写道：
>
> +1
>
> Let's go straight to the right behavior. Drop the option for the wrong
> behavior.
>
> Best,
> Jingsong
>
>
> On Tue, Mar 9, 2021 at 4:29 PM Timo Walther <tw...@apache.org> wrote:
>
>> Hi Leonard,
>>
>> I'm fine with dropping the old buggy behavior immediatly. Users can
>> still implement a UDF with the old bavhior if needed. I hope the new
>> functions will be well-tested so that a fallback to the old functions is
>> not necessary as a workaround. It will definitely avoid confusion for
>> users and avoid spaghetti code in the planner module.
>>
>> Regards,
>> Timo
>>
>> On 09.03.21 08:14, Kurt Young wrote:
>>> Hi Leonard,
>>>
>>> Thanks for this careful consideration. Given the fallback option will
>>> eventually change the behavior twice, which means
>>> potentially break user's job twice, I would also +1 to not introduce it.
>>>
>>> Best,
>>> Kurt
>>>
>>> On Fri, Mar 5, 2021 at 3:00 PM Leonard Xu <xb...@gmail.com> wrote:
>>>
>>>> Hi, all
>>>>
>>>> As the FLIP-162 discussed, we agreed current time functions’ behavior
>> is
>>>> incorrect and plan to introduce the option
>> *t**able.exec.fallback-legacy-time-function
>>>> *to enable user fallback to incorrect behavior.
>>>>
>>>> (1) The option is convenient for users who want to upgrade to 1.13 but
>>>> don't want to change their sql job, user need to config the option
>> value, *this
>>>> is the first time users influenced by these wrong functions.*
>>>>
>>>> (2) But we didn’t consider that the option will be deleted after one or
>>>> two major versions, users have to change their sql job again at that
>> time
>>>> point, *this the second time** users influenced by these wrong
>> functions.*
>>>>
>>>> (3) Besides, maintaining two sets of functions is prone to bugs.
>>>>
>>>> I’ve discussed with some community developers offline, they tend to
>> solve
>>>> these functions at once i.e. Correct the wrong functions directly and do
>>>> not introduce this option.
>>>>
>>>> Considering that we will delete the configuration eventually, comparing
>>>> hurting users twice and bothering them for a long time, I would rather
>> hurt
>>>> users once.
>>>> *Thus I also +1* that we should directly correct these wrong functions
>>>> and remove the wrong functions at the same time.
>>>>
>>>>
>>>> If we can make a consensus in this thread, I think we can remove this
>>>> option support in FLIP-162.
>>>> How do you think?
>>>>
>>>> Best,
>>>> Leonard
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>
>>
>
> --
> Best, Jingsong Lee
Re: [DISCUSS] FLIP-162 follow-up discussion
Posted by Jingsong Li <ji...@gmail.com>.
+1
Let's go straight to the right behavior. Drop the option for the wrong
behavior.
Best,
Jingsong
On Tue, Mar 9, 2021 at 4:29 PM Timo Walther <tw...@apache.org> wrote:
> Hi Leonard,
>
> I'm fine with dropping the old buggy behavior immediatly. Users can
> still implement a UDF with the old bavhior if needed. I hope the new
> functions will be well-tested so that a fallback to the old functions is
> not necessary as a workaround. It will definitely avoid confusion for
> users and avoid spaghetti code in the planner module.
>
> Regards,
> Timo
>
> On 09.03.21 08:14, Kurt Young wrote:
> > Hi Leonard,
> >
> > Thanks for this careful consideration. Given the fallback option will
> > eventually change the behavior twice, which means
> > potentially break user's job twice, I would also +1 to not introduce it.
> >
> > Best,
> > Kurt
> >
> > On Fri, Mar 5, 2021 at 3:00 PM Leonard Xu <xb...@gmail.com> wrote:
> >
> >> Hi, all
> >>
> >> As the FLIP-162 discussed, we agreed current time functions’ behavior
> is
> >> incorrect and plan to introduce the option
> *t**able.exec.fallback-legacy-time-function
> >> *to enable user fallback to incorrect behavior.
> >>
> >> (1) The option is convenient for users who want to upgrade to 1.13 but
> >> don't want to change their sql job, user need to config the option
> value, *this
> >> is the first time users influenced by these wrong functions.*
> >>
> >> (2) But we didn’t consider that the option will be deleted after one or
> >> two major versions, users have to change their sql job again at that
> time
> >> point, *this the second time** users influenced by these wrong
> functions.*
> >>
> >> (3) Besides, maintaining two sets of functions is prone to bugs.
> >>
> >> I’ve discussed with some community developers offline, they tend to
> solve
> >> these functions at once i.e. Correct the wrong functions directly and do
> >> not introduce this option.
> >>
> >> Considering that we will delete the configuration eventually, comparing
> >> hurting users twice and bothering them for a long time, I would rather
> hurt
> >> users once.
> >> *Thus I also +1* that we should directly correct these wrong functions
> >> and remove the wrong functions at the same time.
> >>
> >>
> >> If we can make a consensus in this thread, I think we can remove this
> >> option support in FLIP-162.
> >> How do you think?
> >>
> >> Best,
> >> Leonard
> >>
> >>
> >>
> >>
> >>
> >
>
>
--
Best, Jingsong Lee
Re: [DISCUSS] FLIP-162 follow-up discussion
Posted by Timo Walther <tw...@apache.org>.
Hi Leonard,
I'm fine with dropping the old buggy behavior immediatly. Users can
still implement a UDF with the old bavhior if needed. I hope the new
functions will be well-tested so that a fallback to the old functions is
not necessary as a workaround. It will definitely avoid confusion for
users and avoid spaghetti code in the planner module.
Regards,
Timo
On 09.03.21 08:14, Kurt Young wrote:
> Hi Leonard,
>
> Thanks for this careful consideration. Given the fallback option will
> eventually change the behavior twice, which means
> potentially break user's job twice, I would also +1 to not introduce it.
>
> Best,
> Kurt
>
> On Fri, Mar 5, 2021 at 3:00 PM Leonard Xu <xb...@gmail.com> wrote:
>
>> Hi, all
>>
>> As the FLIP-162 discussed, we agreed current time functions’ behavior is
>> incorrect and plan to introduce the option *t**able.exec.fallback-legacy-time-function
>> *to enable user fallback to incorrect behavior.
>>
>> (1) The option is convenient for users who want to upgrade to 1.13 but
>> don't want to change their sql job, user need to config the option value, *this
>> is the first time users influenced by these wrong functions.*
>>
>> (2) But we didn’t consider that the option will be deleted after one or
>> two major versions, users have to change their sql job again at that time
>> point, *this the second time** users influenced by these wrong functions.*
>>
>> (3) Besides, maintaining two sets of functions is prone to bugs.
>>
>> I’ve discussed with some community developers offline, they tend to solve
>> these functions at once i.e. Correct the wrong functions directly and do
>> not introduce this option.
>>
>> Considering that we will delete the configuration eventually, comparing
>> hurting users twice and bothering them for a long time, I would rather hurt
>> users once.
>> *Thus I also +1* that we should directly correct these wrong functions
>> and remove the wrong functions at the same time.
>>
>>
>> If we can make a consensus in this thread, I think we can remove this
>> option support in FLIP-162.
>> How do you think?
>>
>> Best,
>> Leonard
>>
>>
>>
>>
>>
>