[DISCUSS] Disable conversion between TIMESTAMP and Long in parameters and results of UDXs

[Zhenghua Gao]

Hi,

I wanted to bring up the discuss of Disable conversion between TIMESTAMP
and Long in parameters and results of UDXs.

Since FLINK-12253[1] introduce the new TimestampType and conversion from
and
to long is not supported, the UDXs with Long parameters should not receive
TIMESTAMP fields and vice versa.

The current situation is we use long as internal representation of
TIMESTAMP, the legacy planner and blink planner DO NOT DISABLE this
conversion. Now FLINK-14599[2] would introduce a new internal
representation of TIMESTAMP and it's time to make a decision to DISABLE it.

In addition, our document[3] recommends UDXs users use long as
representation of SQL_TIMESTAMP, which is obsolete too.

Please let me know what you think!

[1] https://issues.apache.org/jira/browse/FLINK-12253
[2] https://issues.apache.org/jira/browse/FLINK-14599
[3]
https://ci.apache.org/projects/flink/flink-docs-release-1.9/dev/table/udfs.html#best-practices-for-implementing-udfs

*Best Regards,*
*Zhenghua Gao*

[Kurt Young]

+1 to disable, we also need to highlight this in 1.10 release notes.

Best,
Kurt


On Fri, Nov 22, 2019 at 5:56 PM Zhenghua Gao <[hidden email]> wrote:

> Hi,
>
> I wanted to bring up the discuss of Disable conversion between TIMESTAMP
> and Long in parameters and results of UDXs.
>
> Since FLINK-12253[1] introduce the new TimestampType and conversion from
> and
> to long is not supported, the UDXs with Long parameters should not receive
> TIMESTAMP fields and vice versa.
>
> The current situation is we use long as internal representation of
> TIMESTAMP, the legacy planner and blink planner DO NOT DISABLE this
> conversion. Now FLINK-14599[2] would introduce a new internal
> representation of TIMESTAMP and it's time to make a decision to DISABLE it.
>
> In addition, our document[3] recommends UDXs users use long as
> representation of SQL_TIMESTAMP, which is obsolete too.
>
> Please let me know what you think!
>
> [1] https://issues.apache.org/jira/browse/FLINK-12253
> [2] https://issues.apache.org/jira/browse/FLINK-14599
> [3]
>
> https://ci.apache.org/projects/flink/flink-docs-release-1.9/dev/table/udfs.html#best-practices-for-implementing-udfs
>
> *Best Regards,*
> *Zhenghua Gao*
>

[Jingsong Li]

+1 to disable, It is already introduced by new type system in TimestampType.
I think it is time to update document too.

Best,
Jingsong Lee

On Fri, Nov 22, 2019 at 6:05 PM Kurt Young <[hidden email]> wrote:

> +1 to disable, we also need to highlight this in 1.10 release notes.
>
> Best,
> Kurt
>
>
> On Fri, Nov 22, 2019 at 5:56 PM Zhenghua Gao <[hidden email]> wrote:
>
> > Hi,
> >
> > I wanted to bring up the discuss of Disable conversion between TIMESTAMP
> > and Long in parameters and results of UDXs.
> >
> > Since FLINK-12253[1] introduce the new TimestampType and conversion from
> > and
> > to long is not supported, the UDXs with Long parameters should not
> receive
> > TIMESTAMP fields and vice versa.
> >
> > The current situation is we use long as internal representation of
> > TIMESTAMP, the legacy planner and blink planner DO NOT DISABLE this
> > conversion. Now FLINK-14599[2] would introduce a new internal
> > representation of TIMESTAMP and it's time to make a decision to DISABLE
> it.
> >
> > In addition, our document[3] recommends UDXs users use long as
> > representation of SQL_TIMESTAMP, which is obsolete too.
> >
> > Please let me know what you think!
> >
> > [1] https://issues.apache.org/jira/browse/FLINK-12253
> > [2] https://issues.apache.org/jira/browse/FLINK-14599
> > [3]
> >
> >
> https://ci.apache.org/projects/flink/flink-docs-release-1.9/dev/table/udfs.html#best-practices-for-implementing-udfs
> >
> > *Best Regards,*
> > *Zhenghua Gao*
> >
>

--
Best, Jingsong Lee

