[GitHub] [beam] kennknowles commented on pull request #12254: Add note on zetaSQL dependencies
kennknowles commented on pull request #12254: URL: https://github.com/apache/beam/pull/12254#issuecomment-659152184 JFYI "Beam-" is mean to refer to a specific Jira id, in case you are fixing a bug. In this case there is no bug (and none needed) so we can leave it 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: us...@infra.apache.org
[GitHub] [beam] kennknowles commented on a change in pull request #12271: [BEAM-10496] Improve nullability analysis for :sdks:java:core
kennknowles commented on a change in pull request #12271:
URL: https://github.com/apache/beam/pull/12271#discussion_r455494897
##
File path:
sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java
##
@@ -609,24 +604,34 @@ public static void printHelp(PrintStream out, Class i
getRequiredGroupNamesToProperties(propertyNamesToGetters);
out.format("%s:%n", currentIface.getName());
- prettyPrintDescription(out,
currentIface.getAnnotation(Description.class));
+ Description ifaceDescription =
currentIface.getAnnotation(Description.class);
+ if (ifaceDescription != null && ifaceDescription.value() != null) {
+prettyPrintDescription(out, ifaceDescription);
+ }
out.println();
List lists = Lists.newArrayList(propertyNamesToGetters.keySet());
lists.sort(String.CASE_INSENSITIVE_ORDER);
for (String propertyName : lists) {
Method method = propertyNamesToGetters.get(propertyName);
+assert method != null
+: "@AssumeAssertion(nullness): method name previously extracted
from iface";
Review comment:
I asked the checkerframework folks for a better solution and they gave
it: `@KeyFor` annotation.
##
File path:
sdks/java/core/src/main/java/org/apache/beam/sdk/util/common/ReflectHelpers.java
##
@@ -74,16 +74,12 @@ public String apply(@Nonnull Method input) {
@Override
public String apply(@Nonnull Method input) {
return String.format(
- "%s#%s", CLASS_NAME.apply(input.getDeclaringClass()),
METHOD_FORMATTER.apply(input));
+ "%s#%s",
+ ((Function, String>)
Class::getName).apply(input.getDeclaringClass()),
Review comment:
Done.
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:
us...@infra.apache.org
[GitHub] [beam] robinyqiu commented on a change in pull request #12266: [BEAM-10494] PubsubSchemaCapableIOProvider config inner class rather than row
robinyqiu commented on a change in pull request #12266:
URL: https://github.com/apache/beam/pull/12266#discussion_r455494296
##
File path:
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaCapableIOProvider.java
##
@@ -155,16 +159,17 @@ private void validateConfigurationSchema(Row
configuration) {
/** An abstraction to create schema aware IOs. */
@Internal
private static class PubsubSchemaIO implements SchemaIO, Serializable {
Review comment:
The `@Internal` flag is a warning to users to not depend on the labeled
class. This class is not even visible to user because it is private, so the
flag is not needed here. Same below.
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:
us...@infra.apache.org
[GitHub] [beam] robinyqiu commented on a change in pull request #12266: [BEAM-10494] PubsubSchemaCapableIOProvider config inner class rather than row
robinyqiu commented on a change in pull request #12266:
URL: https://github.com/apache/beam/pull/12266#discussion_r455492426
##
File path:
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaCapableIOProvider.java
##
@@ -264,4 +265,22 @@ private static boolean fieldPresent(
schema.getField(field).getType(),
Schema.EquivalenceNullablePolicy.IGNORE);
}
}
+
+ @Internal
+ @AutoValue
+ public abstract static class Config implements Serializable {
Review comment:
Can we make this class private or package-private?
##
File path:
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaCapableIOProvider.java
##
@@ -155,16 +159,17 @@ private void validateConfigurationSchema(Row
configuration) {
/** An abstraction to create schema aware IOs. */
@Internal
private static class PubsubSchemaIO implements SchemaIO, Serializable {
Review comment:
The `@Internal` flag is a warning to users to not depend on the labeled
class. This class is not even visible to user because it is private, so the
flag is not needed here.
##
File path:
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaCapableIOProvider.java
##
@@ -264,4 +265,22 @@ private static boolean fieldPresent(
schema.getField(field).getType(),
Schema.EquivalenceNullablePolicy.IGNORE);
}
}
+
+ @Internal
+ @AutoValue
+ public abstract static class Config implements Serializable {
+@Nullable
+public abstract String getTimestampAttributeKey();
+
+@Nullable
+public abstract String getDeadLetterQueue();
+
+private boolean useDlqCheck() {
Review comment:
Rename to `useDeadLetterQueue` for consistency?
##
File path:
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaCapableIOProvider.java
##
@@ -211,7 +216,11 @@ public POutput expand(PCollection input) {
return input
.apply(
RowToPubsubMessage.fromConfig(
- config, useFlatSchema, useTimestampAttribute(config)))
+ new AutoValueSchema()
Review comment:
Actually, I looked at the `RowToPubsubMessage` class and found 2 of the
3 parameters here (`config` and `useFlatSchema`) are not used at all. What was
the reason to keep them there? Can we remove them?
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:
us...@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #12275: [BEAM-10420] Add support for per window invocation of beam:transform:sdf_process_sized_element_and_restrictions:v1
lukecwik commented on pull request #12275: URL: https://github.com/apache/beam/pull/12275#issuecomment-659135889 R: @youngoli @boyuanzz CC: @robertwb 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: us...@infra.apache.org
[GitHub] [beam] robinyqiu commented on a change in pull request #12271: [BEAM-10496] Improve nullability analysis for :sdks:java:core
robinyqiu commented on a change in pull request #12271:
URL: https://github.com/apache/beam/pull/12271#discussion_r455483223
##
File path:
sdks/java/core/src/main/java/org/apache/beam/sdk/util/common/ReflectHelpers.java
##
@@ -74,16 +74,12 @@ public String apply(@Nonnull Method input) {
@Override
public String apply(@Nonnull Method input) {
return String.format(
- "%s#%s", CLASS_NAME.apply(input.getDeclaringClass()),
METHOD_FORMATTER.apply(input));
+ "%s#%s",
+ ((Function, String>)
Class::getName).apply(input.getDeclaringClass()),
Review comment:
Can we just call `input.getDeclaringClass().getName()` here?
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:
us...@infra.apache.org
[GitHub] [beam] youngoli commented on pull request #12215: [BEAM-10310] Adding thread-safe restriction tracker wrapper.
youngoli commented on pull request #12215: URL: https://github.com/apache/beam/pull/12215#issuecomment-659134956 Run Go PostCommit 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: us...@infra.apache.org
[GitHub] [beam] lukecwik opened a new pull request #12275: [BEAM-10420] Add support for per window invocation of beam:transform:sdf_process_sized_element_and_restrictions:v1
lukecwik opened a new pull request #12275: URL: https://github.com/apache/beam/pull/12275 This removes the unused window from pair with restriction allowing for window observing splittable dofns to be exercised. 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/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[](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/)[![Build
[GitHub] [beam] kennknowles merged pull request #12272: [BEAM-10502] Enable checker in zetasketch extension
kennknowles merged pull request #12272: URL: https://github.com/apache/beam/pull/12272 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: us...@infra.apache.org
[GitHub] [beam] robinyqiu commented on pull request #12274: [WIP] testing
robinyqiu commented on pull request #12274: URL: https://github.com/apache/beam/pull/12274#issuecomment-659117349 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: us...@infra.apache.org
[GitHub] [beam] ibzib merged pull request #12243: [BEAM-10460] Increase SparkPortableExecutionTest.testExecution timeout to 10 minutes
ibzib merged pull request #12243: URL: https://github.com/apache/beam/pull/12243 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: us...@infra.apache.org
[GitHub] [beam] ibzib commented on pull request #12243: [BEAM-10460] Increase SparkPortableExecutionTest.testExecution timeout to 10 minutes
ibzib commented on pull request #12243: URL: https://github.com/apache/beam/pull/12243#issuecomment-659098051 Java precommit flakes (of course). BEAM-10472 and BEAM-10508 (new one) 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: us...@infra.apache.org
[GitHub] [beam] ZijieSong946 commented on pull request #12174: [BEAM-10239] Support ZetaSQL NUMERIC type in BeamSQL
ZijieSong946 commented on pull request #12174: URL: https://github.com/apache/beam/pull/12174#issuecomment-659090461 > Internally we got 398 tests passing. > > Note that this PR turned on `LanguageFeature.FEATURE_NUMERIC_TYPE` so there are new tests enabled but are not currently supported. > > I will merge this PR for now. @ZijieSong946 Please link to your mini design doc from comment once that is finished. Acknowledged. 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: us...@infra.apache.org
[GitHub] [beam] ZijieSong946 edited a comment on pull request #12174: [BEAM-10239] Support ZetaSQL NUMERIC type in BeamSQL
ZijieSong946 edited a comment on pull request #12174: URL: https://github.com/apache/beam/pull/12174#issuecomment-659090461 > Internally we got 398 tests passing. > > Note that this PR turned on `LanguageFeature.FEATURE_NUMERIC_TYPE` so there are new tests enabled but are not currently supported. > > I will merge this PR for now. @ZijieSong946 Please link to your mini design doc from comment once that is finished. Acknowledged. Thanks for testing. 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: us...@infra.apache.org
[GitHub] [beam] chamikaramj commented on a change in pull request #12164: [BEAM-10397] add missing environment in windowing strategy for Dataflow
chamikaramj commented on a change in pull request #12164: URL: https://github.com/apache/beam/pull/12164#discussion_r455446367 ## File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py ## @@ -864,7 +867,7 @@ def run_GroupByKey(self, transform_node, options): windowing = transform_node.transform.get_windowing(transform_node.inputs) step.add_property( PropertyNames.SERIALIZED_FN, -self.serialize_windowing_strategy(windowing)) +self.serialize_windowing_strategy(windowing, self._default_environment)) Review comment: We simply use the environment passed here as the default environment for PipelineContext that is used when building the windowing proto: https://github.com/apache/beam/blob/master/sdks/python/apache_beam/runners/dataflow/dataflow_runner.py#L1411 I think x-lang custom windowing fn does not work today. Heejong filed https://issues.apache.org/jira/browse/BEAM-10507. I'll look into this but I think this is less severe and does not affect Kafka. 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: us...@infra.apache.org
[GitHub] [beam] ihji commented on a change in pull request #12164: [BEAM-10397] add missing environment in windowing strategy for Dataflow
ihji commented on a change in pull request #12164: URL: https://github.com/apache/beam/pull/12164#discussion_r455446568 ## File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py ## @@ -864,7 +867,7 @@ def run_GroupByKey(self, transform_node, options): windowing = transform_node.transform.get_windowing(transform_node.inputs) step.add_property( PropertyNames.SERIALIZED_FN, -self.serialize_windowing_strategy(windowing)) +self.serialize_windowing_strategy(windowing, self._default_environment)) Review comment: @robertwb The only problematic scenario is when `WindowingStrategy` comes from different SDK and the proto doesn't have a value in `environment_id` field. In that case, the host SDK will populate the field with its default environment. But I think it shouldn't happen since it's expansion service's responsibility to pass a semantically correct proto. 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: us...@infra.apache.org
[GitHub] [beam] robertwb commented on a change in pull request #12164: [BEAM-10397] add missing environment in windowing strategy for Dataflow
robertwb commented on a change in pull request #12164: URL: https://github.com/apache/beam/pull/12164#discussion_r455444790 ## File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py ## @@ -864,7 +867,7 @@ def run_GroupByKey(self, transform_node, options): windowing = transform_node.transform.get_windowing(transform_node.inputs) step.add_property( PropertyNames.SERIALIZED_FN, -self.serialize_windowing_strategy(windowing)) +self.serialize_windowing_strategy(windowing, self._default_environment)) Review comment: But the default might not be the right thing here if this is from another language, right? I'd like to better understand where this is being used and what can go wrong if it's missing/incorrect. 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: us...@infra.apache.org
[GitHub] [beam] chamikaramj merged pull request #12164: [BEAM-10397] add missing environment in windowing strategy for Dataflow
chamikaramj merged pull request #12164: URL: https://github.com/apache/beam/pull/12164 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: us...@infra.apache.org
[GitHub] [beam] chamikaramj commented on a change in pull request #12164: [BEAM-10397] add missing environment in windowing strategy for Dataflow
chamikaramj commented on a change in pull request #12164: URL: https://github.com/apache/beam/pull/12164#discussion_r455441379 ## File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py ## @@ -864,7 +867,7 @@ def run_GroupByKey(self, transform_node, options): windowing = transform_node.transform.get_windowing(transform_node.inputs) step.add_property( PropertyNames.SERIALIZED_FN, -self.serialize_windowing_strategy(windowing)) +self.serialize_windowing_strategy(windowing, self._default_environment)) Review comment: Heejong clarified that what we pass here is just the default. 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: us...@infra.apache.org
[GitHub] [beam] ihji commented on pull request #12164: [BEAM-10397] add missing environment in windowing strategy for Dataflow
ihji commented on pull request #12164: URL: https://github.com/apache/beam/pull/12164#issuecomment-659084291 @chamikaramj @robertwb Any other feedback before we merge this? 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: us...@infra.apache.org
[GitHub] [beam] robinyqiu opened a new pull request #12274: [WIP] testing
robinyqiu opened a new pull request #12274: URL: https://github.com/apache/beam/pull/12274 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/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)[![Build
[GitHub] [beam] robertwb commented on a change in pull request #12164: [BEAM-10397] add missing environment in windowing strategy for Dataflow
robertwb commented on a change in pull request #12164: URL: https://github.com/apache/beam/pull/12164#discussion_r455430204 ## File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py ## @@ -864,7 +867,7 @@ def run_GroupByKey(self, transform_node, options): windowing = transform_node.transform.get_windowing(transform_node.inputs) step.add_property( PropertyNames.SERIALIZED_FN, -self.serialize_windowing_strategy(windowing)) +self.serialize_windowing_strategy(windowing, self._default_environment)) Review comment: No, we can't assume this. I don't know where this is being consumed. 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: us...@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12273: Replace com.sun.istack.Nullable with checkframework Nullable
kennknowles commented on pull request #12273: URL: https://github.com/apache/beam/pull/12273#issuecomment-659071343 The level of flakiness right now is quite bad. This PR clearly did not cause those known flakes, and it unblocks other value adding work. I don't think kicking the build a bunch of times add value. Going to merge. 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: us...@infra.apache.org
[GitHub] [beam] kennknowles merged pull request #12273: Replace com.sun.istack.Nullable with checkframework Nullable
kennknowles merged pull request #12273: URL: https://github.com/apache/beam/pull/12273 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: us...@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12273: Replace com.sun.istack.Nullable with checkframework Nullable
kennknowles commented on pull request #12273: URL: https://github.com/apache/beam/pull/12273#issuecomment-659070317 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: us...@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12243: [BEAM-10460] Increase SparkPortableExecutionTest.testExecution timeout to 10 minutes
kennknowles commented on pull request #12243: URL: https://github.com/apache/beam/pull/12243#issuecomment-659067344 Is it still deadlocking? I've actually just been doing other things and didn't even read the logs the last few times. 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: us...@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12243: [BEAM-10460] Increase SparkPortableExecutionTest.testExecution timeout to 10 minutes
kennknowles commented on pull request #12243: URL: https://github.com/apache/beam/pull/12243#issuecomment-659067153 Re-opening because why not also just bump this up and eliminate it from consideration. 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: us...@infra.apache.org
[GitHub] [beam] aaltay merged pull request #12252: [BEAM-7390] Add combineglobally code snippets
aaltay merged pull request #12252: URL: https://github.com/apache/beam/pull/12252 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: us...@infra.apache.org
[GitHub] [beam] aaltay merged pull request #12269: Updated output tags
aaltay merged pull request #12269: URL: https://github.com/apache/beam/pull/12269 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: us...@infra.apache.org
[GitHub] [beam] apilloud merged pull request #12240: [BEAM-10462] transforms handle Double.NaN
apilloud merged pull request #12240: URL: https://github.com/apache/beam/pull/12240 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: us...@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12272: [BEAM-10502] Enable checker in zetasketch extension
kennknowles commented on pull request #12272: URL: https://github.com/apache/beam/pull/12272#issuecomment-659050891 Flakes in this run: - https://issues.apache.org/jira/browse/BEAM-10460 `SparkPortableExecutionTest` - https://issues.apache.org/jira/browse/BEAM-10471 `CassandraIOTest > testEstimatedSizeBytes` - https://issues.apache.org/jira/browse/BEAM-10504 `ElasticSearchIOTest > testWriteFullAddressing` and `testWriteWithIndexFn` There were also failures in `spotbugsMain` and `compileTestJava` which I will reproduce locally. 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: us...@infra.apache.org
[GitHub] [beam] robinyqiu merged pull request #12174: [BEAM-10239] Support ZetaSQL NUMERIC type in BeamSQL
robinyqiu merged pull request #12174: URL: https://github.com/apache/beam/pull/12174 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: us...@infra.apache.org
[GitHub] [beam] robinyqiu commented on pull request #12174: [BEAM-10239] Support ZetaSQL NUMERIC type in BeamSQL
robinyqiu commented on pull request #12174: URL: https://github.com/apache/beam/pull/12174#issuecomment-659049262 Internally we got 398 tests passing. Note that this PR turned on `LanguageFeature.FEATURE_NUMERIC_TYPE` so there are new tests enabled but are not currently supported. I will merge this PR for now. @ZijieSong946 Please link to your mini design doc from comment once that is finished. 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: us...@infra.apache.org
[GitHub] [beam] robinyqiu commented on pull request #12261: [BEAM-10490] Support read/write ZetaSQL DATE/TIME types from/to BigQuery
robinyqiu commented on pull request #12261: URL: https://github.com/apache/beam/pull/12261#issuecomment-659048335 Run Java PreCommit 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: us...@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12271: [BEAM-10496] Improve nullability analysis for :sdks:java:core
kennknowles commented on pull request #12271: URL: https://github.com/apache/beam/pull/12271#issuecomment-659040809 Based on the quality of null handling in zetasketch, pinging you on other similar PRs @robinyqiu 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: us...@infra.apache.org
[GitHub] [beam] kennknowles merged pull request #12126: Update docs for Reshuffle to remove the reference for deduplication
kennknowles merged pull request #12126: URL: https://github.com/apache/beam/pull/12126 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: us...@infra.apache.org
[GitHub] [beam] amaliujia commented on a change in pull request #12232: [Beam-9543] Support Match Recognition in Beam SQL
amaliujia commented on a change in pull request #12232:
URL: https://github.com/apache/beam/pull/12232#discussion_r455392148
##
File path:
sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamMatchRelTest.java
##
@@ -0,0 +1,75 @@
+/*
+ * 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.impl.rel;
+
+import static
org.apache.beam.sdk.extensions.sql.impl.rel.BaseRelTest.compilePipeline;
+import static
org.apache.beam.sdk.extensions.sql.impl.rel.BaseRelTest.registerTable;
+
+import org.apache.beam.sdk.extensions.sql.TestUtils;
+import org.apache.beam.sdk.extensions.sql.meta.provider.test.TestBoundedTable;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.testing.PAssert;
+import org.apache.beam.sdk.testing.TestPipeline;
+import org.apache.beam.sdk.values.PCollection;
+import org.apache.beam.sdk.values.Row;
+import org.junit.Rule;
+import org.junit.Test;
+
+public class BeamMatchRelTest {
+
+ @Rule public final TestPipeline pipeline = TestPipeline.create();
+
+ @Test
+ public void matchLogicalPlanTest() {
+Schema schemaType =
+Schema.builder()
+.addInt32Field("id")
+.addStringField("name")
+.addInt32Field("proctime")
+.build();
+
+registerTable(
+"TestTable", TestBoundedTable.of(schemaType).addRows(1, "a", 1, 1,
"b", 2, 1, "c", 3));
+
+String sql =
+"SELECT * "
++ "FROM TestTable "
++ "MATCH_RECOGNIZE ("
++ "PARTITION BY id "
++ "ORDER BY proctime "
++ "PATTERN (A B C) "
Review comment:
I see. Thanks for clarification.
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:
us...@infra.apache.org
[GitHub] [beam] amaliujia commented on pull request #12232: [Beam-9543] Support Match Recognition in Beam SQL
amaliujia commented on pull request #12232: URL: https://github.com/apache/beam/pull/12232#issuecomment-659039166 @Mark-Zeng thanks for your update on this PR. Let me know when you think this PR is ready for review again. 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: us...@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #12240: [BEAM-10462] transforms handle Double.NaN
apilloud commented on pull request #12240: URL: https://github.com/apache/beam/pull/12240#issuecomment-659037813 Failure is BEAM-9187 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: us...@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #12240: [BEAM-10462] transforms handle Double.NaN
apilloud commented on pull request #12240: URL: https://github.com/apache/beam/pull/12240#issuecomment-659037902 Run Java PreCommit 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: us...@infra.apache.org
[GitHub] [beam] tysonjh commented on a change in pull request #8558: [BEAM-7131] Spark: cache executable stage output to prevent re-computation
tysonjh commented on a change in pull request #8558: URL: https://github.com/apache/beam/pull/8558#discussion_r455362373 ## File path: runners/spark/src/main/java/org/apache/beam/runners/spark/translation/SparkTranslationContext.java ## @@ -17,6 +17,7 @@ */ package org.apache.beam.runners.spark.translation; +import com.sun.istack.Nullable; Review comment: Correct, this is not the right nullable. I have local changes to replace the other occurrences of the istack one but Kenn beat to 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12273: Replace com.sun.istack.Nullable with checkframework Nullable
kennknowles commented on pull request #12273: URL: https://github.com/apache/beam/pull/12273#issuecomment-659028721 Added illegal import to checkstyle configuration. Confirmed that with the commit reverted it fails: https://gradle.com/s/5dv5sua4hj2n6 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: us...@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12273: Replace com.sun.istack.Nullable with checkframework Nullable
kennknowles commented on pull request #12273: URL: https://github.com/apache/beam/pull/12273#issuecomment-659026632 I imagine if javax Nullable did the trick, then the checker folks would not have created their own. 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: us...@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12273: Replace com.sun.istack.Nullable with checkframework Nullable
kennknowles commented on pull request #12273: URL: https://github.com/apache/beam/pull/12273#issuecomment-659026520 @tysonjh yea you cannot use javax Nullable at "type use" so you cannot have e.g. `List<@Nullable Foo>`. And there are a few other cases. 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: us...@infra.apache.org
[GitHub] [beam] tysonjh commented on a change in pull request #8558: [BEAM-7131] Spark: cache executable stage output to prevent re-computation
tysonjh commented on a change in pull request #8558: URL: https://github.com/apache/beam/pull/8558#discussion_r455362373 ## File path: runners/spark/src/main/java/org/apache/beam/runners/spark/translation/SparkTranslationContext.java ## @@ -17,6 +17,7 @@ */ package org.apache.beam.runners.spark.translation; +import com.sun.istack.Nullable; Review comment: Correct, use javax nullable instead please. I have local changes to replace the other occurrences of the istack one. 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: us...@infra.apache.org
[GitHub] [beam] robinyqiu commented on a change in pull request #12267: [WIP][BEAM-10408] Schema capable io wrapper pubsub
robinyqiu commented on a change in pull request #12267:
URL: https://github.com/apache/beam/pull/12267#discussion_r455362415
##
File path:
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/pubsub/PubsubJsonTableProvider.java
##
@@ -17,63 +17,25 @@
*/
package org.apache.beam.sdk.extensions.sql.meta.provider.pubsub;
-import static org.apache.beam.sdk.util.RowJsonUtils.newObjectMapperWith;
-
-import com.alibaba.fastjson.JSONObject;
-import com.fasterxml.jackson.core.JsonProcessingException;
import com.google.auto.service.AutoService;
import org.apache.beam.sdk.annotations.Experimental;
import org.apache.beam.sdk.annotations.Internal;
-import org.apache.beam.sdk.extensions.sql.meta.BeamSqlTable;
-import org.apache.beam.sdk.extensions.sql.meta.Table;
-import
org.apache.beam.sdk.extensions.sql.meta.provider.InMemoryMetaTableProvider;
-import org.apache.beam.sdk.extensions.sql.meta.provider.InvalidTableException;
+import
org.apache.beam.sdk.extensions.sql.meta.provider.SchemaCapableIOTableProviderWrapper;
import org.apache.beam.sdk.extensions.sql.meta.provider.TableProvider;
import org.apache.beam.sdk.io.gcp.pubsub.PubsubIO;
import org.apache.beam.sdk.io.gcp.pubsub.PubsubSchemaCapableIOProvider;
-import org.apache.beam.sdk.schemas.io.InvalidConfigurationException;
-import org.apache.beam.sdk.schemas.io.InvalidSchemaException;
-import org.apache.beam.sdk.schemas.io.SchemaIO;
-import org.apache.beam.sdk.util.RowJson.RowJsonDeserializer;
-import org.apache.beam.sdk.values.Row;
/**
- * {@link TableProvider} for {@link PubsubIOJsonTable} which wraps {@link
PubsubIO} for consumption
- * by Beam SQL.
+ * {@link TableProvider} for {@link PubsubIO} for consumption by Beam SQL.
+ *
+ * Passes the {@link PubsubSchemaCapableIOProvider} to the generalized
table provider wrapper,
+ * {@link SchemaCapableIOTableProviderWrapper}, for Pubsub specific behavior.
*/
@Internal
@Experimental
@AutoService(TableProvider.class)
-public class PubsubJsonTableProvider extends InMemoryMetaTableProvider {
-
- @Override
- public String getTableType() {
-return "pubsub";
- }
-
- @Override
- public BeamSqlTable buildBeamSqlTable(Table tableDefinition) {
-JSONObject tableProperties = tableDefinition.getProperties();
-PubsubSchemaCapableIOProvider ioProvider = new
PubsubSchemaCapableIOProvider();
-
-try {
- RowJsonDeserializer deserializer =
- RowJsonDeserializer.forSchema(ioProvider.configurationSchema())
-
.withNullBehavior(RowJsonDeserializer.NullBehavior.ACCEPT_MISSING_OR_NULL);
-
- Row configurationRow =
-
newObjectMapperWith(deserializer).readValue(tableProperties.toString(),
Row.class);
-
- SchemaIO pubsubSchemaIO =
- ioProvider.from(
- tableDefinition.getLocation(), configurationRow,
tableDefinition.getSchema());
-
- return PubsubIOJsonTable.fromSchemaIO(pubsubSchemaIO);
-} catch (InvalidConfigurationException | InvalidSchemaException e) {
- throw new InvalidTableException(e.getMessage());
-} catch (JsonProcessingException e) {
- throw new AssertionError(
- "Failed to re-parse TBLPROPERTIES JSON " +
tableProperties.toString());
-}
+public class PubsubJsonTableProvider extends
SchemaCapableIOTableProviderWrapper {
+ public PubsubJsonTableProvider() {
Review comment:
You will need to remove this as well, otherwise it won't even compile.
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:
us...@infra.apache.org
[GitHub] [beam] ibzib commented on pull request #12270: [BEAM-10025] [Samza] Exclude UsesUnboundedPCollections tests.
ibzib commented on pull request #12270: URL: https://github.com/apache/beam/pull/12270#issuecomment-659021482 testMultiOutputParDoWithSideInputsIsCumulative is an unrelated flake. 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: us...@infra.apache.org
[GitHub] [beam] ibzib merged pull request #12270: [BEAM-10025] [Samza] Exclude UsesUnboundedPCollections tests.
ibzib merged pull request #12270: URL: https://github.com/apache/beam/pull/12270 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: us...@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12268: [BEAM-10090] Add JAXB core/impl as test dependency.
kennknowles commented on pull request #12268: URL: https://github.com/apache/beam/pull/12268#issuecomment-659022248 There's a real build break. #12273 to fix it and/or you can cherrypick the commit here. 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: us...@infra.apache.org
[GitHub] [beam] kennknowles commented on a change in pull request #8558: [BEAM-7131] Spark: cache executable stage output to prevent re-computation
kennknowles commented on a change in pull request #8558: URL: https://github.com/apache/beam/pull/8558#discussion_r455360189 ## File path: runners/spark/src/main/java/org/apache/beam/runners/spark/translation/SparkTranslationContext.java ## @@ -17,6 +17,7 @@ */ package org.apache.beam.runners.spark.translation; +import com.sun.istack.Nullable; Review comment: This is probably not the `Nullable` you want. CC @tysonjh since I found this from a failure on his 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: us...@infra.apache.org
[GitHub] [beam] kennknowles edited a comment on pull request #12272: [BEAM-10502] Enable checker in zetasketch extension
kennknowles edited a comment on pull request #12272: URL: https://github.com/apache/beam/pull/12272#issuecomment-659014899 And may I say @robinyqiu very nice work. This is the cleanest module I have encountered probably in my career. All I did was add trivial annotations, and the module was already perfectly null-correct. That _never_ happens, including my own code. I'm impressed. 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: us...@infra.apache.org
[GitHub] [beam] kennknowles edited a comment on pull request #12272: [BEAM-10502] Enable checker in zetasketch extension
kennknowles edited a comment on pull request #12272: URL: https://github.com/apache/beam/pull/12272#issuecomment-659014899 And may I say @robinyqiu very nice work. This is the cleanest module I have encountered probably in my career. All I did was add trivial annotations, and the module was already perfectly null-correct. That _never_ happens. 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: us...@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12272: [BEAM-10502] Enable checker in zetasketch extension
kennknowles commented on pull request #12272: URL: https://github.com/apache/beam/pull/12272#issuecomment-659014899 And may I say @robinyqiu very nice work. This is the cleanest module I have encountered probably in my career. All I did was add trivial annotations, and the module was already perfectly null-correct. 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: us...@infra.apache.org
[GitHub] [beam] kennknowles opened a new pull request #12272: [BEAM-10502] Enable checker in zetasketch extension
kennknowles opened a new pull request #12272: URL: https://github.com/apache/beam/pull/12272 This enables the checker analyzer in the Zetasketch extension, preventing nullability errors in this module. In this PR is an independent commit that prepares `Combine` for other modules to use nullability analysis. Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily: - [x] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`). - [x] 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. - [x] Update `CHANGES.md` with noteworthy changes. - [x] 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/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[](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/)[![Build
[GitHub] [beam] robinyqiu commented on a change in pull request #12267: [WIP][BEAM-10408] Schema capable io wrapper pubsub
robinyqiu commented on a change in pull request #12267:
URL: https://github.com/apache/beam/pull/12267#discussion_r455351279
##
File path:
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/SchemaCapableIOTableProviderWrapper.java
##
@@ -0,0 +1,136 @@
+/*
+ * 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.meta.provider;
+
+import static org.apache.beam.sdk.util.RowJsonUtils.newObjectMapperWith;
+
+import com.alibaba.fastjson.JSONObject;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import java.io.Serializable;
+import org.apache.beam.sdk.annotations.Experimental;
+import org.apache.beam.sdk.annotations.Internal;
+import org.apache.beam.sdk.extensions.sql.impl.BeamTableStatistics;
+import org.apache.beam.sdk.extensions.sql.meta.BaseBeamTable;
+import org.apache.beam.sdk.extensions.sql.meta.BeamSqlTable;
+import org.apache.beam.sdk.extensions.sql.meta.Table;
+import org.apache.beam.sdk.options.PipelineOptions;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.io.InvalidConfigurationException;
+import org.apache.beam.sdk.schemas.io.InvalidSchemaException;
+import org.apache.beam.sdk.schemas.io.SchemaIO;
+import org.apache.beam.sdk.schemas.io.SchemaIOProvider;
+import org.apache.beam.sdk.transforms.PTransform;
+import org.apache.beam.sdk.util.RowJson;
+import org.apache.beam.sdk.values.PBegin;
+import org.apache.beam.sdk.values.PCollection;
+import org.apache.beam.sdk.values.POutput;
+import org.apache.beam.sdk.values.Row;
+
+/**
+ * A general {@link TableProvider} for IOs for consumption by Beam SQL.
+ *
+ * Can create child classes for IOs to pass {@link
#schemaCapableIOProvider} that is specific to
+ * the IO.
+ */
+@Internal
+@Experimental
+public abstract class SchemaCapableIOTableProviderWrapper extends
InMemoryMetaTableProvider
+implements Serializable {
+ private SchemaIOProvider schemaCapableIOProvider;
+
+ public SchemaCapableIOTableProviderWrapper(SchemaIOProvider
schemaCapableIOProvider) {
+this.schemaCapableIOProvider = schemaCapableIOProvider;
Review comment:
I am guessing it could be this LOC that cause the problem.
I googled about Java service provider and it says a service provider (in
this case it is `PubSubJsonTableProvider`) should have a parameterless
constructor. But I didn't find if the superclass of a service provider (this
file) is also required to have a parameterless constructor.
Could we set
```
private SchemaIOProvider schemaCapableIOProvider = new
PubsubSchemaCapableIOProvider();
```
and remove this constructor here and see if this could fix the problem? I
know it is wrong for the general case, but just curious if that could be the
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:
us...@infra.apache.org
[GitHub] [beam] robinyqiu commented on a change in pull request #12267: [WIP][BEAM-10408] Schema capable io wrapper pubsub
robinyqiu commented on a change in pull request #12267:
URL: https://github.com/apache/beam/pull/12267#discussion_r455351279
##
File path:
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/SchemaCapableIOTableProviderWrapper.java
##
@@ -0,0 +1,136 @@
+/*
+ * 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.meta.provider;
+
+import static org.apache.beam.sdk.util.RowJsonUtils.newObjectMapperWith;
+
+import com.alibaba.fastjson.JSONObject;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import java.io.Serializable;
+import org.apache.beam.sdk.annotations.Experimental;
+import org.apache.beam.sdk.annotations.Internal;
+import org.apache.beam.sdk.extensions.sql.impl.BeamTableStatistics;
+import org.apache.beam.sdk.extensions.sql.meta.BaseBeamTable;
+import org.apache.beam.sdk.extensions.sql.meta.BeamSqlTable;
+import org.apache.beam.sdk.extensions.sql.meta.Table;
+import org.apache.beam.sdk.options.PipelineOptions;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.io.InvalidConfigurationException;
+import org.apache.beam.sdk.schemas.io.InvalidSchemaException;
+import org.apache.beam.sdk.schemas.io.SchemaIO;
+import org.apache.beam.sdk.schemas.io.SchemaIOProvider;
+import org.apache.beam.sdk.transforms.PTransform;
+import org.apache.beam.sdk.util.RowJson;
+import org.apache.beam.sdk.values.PBegin;
+import org.apache.beam.sdk.values.PCollection;
+import org.apache.beam.sdk.values.POutput;
+import org.apache.beam.sdk.values.Row;
+
+/**
+ * A general {@link TableProvider} for IOs for consumption by Beam SQL.
+ *
+ * Can create child classes for IOs to pass {@link
#schemaCapableIOProvider} that is specific to
+ * the IO.
+ */
+@Internal
+@Experimental
+public abstract class SchemaCapableIOTableProviderWrapper extends
InMemoryMetaTableProvider
+implements Serializable {
+ private SchemaIOProvider schemaCapableIOProvider;
+
+ public SchemaCapableIOTableProviderWrapper(SchemaIOProvider
schemaCapableIOProvider) {
+this.schemaCapableIOProvider = schemaCapableIOProvider;
Review comment:
I am guessing it could be this LOC that cause the problem.
I googled about Java service provider and it says a service provider (in
this case it is `PubSubJsonTableProvider`) should have a parameterless
constructor. But I didn't find if the superclass of a service provider (this
file) is also required to have a parameterless constructor.
Could we set
```
private SchemaIOProvider schemaCapableIOProvider = new
PubsubSchemaCapableIOProvider();
```
here and see if this could fix the problem? I know it is wrong for the
general case, but just curious if that could be the 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:
us...@infra.apache.org
[GitHub] [beam] robinyqiu commented on pull request #12247: [BEAM-9179] Refactor: move Java function implementations to a sub-module
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: us...@infra.apache.org
[GitHub] [beam] udim commented on a change in pull request #12009: [BEAM-10258] Support type hint annotations on PTransform's expand()
udim commented on a change in pull request #12009:
URL: https://github.com/apache/beam/pull/12009#discussion_r455342332
##
File path: website/www/site/content/en/documentation/sdks/python-type-safety.md
##
@@ -90,6 +91,15 @@ The following code declares an `int` input and a `str`
output type hint on the `
{{< code_sample
"sdks/python/apache_beam/examples/snippets/snippets_test_py3.py"
type_hints_map_annotations >}}
{{< /highlight >}}
+The following code demonstrates how to use annotations on `PTransform`
subclasses.
+A valid annotation is a `PCollection`, `PBegin`, or `PDone` that wraps an
internal (nested) type.
Review comment:
I would like to split the programming guide update into a separate PR,
since it will include examples from multiple languages and possibly some more
discussion. This will be tracked in BEAM-10495, and you can take it on if you
wish.
The python-type-safety update in this PR already mentions PBegin. Did you
want to add more details?
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:
us...@infra.apache.org
[GitHub] [beam] robinyqiu commented on pull request #12261: [BEAM-10490] Support read/write ZetaSQL DATE/TIME types from/to BigQuery
robinyqiu commented on pull request #12261: URL: https://github.com/apache/beam/pull/12261#issuecomment-659008094 Ah, thanks for catching that. Test added. 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: us...@infra.apache.org
[GitHub] [beam] kennknowles opened a new pull request #12271: [BEAM-10496] Improve nullability analysis for :sdks:java:core
kennknowles opened a new pull request #12271: URL: https://github.com/apache/beam/pull/12271 The total number of nullability issues in `:sdks:java:core` is very high. These are some baby steps towards embracing nullability typing. While not discovering any latent NPE this time, it did uncover improvements to the core SDK: - inlining completely trivial functions and their trivial tests - moving to Java function instead of Guava function - moving to Java streams instead of Guava `FluentIterable` - moving irrelevant null check out of utility function that has no reason to accept null parameters Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily: - [x] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`). - [x] 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. - [x] Update `CHANGES.md` with noteworthy changes. - [x] 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/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [![Build
[GitHub] [beam] kennknowles commented on pull request #12268: [BEAM-10090] Add JAXB core/impl as test dependency.
kennknowles commented on pull request #12268: URL: https://github.com/apache/beam/pull/12268#issuecomment-658995800 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: us...@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12268: [BEAM-10090] Add JAXB core/impl as test dependency.
kennknowles commented on pull request #12268: URL: https://github.com/apache/beam/pull/12268#issuecomment-658994369 Oddly enough, the test failure did not get uploaded in the gradle scan. The scan is just for the license pull. 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: us...@infra.apache.org
[GitHub] [beam] ibzib commented on pull request #12270: [BEAM-10025] [Samza] Exclude UsesUnboundedPCollections tests.
ibzib commented on pull request #12270: URL: https://github.com/apache/beam/pull/12270#issuecomment-658993658 Run Samza ValidatesRunner 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: us...@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #12262: [BEAM-10491] Simplify PeriodicSequence generator to use OffsetRanges with whole whole numbers
pabloem commented on pull request #12262: URL: https://github.com/apache/beam/pull/12262#issuecomment-658991901 thanks Luke! It looks nice! : P 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: us...@infra.apache.org
[GitHub] [beam] pabloem merged pull request #12262: [BEAM-10491] Simplify PeriodicSequence generator to use OffsetRanges with whole whole numbers
pabloem merged pull request #12262: URL: https://github.com/apache/beam/pull/12262 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: us...@infra.apache.org
[GitHub] [beam] iemejia merged pull request #12265: [BEAM-8727] Bump software.amazon.awssdk to 2.13.54
iemejia merged pull request #12265: URL: https://github.com/apache/beam/pull/12265 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: us...@infra.apache.org
[GitHub] [beam] ibzib opened a new pull request #12270: [BEAM-10025] [Samza] Exclude UsesUnboundedPCollections tests.
ibzib opened a new pull request #12270: URL: https://github.com/apache/beam/pull/12270 The Samza runner is already excluding all the other tests annotated `UsesUnboundedPCollections`. Before: 226 tests completed, 1 failed, 2 skipped After: 225 tests completed, 0 failed, 2 skipped R: @apilloud 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/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[](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/)[![Build
[GitHub] [beam] lukecwik commented on pull request #12262: [BEAM-10491] Simplify PeriodicSequence generator to use OffsetRanges with whole whole numbers
lukecwik commented on pull request #12262: URL: https://github.com/apache/beam/pull/12262#issuecomment-658980867 Run Python2_PVR_Flink PreCommit 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: us...@infra.apache.org
[GitHub] [beam] tysonjh commented on pull request #12268: [BEAM-10090] Add JAXB core/impl as test dependency.
tysonjh commented on pull request #12268: URL: https://github.com/apache/beam/pull/12268#issuecomment-658979464 R: @kennknowles I added some license comments just to be cautious. 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: us...@infra.apache.org
[GitHub] [beam] kennknowles commented on a change in pull request #12254: [BEAM-XXXX] Add note on zetaSQL dependencies
kennknowles commented on a change in pull request #12254:
URL: https://github.com/apache/beam/pull/12254#discussion_r455302615
##
File path: website/www/site/content/en/documentation/dsls/sql/overview.md
##
@@ -55,8 +55,12 @@ For more information on the ZetaSQL features in Beam SQL,
see the [Beam ZetaSQL
To switch to Beam ZetaSQL, configure the [pipeline
options](https://beam.apache.org/releases/javadoc/2.15.0/org/apache/beam/sdk/options/PipelineOptions.html)
as follows:
```
-setPlannerName("org.apache.beam.sdk.extensions.sql.zetasql.ZetaSQLQueryPlanner")
+PipelineOptions options = ...;
+options
+.as(BeamSqlPipelineOptions.class)
+
.setPlannerName("org.apache.beam.sdk.extensions.sql.zetasql.ZetaSQLQueryPlanner");
```
+Note, Use of the `ZetaSQLQueryPlanner` requires an additional dependency on
`beam-sdks-java-extensions-sql-zetasql` in addition to the
`beam-sdks-java-extensions-sql` package required for `CalcuiteQueryPlanner`.
Review comment:
Typo: `CalciteQueryPlanner`
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:
us...@infra.apache.org
[GitHub] [beam] tysonjh commented on pull request #12268: [BEAM-10090] Add JAXB core/impl as test dependency.
tysonjh commented on pull request #12268: URL: https://github.com/apache/beam/pull/12268#issuecomment-658963802 Run JavaPortabilityApiJava11 PreCommit 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: us...@infra.apache.org
[GitHub] [beam] tysonjh commented on pull request #12268: [BEAM-10090] Add JAXB core/impl as test dependency.
tysonjh commented on pull request #12268: URL: https://github.com/apache/beam/pull/12268#issuecomment-658963480 Run Java_Examples_Dataflow_Java11 PreCommit 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: us...@infra.apache.org
[GitHub] [beam] tysonjh commented on pull request #12268: [BEAM-10090] Add JAXB core/impl as test dependency.
tysonjh commented on pull request #12268: URL: https://github.com/apache/beam/pull/12268#issuecomment-658963276 Run Java_Examples_Dataflow_Java11 PreCommit please 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: us...@infra.apache.org
[GitHub] [beam] robertwb closed pull request #12259: [BEAM-9561] Run Pandas tests on Beam.
robertwb closed pull request #12259: URL: https://github.com/apache/beam/pull/12259 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: us...@infra.apache.org
[GitHub] [beam] robertwb commented on pull request #12259: [BEAM-9561] Run Pandas tests on Beam.
robertwb commented on pull request #12259: URL: https://github.com/apache/beam/pull/12259#issuecomment-658956391 R: @ibzib Could you take a look at this? 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: us...@infra.apache.org
[GitHub] [beam] davidcavazos edited a comment on pull request #12252: [BEAM-7390] Add combineglobally code snippets
davidcavazos edited a comment on pull request #12252: URL: https://github.com/apache/beam/pull/12252#issuecomment-658939252 I also noticed that the output tags were a little misleading now since we are showing the stdout instead of an output PCollection. Opened #12269 for 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: us...@infra.apache.org
[GitHub] [beam] davidcavazos opened a new pull request #12269: Updated output tags
davidcavazos opened a new pull request #12269: URL: https://github.com/apache/beam/pull/12269 **No content changes** Now that we are displaying the `stdout` content instead of the `PCollection` contents, the previous output tag was a little misleading. This just goes through all the existing examples and updates the tag to remove the `PCollection` part and just leave it as `Output:`. R: @aaltay 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/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/) Python | [![Build
[GitHub] [beam] tysonjh commented on pull request #12268: [BEAM-10090] Add JAXB core/impl as test dependency.
tysonjh commented on pull request #12268: URL: https://github.com/apache/beam/pull/12268#issuecomment-658940970 retest this please 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: us...@infra.apache.org
[GitHub] [beam] davidcavazos commented on pull request #12252: [BEAM-7390] Add combineglobally code snippets
davidcavazos commented on pull request #12252: URL: https://github.com/apache/beam/pull/12252#issuecomment-658939252 I also noticed that the output tags were a little misleading now since we are showing the stdout instead of an output PCollection 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: us...@infra.apache.org
[GitHub] [beam] davidcavazos commented on pull request #12252: [BEAM-7390] Add combineglobally code snippets
davidcavazos commented on pull request #12252: URL: https://github.com/apache/beam/pull/12252#issuecomment-658937924 @aaltay I've addressed the comments 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: us...@infra.apache.org
[GitHub] [beam] tysonjh opened a new pull request #12268: [BEAM-10090] Add JAXB core/impl as test dependency for XmlIO.
tysonjh opened a new pull request #12268: URL: https://github.com/apache/beam/pull/12268 Add JAXB core/impl as test dependency. In Java 9 many EE modules were removed from core Java. This change reintroduces the JAXB modules, updates their minor versions, in preparation for supporting Java 11. **Please** add a meaningful description for your change here 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/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/) Python | [![Build
[GitHub] [beam] robinyqiu commented on pull request #12247: [BEAM-9179] Remove dead code: Java implementation of ZetaSQL functions
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: us...@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #12262: [BEAM-10491] Simplify PeriodicSequence generator to use OffsetRanges with whole whole numbers
lukecwik commented on pull request #12262: URL: https://github.com/apache/beam/pull/12262#issuecomment-658934055 retest this please 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: us...@infra.apache.org
[GitHub] [beam] aaltay merged pull request #12222: Gate GH actions so it only runs in main/upstream repo
aaltay merged pull request #1: URL: https://github.com/apache/beam/pull/1 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: us...@infra.apache.org
[GitHub] [beam] aaltay commented on pull request #12222: Gate GH actions so it only runs in main/upstream repo
aaltay commented on pull request #1: URL: https://github.com/apache/beam/pull/1#issuecomment-658930446 LGTM. Let's merge this. Thank you. 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: us...@infra.apache.org
[GitHub] [beam] aaltay commented on pull request #12252: [BEAM-7390] Add combineglobally code snippets
aaltay commented on pull request #12252: URL: https://github.com/apache/beam/pull/12252#issuecomment-658929492 (Please ping me once the open comments are addressed.) 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: us...@infra.apache.org
[GitHub] [beam] aaltay merged pull request #12217: [BEAM-7390] Add cogroupbykey code snippets
aaltay merged pull request #12217: URL: https://github.com/apache/beam/pull/12217 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: us...@infra.apache.org
[GitHub] [beam] sclukas77 opened a new pull request #12267: [WIP][BEAM-10408] Schema capable io wrapper pubsub
sclukas77 opened a new pull request #12267: URL: https://github.com/apache/beam/pull/12267 Created generalized table and tableprovider wrappers in Beam SQL, implementing for Pubsub. Additional info on project: https://docs.google.com/document/d/1ic3P8EVGHIydHQ-VMDKbN9kEdwm7sBXMo80VrhwksvI/edit#heading=h.x9snb54sjlu9 R: @robinyqiu 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/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[](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/)[](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)[![Build
[GitHub] [beam] robinyqiu commented on pull request #12261: [BEAM-10490] Support read/write ZetaSQL DATE/TIME types from/to BigQuery
robinyqiu commented on pull request #12261: URL: https://github.com/apache/beam/pull/12261#issuecomment-658919794 Run Java PreCommit 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: us...@infra.apache.org
[GitHub] [beam] davidcavazos commented on pull request #12118: [BEAM-7705] Add BigQuery Java samples
davidcavazos commented on pull request #12118: URL: https://github.com/apache/beam/pull/12118#issuecomment-658918522 There are some errors saying `SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".` Does anyone know what they mean and how to fix them? 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: us...@infra.apache.org
[GitHub] [beam] aaltay commented on pull request #12233: [BEAM-10465] Add checksum for Python source distribution and wheels
aaltay commented on pull request #12233: URL: https://github.com/apache/beam/pull/12233#issuecomment-658916388 > @aaltay thanks for merging :) > > Actually I was wrong with statement: "workflows are run only when they merged to master". > It is possible to run workflows not merged to master e.g. in PR which introduces new workflow and it is triggered on 'push' event type this new workflow will run. > > > My specific question, is it possible to trigger workflow similar to PR workflows if changes touch the workflow files? > > It is possible to include `'.github/workflows/**'` in `paths`: > https://github.com/apache/beam/blob/master/.github/workflows/build_wheels.yml#L30 > Then any change in this dir in PR will trigger `Build python wheels` workflow run. > Do you think I should create PR with this change? Yes, I think it would be good to create that PR and test workflows when the workflow files change. 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: us...@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #12074: [BEAM-10313] Add without_defaults to built-in Combiners in Python
pabloem commented on pull request #12074: URL: https://github.com/apache/beam/pull/12074#issuecomment-658915637 Run Python2_PVR_Flink PreCommit 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: us...@infra.apache.org
[GitHub] [beam] robinyqiu commented on a change in pull request #12174: [BEAM-10239] Support ZetaSQL NUMERIC type in BeamSQL
robinyqiu commented on a change in pull request #12174:
URL: https://github.com/apache/beam/pull/12174#discussion_r455235874
##
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/translation/ExpressionConverter.java
##
@@ -805,6 +807,24 @@ private RexNode convertSimpleValueToRexNode(TypeKind kind,
Value value) {
.makeLiteral(
value.getStringValue(),
typeFactory().createSqlType(SqlTypeName.VARCHAR), true);
break;
+ case TYPE_NUMERIC:
+// Cannot simply call makeExactLiteral() for ZetaSQL NUMERIC type
because later it will be
Review comment:
> It might be worth writing up a mini design doc...
That's a great idea!
> I suspect this could also fix the Infinity and NaN issues...
Yes, I agree. We have discussed that a bit offline and we believe this
approach could fix that problem (and other similar problems, if any, due to
different value representation between ZetaSQL and Calcite). I think Zijie will
talk about that in more details in his design doc.
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:
us...@infra.apache.org
[GitHub] [beam] ZijieSong946 commented on a change in pull request #12174: [BEAM-10239] Support ZetaSQL NUMERIC type in BeamSQL
ZijieSong946 commented on a change in pull request #12174:
URL: https://github.com/apache/beam/pull/12174#discussion_r455234860
##
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/translation/ExpressionConverter.java
##
@@ -805,6 +807,24 @@ private RexNode convertSimpleValueToRexNode(TypeKind kind,
Value value) {
.makeLiteral(
value.getStringValue(),
typeFactory().createSqlType(SqlTypeName.VARCHAR), true);
break;
+ case TYPE_NUMERIC:
+// Cannot simply call makeExactLiteral() for ZetaSQL NUMERIC type
because later it will be
+// unparsed to the string representation of the BigDecimal itself
(e.g. "SELECT NUMERIC '0'"
+// will be unparsed to "SELECT 0E-9"), and Calcite does not allow
customize unparsing of
+// SqlNumericLiteral. So we create a wrapper function here such that
we can later recognize
+// it and customize its unparsing in BeamBigQuerySqlDialect.
+ret =
+rexBuilder()
+.makeCall(
+SqlOperators.createSimpleSqlFunction(
+"numeric_literal",
ZetaSqlCalciteTranslationUtils.toCalciteTypeName(kind)),
Review comment:
OK. Done.
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:
us...@infra.apache.org
[GitHub] [beam] robinyqiu commented on a change in pull request #12174: [BEAM-10239] Support ZetaSQL NUMERIC type in BeamSQL
robinyqiu commented on a change in pull request #12174:
URL: https://github.com/apache/beam/pull/12174#discussion_r455233394
##
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/translation/ExpressionConverter.java
##
@@ -805,6 +807,24 @@ private RexNode convertSimpleValueToRexNode(TypeKind kind,
Value value) {
.makeLiteral(
value.getStringValue(),
typeFactory().createSqlType(SqlTypeName.VARCHAR), true);
break;
+ case TYPE_NUMERIC:
+// Cannot simply call makeExactLiteral() for ZetaSQL NUMERIC type
because later it will be
+// unparsed to the string representation of the BigDecimal itself
(e.g. "SELECT NUMERIC '0'"
+// will be unparsed to "SELECT 0E-9"), and Calcite does not allow
customize unparsing of
+// SqlNumericLiteral. So we create a wrapper function here such that
we can later recognize
+// it and customize its unparsing in BeamBigQuerySqlDialect.
+ret =
+rexBuilder()
+.makeCall(
+SqlOperators.createSimpleSqlFunction(
+"numeric_literal",
ZetaSqlCalciteTranslationUtils.toCalciteTypeName(kind)),
Review comment:
Zijie, here you can refer to `NUMERIC_LITERAL_FUNCTION` (of course you
need to make it public) you defined in the other file.
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:
us...@infra.apache.org
[GitHub] [beam] robinyqiu commented on a change in pull request #12174: [BEAM-10239] Support ZetaSQL NUMERIC type in BeamSQL
robinyqiu commented on a change in pull request #12174:
URL: https://github.com/apache/beam/pull/12174#discussion_r455231610
##
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/ZetaSqlTypesUtils.java
##
@@ -0,0 +1,42 @@
+/*
+ * 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;
+
+import java.math.BigDecimal;
+import org.apache.beam.sdk.annotations.Internal;
+
+/** Utils to deal with ZetaSQL type generation. */
+@Internal
+public class ZetaSqlTypesUtils {
Review comment:
I was thinking of moving some other util functions to this file later.
But for now it is only used for test, so I am fine with it in test.
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:
us...@infra.apache.org
[GitHub] [beam] saavannanavati commented on a change in pull request #12009: [BEAM-10258] Support type hint annotations on PTransform's expand()
saavannanavati commented on a change in pull request #12009:
URL: https://github.com/apache/beam/pull/12009#discussion_r455230574
##
File path: website/www/site/content/en/documentation/sdks/python-type-safety.md
##
@@ -90,6 +91,15 @@ The following code declares an `int` input and a `str`
output type hint on the `
{{< code_sample
"sdks/python/apache_beam/examples/snippets/snippets_test_py3.py"
type_hints_map_annotations >}}
{{< /highlight >}}
+The following code demonstrates how to use annotations on `PTransform`
subclasses.
+A valid annotation is a `PCollection`, `PBegin`, or `PDone` that wraps an
internal (nested) type.
Review comment:
Sounds good.
Should we update documentation
[here](https://beam.apache.org/documentation/programming-guide/) and
[here](https://beam.apache.org/documentation/sdks/python-type-safety/) to note
the existence of `PBegin`?
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:
us...@infra.apache.org
[GitHub] [beam] Mark-Zeng commented on a change in pull request #12232: [Beam-9543] Support Match Recognition in Beam SQL
Mark-Zeng commented on a change in pull request #12232:
URL: https://github.com/apache/beam/pull/12232#discussion_r455218522
##
File path:
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/cep/CEPTypeName.java
##
@@ -0,0 +1,33 @@
+/*
+ * 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.impl.cep;
+
+import java.io.Serializable;
+
+public enum CEPTypeName implements Serializable {
Review comment:
There are some differences.
[RexLiteral](https://calcite.apache.org/javadocAggregate/org/apache/calcite/rex/RexLiteral.html)
class seems to have a wider support for time interval. And I just found in the
page that the value of the Double type in a literal node is BigDecimal? weird.
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:
us...@infra.apache.org
[GitHub] [beam] Mark-Zeng commented on a change in pull request #12232: [Beam-9543] Support Match Recognition in Beam SQL
Mark-Zeng commented on a change in pull request #12232:
URL: https://github.com/apache/beam/pull/12232#discussion_r455218522
##
File path:
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/cep/CEPTypeName.java
##
@@ -0,0 +1,33 @@
+/*
+ * 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.impl.cep;
+
+import java.io.Serializable;
+
+public enum CEPTypeName implements Serializable {
Review comment:
There are some differences. [RexLiteral]
(https://calcite.apache.org/javadocAggregate/org/apache/calcite/rex/RexLiteral.html)
class seems to have a wider support for time interval. And I just found in the
page that the value of the Double type in a literal node is BigDecimal? weird.
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:
us...@infra.apache.org
[GitHub] [beam] Mark-Zeng commented on a change in pull request #12232: [Beam-9543] Support Match Recognition in Beam SQL
Mark-Zeng commented on a change in pull request #12232:
URL: https://github.com/apache/beam/pull/12232#discussion_r455213949
##
File path:
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/cep/CEPTypeName.java
##
@@ -0,0 +1,33 @@
+/*
+ * 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.impl.cep;
+
+import java.io.Serializable;
+
+public enum CEPTypeName implements Serializable {
Review comment:
I think it is possible; reusing the schema types seems natural also. I
will update it in my next commit.
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:
us...@infra.apache.org
