Hi, devs, Thanks for all the feedback.
Based on the discussion [1], we seem to have a consensus, so I would like to start a vote. If you have any questions or concerns, please feel free to follow up on this discussion. [1] https://lists.apache.org/thread/3s69dhv3rp4s0kysnslqbvyqo3qf7zq5 Best regards, Jane On Fri, Oct 13, 2023 at 5:36 PM Zakelly Lan <zakelly....@gmail.com> wrote: > Hi Jane, > > Actually, the first example that comes to mind is a simple group > aggregation, such as "SELECT /*+ STATE_TTL('1d') */ userid, SUM(price) > FROM orders GROUP BY userid;". However, as you mentioned, omitting the > key can introduce a lot of ambiguity, especially in nested query > blocks. I agree that we should not support this syntax. > > > Thanks a lot for clarifying this for me. > > > Best, > Zakelly > > > On Fri, Oct 13, 2023 at 4:59 PM Jane Chan <qingyue....@gmail.com> wrote: > > > > Hi Zakelly, > > > > What if an omitted key hint only applied for tables from the current > query > > > block? > > > > > > > Can you elaborate on some examples to illustrate? But if you're asking > > about the usage mentioned in [1], like " SELECT * FROM left /*+ > STATE_TTL( > > '1h' ) */ JOIN right /*+ STATE_TTL('1d') */ ON ...; ", I'd like to > explain > > the reason why this syntax is not proposed in the following aspects: > > > > #1 According to the current hint syntax, hints that follow the > > SqlIdentifier are deemed as **DynamicTableOptions** [2]. However, > > "table.exec.state.ttl" is not a table option. If we want to support this > > approach, a separate thread may be required to discuss whether hints > > that follow an identifier should be limited to table options only. > > > > #2 On the other hand, according to the "Options Propagation" mentioned in > > FLIP-113 [3], > > > > > Let T be the table name that attaches hints: > > > > If T is a view, ignore the hints, we do not allow any query hints in the > > > table hints context, which is ambiguous because the view is actually a > > > query and itself can take a query hint; > > > > Since the "left" and "right" (the example above) could be either a table > > name or a view name (alias to the query block). The hint key cannot be > > omitted if placed on the query block. So, from this point, having a > unified > > syntax that covers all situations is challenging. > > > > [1] > > > https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/queries/hints/#examples > > < > https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/queries/hints/#examples > > > > [2] > > > https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/queries/hints/#dynamic-table-options > > < > https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/queries/hints/#dynamic-table-options > > > > [3] > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-113%3A+Supports+Dynamic+Table+Options+for+Flink+SQL > > < > https://cwiki.apache.org/confluence/display/FLINK/FLIP-113%3A+Supports+Dynamic+Table+Options+for+Flink+SQL > > > > > > Best, > > Jane > > > > On Thu, Oct 12, 2023 at 7:02 PM Zakelly Lan <zakelly....@gmail.com> > wrote: > > > > > Thanks Jane for clarifying this. I'm ok with keeping the current hint > > > pattern with the table name specified by key. Just thinking if there > > > is another simpler way to define the hint. What if an omitted key hint > > > only applied for tables from the current query block? > > > > > > > > > Thanks. > > > > > > > > > Best, > > > Zakelly > > > > > > On Wed, Oct 11, 2023 at 11:25 PM Jane Chan <qingyue....@gmail.com> > wrote: > > > > > > > > Thank you all for the valuable feedback. > > > > > > > > There is a difference when users incorrectly use hints, and there > are two > > > > possible scenarios: > > > > <1> STATE_TTL hint can be applied to the current query block but > with an > > > > invalid key or value. In this case, the validation exception will be > > > > thrown. The HintOptionChecker will throw exceptions if the options > are > > > > empty or the value is an invalid duration. And JoinHintResolver will > > > throw > > > > exceptions if the hint key(table name/alias, etc.) does not exist. So > > > does > > > > for the group aggregate. > > > > > > > > <2> STATE_TTL hint cannot be applied to the current query block, > e.g., > > > > SELECT /*+ STATE_TTL('MyTable' = '2h') */ * a, b, c FROM MyTable. In > this > > > > case, the hint is ignored. This is a by-design behavior according to > > > > FLIP-229 [1]. > > > > > > > > I've made the modifications to the FLIP regarding exception > handling. I > > > > would appreciate it if you could review it again. > > > > > > > > @Xuyang > > > > > > > > > I notice that using STATE_TTL hints wrongly will not throw any > > > > > exceptions. But it seems that in the current join hint scenario, if > > > user > > > > > uses an unknown table name as the chosen side, a validation > exception > > > will > > > > > be thrown. Maybe we should distinguish which exceptions need to be > > > thrown > > > > > explicitly. > > > > > > > > > > > > You're right; the STATE_TTL hint semantic check should throw > exceptions > > > > like join hints. > > > > > > > > @Feng @Yun > > > > > > > > > since there is no exception thrown when the STATE hint is set > > > incorrectly, > > > > > should we somehow show that the TTL setting has taken effect? > > > > > > > > For instance, exhibiting the TTL option within the operator's > > > description? > > > > > > > > > > > > We can throw explicit exceptions for scenario #1. For scenario #2, I > > > prefer > > > > to align the behavior for current query block hints for now (and we > may > > > > open a separate discussion in the future). On the other hand, from > the > > > > implementation aspect, it is not easy to do so. Taking the example of > > > > deduplication mentioned earlier, the hint is lost before it > propagates to > > > > the FlinkLogicalRank node, making it challenging to capture the > > > exception. > > > > > > > > @Zakelly > > > > > > > > > would it be possible to provide a simple hint that allows the > omission > > > of > > > > > the key? For example, something like "SELECT /+ STATE_TTL('1h')/", > > > which > > > > > would specify the TTL for all states in the 'SELECT' clause. > > > > > > > > > > > > I'm afraid the hint key cannot be omitted. E.g., the join operator > is a > > > > TwoInputStreamOperator; if we want to specify different state TTLs > for > > > the > > > > left and right input, we need to inform the planner of the ordinal > info > > > > (LEFT v.s. RIGHT). On the other hand, the SELECT clause may contain > > > several > > > > query blocks, while the hint scope is designed to apply to the > current > > > > query block [2] to prevent the hint on the outer query block from > > > > propagating to the inner query block. For the use case where users > need > > > to > > > > specify the TTL for all states in the 'SELECT' clause, it is > preferable > > > to > > > > modify the compiled plan instead of using hints. > > > > > > > > @ConradJam > > > > I share the same opinion with @Yun that this is a nice-to-have > feature. > > > Big > > > > +1 for the follow-up on FLINK-33230. Users can now use the COMPILE > PLAN > > > > statement to serialize the query to a JSON string or file and then > check > > > > the state metadata to ensure the hint is applied. > > > > > > > > [1] > > > > > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-229%3A+Introduces+Join+Hint+for+Flink+SQL+Batch+Job > > > > < > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-229%3A+Introduces+Join+Hint+for+Flink+SQL+Batch+Job > > > > > > > > [2] > > > > > > > > https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/queries/hints/#query-hints > > > > < > > > > https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/queries/hints/#query-hints > > > > > > > > > > > > Best, > > > > Jane > > > > > > > > > > > > On Wed, Oct 11, 2023 at 8:49 PM Yun Tang <myas...@live.com> wrote: > > > > > > > > > I think showing the TTL for operators is a nice-to-have feature, > not a > > > > > must one in this FLIP. We can still get the information from the > > > operator > > > > > descriptions. > > > > > > > > > > And I think we can continue the TTL showing work based on > FLINK-33230 > > > [1]. > > > > > > > > > > Last but not least, I prefer to throw exceptions if the TTL > > > configuration > > > > > is mistakenly used as it will affect the data correctness. > > > > > > > > > > [1] https://issues.apache.org/jira/browse/FLINK-33230 > > > > > > > > > > Best > > > > > Yun Tang > > > > > ________________________________ > > > > > From: ConradJam <jam.gz...@gmail.com> > > > > > Sent: Wednesday, October 11, 2023 20:30 > > > > > To: dev@flink.apache.org <dev@flink.apache.org> > > > > > Subject: Re: Re: [DISCUSS] FLIP-373: Support Configuring Different > > > State > > > > > TTLs using SQL Hint > > > > > > > > > > +1 TTL shows the state ttl for operators in Flink web ui can be > know > > > what > > > > > operator state > > > > > > > > > > Zakelly Lan <zakelly....@gmail.com> 于2023年10月11日周三 19:14写道： > > > > > > > > > > > Hi Jane, > > > > > > > > > > > > The fine-grained TTL management is extremely useful for > performance > > > > > > tuning, so +1 for the idea. I have a minor suggestion: would it > be > > > > > > possible to provide a simple hint that allows the omission of the > > > key? > > > > > > For example, something like "SELECT /+ STATE_TTL('1h')/", which > would > > > > > > specify the TTL for all states in the 'SELECT' clause. > > > > > > > > > > > > And I also share the same concern as Feng. I am wondering if we > could > > > > > > show the state ttl for operators in Flink UI. > > > > > > > > > > > > > > > > > > Best, > > > > > > Zakelly > > > > > > > > > > > > On Wed, Oct 11, 2023 at 1:27 PM Feng Jin <jinfeng1...@gmail.com> > > > wrote: > > > > > > > > > > > > > > Hi Jane, > > > > > > > > > > > > > > Thank you for providing this explanation. > > > > > > > > > > > > > > Another small question, since there is no exception thrown > when the > > > > > STATE > > > > > > > hint is set incorrectly, > > > > > > > should we somehow show that the TTL setting has taken effect? > > > > > > > For instance, exhibiting the TTL option within the operator's > > > > > > description? > > > > > > > > > > > > > > Best, > > > > > > > Feng > > > > > > > > > > > > > > On Tue, Oct 10, 2023 at 7:51 PM Xuyang <xyzhong...@163.com> > wrote: > > > > > > > > > > > > > > > Hi, Jane. > > > > > > > > > > > > > > > > > > > > > > > > I think this syntax will be easier for users to set operator > > > ttl. So > > > > > > big > > > > > > > > +1. I left some minor comments here. > > > > > > > > > > > > > > > > > > > > > > > > I notice that using STATE_TTL hints wrongly will not throw > any > > > > > > exceptions. > > > > > > > > But it seems that in the current join hint scenario, > > > > > > > > if user uses an unknown table name as the chosen side, a > > > validation > > > > > > > > exception will be thrown. > > > > > > > > Maybe we should distinguish which exceptions need to be > thrown > > > > > > explicitly. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > > > > > Best！ > > > > > > > > Xuyang > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > At 2023-10-10 18:23:55, "Jane Chan" <qingyue....@gmail.com> > > > wrote: > > > > > > > > >Hi Feng, > > > > > > > > > > > > > > > > > >Thank you for your valuable comments. The reason for not > > > including > > > > > the > > > > > > > > >scenarios above is as follows: > > > > > > > > > > > > > > > > > >For <1>, the automatically inferred stateful operators are > not > > > > > easily > > > > > > > > >expressible in SQL. This issue was discussed in FLIP-292, > and > > > > > besides > > > > > > > > >ChangelogNormalize, SinkUpsertMateralizer also faces the > same > > > > > problem. > > > > > > > > > > > > > > > > > >For <2> and <3>, the challenge lies in internal > implementation. > > > > > > During the > > > > > > > > >default_rewrite phase, the row_number expression in > > > LogicalProject > > > > > is > > > > > > > > >transformed into LogicalWindow by Calcite's > > > > > > > > >CoreRules#PROJECT_TO_LOGICAL_PROJECT_AND_WINDOW. However, > > > > > > CalcRelSplitter > > > > > > > > >does not pass the hints as an input argument when creating > > > > > > LogicalWindow, > > > > > > > > >resulting in the loss of the hint at this step. To support > > > this, we > > > > > > may > > > > > > > > >need to rewrite some optimization rules in Calcite, which > could > > > be a > > > > > > > > >follow-up work if required. > > > > > > > > > > > > > > > > > >Best, > > > > > > > > >Jane > > > > > > > > > > > > > > > > > >On Tue, Oct 10, 2023 at 1:40 AM Feng Jin < > jinfeng1...@gmail.com > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > >> Hi Jane, > > > > > > > > >> > > > > > > > > >> Thank you for proposing this FLIP. > > > > > > > > >> > > > > > > > > >> I believe that this FLIP will greatly enhance the > flexibility > > > of > > > > > > setting > > > > > > > > >> state, and by setting different operators' TTL, it can > also > > > > > > increase job > > > > > > > > >> stability, especially in regular join scenarios. > > > > > > > > >> The parameter design is very concise, big +1 for this, > and it > > > is > > > > > > also > > > > > > > > >> relatively easy to use for users. > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> I have a small question: in the FLIP, it only mentions > join > > > and > > > > > > group. > > > > > > > > >> Should we also consider other scenarios? > > > > > > > > >> > > > > > > > > >> 1. the auto generated deduplicate operator[1]. > > > > > > > > >> 2. the deduplicate query[2]. > > > > > > > > >> 3. the topN query[3]. > > > > > > > > >> > > > > > > > > >> [1] > > > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/config/#table-exec-source-cdc-events-duplicate > > > > > > > > >> [2] > > > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/queries/deduplication/ > > > > > > > > >> [3] > > > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/queries/topn/ > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> Best, > > > > > > > > >> Feng > > > > > > > > >> > > > > > > > > >> On Sun, Oct 8, 2023 at 5:53 PM Jane Chan < > > > qingyue....@gmail.com> > > > > > > wrote: > > > > > > > > >> > > > > > > > > >> > Hi devs, > > > > > > > > >> > > > > > > > > > >> > I'd like to initiate a discussion on FLIP-373: Support > > > > > Configuring > > > > > > > > >> > Different State TTLs using SQL Hint [1]. This proposal > is > > > on top > > > > > > of > > > > > > > > the > > > > > > > > >> > FLIP-292 [2] to address typical scenarios with > unambiguous > > > > > > semantics > > > > > > > > and > > > > > > > > >> > hint propagation. > > > > > > > > >> > > > > > > > > > >> > I'm looking forward to your opinions! > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > [1] > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-373%3A+Support+Configuring+Different+State+TTLs+using+SQL+Hint > > > > > > > > >> > [2] > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-292%3A+Enhance+COMPILED+PLAN+to+support+operator-level+state+TTL+configuration > > > > > > > > >> > > > > > > > > > >> > Best, > > > > > > > > >> > Jane > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > Best > > > > > > > > > > ConradJam > > > > > > > > >