[Timo Walther-2]

Hi,

+1 for disabling it in the Blink planner. Once FLIP-65 is implemented
and a UDX is registered with the new
TableEnvironment.createTemporaryFunction() we will also have the
possibility to be fully compliant with the new type system because we
can advertise a new UDF stack with new behavior.

Also the mentioned documentation page will be updated as part of FLIP-65.

Regards,
Timo


On 22.11.19 11:08, Jingsong Li wrote:

> +1 to disable, It is already introduced by new type system in TimestampType.
> I think it is time to update document too.
>
> Best,
> Jingsong Lee
>
> On Fri, Nov 22, 2019 at 6:05 PM Kurt Young <[hidden email]> wrote:
>
>> +1 to disable, we also need to highlight this in 1.10 release notes.
>>
>> Best,
>> Kurt
>>
>>
>> On Fri, Nov 22, 2019 at 5:56 PM Zhenghua Gao <[hidden email]> wrote:
>>
>>> Hi,
>>>
>>> I wanted to bring up the discuss of Disable conversion between TIMESTAMP
>>> and Long in parameters and results of UDXs.
>>>
>>> Since FLINK-12253[1] introduce the new TimestampType and conversion from
>>> and
>>> to long is not supported, the UDXs with Long parameters should not
>> receive
>>> TIMESTAMP fields and vice versa.
>>>
>>> The current situation is we use long as internal representation of
>>> TIMESTAMP, the legacy planner and blink planner DO NOT DISABLE this
>>> conversion. Now FLINK-14599[2] would introduce a new internal
>>> representation of TIMESTAMP and it's time to make a decision to DISABLE
>> it.
>>>
>>> In addition, our document[3] recommends UDXs users use long as
>>> representation of SQL_TIMESTAMP, which is obsolete too.
>>>
>>> Please let me know what you think!
>>>
>>> [1] https://issues.apache.org/jira/browse/FLINK-12253
>>> [2] https://issues.apache.org/jira/browse/FLINK-14599
>>> [3]
>>>
>>>
>> https://ci.apache.org/projects/flink/flink-docs-release-1.9/dev/table/udfs.html#best-practices-for-implementing-udfs
>>>
>>> *Best Regards,*
>>> *Zhenghua Gao*
>>>
>>
>
>

[Jark Wu-2]

Hi,

+1 to disable it in 1.10. I think it's time to disable and correct the
behavior now.

Also cc'ed user mailing list to have broader audiences.

Best,
Jark

On Sat, 23 Nov 2019 at 16:59, Timo Walther <[hidden email]> wrote:

> Hi,
>
> +1 for disabling it in the Blink planner. Once FLIP-65 is implemented
> and a UDX is registered with the new
> TableEnvironment.createTemporaryFunction() we will also have the
> possibility to be fully compliant with the new type system because we
> can advertise a new UDF stack with new behavior.
>
> Also the mentioned documentation page will be updated as part of FLIP-65.
>
> Regards,
> Timo
>
>
> On 22.11.19 11:08, Jingsong Li wrote:
> > +1 to disable, It is already introduced by new type system in
> TimestampType.
> > I think it is time to update document too.
> >
> > Best,
> > Jingsong Lee
> >
> > On Fri, Nov 22, 2019 at 6:05 PM Kurt Young <[hidden email]> wrote:
> >
> >> +1 to disable, we also need to highlight this in 1.10 release notes.
> >>
> >> Best,
> >> Kurt
> >>
> >>
> >> On Fri, Nov 22, 2019 at 5:56 PM Zhenghua Gao <[hidden email]> wrote:
> >>
> >>> Hi,
> >>>
> >>> I wanted to bring up the discuss of Disable conversion between
> TIMESTAMP
> >>> and Long in parameters and results of UDXs.
> >>>
> >>> Since FLINK-12253[1] introduce the new TimestampType and conversion
> from
> >>> and
> >>> to long is not supported, the UDXs with Long parameters should not
> >> receive
> >>> TIMESTAMP fields and vice versa.
> >>>
> >>> The current situation is we use long as internal representation of
> >>> TIMESTAMP, the legacy planner and blink planner DO NOT DISABLE this
> >>> conversion. Now FLINK-14599[2] would introduce a new internal
> >>> representation of TIMESTAMP and it's time to make a decision to DISABLE
> >> it.
> >>>
> >>> In addition, our document[3] recommends UDXs users use long as
> >>> representation of SQL_TIMESTAMP, which is obsolete too.
> >>>
> >>> Please let me know what you think!
> >>>
> >>> [1] https://issues.apache.org/jira/browse/FLINK-12253
> >>> [2] https://issues.apache.org/jira/browse/FLINK-14599
> >>> [3]
> >>>
> >>>
> >>
> https://ci.apache.org/projects/flink/flink-docs-release-1.9/dev/table/udfs.html#best-practices-for-implementing-udfs
> >>>
> >>> *Best Regards,*
> >>> *Zhenghua Gao*
> >>>
> >>
> >
> >
>
>

