Re: [DISCUSS][FLINK-31788][FLINK-33015] Add back Support emitUpdateWithRetract for TableAggregateFunction

2023-09-10 Thread Feng Jin
Thanks Jane for following up on this issue!

+1 for adding it back first.

Supporting emitUpdateWithRetract for TableAggregateFunction is a good
feature, we should support it unless there are better alternatives.


Best,
Feng

On Thu, Sep 7, 2023 at 11:01 PM Lincoln Lee  wrote:

> Thanks to Jane for following up on this issue!  +1 for adding it back
> first.
>
> For the deprecation, considering that users aren't usually motivated to
> upgrade to a major version (1.14, from two years ago, wasn't that old,
> which may be
> part of the reason for not receiving more feedback), I'd recommend holding
> off on removing `TableAggregateFunction` until we have a replacement for
> it,
> e.g., user-defined-operator as Jark mentioned or something else.
>
> Best,
> Lincoln Lee
>
>
> Best,
> Lincoln Lee
>
>
> Jark Wu  于2023年9月7日周四 21:30写道:
>
> > +1 to fix it first.
> >
> > I also agree to deprecate it if there are few people using it,
> > but this should be another discussion thread within dev+user ML.
> >
> > In the future, we are planning to introduce user-defined-operator
> > based on the TVF functionality which I think can fully subsume
> > the UDTAG, cc @Timo Walther .
> >
> > Best,
> > Jark
> >
> > On Thu, 7 Sept 2023 at 11:44, Jane Chan  wrote:
> >
> > > Hi devs,
> > >
> > > Recently, we noticed an issue regarding a feature regression related to
> > > Table API. `org.apache.flink.table.functions.TableAggregateFunction`
> > > provides an API `emitUpdateWithRetract` [1] to cope with updated
> values,
> > > but it's not being called in the code generator. As a result, even if
> > users
> > > override this method, it does not work as intended.
> > >
> > > This issue has been present since version 1.15 (when the old planner
> was
> > > deprecated), but surprisingly, only two users have raised concerns
> about
> > it
> > > [2][3].
> > >
> > > So, I would like to initiate a discussion to bring it back. Of course,
> if
> > > few users use it, we can also consider deprecating it.
> > >
> > > [1]
> > >
> > >
> >
> https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/functions/udfs/#retraction-example
> > > [2] https://lists.apache.org/thread/rnvw8k3636dqhdttpmf1c9colbpw9svp
> > > [3]
> https://www.mail-archive.com/user-zh@flink.apache.org/msg15230.html
> > >
> > > Best,
> > > Jane
> > >
> >
>


Re: [DISCUSS][FLINK-31788][FLINK-33015] Add back Support emitUpdateWithRetract for TableAggregateFunction

2023-09-07 Thread Lincoln Lee
Thanks to Jane for following up on this issue!  +1 for adding it back first.

For the deprecation, considering that users aren't usually motivated to
upgrade to a major version (1.14, from two years ago, wasn't that old,
which may be
part of the reason for not receiving more feedback), I'd recommend holding
off on removing `TableAggregateFunction` until we have a replacement for
it,
e.g., user-defined-operator as Jark mentioned or something else.

Best,
Lincoln Lee


Best,
Lincoln Lee


Jark Wu  于2023年9月7日周四 21:30写道:

> +1 to fix it first.
>
> I also agree to deprecate it if there are few people using it,
> but this should be another discussion thread within dev+user ML.
>
> In the future, we are planning to introduce user-defined-operator
> based on the TVF functionality which I think can fully subsume
> the UDTAG, cc @Timo Walther .
>
> Best,
> Jark
>
> On Thu, 7 Sept 2023 at 11:44, Jane Chan  wrote:
>
> > Hi devs,
> >
> > Recently, we noticed an issue regarding a feature regression related to
> > Table API. `org.apache.flink.table.functions.TableAggregateFunction`
> > provides an API `emitUpdateWithRetract` [1] to cope with updated values,
> > but it's not being called in the code generator. As a result, even if
> users
> > override this method, it does not work as intended.
> >
> > This issue has been present since version 1.15 (when the old planner was
> > deprecated), but surprisingly, only two users have raised concerns about
> it
> > [2][3].
> >
> > So, I would like to initiate a discussion to bring it back. Of course, if
> > few users use it, we can also consider deprecating it.
> >
> > [1]
> >
> >
> https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/functions/udfs/#retraction-example
> > [2] https://lists.apache.org/thread/rnvw8k3636dqhdttpmf1c9colbpw9svp
> > [3] https://www.mail-archive.com/user-zh@flink.apache.org/msg15230.html
> >
> > Best,
> > Jane
> >
>


Re: [DISCUSS][FLINK-31788][FLINK-33015] Add back Support emitUpdateWithRetract for TableAggregateFunction

2023-09-07 Thread Jark Wu
+1 to fix it first.

I also agree to deprecate it if there are few people using it,
but this should be another discussion thread within dev+user ML.

In the future, we are planning to introduce user-defined-operator
based on the TVF functionality which I think can fully subsume
the UDTAG, cc @Timo Walther .

Best,
Jark

On Thu, 7 Sept 2023 at 11:44, Jane Chan  wrote:

> Hi devs,
>
> Recently, we noticed an issue regarding a feature regression related to
> Table API. `org.apache.flink.table.functions.TableAggregateFunction`
> provides an API `emitUpdateWithRetract` [1] to cope with updated values,
> but it's not being called in the code generator. As a result, even if users
> override this method, it does not work as intended.
>
> This issue has been present since version 1.15 (when the old planner was
> deprecated), but surprisingly, only two users have raised concerns about it
> [2][3].
>
> So, I would like to initiate a discussion to bring it back. Of course, if
> few users use it, we can also consider deprecating it.
>
> [1]
>
> https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/functions/udfs/#retraction-example
> [2] https://lists.apache.org/thread/rnvw8k3636dqhdttpmf1c9colbpw9svp
> [3] https://www.mail-archive.com/user-zh@flink.apache.org/msg15230.html
>
> Best,
> Jane
>