flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (FLINK-3754) Add a validation phase before construct RelNode using TableAPI
Date Thu, 28 Apr 2016 07:19:13 GMT

    [ https://issues.apache.org/jira/browse/FLINK-3754?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15261658#comment-15261658
] 

ASF GitHub Bot commented on FLINK-3754:
---------------------------------------

Github user yjshen commented on the pull request:

    https://github.com/apache/flink/pull/1916#issuecomment-215332571
  
    > I think it would be good to eagerly check each method call of the Table API. This
would make debugging easier, because exceptions would be thrown where the error is caused.
Please correct me if I am wrong, but I think we would not lose validation coverage compared
to the coverage this PR if we do eager validation? It might also be easier, because we do
not need the recursive operator traversal (still the expression traversal though). Maybe we
can even directly translate to RelNodes after validation, just like we do right now. I think
a lot of this PR could be used for eager validation, not sure if it would be easily possible
with the SqlNode validation approach. 
    
    Regarding the eager validation you mentioned, I think that could be accomplished by calling
`validate()` each time I am constructing another `Table`, in other words, each time I am calling
a `Table api`, changing the current code from:
    ``` scala
    class Table(
        private[flink] val tableEnv: TableEnvironment,
        private[flink] val planPreparation: PlanPreparation) {
    
      def this(tableEnv: TableEnvironment, logicalPlan: LogicalNode) = {
        this(tableEnv, new PlanPreparation(tableEnv, logicalPlan))
      }
    ```
    to 
    ``` scala
    def this(tableEnv: TableEnvironment, logicalPlan: LogicalNode) = {
        this(tableEnv, {
          val pp = new PlanPreparation(tableEnv, logicalPlan)
          pp.validate()
          pp
        })
    ```
    and also add an additional flag annotate the logical node as `validate` and therefore
avoid the `recursive logical plan traversal` would be enough.  Do I understand your idea correctly?
    On the other hand, I prefer to postponed `RelNode` construction until we are going to
run the query on Flink, since we only need `RelNode` at that time.
    
    > While reviewing the PR, I noticed that some classes seem to be partially derived
from Spark's code base (e.g., TreeNode and RuleExecutor). I know there are some common patterns
that apply in optimizers, but it is good style to give credit to the original source code.

    Can you list which classes are derived from Spark code and add a comment to them pointing
to the source of the code?
    
    OK, I will do a more detailed scan and give credit to all the original source code.


> Add a validation phase before construct RelNode using TableAPI
> --------------------------------------------------------------
>
>                 Key: FLINK-3754
>                 URL: https://issues.apache.org/jira/browse/FLINK-3754
>             Project: Flink
>          Issue Type: Improvement
>          Components: Table API
>    Affects Versions: 1.0.0
>            Reporter: Yijie Shen
>            Assignee: Yijie Shen
>
> Unlike sql string's execution, which have a separate validation phase before RelNode
construction, Table API lacks the counterparts and the validation is scattered in many places.
> I suggest to add a single validation phase and detect problems as early as possible.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message