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.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
