[GitHub] [flink] LadyForest commented on a change in pull request #19007: [FLINK-26495][table-planner] Prohibit hints(dynamic table options) on view

[GitBox <gi...@apache.org>]

LadyForest commented on a change in pull request #19007:
URL: https://github.com/apache/flink/pull/19007#discussion_r828971920



##########
File path: flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/plan/hint/OptionsHintTest.scala
##########
@@ -142,9 +143,12 @@ class OptionsHintTest(param: Param)
   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.

Review 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  encountered`TableKind.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 worked on table? 
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org