Till Westmann has posted comments on this change. Change subject: [ASTERIXDB-2516][RT] add support for deep comparison 1 ......................................................................
Patch Set 1: (10 comments) https://asterix-gerrit.ics.uci.edu/#/c/3145/1/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/common/ILogicalBinaryComparator.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/common/ILogicalBinaryComparator.java: PS1, Line 35: EQUALITY COMPARE PS1, Line 36: INEQUALITY ORDER PS1, Line 39: returnResult "asResult" ? This is basically a cast from int to Result. Could also be a static "of" method in the result enum. PS1, Line 40: if (result == 0) { : return Result.EQ; : } else if (result < 0) { : return Result.LT; : } else { : return Result.GT; : } return result < 0 ? Result.LT : (result == 0 ? Result.EQ : Result.GT) PS1, Line 50: Operation operation Could this be part of the factory and not a parameter for the comparison? https://asterix-gerrit.ics.uci.edu/#/c/3145/1/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/common/LogicalComparatorUtil.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/common/LogicalComparatorUtil.java: PS1, Line 157: type == BuiltinType.ADOUBLE Could we switch on the type tag instead of comparing the object(-reference)s? https://asterix-gerrit.ics.uci.edu/#/c/3145/1/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/AbstractComparisonEvaluator.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/AbstractComparisonEvaluator.java: PS1, Line 109: if (leftConstant != null && rightConstant != null) { : // both are constants : return logicalComparator.compare(leftConstant, rightConstant, operation); : } else if (leftConstant != null) { : // left is constant, right isn't : return logicalComparator.compare(leftConstant, argRight.getByteArray(), argRight.getStartOffset(), : argRight.getLength(), operation); : } else if (rightConstant != null) { : // right is constant, left isn't : return logicalComparator.compare(argLeft.getByteArray(), argLeft.getStartOffset(), argLeft.getLength(), : rightConstant, operation); : } else { : return logicalComparator.compare(argLeft.getByteArray(), argLeft.getStartOffset(), argLeft.getLength(), : argRight.getByteArray(), argRight.getStartOffset(), argRight.getLength(), operation); : } if (leftConstant != null) { if (rightConstant != null) { // both are constants return logicalComparator.compare(leftConstant, rightConstant, operation); } else { // left is constant, right isn't return logicalComparator.compare(leftConstant, argRight.getByteArray(), argRight.getStartOffset(), argRight.getLength(), operation); } } else { if (rightConstant != null) { // right is constant, left isn't return logicalComparator.compare(argLeft.getByteArray(), argLeft.getStartOffset(), argLeft.getLength(), rightConstant, operation); } else { return logicalComparator.compare(argLeft.getByteArray(), argLeft.getStartOffset(), argLeft.getLength(), argRight.getByteArray(), argRight.getStartOffset(), argRight.getLength(), operation); } } https://asterix-gerrit.ics.uci.edu/#/c/3145/1/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/AbstractIfEqualsEvaluator.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/AbstractIfEqualsEvaluator.java: PS1, Line 41: Result comparisonResult = compare(); : if (comparisonResult == Result.MISSING) { : writeMissing(result); : } else if (comparisonResult == Result.NULL) { : writeNull(result); : } else if (comparisonResult == Result.EQ) { : resultStorage.reset(); : writeEqualsResult(); : result.set(resultStorage); : } else { : result.set(argLeft); : } switch(compare()) { case MISSING: writeMissing(result); break; case NULL: writeNull(result); break; case EQ: resultStorage.reset(); writeEqualsResult(); result.set(resultStorage); break; default: // do we need to do something about MISMATCH? result.set(argLeft); } https://asterix-gerrit.ics.uci.edu/#/c/3145/1/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/EqualsDescriptor.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/EqualsDescriptor.java: PS1, Line 39: private IAType leftType; : private IAType rightType; pull-up into superclass? PS1, Line 60: public void setImmutableStates(Object... types) { : leftType = (IAType) types[0]; : rightType = (IAType) types[1]; : } pull-up into superclass? -- To view, visit https://asterix-gerrit.ics.uci.edu/3145 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I623662861e6f3b1fdcff78f8edc0e3216ca10fe1 Gerrit-PatchSet: 1 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-HasComments: Yes
