[jira] [Commented] (FLINK-7636) Introduce Flink RelOptTable, and remove tableSource from all TableSourceScan node constructor

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

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

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

GitHub user beyond1920 opened a pull request:

    https://github.com/apache/flink/pull/4681

    [FLINK-7636][Table API & SQL]Introduce Flink RelOptTable, and remove tableSource from all TableSourceScan node constructor

    ## What is the purpose of the change
    
    There are two ways to fetch TableSource of TableSourceScan node (e.g FlinkLogicalTableSourceScan, PhysicalTableSourceScan and its subclass):
    1. 
    {code}
    val relOptTable: RelOptTable = getTable()
    val tableSourceTable = relOptTable.unwrap(classOf[TableSourceTable[_]])
    val tableSouce = tableSourceTable.tableSource
    {code}
    the result of getTable() is instance of RelOptTableImpl now, and it will not change after RelNode tree is built.
    2.  TableSourceScan node contains a tablesource as constructor parameter, so we could fetch the tablesource directly later.
     
    The returned tableSource is different with each other through above two ways after apply project push(PPD) down or filter push down(FPD).  It is very confusing and will cause problems.
    
    The pr aims to fix the problem by introducing FlinkRelOptTable to replace RelOptTableImpl, and remove tableSource parameter from TableSourceScan's constructor. After PPD or FPD,  a new FlinkRelOptTable instance which contains a new TableSourceTable will be passed to TableSourceScan constructor. 
    
    ## Brief change log
    
      - *Adds FlinkRelOptTable to replace default RelOptTable implementation (RelOptTableImpl)*
      - *Adds FlinkCalciteCatalogReader, which is subclass of  CalciteCatalogReader. It overrides getTable method to return FlinkRelOptTable instance instead of RelOptTableImpl instance*
      - *Removes tableSource parameter from TableSourceScan's constructor. A new FlinkRelOptTable instance which contains a new TableSourceTable will be passed to TableSourceScan constructor.*
    
    ## Verifying this change
    
    This change added tests and can be verified as follows:
    
      - *Added test that validates that FlinkRelOptTable instance is returned once call getTable method of FlinkCalciteCatalogReader*
      - *Other change is already covered by existing tests, such as (TableSourceTest, TableSourceITCase)*
    
    ## Does this pull request potentially affect one of the following parts:
    
      - Dependencies (does it add or upgrade a dependency): (no)
      - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
      - The serializers: (no)
      - The runtime per-record code paths (performance sensitive): (no)
      - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
    
    ## Documentation
    
      - Does this pull request introduce a new feature? (no)

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/beyond1920/flink FLINK-7636

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/4681.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #4681
    
----
commit 7904bd6468f7ae482c4f906e52b29f477fe28602
Author: jingzhang <be...@126.com>
Date:   2017-09-18T10:01:24Z

    [FLINK-7636][Table API & SQL]Introduce Flink RelOptTable, and remove tableSource from all TableSourceScan node constructor

----


> Introduce Flink RelOptTable,  and remove tableSource from all TableSourceScan node constructor
> ----------------------------------------------------------------------------------------------
>
>                 Key: FLINK-7636
>                 URL: https://issues.apache.org/jira/browse/FLINK-7636
>             Project: Flink
>          Issue Type: Improvement
>          Components: Table API & SQL
>            Reporter: jingzhang
>            Assignee: jingzhang
>
> At present, there are two ways to fetch TableSource of a TableSourceScan node (e.g LogicalTableSourceScan, PhysicalTableSourceScan ...):
> 1. 
> {code}
> val relOptTable: RelOptTable = getTable()
> val tableSourceTable = relOptTable.unwrap(classOf[TableSourceTable[_]])
> val tableSouce = tableSourceTable.tableSource
> {code}
> the result of getTable() is instance of RelOptTableImpl now, and it will not change after RelNode tree is built.
> 2. now all TableSourceScan contains a tablesource as constructor parameter, so we could fetch the tablesource directly later.
>  
> The result tableSource is different with each other by above two ways after apply project push(PPD) down or filter push down(FPD).  It is very confusing. 
> we hope to fix the problem by introducing FlinkRelOptTable to replace RelOptTableImpl, and remove tableSource parameter from TableSourceScan's constructor. After PPD or FPD,  a new FlinkRelOptTable instance which contains a new TableSourceTable will be passed to TableSourceScan constructor. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)