It looks like it will be nice to have a config to disable hints. Any other thoughts/concerns before we can close this discussion?
On Fri, Aug 18, 2023 at 7:43 AM Timo Walther <twal...@apache.org> wrote: > > lots of the streaming SQL syntax are extensions of SQL standard > > That is true. But hints are kind of a special case because they are not > even "part of Flink SQL" that's why they are written in a comment syntax. > > Anyway, I feel hints could be sometimes confusing for users because most > of them have no effect for streaming and long-term we could also set > some hints via the CompiledPlan. And if you have multiple teams, > non-skilled users should not play around with hints and leave the > decision to the system that might become smarter over time. > > Regards, > Timo > > > On 17.08.23 18:47, liu ron wrote: > > Hi, Bonnie > > > >> Options hints could be a security concern since users can override > > settings. > > > > I think this still doesn't answer my question > > > > Best, > > Ron > > > > Jark Wu <imj...@gmail.com> 于2023年8月17日周四 19:51写道： > > > >> Sorry, I still don't understand why we need to disable the query hint. > >> It doesn't have the security problems as options hint. Bonnie said it > >> could affect performance, but that depends on users using it explicitly. > >> If there is any performance problem, users can remove the hint. > >> > >> If we want to disable query hint just because it's an extension to SQL > >> standard. > >> I'm afraid we have to introduce a bunch of configuration, because lots > of > >> the streaming SQL syntax are extensions of SQL standard. > >> > >> Best, > >> Jark > >> > >> On Thu, 17 Aug 2023 at 15:43, Timo Walther <twal...@apache.org> wrote: > >> > >>> +1 for this proposal. > >>> > >>> Not every data team would like to enable hints. Also because they are > an > >>> extension to the SQL standard. It might also be the case that custom > >>> rules would be overwritten otherwise. Setting hints could also be the > >>> exclusive task of a DevOp team. > >>> > >>> Regards, > >>> Timo > >>> > >>> > >>> On 17.08.23 09:30, Konstantin Knauf wrote: > >>>> Hi Bonnie, > >>>> > >>>> this makes sense to me, in particular, given that we already have this > >>>> toggle for a different type of hints. > >>>> > >>>> Best, > >>>> > >>>> Konstantin > >>>> > >>>> Am Mi., 16. Aug. 2023 um 19:38 Uhr schrieb Bonnie Arogyam Varghese > >>>> <bvargh...@confluent.io.invalid>: > >>>> > >>>>> Hi Liu, > >>>>> Options hints could be a security concern since users can override > >>>>> settings. However, query hints specifically could affect performance. > >>>>> Since we have a config to disable Options hint, I'm suggesting we > also > >>> have > >>>>> a config to disable Query hints. > >>>>> > >>>>> On Wed, Aug 16, 2023 at 9:41 AM liu ron <ron9....@gmail.com> wrote: > >>>>> > >>>>>> Hi, > >>>>>> > >>>>>> Thanks for driving this proposal. > >>>>>> > >>>>>> Can you explain why you would need to disable query hints because of > >>>>>> security issues? I don't really understand why query hints affects > >>>>>> security. > >>>>>> > >>>>>> Best, > >>>>>> Ron > >>>>>> > >>>>>> Bonnie Arogyam Varghese <bvargh...@confluent.io.invalid> > >> 于2023年8月16日周三 > >>>>>> 23:59写道： > >>>>>> > >>>>>>> Platform providers may want to disable hints completely for > security > >>>>>>> reasons. > >>>>>>> > >>>>>>> Currently, there is a configuration to disable OPTIONS hint - > >>>>>>> > >>>>>>> > >>>>>> > >>>>> > >>> > >> > https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/config/#table-dynamic-table-options-enabled > >>>>>>> > >>>>>>> However, there is no configuration available to disable QUERY hints > >> - > >>>>>>> > >>>>>>> > >>>>>> > >>>>> > >>> > >> > https://nightlies.apache.org/flink/flink-docs-release-1.17/docs/dev/table/sql/queries/hints/#query-hints > >>>>>>> > >>>>>>> The proposal is to add a new configuration: > >>>>>>> > >>>>>>> Name: table.query-options.enabled > >>>>>>> Description: Enable or disable the QUERY hint, if disabled, an > >>>>>>> exception would be thrown if any QUERY hints are specified > >>>>>>> Note: The default value will be set to true. > >>>>>>> > >>>>>> > >>>>> > >>>> > >>>> > >>> > >>> > >> > > > >
