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

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

Github user wuchong commented on a diff in the pull request:

    https://github.com/apache/flink/pull/2921#discussion_r90576889
  
    --- Diff: 
flink-libraries/flink-table/src/main/scala/org/apache/flink/api/table/plan/nodes/dataset/BatchTableSourceScan.scala
 ---
    @@ -32,18 +31,20 @@ class BatchTableSourceScan(
         cluster: RelOptCluster,
         traitSet: RelTraitSet,
         table: RelOptTable,
    -    rowType: RelDataType)
    -  extends BatchScan(cluster, traitSet, table, rowType) {
    +    val tableSource: BatchTableSource[_])
    --- End diff --
    
    Do we have to declare this field as `val` ? It seems that it is only used 
in this class. 


> Decouple BatchTableSourceScan with TableSourceTable
> ---------------------------------------------------
>
>                 Key: FLINK-5185
>                 URL: https://issues.apache.org/jira/browse/FLINK-5185
>             Project: Flink
>          Issue Type: Improvement
>          Components: Table API & SQL
>    Affects Versions: 1.2.0
>            Reporter: Kurt Young
>            Assignee: zhangjing
>            Priority: Minor
>
> As the components' relationship show in this design doc:
> https://docs.google.com/document/d/1PBnEbOcFHlEF1qGGAUgJvINdEXzzFTIRElgvs4-Tdeo/
> We found it's been annoying for {{BatchTableSourceScan}} directly holding 
> {{TableSourceTable}}, and refer to {{TableSource}} further. It's ok if the 
> relationship is immutable, but when we want to change the {{TableSource}} 
> when applying optimizations, it will cause some conflicts and 
> misunderstanding. 
> Since there is only one way to change {{TableSource}}, which is creating a 
> new {{TableSourceTable}} to hold the new {{TableSource}}, and create a new 
> {{BatchTableSourceScan}} pointing to the {{TableSourceTable}} which just 
> created. The annoying part is the {{RelOptTable}} comes from the super class 
> {{TableScan}} still holds the connection to the original {{TableSourceTable}} 
> and {{TableSource}}. It will cause some misunderstanding, which one should 
> the {{Scan}} rely to, and what's difference between these tables. 
> Besides, {{TableSourceTable}} is not very useful in {{BatchTableSourceScan}}, 
> the only thing {{Scan}} cares is the {{RowType}} it returns, since this is 
> and should be decided by {{TableSource}}. So we can let 
> {{BatchTableSourceScan}} directly holding {{TableSource}} instead of holding 
> {{TableSourceTable}}.If some original information are needed, find table 
> through {{RelOptTable}}. 



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

Reply via email to