Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-4704] Move Table API to org.apache.flink.table #2958

Closed
wants to merge 3 commits into from

Conversation

ex00
Copy link

@ex00 ex00 commented Dec 7, 2016

Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration.
If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the How To Contribute guide.
In addition to going through the list, please provide a meaningful description of your changes.

  • General

    • The pull request references the related JIRA issue ("[FLINK-XXX] Jira title text")
    • The pull request addresses only one issue
    • Each commit in the PR has a meaningful commit message (including the JIRA id)
  • Documentation

    • Documentation has been added for new functionality
    • Old documentation affected by the pull request has been updated
    • JavaDoc for public methods has been added
  • Tests & Build

    • Functionality added by the pull request is covered by tests
    • mvn clean verify has been executed successfully locally or a Travis build has passed

The PR about moving table api and sql classes to org.apache.flink.table package

@fhueske
Copy link
Contributor

fhueske commented Dec 13, 2016

Thanks for the PR @ex00!

I would like to suggest to move a few more files:

  • move flink-table examples: org/apache/flink/examples -> org/apache/flink/table/examples (for Java and Scala)
  • move src/main/scala/org/apache/flink/table/windows.scala -> src/main/scala/org/apache/flink/table/api/windows.scala
  • move src/main/scala/org/apache/flink/table/Types.scala -> src/main/scala/org/apache/flink/table/typeutils/Types.scala
  • move src/main/scala/org/apache/flink/table/trees/TreeNode.scala -> src/main/scala/org/apache/flink/table/plan/TreeNode.scala and remove the empty trees package / folder
  • create a new package org.apache.flink.table.calcite and move CalciteConfig.scala, FlinkCalciteSqlValidator.scala, FlinkPlannerImpl.scala, FlinkRelBuilder.scala, FlinkTypeFactory.scala, and FlinkTypeSystem.scala from src/main/scala/org/apache/flink/table to src/main/scala/org/apache/flink/table/calcite

We also need to rebase the PR to the current master and move a few files that have been added in the mean time. There was also on change in the docs that needs to be adapted.

What do you think @ex00 and @twalthr?

@twalthr
Copy link
Contributor

twalthr commented Dec 14, 2016

@fhueske I would move src/main/scala/org/apache/flink/table/Types.scala to src/main/scala/org/apache/flink/table/api/. This class is API only and and not used internally.

And src/main/scala/org/apache/flink/table/table.scala -> src/main/scala/org/apache/flink/table/api/.

@fhueske
Copy link
Contributor

fhueske commented Dec 14, 2016

OK @twalthr. Sounds good to me.

@ex00
Copy link
Author

ex00 commented Dec 14, 2016

Hi @fhueske and @twalthr, thanks for your comments.
I will update PR

Anton Mushin added 2 commits December 14, 2016 13:11
move table api to org.apache.flink.table package
move table api to org.apache.flink.table package
@ex00
Copy link
Author

ex00 commented Dec 14, 2016

PR has been updated.

move table api to org.apache.flink.table package
@fhueske
Copy link
Contributor

fhueske commented Dec 15, 2016

Thanks for the update @ex00!
I'll do a few additional minor refactorings and merge the PR later.

Thanks, Fabian

@ex00
Copy link
Author

ex00 commented Dec 15, 2016

Thank you Fabian!

fhueske pushed a commit to fhueske/flink that referenced this pull request Dec 15, 2016
fhueske pushed a commit to fhueske/flink that referenced this pull request Dec 16, 2016
fhueske pushed a commit to fhueske/flink that referenced this pull request Dec 16, 2016
@fhueske
Copy link
Contributor

fhueske commented Dec 16, 2016

Merging this PR

@twalthr
Copy link
Contributor

twalthr commented Dec 16, 2016

I looked over the changes again. LGTM also from my side.

@asfgit asfgit closed this in ffe9ec8 Dec 16, 2016
@ex00 ex00 deleted the FLINK-4704 branch December 20, 2016 13:36
joseprupi pushed a commit to joseprupi/flink that referenced this pull request Feb 12, 2017
hequn8128 pushed a commit to hequn8128/flink that referenced this pull request Jun 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants