sunjincheng121 commented on a change in pull request #7664: [FLINK-11449][table] Uncouple the Expression class from RexNodes. URL: https://github.com/apache/flink/pull/7664#discussion_r255337933
########## File path: flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/api/BatchTableEnvironment.scala ########## @@ -414,15 +414,15 @@ abstract class BatchTableEnvironment( * @tparam T The type of the [[DataSet]]. */ protected def registerDataSetInternal[T]( - name: String, dataSet: DataSet[T], fields: Array[Expression]): Unit = { + name: String, dataSet: DataSet[T], fields: Array[PlannerExpression]): Unit = { Review comment: The `XXXInternal` methods are internal methods that the user does not perceive, so IMO there is no need to become an Expression? ( just reduce the change code for current PR?) So far I have not found the benefits of PlannerExpression to inherit Expression(Except reduce the some change code), because we already have the visitor mode to establish the conversion bridge from Expression to PlannerExpression. Expression is used at the user API level, and Expression is converted to PlannerExpression at the implementation level. We can indeed inherit PlannerExpression as Expression. Can you elaborate on the benefits of doing this? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services