You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by el...@apache.org on 2015/12/04 03:11:29 UTC

calcite git commit: [CALCITE-983] Handle nulls in ErrorResponse's protobuf representation better

Repository: calcite
Updated Branches:
  refs/heads/master d259cad23 -> cd58fe73f


[CALCITE-983] Handle nulls in ErrorResponse's protobuf representation better

Added some more unit tests to make sure null attributes aren't
causing failures in ErrorResponse (and thus causing the client
to choke). Also fixed a broken hashCode and added toString.


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/cd58fe73
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/cd58fe73
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/cd58fe73

Branch: refs/heads/master
Commit: cd58fe73f29dd576ac2525f22c34e9b3d4df1913
Parents: d259cad
Author: Josh Elser <el...@apache.org>
Authored: Thu Dec 3 21:07:34 2015 -0500
Committer: Josh Elser <el...@apache.org>
Committed: Thu Dec 3 21:07:34 2015 -0500

----------------------------------------------------------------------
 .../apache/calcite/avatica/proto/Responses.java | 100 +++++++++++++++++--
 .../apache/calcite/avatica/remote/Service.java  |  56 ++++++++++-
 avatica/src/main/protobuf/responses.proto       |   1 +
 .../remote/ProtobufTranslationImplTest.java     |   5 +
 4 files changed, 148 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/cd58fe73/avatica/src/main/java/org/apache/calcite/avatica/proto/Responses.java
