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/08/16 03:59:29 UTC
[GitHub] [beam] jhnmora000 opened a new pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
jhnmora000 opened a new pull request #12595:
URL: https://github.com/apache/beam/pull/12595
Add support for Analytic Functions in ZetaSQL:
- Implement AnalyticScan to BeamWindowRel.
- Add tests for ZetaSQL.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
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/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_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.
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
----------------------------------------------------------------
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] amaliujia commented on pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #12595:
URL: https://github.com/apache/beam/pull/12595#issuecomment-675015548
Thanks! I will try this PR on our internal test suite.
----------------------------------------------------------------
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] jhnmora000 commented on pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
jhnmora000 commented on pull request #12595:
URL: https://github.com/apache/beam/pull/12595#issuecomment-674475549
R: @amaliujia @Mark-Zeng
----------------------------------------------------------------
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] amaliujia commented on pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #12595:
URL: https://github.com/apache/beam/pull/12595#issuecomment-682332723
This PR overall looks good!
----------------------------------------------------------------
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] amaliujia commented on pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #12595:
URL: https://github.com/apache/beam/pull/12595#issuecomment-678895260
@jhnmora000 Is this PR ready for review?
----------------------------------------------------------------
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] chamini2 commented on pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
chamini2 commented on pull request #12595:
URL: https://github.com/apache/beam/pull/12595#issuecomment-954064499
Hi, did this get merged on a different PR? I cannot find it
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] amaliujia commented on a change in pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #12595:
URL: https://github.com/apache/beam/pull/12595#discussion_r478837407
##########
File path: sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/translation/ExpressionConverter.java
##########
@@ -978,7 +978,7 @@ private RexNode convertResolvedStructFieldAccessInternal(RexNode referencedExpr,
return rexBuilder().makeFieldAccess(referencedExpr, fieldIdx);
}
- private RexBuilder rexBuilder() {
+ public RexBuilder rexBuilder() {
Review comment:
I am thinking this is not needed. In every Rel converter they can get RexBuilder.
----------------------------------------------------------------
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] aaltay commented on pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #12595:
URL: https://github.com/apache/beam/pull/12595#issuecomment-694467327
@amaliujia - what is the next step on this PR?
----------------------------------------------------------------
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] stale[bot] closed pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #12595:
URL: https://github.com/apache/beam/pull/12595
----------------------------------------------------------------
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] jhnmora000 commented on a change in pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
jhnmora000 commented on a change in pull request #12595:
URL: https://github.com/apache/beam/pull/12595#discussion_r471893296
##########
File path: sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/SqlAnalyzer.java
##########
@@ -177,7 +177,8 @@ static AnalyzerOptions baseAnalyzerOptions() {
LanguageFeature.FEATURE_V_1_1_SELECT_STAR_EXCEPT_REPLACE,
LanguageFeature.FEATURE_TABLE_VALUED_FUNCTIONS,
LanguageFeature.FEATURE_CREATE_TABLE_FUNCTION,
- LanguageFeature.FEATURE_TEMPLATE_FUNCTIONS)));
+ LanguageFeature.FEATURE_TEMPLATE_FUNCTIONS,
+ LanguageFeature.FEATURE_ANALYTIC_FUNCTIONS)));
Review comment:
`Analytic functions not supported [at 1:35]
SELECT item, purchases, category, sum(purchases) over (PARTITION BY category ...
^
com.google.zetasql.SqlException: Analytic functions not supported [at 1:35]
SELECT item, purchases, category, sum(purchases) over (PARTITION BY category ...
^
at com.google.zetasql.Analyzer.analyzeNextStatement(Analyzer.java:206)
at org.apache.beam.sdk.extensions.sql.zetasql.SqlAnalyzer.analyzeNextStatement(SqlAnalyzer.java:126)
at org.apache.beam.sdk.extensions.sql.zetasql.ZetaSQLPlannerImpl.rel(ZetaSQLPlannerImpl.java:106)
at org.apache.beam.sdk.extensions.sql.zetasql.ZetaSQLQueryPlanner.convertToBeamRelInternal(ZetaSQLQueryPlanner.java:168)
at org.apache.beam.sdk.extensions.sql.zetasql.ZetaSQLQueryPlanner.convertToBeamRel(ZetaSQLQueryPlanner.java:156)
at org.apache.beam.sdk.extensions.sql.zetasql.ZetaSQLQueryPlanner.convertToBeamRel(ZetaSQLQueryPlanner.java:140)
at org.apache.beam.sdk.extensions.sql.zetasql.AnalyticFunctionsTest.sumTest(AnalyticFunctionsTest.java:59)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.apache.beam.sdk.testing.TestPipeline$1.evaluate(TestPipeline.java:319)
at org.junit.rules.ExpectedException$ExpectedExceptionStatement.evaluate(ExpectedException.java:266)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:305)
at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:365)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
at org.junit.runners.ParentRunner$4.run(ParentRunner.java:330)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:78)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:328)
at org.junit.runners.ParentRunner.access$100(ParentRunner.java:65)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:292)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:305)
at org.junit.runners.ParentRunner.run(ParentRunner.java:412)
at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:110)
at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:58)
at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:38)
at org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:62)
at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:51)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:32)
at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:93)
at com.sun.proxy.$Proxy2.processTestClass(Unknown Source)
at org.gradle.api.internal.tasks.testing.worker.TestWorker.processTestClass(TestWorker.java:118)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:175)
at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:157)
at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:404)
at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:63)
at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:46)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:55)
at java.lang.Thread.run(Thread.java:748)
Caused by: com.google.zetasql.io.grpc.StatusRuntimeException: INVALID_ARGUMENT: Analytic functions not supported [at 1:35]
SELECT item, purchases, category, sum(purchases) over (PARTITION BY category ...
^
at com.google.zetasql.io.grpc.stub.ClientCalls.toStatusRuntimeException(ClientCalls.java:233)
at com.google.zetasql.io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:214)
at com.google.zetasql.io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:139)
at com.google.zetasql.ZetaSqlLocalServiceGrpc$ZetaSqlLocalServiceBlockingStub.analyze(ZetaSqlLocalServiceGrpc.java:1063)
at com.google.zetasql.Analyzer.analyzeNextStatement(Analyzer.java:204)
... 59 more`
----------------------------------------------------------------
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] jhnmora000 commented on pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
jhnmora000 commented on pull request #12595:
URL: https://github.com/apache/beam/pull/12595#issuecomment-678396032
Hi Rui.
I am working on a commit for navigation/numbering functions support, I hope I can push it tonight or tomorrow. I think it will be ready for review after that.
Thanks,
----------------------------------------------------------------
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] stale[bot] commented on pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #12595:
URL: https://github.com/apache/beam/pull/12595#issuecomment-734494704
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.
----------------------------------------------------------------
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] amaliujia commented on a change in pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #12595:
URL: https://github.com/apache/beam/pull/12595#discussion_r471761646
##########
File path: sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/SqlAnalyzer.java
##########
@@ -177,7 +177,8 @@ static AnalyzerOptions baseAnalyzerOptions() {
LanguageFeature.FEATURE_V_1_1_SELECT_STAR_EXCEPT_REPLACE,
LanguageFeature.FEATURE_TABLE_VALUED_FUNCTIONS,
LanguageFeature.FEATURE_CREATE_TABLE_FUNCTION,
- LanguageFeature.FEATURE_TEMPLATE_FUNCTIONS)));
+ LanguageFeature.FEATURE_TEMPLATE_FUNCTIONS,
+ LanguageFeature.FEATURE_ANALYTIC_FUNCTIONS)));
Review comment:
Is this feature enabled is required?
I am seeing internal test suites throw a bunch of failures and they are all "Function no found". I am guessing it is because this feature is enabled.
----------------------------------------------------------------
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] amaliujia commented on pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #12595:
URL: https://github.com/apache/beam/pull/12595#issuecomment-677868705
Hi John,
Are there more updates you want to append to this PR or this PR is waiting for review?
----------------------------------------------------------------
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] jhnmora000 commented on pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
jhnmora000 commented on pull request #12595:
URL: https://github.com/apache/beam/pull/12595#issuecomment-679351771
Hi Rui.
Yes, please give it a look if you have time. I think it is ready for review.
----------------------------------------------------------------
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] amaliujia commented on a change in pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #12595:
URL: https://github.com/apache/beam/pull/12595#discussion_r471947014
##########
File path: sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/SqlAnalyzer.java
##########
@@ -177,7 +177,8 @@ static AnalyzerOptions baseAnalyzerOptions() {
LanguageFeature.FEATURE_V_1_1_SELECT_STAR_EXCEPT_REPLACE,
LanguageFeature.FEATURE_TABLE_VALUED_FUNCTIONS,
LanguageFeature.FEATURE_CREATE_TABLE_FUNCTION,
- LanguageFeature.FEATURE_TEMPLATE_FUNCTIONS)));
+ LanguageFeature.FEATURE_TEMPLATE_FUNCTIONS,
+ LanguageFeature.FEATURE_ANALYTIC_FUNCTIONS)));
Review comment:
Makes sense. You can continue this PR. I will take care of our internal stuff.
----------------------------------------------------------------
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] jhnmora000 commented on a change in pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
jhnmora000 commented on a change in pull request #12595:
URL: https://github.com/apache/beam/pull/12595#discussion_r471883690
##########
File path: sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/SqlAnalyzer.java
##########
@@ -177,7 +177,8 @@ static AnalyzerOptions baseAnalyzerOptions() {
LanguageFeature.FEATURE_V_1_1_SELECT_STAR_EXCEPT_REPLACE,
LanguageFeature.FEATURE_TABLE_VALUED_FUNCTIONS,
LanguageFeature.FEATURE_CREATE_TABLE_FUNCTION,
- LanguageFeature.FEATURE_TEMPLATE_FUNCTIONS)));
+ LanguageFeature.FEATURE_TEMPLATE_FUNCTIONS,
+ LanguageFeature.FEATURE_ANALYTIC_FUNCTIONS)));
Review comment:
If I recall correctly, I got ' agg OVER (..) ' not supported errors (or similar) when this feature was off.
----------------------------------------------------------------
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] amaliujia commented on a change in pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #12595:
URL: https://github.com/apache/beam/pull/12595#discussion_r478837113
##########
File path: sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/translation/AnalyticScanConverter.java
##########
@@ -0,0 +1,291 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.extensions.sql.zetasql.translation;
+
+import com.google.zetasql.resolvedast.ResolvedNode;
+import com.google.zetasql.resolvedast.ResolvedNodes;
+import com.google.zetasql.resolvedast.ResolvedNodes.ResolvedAnalyticFunctionCall;
+import com.google.zetasql.resolvedast.ResolvedNodes.ResolvedAnalyticScan;
+import com.google.zetasql.resolvedast.ResolvedNodes.ResolvedComputedColumn;
+import com.google.zetasql.resolvedast.ResolvedNodes.ResolvedLiteral;
+import com.google.zetasql.resolvedast.ResolvedNodes.ResolvedWindowFrameExpr;
+import com.google.zetasql.resolvedast.ResolvedOrderByItemEnums;
+import com.google.zetasql.resolvedast.ResolvedWindowFrameEnums;
+import com.google.zetasql.resolvedast.ResolvedWindowFrameExprEnums;
+import java.math.BigDecimal;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.beam.repackaged.core.org.apache.commons.compress.utils.Lists;
+import org.apache.beam.sdk.extensions.sql.impl.rel.BeamWindowRel;
+import org.apache.beam.sdk.extensions.sql.zetasql.ZetaSqlCalciteTranslationUtils;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.rel.RelCollation;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.rel.RelCollations;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.rel.RelFieldCollation;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.rel.RelNode;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.rel.core.Window;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.rel.core.Window.Group;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.rel.core.Window.RexWinAggCall;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.rel.type.RelDataType;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.rex.RexInputRef;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.rex.RexLiteral;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.rex.RexNode;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.rex.RexWindowBound;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.sql.SqlAggFunction;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.sql.SqlLiteral;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.sql.SqlNode;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.sql.SqlRankFunction;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.sql.SqlWindow;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.util.ImmutableBitSet;
+
+/** Converts AnalyticScan into BeamWindowRel. */
+public class AnalyticScanConverter extends RelConverter<ResolvedAnalyticScan> {
+
+ AnalyticScanConverter(ConversionContext context) {
+ super(context);
+ }
+
+ @Override
+ public List<ResolvedNode> getInputs(ResolvedAnalyticScan zetaNode) {
+ return Collections.singletonList(zetaNode.getInputScan());
+ }
+
+ @Override
+ public RelNode convert(ResolvedAnalyticScan zetaNode, List<RelNode> inputs) {
+
+ RelNode inputTable = inputs.get(0);
+
+ RelDataType expectedRowType =
+ getExpressionConverter().rexBuilder().getTypeFactory().copyType(inputTable.getRowType());
+
+ List<RexLiteral> constants = Lists.newArrayList();
+ List<Group> groups = Lists.newArrayList();
+
+ int ordinal = 0;
+ for (ResolvedNodes.ResolvedAnalyticFunctionGroup analyticGroup :
+ zetaNode.getFunctionGroupList()) {
+ ImmutableBitSet partitionKeys =
+ ImmutableBitSet.of(
+ analyticGroup.getPartitionBy() != null
+ ? analyticGroup.getPartitionBy().getPartitionByList().stream()
+ .map(
+ keyColumn -> {
+ return getExpressionConverter()
+ .indexOfProjectionColumnRef(
+ keyColumn.getColumn().getId(), zetaNode.getColumnList());
+ })
+ .collect(Collectors.toList())
+ : Lists.newArrayList());
+ RelCollation relCollation =
+ RelCollations.of(
+ analyticGroup.getOrderBy().getOrderByItemList().stream()
+ .map(
+ orderColumn -> {
+ int columnIndex =
+ getExpressionConverter()
+ .indexOfProjectionColumnRef(
+ orderColumn.getColumnRef().getColumn().getId(),
+ zetaNode.getColumnList());
+ RelFieldCollation.Direction columnDirection =
+ orderColumn.getIsDescending()
+ ? RelFieldCollation.Direction.DESCENDING
+ : RelFieldCollation.Direction.ASCENDING;
+ RelFieldCollation.NullDirection columnNull =
+ orderColumn.getNullOrder()
+ == ResolvedOrderByItemEnums.NullOrderMode.NULLS_FIRST
+ ? RelFieldCollation.NullDirection.FIRST
+ : orderColumn.getNullOrder()
+ == ResolvedOrderByItemEnums.NullOrderMode.NULLS_LAST
+ ? RelFieldCollation.NullDirection.LAST
+ : RelFieldCollation.NullDirection.UNSPECIFIED;
+ return new RelFieldCollation(columnIndex, columnDirection, columnNull);
+ })
+ .collect(Collectors.toList()));
+
+ for (ResolvedComputedColumn aggColumn : analyticGroup.getAnalyticFunctionList()) {
+ ResolvedAnalyticFunctionCall aggCall = (ResolvedAnalyticFunctionCall) aggColumn.getExpr();
+
+ if (aggCall.getDistinct()) {
+ throw new UnsupportedOperationException("Does not support DISTINTC");
+ }
+
+ SqlAggFunction sqlAggFunction =
+ (SqlAggFunction)
+ SqlOperatorMappingTable.ZETASQL_FUNCTION_TO_CALCITE_SQL_OPERATOR.get(
+ aggCall.getFunction().getName());
+ if (sqlAggFunction == null) {
+ throw new UnsupportedOperationException(
+ "Does not support ZetaSQL aggregate function: " + aggCall.getFunction().getName());
+ }
+
+ RelDataType columnReturnType =
+ ZetaSqlCalciteTranslationUtils.toCalciteType(
+ aggColumn.getColumn().getType(), true, getCluster().getRexBuilder());
+
+ List<RexNode> argsColumns =
+ aggCall.getArgumentList().stream()
+ .map(
+ argument -> {
+ return getExpressionConverter().convertRexNodeFromResolvedExpr(argument);
+ })
+ .collect(Collectors.toList());
+
+ RexWinAggCall rexWinAggCall =
+ new RexWinAggCall(
+ sqlAggFunction,
+ columnReturnType,
+ argsColumns,
+ ordinal++,
+ aggCall.getDistinct(),
+ false);
+
+ RexNode rexPreceding = null;
+ RexNode rexFollowing = null;
+
+ if (aggCall.getWindowFrame() != null
+ && (aggCall.getWindowFrame().getStartExpr().getBoundaryType()
+ == ResolvedWindowFrameExprEnums.BoundaryType.OFFSET_PRECEDING
+ || aggCall.getWindowFrame().getStartExpr().getBoundaryType()
+ == ResolvedWindowFrameExprEnums.BoundaryType.OFFSET_FOLLOWING)) {
+ constants.add(
+ RexLiteral.fromJdbcString(
+ SqlOperators.BIGINT,
+ SqlTypeName.DECIMAL,
+ obtainValue(aggCall.getWindowFrame().getStartExpr()).toString()));
+
+ rexPreceding =
+ getExpressionConverter()
+ .rexBuilder()
Review comment:
you can get RexBuilder by `getCluster().getRexBuilder()`?
----------------------------------------------------------------
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] amaliujia commented on pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #12595:
URL: https://github.com/apache/beam/pull/12595#issuecomment-694485388
@aaltay this change will break internal test suite. Need to take a closer to know why the suite is broken and how to fix.
----------------------------------------------------------------
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] stale[bot] commented on pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #12595:
URL: https://github.com/apache/beam/pull/12595#issuecomment-743934703
This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
----------------------------------------------------------------
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] aaltay commented on pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #12595:
URL: https://github.com/apache/beam/pull/12595#issuecomment-690808206
@amaliujia - Could this be merged?
----------------------------------------------------------------
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] ibzib commented on pull request #12595: [BEAM-9198] BeamSQL aggregation analytics functionality : Add support for Analytic Functions in ZetaSQL
Posted by GitBox <gi...@apache.org>.
ibzib commented on pull request #12595:
URL: https://github.com/apache/beam/pull/12595#issuecomment-954075480
> Hi, did this get merged on a different PR? I cannot find it
I don't think so, since all of the standard analytic functions are still disabled: https://github.com/apache/beam/blob/4cdbe8dc3a4be8fd3e6ee74cef43af3fb03761b1/sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/SupportedZetaSqlBuiltinFunctions.java#L479-L496
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org