You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/07/13 23:39:49 UTC
[GitHub] [beam] robinyqiu opened a new pull request #12247: [BEAM-9179] Remove dead code: Java implementation of ZetaSQL functions
robinyqiu opened a new pull request #12247:
URL: https://github.com/apache/beam/pull/12247
After we switched to `BeamZetaSqlCalcRel`, the Java function implementation is no longer needed. But the code stayed there without being used, so I am removing them now, along with some other unused files.
Also refactor the code base:
- Move files only used in the ZetaSQL => Calcite translation to the `translation` sub-directory, including `SqlOperators`, `Sql*OperatorRewriter`, `SqlWindowTableFunction`
- Move the allowlist in `SqlStdOperatorMappingTable` to a new file `SupportedZetaSqlBuiltinFunctions` (used only by `SqlAnalyzer`); and move the rest of the file into `translation` sub-directory (used only in translation)
@apilloud
@ibzib (I moved your `SqlWindowTableFunction` file)
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
--- | --- | --- | --- | --- | --- | ---
Go | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] robinyqiu commented on pull request #12247: [BEAM-9179] Remove dead code: Java implementation of ZetaSQL functions
Posted by GitBox <gi...@apache.org>.
robinyqiu commented on pull request #12247:
URL: https://github.com/apache/beam/pull/12247#issuecomment-658934753
> I know of at least one customer use case where they are disabling BeamZetaSqlCalcRel so they can avoid JNI. We also have plans to turn BeamCalcRel back on for simple operators to enable better performance.
That make sense. Thanks for sharing this information. But I do believe it makes our code cleaner to move those files which are only used on the BeamZetaSqlCalcRel disabled path together. So how about I move those code to a new `/impl` subdirectory, instead of deleting them?
> I believe the preferred method is git pull --rebase or git rebase upstream/master.
Sure, I can do that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] robinyqiu merged pull request #12247: [BEAM-9179] Refactor: move Java function implementations to a sub-module
Posted by GitBox <gi...@apache.org>.
robinyqiu merged pull request #12247:
URL: https://github.com/apache/beam/pull/12247
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] robinyqiu commented on pull request #12247: [BEAM-9179] Refactor: move Java function implementations to a sub-module
Posted by GitBox <gi...@apache.org>.
robinyqiu commented on pull request #12247:
URL: https://github.com/apache/beam/pull/12247#issuecomment-659010570
Tested internally.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] robinyqiu commented on pull request #12247: [BEAM-9179] Remove dead code: Java implementation of ZetaSQL functions
Posted by GitBox <gi...@apache.org>.
robinyqiu commented on pull request #12247:
URL: https://github.com/apache/beam/pull/12247#issuecomment-657880976
I moved `SqlOperators` to the `translation` sub-directory and changed its content. For some reason GitHub does not recognize that. Instead it treats the change as a file deletion + file addition.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #12247: [BEAM-9179] Remove dead code: Java implementation of ZetaSQL functions
Posted by GitBox <gi...@apache.org>.
apilloud commented on pull request #12247:
URL: https://github.com/apache/beam/pull/12247#issuecomment-657905365
I would be fine with deleting anything that is obviously broken and cleanup, but this isn't dead code. I know of at least one customer use case where they are disabling `BeamZetaSqlCalcRel` so they can avoid JNI. We also have plans to turn `BeamCalcRel` back on for simple operators to enable better performance.
Also looks like you are doing `git pull` with the default config, which is creating merge commits on your branch. I believe the preferred method is `git pull --rebase` or `git rebase upstream/master`. (There is a way to make that the default, I don't remember of the top of my head.)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org