----------------------------------------------------------------------
diff --git a/avatica/src/main/java/org/apache/calcite/avatica/proto/Responses.java b/avatica/src/main/java/org/apache/calcite/avatica/proto/Responses.java
index f364021..7df84b7 100644
--- a/avatica/src/main/java/org/apache/calcite/avatica/proto/Responses.java
+++ b/avatica/src/main/java/org/apache/calcite/avatica/proto/Responses.java
@@ -8499,6 +8499,15 @@ package org.apache.calcite.avatica.proto;
         getExceptionsBytes(int index);
 
     /**
+     * <code>optional bool has_exceptions = 7;</code>
+     *
+     * <pre>
+     * are there stacktraces contained?
+     * </pre>
+     */
+    boolean getHasExceptions();
+
+    /**
      * <code>optional string error_message = 2;</code>
      *
      * <pre>
@@ -8582,6 +8591,7 @@ package org.apache.calcite.avatica.proto;
     }
     private ErrorResponse() {
       exceptions_ = com.google.protobuf.LazyStringArrayList.EMPTY;
+      hasExceptions_ = false;
       errorMessage_ = "";
       severity_ = 0;
       errorCode_ = 0;
@@ -8657,6 +8667,11 @@ package org.apache.calcite.avatica.proto;
 
               break;
             }
+            case 56: {
+
+              hasExceptions_ = input.readBool();
+              break;
+            }
           }
         }
       } catch (com.google.protobuf.InvalidProtocolBufferException e) {
@@ -8730,6 +8745,19 @@ package org.apache.calcite.avatica.proto;
       return exceptions_.getByteString(index);
     }
 
+    public static final int HAS_EXCEPTIONS_FIELD_NUMBER = 7;
+    private boolean hasExceptions_;
+    /**
+     * <code>optional bool has_exceptions = 7;</code>
+     *
+     * <pre>
+     * are there stacktraces contained?
+     * </pre>
+     */
+    public boolean getHasExceptions() {
+      return hasExceptions_;
+    }
+
     public static final int ERROR_MESSAGE_FIELD_NUMBER = 2;
     private volatile java.lang.Object errorMessage_;
     /**
@@ -8894,6 +8922,9 @@ package org.apache.calcite.avatica.proto;
       if (metadata_ != null) {
         output.writeMessage(6, getMetadata());
       }
+      if (hasExceptions_ != false) {
+        output.writeBool(7, hasExceptions_);
+      }
     }
 
     public int getSerializedSize() {
@@ -8927,6 +8958,10 @@ package org.apache.calcite.avatica.proto;
         size += com.google.protobuf.CodedOutputStream
           .computeMessageSize(6, getMetadata());
       }
+      if (hasExceptions_ != false) {
+        size += com.google.protobuf.CodedOutputStream
+          .computeBoolSize(7, hasExceptions_);
+      }
       memoizedSize = size;
       return size;
     }
@@ -9044,6 +9079,8 @@ package org.apache.calcite.avatica.proto;
         super.clear();
         exceptions_ = com.google.protobuf.LazyStringArrayList.EMPTY;
         bitField0_ = (bitField0_ & ~0x00000001);
+        hasExceptions_ = false;
+
         errorMessage_ = "";
 
         severity_ = 0;
@@ -9087,6 +9124,7 @@ package org.apache.calcite.avatica.proto;
           bitField0_ = (bitField0_ & ~0x00000001);
         }
         result.exceptions_ = exceptions_;
+        result.hasExceptions_ = hasExceptions_;
         result.errorMessage_ = errorMessage_;
         result.severity_ = severity_;
         result.errorCode_ = errorCode_;
@@ -9122,6 +9160,9 @@ package org.apache.calcite.avatica.proto;
           }
           onChanged();
         }
+        if (other.getHasExceptions() != false) {
+          setHasExceptions(other.getHasExceptions());
+        }
         if (!other.getErrorMessage().isEmpty()) {
           errorMessage_ = other.errorMessage_;
           onChanged();
@@ -9296,6 +9337,44 @@ package org.apache.calcite.avatica.proto;
         return this;
       }
 
+      private boolean hasExceptions_ ;
+      /**
+       * <code>optional bool has_exceptions = 7;</code>
+       *
+       * <pre>
+       * are there stacktraces contained?
+       * </pre>
+       */
+      public boolean getHasExceptions() {
+        return hasExceptions_;
+      }
+      /**
+       * <code>optional bool has_exceptions = 7;</code>
+       *
+       * <pre>
+       * are there stacktraces contained?
+       * </pre>
+       */
+      public Builder setHasExceptions(boolean value) {
+        
+        hasExceptions_ = value;
+        onChanged();
+        return this;
+      }
+      /**
+       * <code>optional bool has_exceptions = 7;</code>
+       *
+       * <pre>
+       * are there stacktraces contained?
+       * </pre>
+       */
+      public Builder clearHasExceptions() {
+        
+        hasExceptions_ = false;
+        onChanged();
+        return this;
+      }
+
       private java.lang.Object errorMessage_ = "";
       /**
        * <code>optional string error_message = 2;</code>
@@ -10990,15 +11069,16 @@ package org.apache.calcite.avatica.proto;
       "a\030\003 \001(\0132\014.RpcMetadata\"c\n\030DatabasePropert" +
       "yResponse\022\'\n\005props\030\001 \003(\0132\030.DatabasePrope" +
       "rtyElement\022\036\n\010metadata\030\002 \001(\0132\014.RpcMetada" +
-      "ta\"\236\001\n\rErrorResponse\022\022\n\nexceptions\030\001 \003(\t",
-      "\022\025\n\rerror_message\030\002 \001(\t\022\033\n\010severity\030\003 \001(" +
-      "\0162\t.Severity\022\022\n\nerror_code\030\004 \001(\r\022\021\n\tsql_" +
-      "state\030\005 \001(\t\022\036\n\010metadata\030\006 \001(\0132\014.RpcMetad" +
-      "ata\"f\n\023SyncResultsResponse\022\031\n\021missing_st" +
-      "atement\030\001 \001(\010\022\024\n\014more_results\030\002 \001(\010\022\036\n\010m" +
-      "etadata\030\003 \001(\0132\014.RpcMetadata\"%\n\013RpcMetada" +
-      "ta\022\026\n\016server_address\030\001 \001(\tB\"\n org.apache" +
-      ".calcite.avatica.protob\006proto3"
+      "ta\"\266\001\n\rErrorResponse\022\022\n\nexceptions\030\001 \003(\t",
+      "\022\026\n\016has_exceptions\030\007 \001(\010\022\025\n\rerror_messag" +
+      "e\030\002 \001(\t\022\033\n\010severity\030\003 \001(\0162\t.Severity\022\022\n\n" +
+      "error_code\030\004 \001(\r\022\021\n\tsql_state\030\005 \001(\t\022\036\n\010m" +
+      "etadata\030\006 \001(\0132\014.RpcMetadata\"f\n\023SyncResul" +
+      "tsResponse\022\031\n\021missing_statement\030\001 \001(\010\022\024\n" +
+      "\014more_results\030\002 \001(\010\022\036\n\010metadata\030\003 \001(\0132\014." +
+      "RpcMetadata\"%\n\013RpcMetadata\022\026\n\016server_add" +
+      "ress\030\001 \001(\tB\"\n org.apache.calcite.avatica" +
+      ".protob\006proto3"
     };
     com.google.protobuf.Descriptors.FileDescriptor.InternalDescriptorAssigner assigner =
         new com.google.protobuf.Descriptors.FileDescriptor.    InternalDescriptorAssigner() {
@@ -11084,7 +11164,7 @@ package org.apache.calcite.avatica.proto;
     internal_static_ErrorResponse_fieldAccessorTable = new
       com.google.protobuf.GeneratedMessage.FieldAccessorTable(
         internal_static_ErrorResponse_descriptor,
-        new java.lang.String[] { "Exceptions", "ErrorMessage", "Severity", "ErrorCode", "SqlState", "Metadata", });
+        new java.lang.String[] { "Exceptions", "HasExceptions", "ErrorMessage", "Severity", "ErrorCode", "SqlState", "Metadata", });
     internal_static_SyncResultsResponse_descriptor =
       getDescriptor().getMessageTypes().get(12);
     internal_static_SyncResultsResponse_fieldAccessorTable = new

http://git-wip-us.apache.org/repos/asf/calcite/blob/cd58fe73/avatica/src/main/java/org/apache/calcite/avatica/remote/Service.java
----------------------------------------------------------------------
diff --git a/avatica/src/main/java/org/apache/calcite/avatica/remote/Service.java b/avatica/src/main/java/org/apache/calcite/avatica/remote/Service.java
index c1dc3d3..d7b450c 100644
--- a/avatica/src/main/java/org/apache/calcite/avatica/remote/Service.java
+++ b/avatica/src/main/java/org/apache/calcite/avatica/remote/Service.java
@@ -2789,13 +2789,32 @@ public interface Service {
       Responses.ErrorResponse msg = (Responses.ErrorResponse) genericMsg;
       Descriptor desc = msg.getDescriptorForType();
 
+      List<String> exceptions = null;
+      if (msg.getHasExceptions()) {
+        exceptions = msg.getExceptionsList();
+      }
+
+      String errorMessage = null;
+      if (ProtobufService.hasField(msg, desc, Responses.ErrorResponse.ERROR_MESSAGE_FIELD_NUMBER)) {
+        errorMessage = msg.getErrorMessage();
+      }
+
+      String sqlState = null;
+      if (ProtobufService.hasField(msg, desc, Responses.ErrorResponse.SQL_STATE_FIELD_NUMBER)) {
+        sqlState = msg.getSqlState();
+      }
+
+      AvaticaSeverity severity = null;
+      if (ProtobufService.hasField(msg, desc, Responses.ErrorResponse.SEVERITY_FIELD_NUMBER)) {
+        severity = AvaticaSeverity.fromProto(msg.getSeverity());
+      }
+
       RpcMetadataResponse metadata = null;
       if (ProtobufService.hasField(msg, desc, Responses.ErrorResponse.METADATA_FIELD_NUMBER)) {
         metadata = RpcMetadataResponse.fromProto(msg.getMetadata());
       }
 
-      return new ErrorResponse(msg.getExceptionsList(), msg.getErrorMessage(),
-          msg.getErrorCode(), msg.getSqlState(), AvaticaSeverity.fromProto(msg.getSeverity()),
+      return new ErrorResponse(exceptions, errorMessage, msg.getErrorCode(), sqlState, severity,
           metadata);
     }
 
@@ -2806,14 +2825,43 @@ public interface Service {
         builder.setMetadata(rpcMetadata.serialize());
       }
 
-      return builder.addAllExceptions(exceptions).setErrorMessage(errorMessage)
-          .setErrorCode(errorCode).setSqlState(sqlState).setSeverity(severity.toProto()).build();
+      if (null != exceptions) {
+        builder.setHasExceptions(true);
+        builder.addAllExceptions(exceptions);
+      } else {
+        builder.setHasExceptions(false);
+      }
+
+      if (null != errorMessage) {
+        builder.setErrorMessage(errorMessage);
+      }
+
+      if (null != sqlState) {
+        builder.setSqlState(sqlState);
+      }
+
+      if (null != severity) {
+        builder.setSeverity(severity.toProto());
+      }
+
+      return builder.setErrorCode(errorCode).build();
+    }
+
+    @Override public String toString() {
+      StringBuilder sb = new StringBuilder(32);
+      sb.append("ErrorResponse[errorCode=").append(errorCode)
+          .append(", sqlState=").append(sqlState)
+          .append(", severity=").append(severity)
+          .append(", errorMessage=").append(errorMessage)
+          .append(", exceptions=").append(exceptions);
+      return sb.toString();
     }
 
     @Override public int hashCode() {
       final int prime = 31;
       int result = 1;
       result = prime * result + ((exceptions == null) ? 0 : exceptions.hashCode());
+      result = prime * result + ((errorMessage == null) ? 0 : errorMessage.hashCode());
       result = prime * result + errorCode;
       result = prime * result + ((sqlState == null) ? 0 : sqlState.hashCode());
       result = prime * result + ((severity == null) ? 0 : severity.hashCode());

http://git-wip-us.apache.org/repos/asf/calcite/blob/cd58fe73/avatica/src/main/protobuf/responses.proto
----------------------------------------------------------------------
diff --git a/avatica/src/main/protobuf/responses.proto b/avatica/src/main/protobuf/responses.proto
index 98ccf7f..65c702e 100644
--- a/avatica/src/main/protobuf/responses.proto
+++ b/avatica/src/main/protobuf/responses.proto
@@ -96,6 +96,7 @@ message DatabasePropertyResponse {
 // Send contextual information about some error over the wire from the server.
 message ErrorResponse {
   repeated string exceptions = 1; // exception stacktraces, many for linked exceptions.
+  bool has_exceptions = 7; // are there stacktraces contained?
   string error_message = 2; // human readable description
   Severity severity = 3;
   uint32 error_code = 4; // numeric identifier for error

http://git-wip-us.apache.org/repos/asf/calcite/blob/cd58fe73/avatica/src/test/java/org/apache/calcite/avatica/remote/ProtobufTranslationImplTest.java
----------------------------------------------------------------------
diff --git a/avatica/src/test/java/org/apache/calcite/avatica/remote/ProtobufTranslationImplTest.java b/avatica/src/test/java/org/apache/calcite/avatica/remote/ProtobufTranslationImplTest.java
index 8e0d738..998bba4 100644
--- a/avatica/src/test/java/org/apache/calcite/avatica/remote/ProtobufTranslationImplTest.java
+++ b/avatica/src/test/java/org/apache/calcite/avatica/remote/ProtobufTranslationImplTest.java
@@ -319,6 +319,11 @@ public class ProtobufTranslationImplTest<T> {
     // More results, no missing statement
     responses.add(new SyncResultsResponse(true, false, rpcMetadata));
 
+    // Some tests to make sure ErrorResponse doesn't fail.
+    responses.add(new ErrorResponse((List<String>) null, null, 0, null, null, null));
+    responses.add(new ErrorResponse(Arrays.asList("stacktrace1", "stacktrace2"), null, 0, null,
+        null, null));
+
     return responses;
   }