[FLINK-26495][table-planner] Prohibit hints(dynamic table options) on view #19007
Conversation
|
@flinkbot run azure |
|
|
@flinkbot run azure |
|
Hi, @JingsongLi, do you have time to take a look? |
|
Thanks @LadyForest for the contribution, can you add a case:
Explain select view2. |
| def testOptionsHintOnTableApiView(): Unit = { | ||
| val view1 = util.tableEnv.sqlQuery("select * from t1 join t2 on t1.a = t2.d") | ||
| util.tableEnv.createTemporaryView("view1", view1) | ||
| // The table hints on view expect to be ignored. |
There was a problem hiding this comment.
It appears that the previous behavior was by design, so we need to ensure that it is fully corrected.
There was a problem hiding this comment.
It appears that the previous behavior was by design.
Yes indeed. The hints are ignored quietly during planning phase. But it seems like our users are complaining the hints don't work. If we don't support hints on view, I think we'd better make it an explicit behavior. I'm afraid that the current design is somewhat not so much user-friendly. Beyond that, the current design is somewhat inconsistent, you can take a look at CatalogSourceTable#computeContextResolvedTable at L#144, it also throws exception when encounteredTableKind.VIEW. However, since the view expanding does not go through here, so this snippet of code does not get a chance to be called.
so we need to ensure that it is fully corrected
What do you mean by "ensure that it is fully corrected". Do you want to express we should ensure the exception is indeed thrown, or to ensure we don't throw exception when hints are imposed on table?
Sure |
|
@flinkbot run azure |
… view * Throw exception when expanding subquery * Fix the assertion order in CliClientITCase
What is the purpose of the change
This PR aims to throw an explicit exception when querying a view with hints to improve user experience. Currently, the hints on view are swallowed quietly during the planning phase without any trace, and users complain that the hints don't work on view.
Brief changelog
ExpandingPreparingTableto throw aValidationExceptionCliClientITCaseto correct the assertion order for expected and actualVerifying this change
This change added tests and can be verified as follows:
TableEnvironmentTest#testQueryViewWithHintsOptionHintsTest#testOptionsHintOnTableApiViewand#testOptionsHintOnSQLViewview.q, which can be verified by runningCliClientITCaseDoes this pull request potentially affect one of the following parts:
@Public(Evolving): (yes / no)Documentation