diff --git a/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/predicates/ComparablePredicate.scala b/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/predicates/ComparablePredicate.scala index 67bfb41a4065..dcd178570020 100644 --- a/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/predicates/ComparablePredicate.scala +++ b/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/predicates/ComparablePredicate.scala @@ -22,7 +22,7 @@ package org.neo4j.cypher.internal.runtime.interpreted.commands.predicates import org.neo4j.cypher.internal.runtime.interpreted.ExecutionContext import org.neo4j.cypher.internal.runtime.interpreted.commands.expressions.{Expression, Literal, Variable} import org.neo4j.cypher.internal.runtime.interpreted.pipes.QueryState -import org.neo4j.values.AnyValues +import org.neo4j.values.{AnyValues, Comparison} import org.neo4j.values.storable._ abstract sealed class ComparablePredicate(val left: Expression, val right: Expression) extends Predicate { @@ -37,25 +37,25 @@ abstract sealed class ComparablePredicate(val left: Expression, val right: Expre else (l, r) match { case (d: FloatingPointValue, _) if d.doubleValue().isNaN => None case (_, d: FloatingPointValue) if d.doubleValue().isNaN => None - case (n1: NumberValue, n2: NumberValue) => compare(nullToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) - case (n1: TextValue, n2: TextValue) => compare(nullToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) - case (n1: BooleanValue, n2: BooleanValue) => compare(nullToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) - case (n1: PointValue, n2: PointValue) => compare(nullToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) - case (n1: DateValue, n2: DateValue) => compare(nullToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) - case (n1: LocalTimeValue, n2: LocalTimeValue) => compare(nullToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) - case (n1: TimeValue, n2: TimeValue) => compare(nullToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) - case (n1: LocalDateTimeValue, n2: LocalDateTimeValue) => compare(nullToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) - case (n1: DateTimeValue, n2: DateTimeValue) => compare(nullToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) + case (n1: NumberValue, n2: NumberValue) => compare(undefinedToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) + case (n1: TextValue, n2: TextValue) => compare(undefinedToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) + case (n1: BooleanValue, n2: BooleanValue) => compare(undefinedToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) + case (n1: PointValue, n2: PointValue) => compare(undefinedToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) + case (n1: DateValue, n2: DateValue) => compare(undefinedToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) + case (n1: LocalTimeValue, n2: LocalTimeValue) => compare(undefinedToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) + case (n1: TimeValue, n2: TimeValue) => compare(undefinedToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) + case (n1: LocalDateTimeValue, n2: LocalDateTimeValue) => compare(undefinedToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) + case (n1: DateTimeValue, n2: DateTimeValue) => compare(undefinedToNone(AnyValues.TERNARY_COMPARATOR.ternaryCompare(n1, n2))) case _ => None } res } - private def nullToNone(i: java.lang.Integer) : Option[Int] = { + private def undefinedToNone(i: Comparison) : Option[Int] = { // Do NOT use Option here (as suggested by the warning). // This would lead to NullPointerExceptions when i == null - if(i == null) None - else Some(i) + if(i == Comparison.UNDEFINED) None + else Some(i.value()) } def sign: String diff --git a/community/values/src/main/java/org/neo4j/values/AnyValueComparator.java b/community/values/src/main/java/org/neo4j/values/AnyValueComparator.java index 74a439d4a215..e8a6d40bf5b6 100644 --- a/community/values/src/main/java/org/neo4j/values/AnyValueComparator.java +++ b/community/values/src/main/java/org/neo4j/values/AnyValueComparator.java @@ -20,9 +20,9 @@ package org.neo4j.values; import java.util.Comparator; -import java.util.function.BiFunction; import org.neo4j.values.storable.Value; +import org.neo4j.values.storable.ValueComparator; import org.neo4j.values.storable.Values; import org.neo4j.values.virtual.VirtualValueGroup; @@ -31,19 +31,16 @@ */ class AnyValueComparator implements Comparator, TernaryComparator { - private final Comparator valueComparator; - private final TernaryComparator ternaryValueComparator; private final Comparator virtualValueGroupComparator; + private final ValueComparator valueComparator; - AnyValueComparator( Comparator valueComparator, TernaryComparator ternaryValueComparator, - Comparator virtualValueGroupComparator ) + AnyValueComparator( ValueComparator valueComparator, Comparator virtualValueGroupComparator ) { - this.valueComparator = valueComparator; - this.ternaryValueComparator = ternaryValueComparator; this.virtualValueGroupComparator = virtualValueGroupComparator; + this.valueComparator = valueComparator; } - private Integer cmp( AnyValue v1, AnyValue v2, BiFunction compareValues ) + private Comparison cmp( AnyValue v1, AnyValue v2, boolean ternary ) { assert v1 != null && v2 != null : "null values are not supported, use NoValue.NO_VALUE instead"; @@ -51,15 +48,15 @@ private Integer cmp( AnyValue v1, AnyValue v2, BiFunction // front if ( v1 == v2 ) { - return 0; + return Comparison.EQUAL; } if ( v1 == Values.NO_VALUE ) { - return 1; + return Comparison.GREATER_THAN; } if ( v2 == Values.NO_VALUE ) { - return -1; + return Comparison.SMALLER_THAN; } // We must handle sequences as a special case, as they can be both storable and virtual @@ -68,15 +65,15 @@ private Integer cmp( AnyValue v1, AnyValue v2, BiFunction if ( isSequence1 && isSequence2 ) { - return compareSequences( (SequenceValue)v1, (SequenceValue)v2 ); + return Comparison.from( compareSequences( (SequenceValue) v1, (SequenceValue) v2 ) ); } else if ( isSequence1 ) { - return compareSequenceAndNonSequence( (SequenceValue)v1, v2 ); + return Comparison.from( compareSequenceAndNonSequence( (SequenceValue) v1, v2 ) ); } else if ( isSequence2 ) { - return -compareSequenceAndNonSequence( (SequenceValue)v2, v1 ); + return Comparison.from( -compareSequenceAndNonSequence( (SequenceValue) v2, v1 ) ); } // Handle remaining AnyValues @@ -91,29 +88,35 @@ else if ( isSequence2 ) // Do not turn this into ?-operator if ( isValue1 ) { - // This return Integer (null possible!) - return compareValues.apply( (Value) v1, (Value) v2 ); + if ( ternary ) + { + return valueComparator.ternaryCompare( (Value) v1, (Value) v2 ); + } + else + { + return Comparison.from( valueComparator.compare( (Value) v1, (Value) v2 ) ); + } } else { // This returns int - return compareVirtualValues( (VirtualValue)v1, (VirtualValue)v2 ); + return Comparison.from( compareVirtualValues( (VirtualValue) v1, (VirtualValue) v2 ) ); } } - return x; + return Comparison.from( x ); } @Override public int compare( AnyValue v1, AnyValue v2 ) { - return cmp( v1, v2, valueComparator::compare ); + return cmp( v1, v2, false ).value(); } @Override - public Integer ternaryCompare( AnyValue v1, AnyValue v2 ) + public Comparison ternaryCompare( AnyValue v1, AnyValue v2 ) { - return cmp( v1, v2, ternaryValueComparator::ternaryCompare ); + return cmp( v1, v2, true ); } @Override @@ -151,7 +154,7 @@ private int compareSequenceAndNonSequence( SequenceValue v1, AnyValue v2 ) } else { - return virtualValueGroupComparator.compare( VirtualValueGroup.LIST, ((VirtualValue)v2).valueGroup() ); + return virtualValueGroupComparator.compare( VirtualValueGroup.LIST, ((VirtualValue) v2).valueGroup() ); } } diff --git a/community/values/src/main/java/org/neo4j/values/AnyValues.java b/community/values/src/main/java/org/neo4j/values/AnyValues.java index f88e51da1cbd..f4a4c3effea3 100644 --- a/community/values/src/main/java/org/neo4j/values/AnyValues.java +++ b/community/values/src/main/java/org/neo4j/values/AnyValues.java @@ -67,7 +67,7 @@ public final class AnyValues *
  • VOID (i.e. the type of null) * */ - private static final AnyValueComparator comp = new AnyValueComparator( Values.COMPARATOR, Values.TERNARY_COMPARATOR, VirtualValueGroup::compareTo ); + private static final AnyValueComparator comp = new AnyValueComparator( Values.COMPARATOR, VirtualValueGroup::compareTo ); public static final Comparator COMPARATOR = comp; public static final TernaryComparator TERNARY_COMPARATOR = comp; diff --git a/community/values/src/main/java/org/neo4j/values/Comparison.java b/community/values/src/main/java/org/neo4j/values/Comparison.java new file mode 100644 index 000000000000..b4cb0d35f435 --- /dev/null +++ b/community/values/src/main/java/org/neo4j/values/Comparison.java @@ -0,0 +1,103 @@ +/* + * Copyright (c) 2002-2018 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Neo4j is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.neo4j.values; + +/** + * Defines the result of a ternary comparison. + *

    + * In a ternary comparison the result may not only be greater than, equal or smaller than but the + * result can also be undefined. + */ +public enum Comparison +{ + GREATER_THAN + { + @Override + public int value() + { + return 1; + } + + }, + EQUAL + { + @Override + public int value() + { + return 0; + } + + }, + SMALLER_THAN + { + @Override + public int value() + { + return -1; + } + + }, + UNDEFINED + { + @Override + public int value() + { + throw new IllegalStateException( + "This value is undefined and can't handle primitive comparisons" ); + } + + }; + + /** + * Integer representation of comparison + *

    + * Returns a positive integer if {@link Comparison#GREATER_THAN} than, negative integer for + * {@link Comparison#SMALLER_THAN}, + * and zero for {@link Comparison#EQUAL} + * + * @return a positive number if result is greater than, a negative number if the result is smaller than or zero + * if equal. + * @throws IllegalStateException if the result is undefined. + */ + public abstract int value(); + + /** + * Maps an integer value to comparison result. + * + * @param i the integer to be mapped to a Comparison + * @return {@link Comparison#GREATER_THAN} than if positive, {@link Comparison#SMALLER_THAN} if negative or + * {@link Comparison#EQUAL} if zero + */ + public static Comparison from( int i ) + { + if ( i > 0 ) + { + return GREATER_THAN; + } + else if ( i < 0 ) + { + return SMALLER_THAN; + } + else + { + return EQUAL; + } + } +} diff --git a/community/values/src/main/java/org/neo4j/values/TernaryComparator.java b/community/values/src/main/java/org/neo4j/values/TernaryComparator.java index a43efc36bc88..2b5660b1a995 100644 --- a/community/values/src/main/java/org/neo4j/values/TernaryComparator.java +++ b/community/values/src/main/java/org/neo4j/values/TernaryComparator.java @@ -20,7 +20,7 @@ package org.neo4j.values; /** - * Comparator that allows {@code null}. + * Comparator that allows undefined comparison. */ @FunctionalInterface public interface TernaryComparator @@ -31,5 +31,5 @@ public interface TernaryComparator * 0 if o1 and o2 are equal, and * {@code null} if they are not comparable. */ - Integer ternaryCompare( T o1, T o2 ); + Comparison ternaryCompare( T o1, T o2 ); } diff --git a/community/values/src/main/java/org/neo4j/values/storable/PointValue.java b/community/values/src/main/java/org/neo4j/values/storable/PointValue.java index c9261bce0346..b34ef042bb0b 100644 --- a/community/values/src/main/java/org/neo4j/values/storable/PointValue.java +++ b/community/values/src/main/java/org/neo4j/values/storable/PointValue.java @@ -28,6 +28,7 @@ import org.neo4j.graphdb.spatial.Coordinate; import org.neo4j.graphdb.spatial.Point; import org.neo4j.values.AnyValue; +import org.neo4j.values.Comparison; import org.neo4j.values.ValueMapper; import org.neo4j.values.utils.InvalidValuesArgumentException; import org.neo4j.values.utils.PrettyPrinter; @@ -156,13 +157,13 @@ int unsafeCompareTo( Value otherValue ) } @Override - Integer unsafeTernaryCompareTo( Value otherValue ) + Comparison unsafeTernaryCompareTo( Value otherValue ) { PointValue other = (PointValue) otherValue; if ( this.crs.getCode() != other.crs.getCode() || this.coordinate.length != other.coordinate.length ) { - return null; + return Comparison.UNDEFINED; } int result = 0; @@ -173,13 +174,13 @@ Integer unsafeTernaryCompareTo( Value otherValue ) { if ( (cmpVal < 0 && result > 0) || (cmpVal > 0 && result < 0) ) { - return null; + return Comparison.UNDEFINED; } result = cmpVal; } } - return result; + return Comparison.from( result ); } @Override @@ -257,19 +258,17 @@ public boolean withinRange( PointValue lower, boolean includeLower, PointValue u { if ( lower != null ) { - Integer compareLower = this.unsafeTernaryCompareTo( lower ); - if ( compareLower == null || compareLower < 0 || compareLower == 0 && !includeLower ) + Comparison compareLower = this.unsafeTernaryCompareTo( lower ); + if ( compareLower == Comparison.UNDEFINED || compareLower == Comparison.SMALLER_THAN || compareLower == Comparison.EQUAL && !includeLower ) { return false; } } if ( upper != null ) { - Integer compareUpper = this.unsafeTernaryCompareTo( upper ); - if ( compareUpper == null || compareUpper > 0 || compareUpper == 0 && !includeUpper ) - { - return false; - } + Comparison compareUpper = this.unsafeTernaryCompareTo( upper ); + return compareUpper != Comparison.UNDEFINED && compareUpper != Comparison.GREATER_THAN && + (compareUpper != Comparison.EQUAL || includeUpper); } return true; } diff --git a/community/values/src/main/java/org/neo4j/values/storable/Value.java b/community/values/src/main/java/org/neo4j/values/storable/Value.java index 4c11b714f5ec..d46ca0eb4f3b 100644 --- a/community/values/src/main/java/org/neo4j/values/storable/Value.java +++ b/community/values/src/main/java/org/neo4j/values/storable/Value.java @@ -30,6 +30,7 @@ import org.neo4j.graphdb.spatial.Geometry; import org.neo4j.values.AnyValue; import org.neo4j.values.AnyValueWriter; +import org.neo4j.values.Comparison; import org.neo4j.values.SequenceValue; import org.neo4j.values.utils.InvalidValuesArgumentException; @@ -187,9 +188,9 @@ public Boolean ternaryEquals( AnyValue other ) * Should return {@code null} for values that cannot be compared * under Comparability semantics. */ - Integer unsafeTernaryCompareTo( Value other ) + Comparison unsafeTernaryCompareTo( Value other ) { - return unsafeCompareTo( other ); + return Comparison.from( unsafeCompareTo( other ) ); } @Override diff --git a/community/values/src/main/java/org/neo4j/values/storable/ValueComparator.java b/community/values/src/main/java/org/neo4j/values/storable/ValueComparator.java index cd41c2715eef..09078a2ec743 100644 --- a/community/values/src/main/java/org/neo4j/values/storable/ValueComparator.java +++ b/community/values/src/main/java/org/neo4j/values/storable/ValueComparator.java @@ -21,12 +21,13 @@ import java.util.Comparator; +import org.neo4j.values.Comparison; import org.neo4j.values.TernaryComparator; /** * Comparator for values. Usable for sorting values, for example during index range scans. */ -class ValueComparator implements Comparator, TernaryComparator +public final class ValueComparator implements Comparator, TernaryComparator { private final Comparator valueGroupComparator; @@ -54,7 +55,7 @@ public int compare( Value v1, Value v2 ) } @Override - public Integer ternaryCompare( Value v1, Value v2 ) + public Comparison ternaryCompare( Value v1, Value v2 ) { assert v1 != null && v2 != null : "null values are not supported, use NoValue.NO_VALUE instead"; @@ -67,13 +68,13 @@ public Integer ternaryCompare( Value v1, Value v2 ) { return v1.unsafeTernaryCompareTo( v2 ); } - return x; + return Comparison.from( x ); } @Override public boolean equals( Object obj ) { - return obj != null && obj instanceof ValueComparator; + return obj instanceof ValueComparator; } @Override diff --git a/community/values/src/main/java/org/neo4j/values/storable/Values.java b/community/values/src/main/java/org/neo4j/values/storable/Values.java index f7a32aa957ba..a9e2a373660a 100644 --- a/community/values/src/main/java/org/neo4j/values/storable/Values.java +++ b/community/values/src/main/java/org/neo4j/values/storable/Values.java @@ -94,9 +94,7 @@ private Values() * * To get Comparability semantics, use .ternaryCompare */ - private static final ValueComparator comp = new ValueComparator( ValueGroup::compareTo ); - public static final Comparator COMPARATOR = comp; - public static final TernaryComparator TERNARY_COMPARATOR = comp; + public static final ValueComparator COMPARATOR = new ValueComparator( ValueGroup::compareTo ); public static boolean isNumberValue( Object value ) { diff --git a/enterprise/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/spi/v3_4/codegen/Templates.scala b/enterprise/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/spi/v3_4/codegen/Templates.scala index bf0171a370d1..b63fdb22508e 100644 --- a/enterprise/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/spi/v3_4/codegen/Templates.scala +++ b/enterprise/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/spi/v3_4/codegen/Templates.scala @@ -45,9 +45,9 @@ import org.neo4j.kernel.api.SilentTokenNameLookup import org.neo4j.kernel.impl.api.RelationshipDataExtractor import org.neo4j.kernel.impl.core.EmbeddedProxySPI import org.neo4j.kernel.impl.util.ValueUtils -import org.neo4j.values.{AnyValue, AnyValues} -import org.neo4j.values.storable.{Value, Values} +import org.neo4j.values.storable.{Value, ValueComparator, Values} import org.neo4j.values.virtual._ +import org.neo4j.values.{AnyValue, AnyValues} /** * Contains common code generation constructs. @@ -202,7 +202,7 @@ object Templates { val newRelationshipDataExtractor = Expression .invoke(Expression.newInstance(typeRef[RelationshipDataExtractor]), MethodReference.constructorReference(typeRef[RelationshipDataExtractor])) - val valueComparator = Expression.getStatic(staticField[Values, Comparator[Value]]("COMPARATOR")) + val valueComparator = Expression.getStatic(staticField[Values, ValueComparator]("COMPARATOR")) val anyValueComparator = Expression.getStatic(staticField[AnyValues, Comparator[AnyValue]]("COMPARATOR")) def constructor(classHandle: ClassHandle) = MethodTemplate.constructor(