[Zhenghua Gao]

Since it is unanimously agreed that we should disable conversion between
Timestmap and
long in parameters and results of UDXs, in PR [1] we will disable it in
blink planner. And we
will add a release note in FLINK-14599 [2] of this incompatible
modification.

<https://github.com/apache/flink/pull/10268>

[1] https://github.com/apache/flink/pull/10268
[2] https://issues.apache.org/jira/browse/FLINK-14599

*Best Regards,*
*Zhenghua Gao*


On Sun, Nov 24, 2019 at 8:44 PM Jark Wu <[hidden email]> wrote:

> Hi,
>
> +1 to disable it in 1.10. I think it's time to disable and correct the
> behavior now.
>
> Also cc'ed user mailing list to have broader audiences.
>
> Best,
> Jark
>
> On Sat, 23 Nov 2019 at 16:59, Timo Walther <[hidden email]> wrote:
>
>> Hi,
>>
>> +1 for disabling it in the Blink planner. Once FLIP-65 is implemented
>> and a UDX is registered with the new
>> TableEnvironment.createTemporaryFunction() we will also have the
>> possibility to be fully compliant with the new type system because we
>> can advertise a new UDF stack with new behavior.
>>
>> Also the mentioned documentation page will be updated as part of FLIP-65.
>>
>> Regards,
>> Timo
>>
>>
>> On 22.11.19 11:08, Jingsong Li wrote:
>> > +1 to disable, It is already introduced by new type system in
>> TimestampType.
>> > I think it is time to update document too.
>> >
>> > Best,
>> > Jingsong Lee
>> >
>> > On Fri, Nov 22, 2019 at 6:05 PM Kurt Young <[hidden email]> wrote:
>> >
>> >> +1 to disable, we also need to highlight this in 1.10 release notes.
>> >>
>> >> Best,
>> >> Kurt
>> >>
>> >>
>> >> On Fri, Nov 22, 2019 at 5:56 PM Zhenghua Gao <[hidden email]> wrote:
>> >>
>> >>> Hi,
>> >>>
>> >>> I wanted to bring up the discuss of Disable conversion between
>> TIMESTAMP
>> >>> and Long in parameters and results of UDXs.
>> >>>
>> >>> Since FLINK-12253[1] introduce the new TimestampType and conversion
>> from
>> >>> and
>> >>> to long is not supported, the UDXs with Long parameters should not
>> >> receive
>> >>> TIMESTAMP fields and vice versa.
>> >>>
>> >>> The current situation is we use long as internal representation of
>> >>> TIMESTAMP, the legacy planner and blink planner DO NOT DISABLE this
>> >>> conversion. Now FLINK-14599[2] would introduce a new internal
>> >>> representation of TIMESTAMP and it's time to make a decision to
>> DISABLE
>> >> it.
>> >>>
>> >>> In addition, our document[3] recommends UDXs users use long as
>> >>> representation of SQL_TIMESTAMP, which is obsolete too.
>> >>>
>> >>> Please let me know what you think!
>> >>>
>> >>> [1] https://issues.apache.org/jira/browse/FLINK-12253
>> >>> [2] https://issues.apache.org/jira/browse/FLINK-14599
>> >>> [3]
>> >>>
>> >>>
>> >>
>> https://ci.apache.org/projects/flink/flink-docs-release-1.9/dev/table/udfs.html#best-practices-for-implementing-udfs
>> >>>
>> >>> *Best Regards,*
>> >>> *Zhenghua Gao*
>> >>>
>> >>
>> >
>> >
>>
>>