You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@beam.apache.org by GitBox <gi...@apache.org> on 2020/03/13 01:08:27 UTC

[GitHub] [beam] TheNeuralBit opened a new pull request #11119: [BEAM-9498] Include descriptor and type of unsupported fields in RowJson exception

TheNeuralBit opened a new pull request #11119: [BEAM-9498] Include descriptor and type of unsupported fields in RowJson exception
URL: https://github.com/apache/beam/pull/11119
 
 
   Post-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   Lang | SDK | Apex | Dataflow | Flink | Gearpump | Samza | Spark
   --- | --- | --- | --- | --- | --- | --- | ---
   Go | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
   Java | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
   Python | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
   XLang | --- | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
   
   Pre-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   --- |Java | Python | Go | Website
   --- | --- | --- | --- | ---
   Non-portable | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) 
   Portable | --- | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
   
   See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] TheNeuralBit merged pull request #11119: [BEAM-9498] Include descriptor and type of unsupported fields in RowJson exception

Posted by GitBox <gi...@apache.org>.
TheNeuralBit merged pull request #11119: [BEAM-9498] Include descriptor and type of unsupported fields in RowJson exception
URL: https://github.com/apache/beam/pull/11119
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] apilloud commented on a change in pull request #11119: [BEAM-9498] Include descriptor and type of unsupported fields in RowJson exception

Posted by GitBox <gi...@apache.org>.
apilloud commented on a change in pull request #11119: [BEAM-9498] Include descriptor and type of unsupported fields in RowJson exception
URL: https://github.com/apache/beam/pull/11119#discussion_r393391148
 
 

 ##########
 File path: sdks/java/core/src/main/java/org/apache/beam/sdk/util/RowJson.java
 ##########
 @@ -84,41 +85,76 @@
   private static final ImmutableSet<TypeName> SUPPORTED_TYPES =
       ImmutableSet.of(BYTE, INT16, INT32, INT64, FLOAT, DOUBLE, BOOLEAN, STRING, DECIMAL);
 
+  /**
+   * Throws {@link UnsupportedRowJsonException} if {@code schema} contains an unsupported field
+   * type.
+   */
   public static void verifySchemaSupported(Schema schema) {
-    schema.getFields().forEach(RowJson::verifyFieldTypeSupported);
+    ImmutableList<UnsupportedField> unsupportedFields = findUnsupportedFields(schema);
+    if (!unsupportedFields.isEmpty()) {
+      throw new UnsupportedRowJsonException(
+          String.format(
+              "Field type%s %s not supported when converting between JSON and Rows. Supported types are: %s",
+              unsupportedFields.size() > 1 ? "s" : "",
+              unsupportedFields.toString(),
+              SUPPORTED_TYPES.toString()));
+    }
+  }
+
+  private static class UnsupportedField {
+    final String descriptor;
+    final TypeName typeName;
+
+    UnsupportedField(String descriptor, TypeName typeName) {
+      this.descriptor = descriptor;
+      this.typeName = typeName;
+    }
+
+    @Override
+    public String toString() {
+      return this.descriptor + "=" + this.typeName;
+    }
+  }
+
+  private static ImmutableList<UnsupportedField> findUnsupportedFields(Schema schema) {
+    return schema.getFields().stream()
+        .flatMap((field) -> findUnsupportedFields(field).stream())
+        .collect(toImmutableList());
   }
 
