flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From fhueske <...@git.apache.org>
Subject [GitHub] flink pull request: [FLINK-3754][Table]Add a validation phase befo...
Date Wed, 27 Apr 2016 16:41:56 GMT
Github user fhueske commented on the pull request:

    https://github.com/apache/flink/pull/1916#issuecomment-215142967
  
    Hi @yjshen, thanks for your patience. I also finished a first pass over the PR. 
    
    I'd like to propose a third alternative, in addition to the custom validation phase (this
PR) and generating `SqlNode`s and using Calcite's validator. Both approaches would mean that
the validation happens before the logical plan is translated into a `RelNode`. 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 `RelNode`s 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. 
    What do you think about eagerly validation, @yjshen and @twalthr?
    
    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?
    
    Thanks, Fabian


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message