Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-5727

Unify some API of batch and stream TableEnvironment

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Done
    • Affects Version/s: None
    • Fix Version/s: 1.3.0
    • Component/s: Table API & SQL
    • Labels:
      None

      Description

      Some API's implementation of current BatchTableEnvironment and StreamTableEnvironment is identical, like sql. And other API like registerTableSource and scan can be unified to the base class. (we can change ingest from StreamTableEnvironment to scan also)

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user KurtYoung opened a pull request:

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

          FLINK-5727 [table] Unify some API of batch and stream TableEnvironment

          This PR contains 3 changes:

          1. move `sql` to base class
          2. unify `scan` and `ingest` to `scan`, and move to base class
          3. move `registerTableSource` to base class and change the parameter type from specific `BatchTableSource` or `StreamTableSource` to their base class: `TableSource`

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

          $ git pull https://github.com/KurtYoung/flink flink-5727

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

          https://github.com/apache/flink/pull/3281.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 #3281


          commit 8303b0c1c6320713daf212c6da4d72499c459e42
          Author: Kurt Young <ykt836@gmail.com>
          Date: 2017-02-07T06:15:02Z

          FLINK-5727 [table] Unify some API of batch and stream TableEnvironment


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user KurtYoung opened a pull request: https://github.com/apache/flink/pull/3281 FLINK-5727 [table] Unify some API of batch and stream TableEnvironment This PR contains 3 changes: 1. move `sql` to base class 2. unify `scan` and `ingest` to `scan`, and move to base class 3. move `registerTableSource` to base class and change the parameter type from specific `BatchTableSource` or `StreamTableSource` to their base class: `TableSource` You can merge this pull request into a Git repository by running: $ git pull https://github.com/KurtYoung/flink flink-5727 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3281.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 #3281 commit 8303b0c1c6320713daf212c6da4d72499c459e42 Author: Kurt Young <ykt836@gmail.com> Date: 2017-02-07T06:15:02Z FLINK-5727 [table] Unify some API of batch and stream TableEnvironment
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user fhueske commented on the issue:

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

          Hi @KurtYoung,

          thanks for the refactoring. Moving `sql()` and `scan()` to `TableEnvironment` makes sense.

          However, I'd like to keep `registerTableSource` at the `BatchTableEnvironment` and `StreamTableEnvironment` because this separation ensures that a `BatchTableSource` is not registered at a `StreamTableEnvironment` and vice versa.

          What do you think?

          Best, Fabian

          Show
          githubbot ASF GitHub Bot added a comment - Github user fhueske commented on the issue: https://github.com/apache/flink/pull/3281 Hi @KurtYoung, thanks for the refactoring. Moving `sql()` and `scan()` to `TableEnvironment` makes sense. However, I'd like to keep `registerTableSource` at the `BatchTableEnvironment` and `StreamTableEnvironment` because this separation ensures that a `BatchTableSource` is not registered at a `StreamTableEnvironment` and vice versa. What do you think? Best, Fabian
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user KurtYoung commented on the issue:

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

          Hi @fhueske , thanks for your reviewing and comments, it makes sense to me. The motivation behind this refactoring is we want to eliminate most differences between batch and stream execution environment, and use `TableEnvironment` only when they writing programs.

          How about we still keep `registerTableSource` in `TableEnvironment` and leave the implementation to the specific env, and do some type validation before registration.

          Show
          githubbot ASF GitHub Bot added a comment - Github user KurtYoung commented on the issue: https://github.com/apache/flink/pull/3281 Hi @fhueske , thanks for your reviewing and comments, it makes sense to me. The motivation behind this refactoring is we want to eliminate most differences between batch and stream execution environment, and use `TableEnvironment` only when they writing programs. How about we still keep `registerTableSource` in `TableEnvironment` and leave the implementation to the specific env, and do some type validation before registration.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user fhueske commented on the issue:

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

          Thanks for the update @KurtYoung.

          +1 to merge

          Show
          githubbot ASF GitHub Bot added a comment - Github user fhueske commented on the issue: https://github.com/apache/flink/pull/3281 Thanks for the update @KurtYoung. +1 to merge
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user fhueske commented on the issue:

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

          merging

          Show
          githubbot ASF GitHub Bot added a comment - Github user fhueske commented on the issue: https://github.com/apache/flink/pull/3281 merging
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3281
          Hide
          fhueske Fabian Hueske added a comment -

          Done for 1.3.0 with 2d0cb135972301926d62366dd3bb8b888d6d972d

          Show
          fhueske Fabian Hueske added a comment - Done for 1.3.0 with 2d0cb135972301926d62366dd3bb8b888d6d972d

            People

            • Assignee:
              ykt836 Kurt Young
              Reporter:
              ykt836 Kurt Young
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development