ARROW-6472: [Java] ValueVector#accept may has potential cast exception

alippai · Sep 24, 2019 · 12e4aa2 · 12e4aa2
1 parent 35b6d07
commit 12e4aa2
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 6 deletions.
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/compare/ApproxEqualsVisitor.java b/java/vector/src/main/java/org/apache/arrow/vector/compare/ApproxEqualsVisitor.java
@@ -69,8 +69,14 @@ public void setDoubleDiffFunction(DiffFunctionDouble doubleDiffFunction) {
   @Override
   public Boolean visit(BaseFixedWidthVector left, Range range) {
     if (left instanceof Float4Vector) {
+      if (!validate(left)) {
+        return false;
+      }
       return float4ApproxEquals(range);
     } else if (left instanceof Float8Vector) {
+      if (!validate(left)) {
+        return false;
+      }
       return float8ApproxEquals(range);
     } else {
       return super.visit(left, range);

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/compare/RangeEqualsVisitor.java b/java/vector/src/main/java/org/apache/arrow/vector/compare/RangeEqualsVisitor.java
@@ -58,7 +58,11 @@ public RangeEqualsVisitor(ValueVector left, ValueVector right, boolean isTypeChe
     Preconditions.checkArgument(right != null,
         "right vector cannot be null");
 
-    // types cannot change for a visitor instance. so, the check is done only once.
+    // type usually checks only once unless the left vector is changed.
+    checkType();
+  }
+
+  private void checkType() {
     if (!isTypeCheckNeeded) {
       typeCompareResult = true;
     } else if (left == right) {
@@ -68,6 +72,17 @@ public RangeEqualsVisitor(ValueVector left, ValueVector right, boolean isTypeChe
     }
   }
 
+  /**
+   * Validate the passed left vector, if it is changed, reset and check type.
+   */
+  protected boolean validate(ValueVector left) {
+    if (left != this.left) {
+      this.left = left;
+      checkType();
+    }
+    return typeCompareResult;
+  }
+
   /**
    * Constructs a new instance.
    *
@@ -79,7 +94,7 @@ public RangeEqualsVisitor(ValueVector left, ValueVector right) {
   }
 
   /**
-   * Check range equals without passing IN param in VectorVisitor.
+   * Check range equals.
    */
   public boolean rangeEquals(Range range) {
     if (!typeCompareResult) {
@@ -107,42 +122,59 @@ public ValueVector getRight() {
     return right;
   }
 
-  public boolean isTypeCheckNeeded() {
-    return isTypeCheckNeeded;
-  }
-
   @Override
   public Boolean visit(BaseFixedWidthVector left, Range range) {
+    if (!validate(left)) {
+      return false;
+    }
     return compareBaseFixedWidthVectors(range);
   }
 
   @Override
   public Boolean visit(BaseVariableWidthVector left, Range range) {
+    if (!validate(left)) {
+      return false;
+    }
     return compareBaseVariableWidthVectors(range);
   }
 
   @Override
   public Boolean visit(ListVector left, Range range) {
+    if (!validate(left)) {
+      return false;
+    }
     return compareListVectors(range);
   }
 
   @Override
   public Boolean visit(FixedSizeListVector left, Range range) {
+    if (!validate(left)) {
+      return false;
+    }
     return compareFixedSizeListVectors(range);
   }
 
   @Override
   public Boolean visit(NonNullableStructVector left, Range range) {
+    if (!validate(left)) {
+      return false;
+    }
     return compareStructVectors(range);
   }
 
   @Override
   public Boolean visit(UnionVector left, Range range) {
+    if (!validate(left)) {
+      return false;
+    }
     return compareUnionVectors(range);
   }
 
   @Override
   public Boolean visit(ZeroVector left, Range range) {
+    if (!validate(left)) {
+      return false;
+    }
     return true;
   }