Github user fhueske commented on the issue:

    https://github.com/apache/flink/pull/2810
  
    Hi @tonycox, @beyond1920, and @KurtYoung,
    
    I think I found a good way to split the issue. Let me first summarize what 
both of your PRs implement:
    
    @tonycox (PR #2810)
    - good test coverage with tests that check the optimized plan 
(`TableTestBase`)
    - adapted `RowCsvInputFormat` + `CsvTableSource` to support projection
    - first changes for projection pushdown in streaming
    - good cost function
    
    @beyond1920 (PR #2923)
    - Extracts fields from `RexProgram` and converts `RexProgram` (seems to 
reused in PR #2810)
    - Rules work on `DataSetRel` instead of `LogicalRel` (#2810 rules seem to 
be inspired by this rule)
    - too many ITCases
    - no changes for streaming
    - no projection support for `CsvTableSource`.
    
    From my point of view PR #2923 provides a solid basis for this feature 
which seem to be partially reused in PR #2810 but lacks a few things that have 
been implemented in PR #2810. 
    Therefore, I would propose the following:
    
    PR #2923 is modified to cover the following:
    - `ProjectableTableSource`
    - `RexProgramProjectExtractor` + missing unit test
    - Rule to push projection into `BatchTableSourceScan`
    - Remove all ITCases except for one (the other PR will add more tests)
    - Adapt the cost model for the `BatchTableSourceScan` as in PR #2810.
    
    PR #2810 builds on PR#2923 and adds the following:
    - Translation for `StreamTableSourceScan` + one ITCase
    - `CsvTableSource` + `RowCsvInputFormat` changes (including tests and tests 
refactoring)
    - The extensive plan tests based on `TableTestBase` for batch and streaming
    
    If we agree on this plan, it would be good if PR #2923 would be adapted as 
soon as possible such that PR #2810 can be built on top.
    
    What do you think @tonycox, @beyond1920, @KurtYoung?
    
    Best, 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to