Hey Jing,
If you went through the discussion, you would see it has never been
shifted towards "ignore". The only concern in the discussion was we'd
have too many options and that lookup joins require them. It was never
questioned we should not throw an exception that was suggested in the
first message:
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.
until you commented on the PR, which confused me. The oracle's
OPTIMIZER_IGNORE_HINTS was shown as an example of a system that does let
you disable hints. It was never said let's support the same behaviour.
Just that, yeah, I am fine now with the adding the option. Let's name it
the same way.
On one
hand, there is no clear reason why we should disable(throwing exception) it
globally, and on the other hand, some functionalities, e.g. lookup join
pointed out by Ron, are depending on it.
What's the difference between ignore and throw in this case? They
wouldn't work in either case.
Would you like to elaborate the
must-have requirement for the "disabled" scenario?
As platform provider we'd like to take as much control of the query as
possible. Query hints expose internals which we would like to take
control of. Moreover we don't suggest to disable them by default. We
just propose to have that possibility if you need it. If you don't want
to, you don't need to use it. I also don't get the argument we'd have
too many options. We already have many detailed options and on the other
hand query hints themselves make the system more complicated to operate.
I think my proposal to support both THROW and IGNORE semantics should
satisfy both mine requirements and your concerns, so to be honest I am
disappointed that you don't want to reach a compromise, but you just
block the effort.
Best,
Dawid
On 05/10/2023 05:08, Jing Ge wrote:
Hi Dawid,
Thanks for the clarification. If you could go through the discussion, you
would be aware that the focus has been moved from "disable" to "ignore".
There was an alignment only on "ignore hints". Your suggestion bypassed the
alignment and mixed everything together. That confused me a bit. On one
hand, there is no clear reason why we should disable(throwing exception) it
globally, and on the other hand, some functionalities, e.g. lookup join
pointed out by Ron, are depending on it. Would you like to elaborate the
must-have requirement for the "disabled" scenario? Thanks!
Best regards,
Jing
On Thu, Oct 5, 2023 at 12:23 AM Sergey Nuyanzin<snuyan...@gmail.com> wrote:
Hi Dawid,
Thanks for bringing this.
I would agree with enum approach
ignored option would allow to follow Oracle's behavior as well
table.optimizer.query-options = ENABLED/DISABLED/IGNORED
nit: Can we have "hint" in config option name
e.g. table.optimizer.query-options-hints ?
On Tue, Oct 3, 2023 at 5:58 PM Dawid Wysakowicz<dwysakow...@apache.org>
wrote:
Hey all,
My understanding was that from the first message we were discussing
throwing an exception. Oracle was only shown as an example of a system
that have a flag for hints behaviour.
Let's get back to the discussion and agree on the behavior. My
suggestion is to introduce an enum instead of a boolean flag since
apparently there are different requirements. My take is that it is worth
to have an option to throw an exception if hints are disabled and are
provided in the SQL query. This is the same behavior as disabling
OPTIONS hint we already have[1]
Since you both @Jing and @Sergey would rather like to have an option to
ignore them we can introduce
table.optimizer.query-options = ENABLED/DISABLED/IGNORED
ENABLED: hints just work
DISABLED: throw an exception
IGNORED: ignore hints
Are you two fine with that option @Jing @Sergey?
Since this thread had a few misunderstandings already, I'd suggest to
convert it to a FLIP and follow with a VOTE shortly. @Bonnie would you
like to help with that?
Best,
Dawid
[1]
https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/config/#table-dynamic-table-options-enabled
On 02/10/2023 18:18, Jing Ge wrote:
Hi,
I have the same concern as Sergey and would like to make sure the
purpose
of this discussion is to globally ignore hints without changing any
other
behaviours, if I am not mistaken. Thanks!
Best regards,
Jing
On Mon, Oct 2, 2023 at 3:40 PM Sergey Nuyanzin<snuyan...@gmail.com>
wrote:
Hi Bonnie,
I think changing it to something like <name>.enabled
could lead to misunderstanding
for instance when we set this flag to false what should it mean?
1. Just ignore hints and execute a query like the same query however
with
removed hints
2. Fail on validation because hints are disabled
3. something else
I tend to think that we are talking about just ignoring hints, so
option 1
(and Oracle follows option 1 as well as mentioned at [1]).
So I would suggest to keep ignore in property name to make it more
clear
Please let me know if I miss anything
[1]
https://docs.oracle.com/en/database/oracle/oracle-database/23/refrn/OPTIMIZER_IGNORE_HINTS.html#GUID-D62CA6D8-D0D8-4A20-93EA-EEB4B3144347
On Fri, Sep 29, 2023 at 7:20 PM Bonnie Arogyam Varghese
<bvargh...@confluent.io.invalid> wrote:
Hi Jark,
A minor suggestion. Would it be ok if we changed the config name
to `
table.optimizer.query-options.enabled`?
This is inline with other existing configurations such as:
table.dynamic-table-options.enabled
table.optimizer.distinct-agg.split.enabled
table.optimizer.dynamic-filtering.enabled
On Wed, Sep 27, 2023 at 9:57 AM Bonnie Arogyam Varghese <
bvargh...@confluent.io> wrote:
Hi Martjin,
Yes, the suggestion for the configuration name made by Jark sounds
good.
Also, thanks to everyone who participated in this discussion.
On Tue, Sep 19, 2023 at 2:40 PM Martijn Visser <
martijnvis...@apache.org
wrote:
Hey Jark,
Sounds fine to me.
@Bonnie does that also work for you?
Best regards,
Martijn
On Fri, Sep 15, 2023 at 1:28 PM Jark Wu<imj...@gmail.com> wrote:
Hi Martijn,
Thanks for the investigation. I found the blog[1] shows a use case
that they use "OPTIMIZER_IGNORE_HINTS" to check if embedded
hints can be removed in legacy code. I think this is a useful tool
to
improve queries without complex hints strewn throughout the code.
Therefore, I'm fine to support this now.
Maybe we can follow Oracle to name the config
"table.optimizer.ignore-query-hints=false"?
Best,
Jark
[1]:
https://dbaora.com/optimizer_ignore_hints-oracle-database-18c/
On Fri, 15 Sept 2023 at 17:57, Martijn Visser <
martijnvis...@apache.org
wrote:
Hi Jark,
Oracle has/had a generic "OPTIMIZER_IGNORE_HINTS" [1]. It looks
like
MSSQL
has configuration options to disable specific hints [2] instead
of a
generic solution.
[1]
https://docs.oracle.com/en/database/oracle/oracle-database/23/refrn/OPTIMIZER_IGNORE_HINTS.html#GUID-D62CA6D8-D0D8-4A20-93EA-EEB4B3144347
[2]
https://www.mssqltips.com/sqlservertip/4175/disabling-sql-server-optimizer-rules-with-queryruleoff/
Best regards,
Martijn
On Fri, Sep 15, 2023 at 10:53 AM Jark Wu<imj...@gmail.com>
wrote:
Hi Martijn,
I can understand that.
Is there any database/system that supports disabling/enabling
query
hints
with a configuration? This might help us to understand the use
case,
and follow the approach.
Best,
Jark
On Fri, 15 Sept 2023 at 15:34, Martijn Visser <
martijnvis...@apache.org>
wrote:
Hi all,
I think Jark has a valid point with:
Does this mean that in the future we might add an option to
disable
each
feature?
I don't think that's a reasonable outcome indeed, but we are
currently
in a
situation where we don't have clear guidelines on when to add
a
configuration option, and when not to add one. From a platform
perspective,
there might not be an imminent or obvious security
implication,
but you
want to minimize a potential attack surface.
We should try to remove the unnecessary configuration from
the
list
in
Flink 2.0.
I agree with that too.
With these things in mind, my proposal would be the following:
* Add a configuration option for this situation, given that we
don't
have
clear guidelines on when to add/not add a new config option.
* Since we want to overhaul the configuration layer anyway in
Flink
2.0,
we
clean-up the configuration list by not having an option for
each
item,
but
either a generic option that allows you to disable one or more
features
(by
providing a list as the configuration option), or we already
bundle
multiple configuration options into a specific category, e.g.
so
that
you
can have a default Flink without any hardening, a read-only
Flink, a
fully-hardened Flink etc)
Best regards,
Martijn
On Mon, Sep 11, 2023 at 4:51 PM Jim Hughes
<jhug...@confluent.io.invalid
wrote:
Hi Jing and Jark!
I can definitely appreciate the desire to have fewer
configurations.
Do you have a suggested alternative for platform providers
to
limit
or
restrict the hints that Bonnie is talking about?
As one possibility, maybe one configuration could be set to
control
all
hints.
Cheers,
Jim
On Sat, Sep 9, 2023 at 6:16 AM Jark Wu<imj...@gmail.com>
wrote:
I agree with Jing,
My biggest concern is this makes the boundary of adding an
option
very
unclear.
It's not a strong reason to add a config just because of
it
doesn't
affect
existing
users. Does this mean that in the future we might add an
option to
disable
each feature?
Flink already has a very long list of configurations
[1][2]
and
this
is
very scary
and not easy to use. We should try to remove the
unnecessary
configuration
from
the list in Flink 2.0. However, from my perspective,
adding
this
option
makes us far
away from this direction.
Best,
Jark
[1]
https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/config/
[2]
https://nightlies.apache.org/flink/flink-docs-master/docs/deployment/config/
On Sat, 9 Sept 2023 at 17:33, Jing Ge
<j...@ververica.com.invalid>
wrote:
Hi,
Thanks for bringing this to our attention. At the first
glance,
it
looks
reasonable to offer a new configuration to
enable/disable
SQL
hints
globally. However, IMHO, it is not the right timing to
do
it now,
because
we should not only think as platform providers but also
as
end
users(the
number of end users are much bigger than platform
providers):
1. Users don't need it because users have the choice to
use
hints
or
not,
just like Jark pointed out. With this configuration,
there
will
be
a
fight
between platform providers and users which will cause
more
confusions
and
conflicts. And users will probably win, IMHO, because
they
are
the
end
customers that use Flink to create business values.
2. SQL hints could be considered as an additional
feature
for
users
to
control, to optimize the execution plan without touching
the
internal
logic, i.e. features for advanced use cases and i.e.
don't
use it
if
you
don't understand it.
3. Before the system is smart enough to take over(where
we
are
now,
fortunately and unfortunately :-))), there should be a
way
for
users
to
do
such tuning, even if it is a temporary phase from a
long-term's perspective, i.e. just because it is a
temporary
solution,
does
not mean it is not necessary for now.
4. What if users write wrong hints? Well, the code
review
process
is
recommended. Someone who truly understands hints should
double
check
it
before hints are merged to the master or submitted to
the
production
env.
Just like a common software development process.
Just my two cents.
Best regards,
Jing
On Thu, Sep 7, 2023 at 10:02 PM Bonnie Arogyam Varghese
<bvargh...@confluent.io.invalid> wrote:
Hi Liu,
The default will be set to enabled which is the
current
behavior.
The
option will allow users/platform providers to disable
it
if
they
want
to.
On Wed, Sep 6, 2023 at 6:39 PM liu ron <
ron9....@gmail.com>
wrote:
Hi, Boonie
I'm with Jark on why disable hint is needed if it
won't
affect
security.
If
users don't need to use hint, then they won't care
about it
and I
don't
think it's going to be a nuisance. On top of that,
Lookup
Join
Hint
is
very
useful for streaming jobs, and disabling the hint
would
result
in
users
not
being able to use it.
Best,
Ron
Bonnie Arogyam Varghese <bvargh...@confluent.io
.invalid>
于2023年9月6日周三
23:52写道：
Hi Liu Ron,
To answer your question,
Security might not be the main reason for
disabling this
option
but
other arguments brought forward by Timo. Let me
know
if you
have
any
further questions or concerns.
On Tue, Sep 5, 2023 at 9:35 PM Bonnie Arogyam
Varghese <
bvargh...@confluent.io> wrote:
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.
--
Best regards,
Sergey
--
Best regards,
Sergey