-  static void verifyFieldTypeSupported(Field field) {
+  private static ImmutableList<UnsupportedField> findUnsupportedFields(Field field) {
     FieldType fieldType = field.getType();
-    verifyFieldTypeSupported(fieldType);
+    return findUnsupportedFields(fieldType, field.getName());
   }
 
-  static void verifyFieldTypeSupported(FieldType fieldType) {
+  private static ImmutableList<UnsupportedField> findUnsupportedFields(
+      FieldType fieldType, String fieldName) {
     TypeName fieldTypeName = fieldType.getTypeName();
 
     if (fieldTypeName.isCompositeType()) {
       Schema rowFieldSchema = fieldType.getRowSchema();
 
 Review comment:
   nit: drop the variable, it doesn't add value.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] apilloud commented on a change in pull request #11119: [BEAM-9498] Include descriptor and type of unsupported fields in RowJson exception

Posted by GitBox <gi...@apache.org>.
apilloud commented on a change in pull request #11119: [BEAM-9498] Include descriptor and type of unsupported fields in RowJson exception
URL: https://github.com/apache/beam/pull/11119#discussion_r393390344
 
 

 ##########
 File path: sdks/java/core/src/main/java/org/apache/beam/sdk/util/RowJson.java
 ##########
 @@ -84,41 +85,76 @@
   private static final ImmutableSet<TypeName> SUPPORTED_TYPES =
       ImmutableSet.of(BYTE, INT16, INT32, INT64, FLOAT, DOUBLE, BOOLEAN, STRING, DECIMAL);
 
+  /**
+   * Throws {@link UnsupportedRowJsonException} if {@code schema} contains an unsupported field
+   * type.
+   */
   public static void verifySchemaSupported(Schema schema) {
-    schema.getFields().forEach(RowJson::verifyFieldTypeSupported);
+    ImmutableList<UnsupportedField> unsupportedFields = findUnsupportedFields(schema);
+    if (!unsupportedFields.isEmpty()) {
+      throw new UnsupportedRowJsonException(
+          String.format(
+              "Field type%s %s not supported when converting between JSON and Rows. Supported types are: %s",
+              unsupportedFields.size() > 1 ? "s" : "",
+              unsupportedFields.toString(),
+              SUPPORTED_TYPES.toString()));
+    }
+  }
+
+  private static class UnsupportedField {
+    final String descriptor;
+    final TypeName typeName;
+
+    UnsupportedField(String descriptor, TypeName typeName) {
+      this.descriptor = descriptor;
+      this.typeName = typeName;
+    }
+
+    @Override
+    public String toString() {
+      return this.descriptor + "=" + this.typeName;
+    }
+  }
+
+  private static ImmutableList<UnsupportedField> findUnsupportedFields(Schema schema) {
+    return schema.getFields().stream()
+        .flatMap((field) -> findUnsupportedFields(field).stream())
+        .collect(toImmutableList());
   }
 
-  static void verifyFieldTypeSupported(Field field) {
+  private static ImmutableList<UnsupportedField> findUnsupportedFields(Field field) {
     FieldType fieldType = field.getType();
 
 Review comment:
   nit: drop the variable, it doesn't add value.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] TheNeuralBit commented on a change in pull request #11119: [BEAM-9498] Include descriptor and type of unsupported fields in RowJson exception

Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #11119: [BEAM-9498] Include descriptor and type of unsupported fields in RowJson exception
URL: https://github.com/apache/beam/pull/11119#discussion_r394028731
 
 

 ##########
 File path: sdks/java/core/src/main/java/org/apache/beam/sdk/util/RowJson.java
 ##########
 @@ -84,41 +85,76 @@
   private static final ImmutableSet<TypeName> SUPPORTED_TYPES =
       ImmutableSet.of(BYTE, INT16, INT32, INT64, FLOAT, DOUBLE, BOOLEAN, STRING, DECIMAL);
 
+  /**
+   * Throws {@link UnsupportedRowJsonException} if {@code schema} contains an unsupported field
+   * type.
+   */
   public static void verifySchemaSupported(Schema schema) {
-    schema.getFields().forEach(RowJson::verifyFieldTypeSupported);
+    ImmutableList<UnsupportedField> unsupportedFields = findUnsupportedFields(schema);
+    if (!unsupportedFields.isEmpty()) {
+      throw new UnsupportedRowJsonException(
+          String.format(
+              "Field type%s %s not supported when converting between JSON and Rows. Supported types are: %s",
+              unsupportedFields.size() > 1 ? "s" : "",
+              unsupportedFields.toString(),
+              SUPPORTED_TYPES.toString()));
+    }
+  }
+
+  private static class UnsupportedField {
+    final String descriptor;
+    final TypeName typeName;
+
+    UnsupportedField(String descriptor, TypeName typeName) {
+      this.descriptor = descriptor;
+      this.typeName = typeName;
+    }
+
+    @Override
+    public String toString() {
+      return this.descriptor + "=" + this.typeName;
+    }
+  }
+
+  private static ImmutableList<UnsupportedField> findUnsupportedFields(Schema schema) {
+    return schema.getFields().stream()
+        .flatMap((field) -> findUnsupportedFields(field).stream())
+        .collect(toImmutableList());
   }
 
-  static void verifyFieldTypeSupported(Field field) {
+  private static ImmutableList<UnsupportedField> findUnsupportedFields(Field field) {
     FieldType fieldType = field.getType();
 
 Review comment:
   Done! thanks

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services