From 073bc78d957d3b3571c3976a6e420b007ad47224 Mon Sep 17 00:00:00 2001 From: Pindikura Ravindra Date: Sat, 31 Aug 2019 16:10:36 +0530 Subject: [PATCH] Test: Move out Range from the visitor params --- .../deduplicate/DeduplicationUtils.java | 14 +- .../vector/compare/ApproxEqualsVisitor.java | 49 ++-- .../apache/arrow/vector/compare/Range.java | 85 +++++++ .../vector/compare/RangeEqualsParameter.java | 146 ----------- .../vector/compare/RangeEqualsVisitor.java | 233 ++++++++++-------- .../vector/compare/VectorEqualsVisitor.java | 32 ++- .../dictionary/DictionaryHashTable.java | 17 +- .../apache/arrow/vector/TestValueVector.java | 11 +- .../compare/TestRangeEqualsVisitor.java | 135 +++------- 9 files changed, 309 insertions(+), 413 deletions(-) create mode 100644 java/vector/src/main/java/org/apache/arrow/vector/compare/Range.java delete mode 100644 java/vector/src/main/java/org/apache/arrow/vector/compare/RangeEqualsParameter.java diff --git a/java/algorithm/src/main/java/org/apache/arrow/algorithm/deduplicate/DeduplicationUtils.java b/java/algorithm/src/main/java/org/apache/arrow/algorithm/deduplicate/DeduplicationUtils.java index 8ae23d0bd35..8c0c4af502d 100644 --- a/java/algorithm/src/main/java/org/apache/arrow/algorithm/deduplicate/DeduplicationUtils.java +++ b/java/algorithm/src/main/java/org/apache/arrow/algorithm/deduplicate/DeduplicationUtils.java @@ -22,7 +22,7 @@ import org.apache.arrow.vector.BitVectorHelper; import org.apache.arrow.vector.IntVector; import org.apache.arrow.vector.ValueVector; -import org.apache.arrow.vector.compare.RangeEqualsParameter; +import org.apache.arrow.vector.compare.Range; import org.apache.arrow.vector.compare.RangeEqualsVisitor; import io.netty.buffer.ArrowBuf; @@ -44,15 +44,11 @@ public static void populateRunStartIndicators(V vector, runStarts.setZero(0, bufSize); BitVectorHelper.setValidityBitToOne(runStarts, 0); - RangeEqualsVisitor visitor = new RangeEqualsVisitor(); - RangeEqualsParameter param = new RangeEqualsParameter() - .setRight(vector) - .setLeft(vector) - .setLength(1) - .setTypeCheckNeeded(false); + RangeEqualsVisitor visitor = new RangeEqualsVisitor(vector, vector, false); + Range range = new Range(0, 0, 1); for (int i = 1; i < vector.getValueCount(); i++) { - param.setLeftStart(i).setRightStart(i - 1); - if (!visitor.rangeEquals(param)) { + range.setLeftStart(i).setRightStart(i - 1); + if (!visitor.rangeEquals(range)) { BitVectorHelper.setValidityBitToOne(runStarts, i); } } 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 index b8924eac5cb..1c12768547f 100644 --- 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 @@ -20,6 +20,7 @@ import org.apache.arrow.vector.BaseFixedWidthVector; import org.apache.arrow.vector.Float4Vector; import org.apache.arrow.vector.Float8Vector; +import org.apache.arrow.vector.ValueVector; /** * Visitor to compare floating point. @@ -40,7 +41,16 @@ public class ApproxEqualsVisitor extends RangeEqualsVisitor { private DiffFunction doubleDiffFunction = (Double value1, Double value2) -> Math.abs(value1 - value2); - public ApproxEqualsVisitor(float floatEpsilon, double doubleEpsilon) { + /** + * Constructs a new instance. + * + * @param left left vector + * @param right right vector + * @param floatEpsilon difference for float values + * @param doubleEpsilon difference for double values + */ + public ApproxEqualsVisitor(ValueVector left, ValueVector right, float floatEpsilon, double doubleEpsilon) { + super(left, right, true); this.floatEpsilon = floatEpsilon; this.doubleEpsilon = doubleEpsilon; } @@ -54,29 +64,28 @@ public void setDoubleDiffFunction(DiffFunction doubleDiffFunction) { } @Override - public Boolean visit(BaseFixedWidthVector left, RangeEqualsParameter parameter) { - parameter.setLeft(left); + public Boolean visit(BaseFixedWidthVector left, Range range) { if (left instanceof Float4Vector) { - return parameter.validate() && float4ApproxEquals(parameter); + return float4ApproxEquals(range); } else if (left instanceof Float8Vector) { - return parameter.validate() && float8ApproxEquals(parameter); + return float8ApproxEquals(range); } else { - return super.visit(left, parameter); + return super.visit(left, range); } } @Override - protected ApproxEqualsVisitor createInnerVisitor() { - return new ApproxEqualsVisitor(floatEpsilon, doubleEpsilon); + protected ApproxEqualsVisitor createInnerVisitor(ValueVector left, ValueVector right) { + return new ApproxEqualsVisitor(left, right, floatEpsilon, doubleEpsilon); } - private boolean float4ApproxEquals(RangeEqualsParameter parameter) { - Float4Vector leftVector = (Float4Vector) parameter.getLeft(); - Float4Vector rightVector = (Float4Vector) parameter.getRight(); + private boolean float4ApproxEquals(Range range) { + Float4Vector leftVector = (Float4Vector) getLeft(); + Float4Vector rightVector = (Float4Vector) getRight(); - for (int i = 0; i < parameter.getLength(); i++) { - int leftIndex = parameter.getLeftStart() + i; - int rightIndex = parameter.getRightStart() + i; + for (int i = 0; i < range.getLength(); i++) { + int leftIndex = range.getLeftStart() + i; + int rightIndex = range.getRightStart() + i; boolean isNull = leftVector.isNull(leftIndex); @@ -101,13 +110,13 @@ private boolean float4ApproxEquals(RangeEqualsParameter parameter) { return true; } - private boolean float8ApproxEquals(RangeEqualsParameter parameter) { - Float8Vector leftVector = (Float8Vector) parameter.getLeft(); - Float8Vector rightVector = (Float8Vector) parameter.getRight(); + private boolean float8ApproxEquals(Range range) { + Float8Vector leftVector = (Float8Vector) getLeft(); + Float8Vector rightVector = (Float8Vector) getRight(); - for (int i = 0; i < parameter.getLength(); i++) { - int leftIndex = parameter.getLeftStart() + i; - int rightIndex = parameter.getRightStart() + i; + for (int i = 0; i < range.getLength(); i++) { + int leftIndex = range.getLeftStart() + i; + int rightIndex = range.getRightStart() + i; boolean isNull = leftVector.isNull(leftIndex); diff --git a/java/vector/src/main/java/org/apache/arrow/vector/compare/Range.java b/java/vector/src/main/java/org/apache/arrow/vector/compare/Range.java new file mode 100644 index 00000000000..0de99ab011f --- /dev/null +++ b/java/vector/src/main/java/org/apache/arrow/vector/compare/Range.java @@ -0,0 +1,85 @@ +/* + * 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.arrow.vector.compare; + +/** + * Wrapper for the parameters of comparing a range of values in two vectors. + */ +public class Range { + + /** + * Start position in the left vector. + */ + private int leftStart = -1; + + /** + * Start position in the right vector. + */ + private int rightStart = -1; + + /** + * Length of the range. + */ + private int length = -1; + + + /** + * Constructs a new instance. + */ + public Range() {} + + /** + * Constructs a new instance. + * + * @param leftStart start index in left vector + * @param rightStart start index in right vector + * @param length length of range + */ + public Range(int leftStart, int rightStart, int length) { + this.leftStart = leftStart; + this.rightStart = rightStart; + this.length = length; + } + + public int getLeftStart() { + return leftStart; + } + + public int getRightStart() { + return rightStart; + } + + public int getLength() { + return length; + } + + public Range setLeftStart(int leftStart) { + this.leftStart = leftStart; + return this; + } + + public Range setRightStart(int rightStart) { + this.rightStart = rightStart; + return this; + } + + public Range setLength(int length) { + this.length = length; + return this; + } +} diff --git a/java/vector/src/main/java/org/apache/arrow/vector/compare/RangeEqualsParameter.java b/java/vector/src/main/java/org/apache/arrow/vector/compare/RangeEqualsParameter.java deleted file mode 100644 index e1a58ce24e8..00000000000 --- a/java/vector/src/main/java/org/apache/arrow/vector/compare/RangeEqualsParameter.java +++ /dev/null @@ -1,146 +0,0 @@ -/* - * 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.arrow.vector.compare; - -import org.apache.arrow.util.Preconditions; -import org.apache.arrow.vector.ValueVector; - -/** - * Wrapper for the parameters of comparing a range of values in two vectors. - */ -public class RangeEqualsParameter { - - /** - * The left vector. - */ - private ValueVector left; - - /** - * The right vector. - */ - private ValueVector right; - - /** - * Start position in the left vector. - */ - private int leftStart = -1; - - /** - * Start position in the right vector. - */ - private int rightStart = -1; - - /** - * Length of the range. - */ - private int length = -1; - - /** - * If type check is required. - */ - private boolean typeCheckNeeded; - - /** - * Checks the type of the left and right vectors. - * @return true if left and right vectors have the same type, and false otherwise. - */ - public boolean checkType() { - if (!typeCheckNeeded) { - return true; - } - return left.getField().getType().equals(right.getField().getType()); - } - - /** - * Do some validation work, like type check and indices check. - */ - public boolean validate() { - if (!checkType()) { - return false; - } - - Preconditions.checkArgument(leftStart >= 0, - "leftStart %s must be non negative.", leftStart); - Preconditions.checkArgument((leftStart + length) <= left.getValueCount(), - "(leftStart + length) %s out of range[0, %s].", 0, left.getValueCount()); - Preconditions.checkArgument(rightStart >= 0, - "rightStart %s must be non negative.", rightStart); - Preconditions.checkArgument((rightStart + length) <= right.getValueCount(), - "(rightStart + length) %s out of range[0, %s].", 0, right.getValueCount()); - Preconditions.checkArgument(left != null, - "left vector cannot be null"); - Preconditions.checkArgument(right != null, - "right vector cannot be null"); - - return true; - } - - public ValueVector getLeft() { - return left; - } - - public ValueVector getRight() { - return right; - } - - public int getLeftStart() { - return leftStart; - } - - public int getRightStart() { - return rightStart; - } - - public int getLength() { - return length; - } - - public boolean isTypeCheckNeeded() { - return typeCheckNeeded; - } - - public RangeEqualsParameter setLeft(ValueVector left) { - this.left = left; - return this; - } - - public RangeEqualsParameter setRight(ValueVector right) { - this.right = right; - return this; - } - - public RangeEqualsParameter setLeftStart(int leftStart) { - this.leftStart = leftStart; - return this; - } - - public RangeEqualsParameter setRightStart(int rightStart) { - this.rightStart = rightStart; - return this; - } - - public RangeEqualsParameter setLength(int length) { - this.length = length; - return this; - } - - public RangeEqualsParameter setTypeCheckNeeded(boolean typeCheckNeeded) { - this.typeCheckNeeded = typeCheckNeeded; - return this; - } -} 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 index 3a7d1f2c05c..5d43031ffb5 100644 --- 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 @@ -20,9 +20,11 @@ import java.util.List; import org.apache.arrow.memory.util.ByteFunctionHelpers; +import org.apache.arrow.util.Preconditions; import org.apache.arrow.vector.BaseFixedWidthVector; import org.apache.arrow.vector.BaseVariableWidthVector; import org.apache.arrow.vector.FieldVector; +import org.apache.arrow.vector.ValueVector; import org.apache.arrow.vector.ZeroVector; import org.apache.arrow.vector.complex.BaseRepeatedValueVector; import org.apache.arrow.vector.complex.FixedSizeListVector; @@ -33,55 +35,115 @@ /** * Visitor to compare a range of values for vectors. */ -public class RangeEqualsVisitor implements VectorVisitor { +public class RangeEqualsVisitor implements VectorVisitor { + private ValueVector left; + private ValueVector right; + private boolean isTypeCheckNeeded; + private boolean typeCompareResult; + + /** + * Constructs a new instance. + * + * @param left left vector + * @param right right vector + * @param isTypeCheckNeeded type check needed + */ + public RangeEqualsVisitor(ValueVector left, ValueVector right, boolean isTypeCheckNeeded) { + this.left = left; + this.right = right; + this.isTypeCheckNeeded = isTypeCheckNeeded; + + Preconditions.checkArgument(left != null, + "left vector cannot be null"); + Preconditions.checkArgument(right != null, + "right vector cannot be null"); + + // types cannot change for a visitor instance. so, the check is done only once. + if (!isTypeCheckNeeded) { + typeCompareResult = true; + } else if (left == right) { + typeCompareResult = true; + } else { + typeCompareResult = left.getField().getType().equals(right.getField().getType()); + } + } + + /** + * Constructs a new instance. + * + * @param left left vector + * @param right right vector + */ + public RangeEqualsVisitor(ValueVector left, ValueVector right) { + this(left, right, true); + } /** * Check range equals without passing IN param in VectorVisitor. */ - public boolean rangeEquals(RangeEqualsParameter parameter) { - return parameter.getLeft().accept(this, parameter); + public boolean rangeEquals(Range range) { + if (!typeCompareResult) { + return false; + } + + Preconditions.checkArgument(range.getLeftStart() >= 0, + "leftStart %s must be non negative.", range.getLeftStart()); + Preconditions.checkArgument(range.getRightStart() >= 0, + "rightStart %s must be non negative.", range.getRightStart()); + + Preconditions.checkArgument(range.getRightStart() + range.getLength() <= right.getValueCount(), + "(rightStart + length) %s out of range[0, %s].", 0, right.getValueCount()); + Preconditions.checkArgument(range.getLeftStart() + range.getLength() <= left.getValueCount(), + "(leftStart + length) %s out of range[0, %s].", 0, left.getValueCount()); + + return left.accept(this, range); + } + + public ValueVector getLeft() { + return left; + } + + public ValueVector getRight() { + return right; + } + + public boolean isTypeCheckNeeded() { + return isTypeCheckNeeded; } @Override - public Boolean visit(BaseFixedWidthVector left, RangeEqualsParameter parameter) { - parameter.setLeft(left); - return parameter.validate() && compareBaseFixedWidthVectors(parameter); + public Boolean visit(BaseFixedWidthVector left, Range range) { + return compareBaseFixedWidthVectors(range); } @Override - public Boolean visit(BaseVariableWidthVector left, RangeEqualsParameter parameter) { - parameter.setLeft(left); - return parameter.validate() && compareBaseVariableWidthVectors(parameter); + public Boolean visit(BaseVariableWidthVector left, Range range) { + return compareBaseVariableWidthVectors(range); } @Override - public Boolean visit(ListVector left, RangeEqualsParameter parameter) { - parameter.setLeft(left); - return parameter.validate() && compareListVectors(parameter); + public Boolean visit(ListVector left, Range range) { + return compareListVectors(range); } @Override - public Boolean visit(FixedSizeListVector left, RangeEqualsParameter parameter) { - parameter.setLeft(left); - return parameter.validate() && compareFixedSizeListVectors(parameter); + public Boolean visit(FixedSizeListVector left, Range range) { + return compareFixedSizeListVectors(range); } @Override - public Boolean visit(NonNullableStructVector left, RangeEqualsParameter parameter) { - parameter.setLeft(left); - return parameter.validate() && compareStructVectors(parameter); + public Boolean visit(NonNullableStructVector left, Range range) { + return compareStructVectors(range); } @Override - public Boolean visit(UnionVector left, RangeEqualsParameter parameter) { - parameter.setLeft(left); - return parameter.validate() && compareUnionVectors(parameter); + public Boolean visit(UnionVector left, Range range) { + return compareUnionVectors(range); } @Override - public Boolean visit(ZeroVector left, RangeEqualsParameter parameter) { - parameter.setLeft(left); - return parameter.validate(); + public Boolean visit(ZeroVector left, Range range) { + return true; } /** @@ -89,13 +151,13 @@ public Boolean visit(ZeroVector left, RangeEqualsParameter parameter) { * It is used for complex vector types. * @return the visitor for child vecors. */ - protected RangeEqualsVisitor createInnerVisitor() { - return new RangeEqualsVisitor(); + protected RangeEqualsVisitor createInnerVisitor(ValueVector leftInner, ValueVector rightInner) { + return new RangeEqualsVisitor(leftInner, rightInner, isTypeCheckNeeded); } - protected boolean compareUnionVectors(RangeEqualsParameter parameter) { - UnionVector leftVector = (UnionVector) parameter.getLeft(); - UnionVector rightVector = (UnionVector) parameter.getRight(); + protected boolean compareUnionVectors(Range range) { + UnionVector leftVector = (UnionVector) left; + UnionVector rightVector = (UnionVector) right; List leftChildren = leftVector.getChildrenFromFields(); List rightChildren = rightVector.getChildrenFromFields(); @@ -104,41 +166,27 @@ protected boolean compareUnionVectors(RangeEqualsParameter parameter) { return false; } - RangeEqualsVisitor visitor = createInnerVisitor(); - RangeEqualsParameter innerParam = new RangeEqualsParameter() - .setLeftStart(parameter.getLeftStart()) - .setRightStart(parameter.getRightStart()) - .setLength(parameter.getLength()) - .setTypeCheckNeeded(parameter.isTypeCheckNeeded()); - for (int k = 0; k < leftChildren.size(); k++) { - innerParam.setRight(rightChildren.get(k)); - if (!leftChildren.get(k).accept(visitor, innerParam)) { + RangeEqualsVisitor visitor = createInnerVisitor(leftChildren.get(k), rightChildren.get(k)); + if (!visitor.rangeEquals(range)) { return false; } } return true; } - protected boolean compareStructVectors(RangeEqualsParameter parameter) { - NonNullableStructVector leftVector = (NonNullableStructVector) parameter.getLeft(); - NonNullableStructVector rightVector = (NonNullableStructVector) parameter.getRight(); + protected boolean compareStructVectors(Range range) { + NonNullableStructVector leftVector = (NonNullableStructVector) left; + NonNullableStructVector rightVector = (NonNullableStructVector) right; List leftChildNames = leftVector.getChildFieldNames(); if (!leftChildNames.equals(rightVector.getChildFieldNames())) { return false; } - RangeEqualsVisitor visitor = createInnerVisitor(); - RangeEqualsParameter innerParam = new RangeEqualsParameter() - .setLeftStart(parameter.getLeftStart()) - .setRightStart(parameter.getRightStart()) - .setLength(parameter.getLength()) - .setTypeCheckNeeded(parameter.isTypeCheckNeeded()); - for (String name : leftChildNames) { - innerParam.setRight(rightVector.getChild(name)); - if (!leftVector.getChild(name).accept(visitor, innerParam)) { + RangeEqualsVisitor visitor = createInnerVisitor(leftVector.getChild(name), rightVector.getChild(name)); + if (!visitor.rangeEquals(range)) { return false; } } @@ -146,17 +194,17 @@ protected boolean compareStructVectors(RangeEqualsParameter parameter) { return true; } - protected boolean compareBaseFixedWidthVectors(RangeEqualsParameter parameter) { - BaseFixedWidthVector leftVector = (BaseFixedWidthVector) parameter.getLeft(); - BaseFixedWidthVector rightVector = (BaseFixedWidthVector) parameter.getRight(); + protected boolean compareBaseFixedWidthVectors(Range range) { + BaseFixedWidthVector leftVector = (BaseFixedWidthVector) left; + BaseFixedWidthVector rightVector = (BaseFixedWidthVector) right; - for (int i = 0; i < parameter.getLength(); i++) { - int leftIndex = parameter.getLeftStart() + i; - int rightIndex = parameter.getRightStart() + i; + for (int i = 0; i < range.getLength(); i++) { + int leftIndex = range.getLeftStart() + i; + int rightIndex = range.getRightStart() + i; - boolean isNull = parameter.getLeft().isNull(leftIndex); + boolean isNull = leftVector.isNull(leftIndex); - if (isNull != parameter.getRight().isNull(rightIndex)) { + if (isNull != rightVector.isNull(rightIndex)) { return false; } @@ -179,13 +227,13 @@ protected boolean compareBaseFixedWidthVectors(RangeEqualsParameter parameter) { return true; } - protected boolean compareBaseVariableWidthVectors(RangeEqualsParameter parameter) { - BaseVariableWidthVector leftVector = (BaseVariableWidthVector) parameter.getLeft(); - BaseVariableWidthVector rightVector = (BaseVariableWidthVector) parameter.getRight(); + protected boolean compareBaseVariableWidthVectors(Range range) { + BaseVariableWidthVector leftVector = (BaseVariableWidthVector) left; + BaseVariableWidthVector rightVector = (BaseVariableWidthVector) right; - for (int i = 0; i < parameter.getLength(); i++) { - int leftIndex = parameter.getLeftStart() + i; - int rightIndex = parameter.getRightStart() + i; + for (int i = 0; i < range.getLength(); i++) { + int leftIndex = range.getLeftStart() + i; + int rightIndex = range.getRightStart() + i; boolean isNull = leftVector.isNull(leftIndex); if (isNull != rightVector.isNull(rightIndex)) { @@ -212,19 +260,16 @@ protected boolean compareBaseVariableWidthVectors(RangeEqualsParameter parameter return true; } - protected boolean compareListVectors(RangeEqualsParameter parameter) { - ListVector leftVector = (ListVector) parameter.getLeft(); - ListVector rightVector = (ListVector) parameter.getRight(); + protected boolean compareListVectors(Range range) { + ListVector leftVector = (ListVector) left; + ListVector rightVector = (ListVector) right; - RangeEqualsVisitor visitor = createInnerVisitor(); - RangeEqualsParameter innerParam = new RangeEqualsParameter() - .setLeft(leftVector.getDataVector()) - .setRight(rightVector.getDataVector()) - .setTypeCheckNeeded(parameter.isTypeCheckNeeded()); + RangeEqualsVisitor innerVisitor = createInnerVisitor(leftVector.getDataVector(), rightVector.getDataVector()); + Range innerRange = new Range(); - for (int i = 0; i < parameter.getLength(); i++) { - int leftIndex = parameter.getLeftStart() + i; - int rightIndex = parameter.getRightStart() + i; + for (int i = 0; i < range.getLength(); i++) { + int leftIndex = range.getLeftStart() + i; + int rightIndex = range.getRightStart() + i; boolean isNull = leftVector.isNull(leftIndex); if (isNull != rightVector.isNull(rightIndex)) { @@ -244,10 +289,11 @@ protected boolean compareListVectors(RangeEqualsParameter parameter) { return false; } - innerParam.setRightStart(startIndexRight) - .setLeftStart(startIndexLeft) - .setLength(endIndexLeft - startIndexLeft); - if (!leftVector.getDataVector().accept(visitor, innerParam)) { + innerRange = innerRange + .setRightStart(startIndexRight) + .setLeftStart(startIndexLeft) + .setLength(endIndexLeft - startIndexLeft); + if (!innerVisitor.rangeEquals(innerRange)) { return false; } } @@ -255,31 +301,27 @@ protected boolean compareListVectors(RangeEqualsParameter parameter) { return true; } - protected boolean compareFixedSizeListVectors(RangeEqualsParameter parameter) { - FixedSizeListVector leftVector = (FixedSizeListVector) parameter.getLeft(); - FixedSizeListVector rightVector = (FixedSizeListVector) parameter.getRight(); + protected boolean compareFixedSizeListVectors(Range range) { + FixedSizeListVector leftVector = (FixedSizeListVector) left; + FixedSizeListVector rightVector = (FixedSizeListVector) right; if (leftVector.getListSize() != rightVector.getListSize()) { return false; } - RangeEqualsVisitor visitor = createInnerVisitor(); - RangeEqualsParameter innerParam = new RangeEqualsParameter() - .setLeft(leftVector.getDataVector()) - .setRight(rightVector.getDataVector()) - .setTypeCheckNeeded(parameter.isTypeCheckNeeded()); + int listSize = leftVector.getListSize(); + RangeEqualsVisitor innerVisitor = createInnerVisitor(leftVector, rightVector); + Range innerRange = new Range(0, 0, listSize); - for (int i = 0; i < parameter.getLength(); i++) { - int leftIndex = parameter.getLeftStart() + i; - int rightIndex = parameter.getRightStart() + i; + for (int i = 0; i < range.getLength(); i++) { + int leftIndex = range.getLeftStart() + i; + int rightIndex = range.getRightStart() + i; boolean isNull = leftVector.isNull(leftIndex); if (isNull != rightVector.isNull(rightIndex)) { return false; } - int listSize = leftVector.getListSize(); - if (!isNull) { final int startIndexLeft = leftIndex * listSize; final int endIndexLeft = (leftIndex + 1) * listSize; @@ -291,10 +333,9 @@ protected boolean compareFixedSizeListVectors(RangeEqualsParameter parameter) { return false; } - innerParam.setRightStart(startIndexRight) - .setLeftStart(startIndexLeft) - .setLength(endIndexLeft - startIndexLeft); - if (!leftVector.getDataVector().accept(visitor, innerParam)) { + innerRange = innerRange.setLeftStart(startIndexLeft) + .setRightStart(startIndexRight); + if (!innerVisitor.rangeEquals(innerRange)) { return false; } } diff --git a/java/vector/src/main/java/org/apache/arrow/vector/compare/VectorEqualsVisitor.java b/java/vector/src/main/java/org/apache/arrow/vector/compare/VectorEqualsVisitor.java index 31383afc64f..2ba9509a825 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/compare/VectorEqualsVisitor.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/compare/VectorEqualsVisitor.java @@ -22,35 +22,31 @@ /** * Visitor to compare vectors equal. */ -public class VectorEqualsVisitor extends RangeEqualsVisitor { +public class VectorEqualsVisitor { - private final boolean typeCheckNeeded; - - public VectorEqualsVisitor() { - this(true); - } - - public VectorEqualsVisitor(boolean typeCheckNeeded) { - this.typeCheckNeeded = typeCheckNeeded; + /** + * Checks if two vectors are equals. + * @param left the left vector to compare. + * @param right the right vector to compare. + * @return true if the vectors are equal, and false otherwise. + */ + public static boolean vectorEquals(ValueVector left, ValueVector right) { + return vectorEquals(left, right, true); } /** * Checks if two vectors are equals. * @param left the left vector to compare. * @param right the right vector to compare. + * @param isTypeCheckNeeded check equality of types * @return true if the vectors are equal, and false otherwise. */ - public boolean vectorEquals(ValueVector left, ValueVector right) { + public static boolean vectorEquals(ValueVector left, ValueVector right, boolean isTypeCheckNeeded) { if (left.getValueCount() != right.getValueCount()) { return false; } - RangeEqualsParameter param = new RangeEqualsParameter() - .setLeft(left) - .setRight(right) - .setLeftStart(0) - .setRightStart(0) - .setLength(left.getValueCount()) - .setTypeCheckNeeded(typeCheckNeeded); - return super.rangeEquals(param); + + RangeEqualsVisitor visitor = new RangeEqualsVisitor(left, right, isTypeCheckNeeded); + return visitor.rangeEquals(new Range(0, 0, left.getValueCount())); } } diff --git a/java/vector/src/main/java/org/apache/arrow/vector/dictionary/DictionaryHashTable.java b/java/vector/src/main/java/org/apache/arrow/vector/dictionary/DictionaryHashTable.java index 8847c8bfc51..c4e003d2998 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/dictionary/DictionaryHashTable.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/dictionary/DictionaryHashTable.java @@ -18,7 +18,7 @@ package org.apache.arrow.vector.dictionary; import org.apache.arrow.vector.ValueVector; -import org.apache.arrow.vector.compare.RangeEqualsParameter; +import org.apache.arrow.vector.compare.Range; import org.apache.arrow.vector.compare.RangeEqualsVisitor; /** @@ -138,19 +138,16 @@ public int getIndex(int indexInArray, ValueVector toEncode) { int hash = toEncode.hashCode(indexInArray); int index = indexFor(hash, table.length); - RangeEqualsVisitor equalVisitor = new RangeEqualsVisitor(); - RangeEqualsParameter param = new RangeEqualsParameter() - .setLeft(dictionary) - .setRight(toEncode) - .setTypeCheckNeeded(false) - .setLength(1); + RangeEqualsVisitor equalVisitor = new RangeEqualsVisitor(dictionary, toEncode, false); + Range range = new Range(0, 0, 1); for (DictionaryHashTable.Entry e = table[index]; e != null ; e = e.next) { if (e.hash == hash) { int dictIndex = e.index; - param.setRightStart(indexInArray) - .setLeftStart(dictIndex); - if (equalVisitor.rangeEquals(param)) { + + range = range.setRightStart(indexInArray) + .setLeftStart(dictIndex); + if (equalVisitor.rangeEquals(range)) { return dictIndex; } } diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java b/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java index 3f89b85fcc4..d2f9fc194da 100644 --- a/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java +++ b/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java @@ -36,7 +36,7 @@ import org.apache.arrow.memory.BufferAllocator; import org.apache.arrow.memory.RootAllocator; import org.apache.arrow.memory.util.ArrowBufPointer; -import org.apache.arrow.vector.compare.RangeEqualsParameter; +import org.apache.arrow.vector.compare.Range; import org.apache.arrow.vector.compare.RangeEqualsVisitor; import org.apache.arrow.vector.compare.VectorEqualsVisitor; import org.apache.arrow.vector.complex.ListVector; @@ -2635,14 +2635,7 @@ public void testEqualsWithIndexOutOfRange() { vector2.setSafe(0, 1); vector2.setSafe(1, 2); - RangeEqualsVisitor visitor = new RangeEqualsVisitor(); - RangeEqualsParameter param = new RangeEqualsParameter() - .setLeft(vector2) - .setRight(vector1) - .setRightStart(3) - .setLeftStart(2) - .setLength(1); - assertTrue(visitor.rangeEquals(param)); + assertTrue(new RangeEqualsVisitor(vector1, vector2).rangeEquals(new Range(2, 3, 1))); } } diff --git a/java/vector/src/test/java/org/apache/arrow/vector/compare/TestRangeEqualsVisitor.java b/java/vector/src/test/java/org/apache/arrow/vector/compare/TestRangeEqualsVisitor.java index 604d3f62ba6..04d73e231cd 100644 --- a/java/vector/src/test/java/org/apache/arrow/vector/compare/TestRangeEqualsVisitor.java +++ b/java/vector/src/test/java/org/apache/arrow/vector/compare/TestRangeEqualsVisitor.java @@ -81,9 +81,8 @@ public void testIntVectorEqualsWithNull() { vector1.setSafe(1, 2); vector2.setSafe(0, 1); - VectorEqualsVisitor visitor = new VectorEqualsVisitor(); - assertFalse(visitor.vectorEquals(vector1, vector2)); + assertFalse(VectorEqualsVisitor.vectorEquals(vector1, vector2)); } } @@ -109,14 +108,8 @@ public void testBaseFixedWidthVectorRangeEqual() { vector2.setSafe(3,4); vector2.setSafe(4,55); - RangeEqualsParameter param = new RangeEqualsParameter() - .setLeft(vector1) - .setRight(vector2) - .setLeftStart(1) - .setRightStart(1) - .setLength(3); - RangeEqualsVisitor visitor = new RangeEqualsVisitor(); - assertTrue(visitor.rangeEquals(param)); + RangeEqualsVisitor visitor = new RangeEqualsVisitor(vector1, vector2); + assertTrue(visitor.rangeEquals(new Range(1, 1, 3))); } } @@ -143,14 +136,8 @@ public void testBaseVariableVectorRangeEquals() { vector2.setSafe(4, STR1, 0, STR1.length); vector2.setValueCount(5); - RangeEqualsVisitor visitor = new RangeEqualsVisitor(); - RangeEqualsParameter param = new RangeEqualsParameter() - .setLeft(vector1) - .setRight(vector2) - .setLeftStart(1) - .setRightStart(1) - .setLength(3); - assertTrue(visitor.rangeEquals(param)); + RangeEqualsVisitor visitor = new RangeEqualsVisitor(vector1, vector2); + assertTrue(visitor.rangeEquals(new Range(1, 1, 3))); } } @@ -181,14 +168,8 @@ public void testListVectorRangeEquals() { writeListVector(writer2, new int[] {0, 0}); writer2.setValueCount(5); - RangeEqualsVisitor visitor = new RangeEqualsVisitor(); - RangeEqualsParameter param = new RangeEqualsParameter() - .setLeft(vector1) - .setRight(vector2) - .setLeftStart(1) - .setRightStart(1) - .setLength(3); - assertTrue(visitor.rangeEquals(param)); + RangeEqualsVisitor visitor = new RangeEqualsVisitor(vector1, vector2); + assertTrue(visitor.rangeEquals(new Range(1, 1, 3))); } } @@ -221,14 +202,8 @@ public void testStructVectorRangeEquals() { writeStructVector(writer2, 0, 0L); writer2.setValueCount(5); - RangeEqualsVisitor visitor = new RangeEqualsVisitor(); - RangeEqualsParameter param = new RangeEqualsParameter() - .setLeft(vector1) - .setRight(vector2) - .setLeftStart(1) - .setRightStart(1) - .setLength(3); - assertTrue(visitor.rangeEquals(param)); + RangeEqualsVisitor visitor = new RangeEqualsVisitor(vector1, vector2); + assertTrue(visitor.rangeEquals(new Range(1, 1, 3))); } } @@ -265,14 +240,8 @@ public void testUnionVectorRangeEquals() { vector2.setSafe(2, intHolder); vector2.setValueCount(3); - RangeEqualsVisitor visitor = new RangeEqualsVisitor(); - RangeEqualsParameter param = new RangeEqualsParameter() - .setLeft(vector1) - .setRight(vector2) - .setLeftStart(1) - .setRightStart(1) - .setLength(2); - assertTrue(visitor.rangeEquals(param)); + RangeEqualsVisitor visitor = new RangeEqualsVisitor(vector1, vector2); + assertTrue(visitor.rangeEquals(new Range(1, 1, 2))); } } @@ -282,11 +251,8 @@ public void testEqualsWithOutTypeCheck() { try (final IntVector intVector = new IntVector("int", allocator); final ZeroVector zeroVector = new ZeroVector()) { - VectorEqualsVisitor zeroVisitor = new VectorEqualsVisitor(false); - assertTrue(zeroVisitor.vectorEquals(intVector, zeroVector)); - - VectorEqualsVisitor intVisitor = new VectorEqualsVisitor(false); - assertTrue(intVisitor.vectorEquals(zeroVector, intVector)); + assertTrue(VectorEqualsVisitor.vectorEquals(intVector, zeroVector, false)); + assertTrue(VectorEqualsVisitor.vectorEquals(zeroVector, intVector, false)); } } @@ -314,18 +280,13 @@ public void testFloat4ApproxEquals() { vector3.setSafe(0, 1.1f + epsilon * 2); vector3.setSafe(1, 2.2f + epsilon * 2); - ApproxEqualsVisitor visitor = new ApproxEqualsVisitor(epsilon, epsilon); - RangeEqualsParameter param = new RangeEqualsParameter() - .setRight(vector1) - .setLeft(vector2) - .setLeftStart(0) - .setRightStart(0) - .setLength(vector1.getValueCount()); + Range range = new Range(0, 0, vector1.getValueCount()); - assertTrue(visitor.rangeEquals(param)); + ApproxEqualsVisitor visitor12 = new ApproxEqualsVisitor(vector1, vector2, epsilon, epsilon); + assertTrue(visitor12.rangeEquals(range)); - param.setLeft(vector3); - assertFalse(visitor.rangeEquals(param)); + ApproxEqualsVisitor visitor13 = new ApproxEqualsVisitor(vector1, vector3, epsilon, epsilon); + assertFalse(visitor13.rangeEquals(range)); } } @@ -353,18 +314,9 @@ public void testFloat8ApproxEquals() { vector3.setSafe(0, 1.1 + epsilon * 2); vector3.setSafe(1, 2.2 + epsilon * 2); - ApproxEqualsVisitor visitor = new ApproxEqualsVisitor(epsilon, epsilon); - RangeEqualsParameter param = new RangeEqualsParameter() - .setRight(vector1) - .setLeft(vector2) - .setLeftStart(0) - .setRightStart(0) - .setLength(vector1.getValueCount()); - - assertTrue(visitor.rangeEquals(param)); - - param.setLeft(vector3); - assertFalse(visitor.rangeEquals(param)); + Range range = new Range(0, 0, vector1.getValueCount()); + assertTrue(new ApproxEqualsVisitor(vector1, vector2, epsilon, epsilon).rangeEquals(range)); + assertFalse(new ApproxEqualsVisitor(vector1, vector3, epsilon, epsilon).rangeEquals(range)); } } @@ -407,18 +359,9 @@ public void testStructVectorApproxEquals() { writeStructVector(leftWriter2, 2.02f - epsilon * 2, 4.04 - epsilon * 2); leftWriter2.setValueCount(2); - ApproxEqualsVisitor visitor = new ApproxEqualsVisitor(epsilon, epsilon); - RangeEqualsParameter param = new RangeEqualsParameter() - .setRight(right) - .setLeft(left1) - .setLeftStart(0) - .setRightStart(0) - .setLength(right.getValueCount()); - - assertTrue(visitor.rangeEquals(param)); - - param.setLeft(left2); - assertFalse(visitor.rangeEquals(param)); + Range range = new Range(0, 0, right.getValueCount()); + assertTrue(new ApproxEqualsVisitor(left1, right, epsilon, epsilon).rangeEquals(range)); + assertFalse(new ApproxEqualsVisitor(left2, right, epsilon, epsilon).rangeEquals(range)); } } @@ -462,18 +405,9 @@ public void testUnionVectorApproxEquals() { left2.setSafe(1, float8Holder); left2.setValueCount(2); - ApproxEqualsVisitor visitor = new ApproxEqualsVisitor(epsilon, epsilon); - RangeEqualsParameter param = new RangeEqualsParameter() - .setRight(right) - .setLeft(left1) - .setLeftStart(0) - .setRightStart(0) - .setLength(right.getValueCount()); - - assertTrue(visitor.rangeEquals(param)); - - param.setLeft(left2); - assertFalse(visitor.rangeEquals(param)); + Range range = new Range(0, 0, right.getValueCount()); + assertTrue(new ApproxEqualsVisitor(left1, right, epsilon, epsilon).rangeEquals(range)); + assertFalse(new ApproxEqualsVisitor(left2, right, epsilon, epsilon).rangeEquals(range)); } } @@ -503,18 +437,9 @@ public void testListVectorApproxEquals() { writeListVector(leftWriter2, new double[] {1.01 + epsilon * 2, 2.02 - epsilon * 2}); leftWriter2.setValueCount(2); - ApproxEqualsVisitor visitor = new ApproxEqualsVisitor(epsilon, epsilon); - RangeEqualsParameter param = new RangeEqualsParameter() - .setRight(right) - .setLeft(left1) - .setLeftStart(0) - .setRightStart(0) - .setLength(right.getValueCount()); - - assertTrue(visitor.rangeEquals(param)); - - param.setLeft(left2); - assertFalse(visitor.rangeEquals(param)); + Range range = new Range(0, 0, right.getValueCount()); + assertTrue(new ApproxEqualsVisitor(left1, right, epsilon, epsilon).rangeEquals(range)); + assertFalse(new ApproxEqualsVisitor(left2, right, epsilon, epsilon).rangeEquals(range)); } }