Commit c3e14d12 authored by Satia Herfert's avatar Satia Herfert
Browse files

Adjust NaN behavior to new CLG rules.

parent 4e3ba6d9
No related merge requests found
Showing with 253 additions and 61 deletions
+253 -61
......@@ -642,7 +642,9 @@ object IntermediateRepresentation {
def trueValue: IntermediateRepresentation = getStatic[Values, BooleanValue]("TRUE")
def falseValue: IntermediateRepresentation = getStatic[Values, BooleanValue]("FALSE")
def falseValue: IntermediateRepresentation = getStatic[Values, BooleanValue]("FALSE")
def isNaN(value: IntermediateRepresentation): IntermediateRepresentation = and(instanceOf[FloatingPointValue](value), invoke(cast[FloatingPointValue](value), method[FloatingPointValue, Boolean]("isNaN")))
def constant(value: Any): IntermediateRepresentation = Constant(value)
......
......@@ -161,6 +161,7 @@ public abstract class IndexProviderCompatibilityTestSuite
Arrays.asList(
Values.of( "string1" ),
Values.of( 42 ),
Values.of( Double.NaN ),
Values.of( true ),
Values.of( new char[]{'a', 'z'} ),
Values.of( new String[]{"arrayString1", "arraysString2"} ),
......
......@@ -35,11 +35,13 @@ import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.temporal.ChronoUnit;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import org.neo4j.internal.kernel.api.IndexQuery;
import org.neo4j.internal.schema.IndexOrder;
......@@ -815,26 +817,99 @@ public abstract class SimpleIndexAccessorCompatibility extends IndexAccessorComp
shouldRangeSeekInOrder( IndexOrder.DESCENDING, o0, o1, o2, o3, o4, o5 );
}
private void shouldRangeSeekInOrder( IndexOrder order, Object o0, Object o1, Object o2, Object o3, Object o4, Object o5 ) throws Exception
@Test
public void shouldRangeSeekAscendingWithoutFindingNanForOpenEnd() throws Exception
{
Object o0 = 0;
Object o1 = 1.0;
Object o2 = 2.5;
Object o3 = 3;
Object o4 = 4;
Object o5 = 5;
Object o6 = Double.POSITIVE_INFINITY;
Object o7 = Double.NaN;
shouldRangeSeekInOrderWithExpectedSize( IndexOrder.ASCENDING, RangeSeekMode.OPEN_END, 7, o0, o1, o2, o3, o4, o5, o6, o7 );
}
@Test
public void shouldRangeSeekDescendingWithoutFindingNanForOpenEnd() throws Exception
{
Object o0 = 0;
Object o1 = 1.0;
Object o2 = 2.5;
Object o3 = 3;
Object o4 = 4;
Object o5 = 5;
Object o6 = Double.POSITIVE_INFINITY;
Object o7 = Double.NaN;
shouldRangeSeekInOrderWithExpectedSize( IndexOrder.DESCENDING, RangeSeekMode.OPEN_END, 7, o0, o1, o2, o3, o4, o5, o6, o7 );
}
@Test
public void shouldRangeSeekAscendingWithoutFindingNanForOpenStart() throws Exception
{
Object o0 = Double.NaN;
Object o1 = 1.0;
Object o2 = 2.5;
Object o3 = 3;
Object o4 = 4;
Object o5 = 5;
Object o6 = Double.POSITIVE_INFINITY;
shouldRangeSeekInOrderWithExpectedSize( IndexOrder.ASCENDING, RangeSeekMode.OPEN_START, 6, o0, o1, o2, o3, o4, o5, o6 );
}
@Test
public void shouldRangeSeekDescendingWithoutFindingNanForOpenStart() throws Exception
{
Object o0 = Double.NaN;
Object o1 = 1.0;
Object o2 = 2.5;
Object o3 = 3;
Object o4 = 4;
Object o5 = 5;
Object o6 = Double.POSITIVE_INFINITY;
shouldRangeSeekInOrderWithExpectedSize( IndexOrder.DESCENDING, RangeSeekMode.OPEN_START, 6, o0, o1, o2, o3, o4, o5, o6 );
}
private void shouldRangeSeekInOrder( IndexOrder order, Object... objects ) throws Exception
{
shouldRangeSeekInOrderWithExpectedSize( order, RangeSeekMode.CLOSED, objects.length, objects );
}
private void shouldRangeSeekInOrderWithExpectedSize( IndexOrder order, RangeSeekMode rangeSeekMode, int expectedSize, Object... objects ) throws Exception
{
IndexQuery range = range( 100, Values.of( o0 ), true, Values.of( o5 ), true );
IndexQuery range;
switch ( rangeSeekMode )
{
case CLOSED:
range = range( 100, Values.of( objects[0] ), true, Values.of( objects[objects.length - 1] ), true );
break;
case OPEN_END:
range = range( 100, Values.of( objects[0] ), true, null, false );
break;
case OPEN_START:
range = range( 100, null, false, Values.of( objects[objects.length - 1] ), true );
break;
default:
throw new IllegalStateException();
}
IndexOrder[] indexOrders = orderCapability( range );
Assume.assumeTrue( "Assume support for order " + order, ArrayUtils.contains( indexOrders, order ) );
updateAndCommit( asList(
add( 1, descriptor.schema(), o0 ),
add( 1, descriptor.schema(), o5 ),
add( 1, descriptor.schema(), o1 ),
add( 1, descriptor.schema(), o4 ),
add( 1, descriptor.schema(), o2 ),
add( 1, descriptor.schema(), o3 )
) );
List<IndexEntryUpdate<?>> additions = Arrays.stream( objects ).map( o -> add( 1, descriptor.schema(), o ) ).collect( Collectors.toList() );
Collections.shuffle( additions, random.random() );
updateAndCommit( additions );
SimpleNodeValueClient client = new SimpleNodeValueClient();
try ( AutoCloseable ignored = query( client, order, range ) )
{
List<Long> seenIds = assertClientReturnValuesInOrder( client, order );
assertThat( seenIds.size(), equalTo( 6 ) );
assertThat( seenIds.size(), equalTo( expectedSize ) );
}
}
......@@ -1163,4 +1238,11 @@ public abstract class SimpleIndexAccessorCompatibility extends IndexAccessorComp
assertThat( query( IndexQuery.exists( 1 ) ), equalTo( asList( 1L, 2L, 3L ) ) );
}
}
private enum RangeSeekMode
{
CLOSED,
OPEN_END,
OPEN_START
}
}
......@@ -56,7 +56,7 @@ import org.neo4j.kernel.impl.core.TransactionalEntityFactory
import org.neo4j.kernel.impl.util.ValueUtils.{fromNodeEntity, fromRelationshipEntity}
import org.neo4j.kernel.impl.util.{DefaultValueMapper, ValueUtils}
import org.neo4j.storageengine.api.RelationshipVisitor
import org.neo4j.values.storable.{TextValue, Value, Values}
import org.neo4j.values.storable.{FloatingPointValue, TextValue, Value, Values}
import org.neo4j.values.virtual._
import org.neo4j.values.{AnyValue, ValueMapper}
......@@ -217,7 +217,7 @@ sealed class TransactionBoundQueryContext(val transactionalContext: Transactiona
val impossiblePredicate =
predicates.exists {
case p: IndexQuery.ExactPredicate => p.value() eq Values.NO_VALUE
case p: IndexQuery.ExactPredicate => (p.value() eq Values.NO_VALUE) || (p.value().isInstanceOf[FloatingPointValue] && p.value().asInstanceOf[FloatingPointValue].isNaN)
case _: IndexQuery.ExistsPredicate if predicates.length > 1 => false
case p: IndexQuery =>
!RANGE_SEEKABLE_VALUE_GROUPS.contains(p.valueGroup())
......
......@@ -48,7 +48,6 @@ class ComparablePredicateTest extends CypherFunSuite {
Long.MaxValue,
Double.MaxValue,
Double.PositiveInfinity,
Double.NaN,
null
).flatMap {
case null => Seq(null)
......
......@@ -139,6 +139,10 @@ public final class CypherBoolean
public static Value lessThan( AnyValue lhs, AnyValue rhs )
{
if ( AnyValue.isNanAndNumber(lhs, rhs) )
{
return FALSE;
}
Comparison comparison = AnyValues.TERNARY_COMPARATOR.ternaryCompare( lhs, rhs );
switch ( comparison )
{
......@@ -158,6 +162,10 @@ public final class CypherBoolean
public static Value lessThanOrEqual( AnyValue lhs, AnyValue rhs )
{
if ( AnyValue.isNanAndNumber(lhs, rhs) )
{
return FALSE;
}
Comparison comparison = AnyValues.TERNARY_COMPARATOR.ternaryCompare( lhs, rhs );
switch ( comparison )
{
......@@ -177,6 +185,10 @@ public final class CypherBoolean
public static Value greaterThan( AnyValue lhs, AnyValue rhs )
{
if ( AnyValue.isNanAndNumber(lhs, rhs) )
{
return FALSE;
}
Comparison comparison = AnyValues.TERNARY_COMPARATOR.ternaryCompare( lhs, rhs );
switch ( comparison )
{
......@@ -196,6 +208,10 @@ public final class CypherBoolean
public static Value greaterThanOrEqual( AnyValue lhs, AnyValue rhs )
{
if ( AnyValue.isNanAndNumber(lhs, rhs) )
{
return FALSE;
}
Comparison comparison = AnyValues.TERNARY_COMPARATOR.ternaryCompare( lhs, rhs );
switch ( comparison )
{
......@@ -243,11 +259,6 @@ public final class CypherBoolean
return seenUndefined ? NO_VALUE : Values.FALSE;
}
private static boolean isNan( AnyValue value )
{
return value instanceof FloatingPointValue && ((FloatingPointValue) value).isNaN();
}
private static final class BooleanMapper implements ValueMapper<Value>
{
@Override
......
......@@ -78,7 +78,7 @@ public abstract class IndexQuery
Number from, boolean fromInclusive,
Number to, boolean toInclusive )
{
return new NumberRangePredicate( propertyKeyId,
return NumberRangePredicate.create( propertyKeyId,
from == null ? null : Values.numberValue( from ), fromInclusive,
to == null ? null : Values.numberValue( to ), toInclusive );
}
......@@ -105,7 +105,7 @@ public abstract class IndexQuery
switch ( valueGroup )
{
case NUMBER:
return new NumberRangePredicate( propertyKeyId,
return NumberRangePredicate.create( propertyKeyId,
(NumberValue)from, fromInclusive,
(NumberValue)to, toInclusive );
......@@ -263,7 +263,7 @@ public abstract class IndexQuery
public static final class ExistsPredicate extends IndexQuery
{
ExistsPredicate( int propertyKeyId )
private ExistsPredicate( int propertyKeyId )
{
super( propertyKeyId );
}
......@@ -297,7 +297,7 @@ public abstract class IndexQuery
{
private final Value exactValue;
ExactPredicate( int propertyKeyId, Object value )
private ExactPredicate( int propertyKeyId, Object value )
{
super( propertyKeyId );
this.exactValue = value instanceof Value ? (Value)value : Values.of( value );
......@@ -419,7 +419,8 @@ public abstract class IndexQuery
{
private final CoordinateReferenceSystem crs;
GeometryRangePredicate( int propertyKeyId, CoordinateReferenceSystem crs, PointValue from, boolean fromInclusive, PointValue to, boolean toInclusive )
private GeometryRangePredicate( int propertyKeyId, CoordinateReferenceSystem crs, PointValue from, boolean fromInclusive, PointValue to,
boolean toInclusive )
{
super( propertyKeyId, ValueGroup.GEOMETRY, from, fromInclusive, to, toInclusive );
this.crs = crs;
......@@ -470,28 +471,9 @@ public abstract class IndexQuery
}
}
public static final class NumberRangePredicate extends RangePredicate<NumberValue>
{
NumberRangePredicate( int propertyKeyId, NumberValue from, boolean fromInclusive, NumberValue to,
boolean toInclusive )
{
super( propertyKeyId, ValueGroup.NUMBER, from, fromInclusive, to, toInclusive );
}
public Number from()
{
return from == null ? null : from.asObject();
}
public Number to()
{
return to == null ? null : to.asObject();
}
}
public static final class TextRangePredicate extends RangePredicate<TextValue>
{
TextRangePredicate( int propertyKeyId, TextValue from, boolean fromInclusive, TextValue to,
private TextRangePredicate( int propertyKeyId, TextValue from, boolean fromInclusive, TextValue to,
boolean toInclusive )
{
super( propertyKeyId, ValueGroup.TEXT, from, fromInclusive, to, toInclusive );
......@@ -510,7 +492,7 @@ public abstract class IndexQuery
public abstract static class StringPredicate extends IndexQuery
{
StringPredicate( int propertyKeyId )
private StringPredicate( int propertyKeyId )
{
super( propertyKeyId );
}
......@@ -538,7 +520,7 @@ public abstract class IndexQuery
{
private final TextValue prefix;
StringPrefixPredicate( int propertyKeyId, TextValue prefix )
private StringPrefixPredicate( int propertyKeyId, TextValue prefix )
{
super( propertyKeyId );
//we know utf8 values are coming from the index so optimize for that
......@@ -567,7 +549,7 @@ public abstract class IndexQuery
{
private final TextValue contains;
StringContainsPredicate( int propertyKeyId, TextValue contains )
private StringContainsPredicate( int propertyKeyId, TextValue contains )
{
super( propertyKeyId );
//we know utf8 values are coming from the index so optimize for that
......@@ -596,7 +578,7 @@ public abstract class IndexQuery
{
private final TextValue suffix;
StringSuffixPredicate( int propertyKeyId, TextValue suffix )
private StringSuffixPredicate( int propertyKeyId, TextValue suffix )
{
super( propertyKeyId );
//we know utf8 values are coming from the index so optimize for that
......@@ -625,7 +607,7 @@ public abstract class IndexQuery
{
private final String query;
FulltextSearchPredicate( String query )
private FulltextSearchPredicate( String query )
{
super( TokenRead.NO_TOKEN );
this.query = query;
......
/*
* Copyright (c) 2002-2019 "Neo4j,"
* Neo4j Sweden AB [http://neo4j.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 <http://www.gnu.org/licenses/>.
*/
package org.neo4j.internal.kernel.api;
import org.neo4j.values.storable.NumberValue;
import org.neo4j.values.storable.ValueGroup;
import org.neo4j.values.storable.Values;
public final class NumberRangePredicate extends IndexQuery.RangePredicate<NumberValue>
{
private NumberRangePredicate( int propertyKeyId, NumberValue from, boolean fromInclusive, NumberValue to,
boolean toInclusive )
{
super( propertyKeyId, ValueGroup.NUMBER, from, fromInclusive, to, toInclusive );
}
static org.neo4j.internal.kernel.api.NumberRangePredicate create( int propertyKeyId, NumberValue from, boolean fromInclusive, NumberValue to,
boolean toInclusive )
{
// For range queries with numbers we need to redefine the upper bound from NaN to positive infinity.
// The reason is that we do not want to find NaNs for seeks, but for full scans we do.
if ( to == null )
{
to = Values.doubleValue( Double.POSITIVE_INFINITY );
toInclusive = true;
}
return new org.neo4j.internal.kernel.api.NumberRangePredicate( propertyKeyId, from, fromInclusive, to, toInclusive );
}
public Number from()
{
return from == null ? null : from.asObject();
}
public Number to()
{
return to == null ? null : to.asObject();
}
}
......@@ -43,7 +43,7 @@ class NumberType extends Type
NumberType( byte typeId )
{
super( ValueGroup.NUMBER, typeId, Values.of( Double.NEGATIVE_INFINITY ), Values.of( Double.POSITIVE_INFINITY ) );
super( ValueGroup.NUMBER, typeId, Values.of( Double.NEGATIVE_INFINITY ), Values.of( Double.NaN ) );
}
@Override
......
......@@ -19,6 +19,9 @@
*/
package org.neo4j.values;
import org.neo4j.values.storable.FloatingPointValue;
import org.neo4j.values.storable.NumberValue;
public abstract class AnyValue
{
private int hash;
......@@ -94,4 +97,22 @@ public abstract class AnyValue
{
return ((value + 7) / 8) * 8;
}
/**
* @return {@code true} if at least one operand is NaN and the other is a number
*/
public static boolean isNanAndNumber( AnyValue value1, AnyValue value2 )
{
return (value1 instanceof FloatingPointValue && ((FloatingPointValue) value1).isNaN() && value2 instanceof NumberValue)
|| (value2 instanceof FloatingPointValue && ((FloatingPointValue) value2).isNaN() && value1 instanceof NumberValue);
}
/**
* @return {@code true} if at least one operand is NaN
*/
public static boolean hasNaNOperand( AnyValue value1, AnyValue value2 )
{
return (value1 instanceof FloatingPointValue && ((FloatingPointValue) value1).isNaN())
|| (value2 instanceof FloatingPointValue && ((FloatingPointValue) value2).isNaN());
}
}
......@@ -173,6 +173,10 @@ public abstract class Value extends AnyValue
{
return ((SequenceValue) this).ternaryEquality( (SequenceValue) other );
}
if ( hasNaNOperand( this, other ) )
{
return Equality.FALSE;
}
if ( other instanceof Value && ((Value) other).valueGroup() == valueGroup() )
{
Value otherValue = (Value) other;
......
......@@ -27,7 +27,7 @@ import java.util.concurrent.ThreadLocalRandom;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.neo4j.values.storable.NumberValues.hash;
import static org.neo4j.values.utils.AnyValueTestUtil.assertIncomparable;
import static org.neo4j.values.utils.AnyValueTestUtil.assertNotEqual;
import static org.neo4j.values.virtual.VirtualValueTestUtil.toAnyValue;
class NumberValuesTest
......@@ -49,9 +49,9 @@ class NumberValuesTest
@Test
void shouldHandleNaNCorrectly()
{
assertIncomparable( toAnyValue(Double.NaN), toAnyValue( Double.NaN ) );
assertIncomparable( toAnyValue( 1 ), toAnyValue( Double.NaN ) );
assertIncomparable( toAnyValue( Double.NaN ), toAnyValue( 1 ) );
assertNotEqual( toAnyValue(Double.NaN), toAnyValue( Double.NaN ) );
assertNotEqual( toAnyValue( 1 ), toAnyValue( Double.NaN ) );
assertNotEqual( toAnyValue( Double.NaN ), toAnyValue( 1 ) );
}
@Test
......
......@@ -21,11 +21,18 @@ package org.neo4j.values.storable;
import org.junit.jupiter.api.Test;
import java.util.Arrays;
import java.util.List;
import org.neo4j.string.UTF8;
import static java.time.ZoneOffset.UTC;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.neo4j.values.storable.DurationValue.duration;
import static org.neo4j.values.storable.LocalTimeValue.localTime;
import static org.neo4j.values.storable.TimeValue.time;
import static org.neo4j.values.storable.Values.booleanArray;
import static org.neo4j.values.storable.Values.booleanValue;
import static org.neo4j.values.storable.Values.byteArray;
......@@ -46,6 +53,7 @@ import static org.neo4j.values.storable.Values.stringArray;
import static org.neo4j.values.storable.Values.stringValue;
import static org.neo4j.values.storable.Values.utf8Value;
import static org.neo4j.values.utils.AnyValueTestUtil.assertEqual;
import static org.neo4j.values.utils.AnyValueTestUtil.assertNotEqual;
class ValuesTest
{
......@@ -116,4 +124,30 @@ class ValuesTest
assertFalse( areOfSameClasses );
assertTrue( areOfSameValueType );
}
@Test
void differentTypesShouldNotBeEqual()
{
// This includes NaN
List<Value> items = Arrays.asList(
stringValue( "foo" ),
intValue( 42 ),
booleanValue( false ),
doubleValue( Double.NaN ),
time( 14, 0, 0, 0, UTC ),
localTime( 14, 0, 0, 0 ),
duration( 0, 0, 0, 1_000_000_000 ),
byteArray( new byte[]{} ),
charArray( new char[]{'x'} ) );
for ( int i = 0; i < items.size(); i++ )
{
for ( int j = 0; j < items.size(); j++ )
{
if ( i != j )
{
assertNotEqual( items.get( i ), items.get( j ) );
}
}
}
}
}
......@@ -45,23 +45,23 @@ public class AnyValueTestUtil
{
assertEquals( a, b, a + " should be equivalent to " + b );
assertEquals( b, a, a + " should be equivalent to " + b );
assertEquals( a.ternaryEquals( b ), Equality.TRUE, a + " should be equal to " + b );
assertEquals( b.ternaryEquals( a ), Equality.TRUE, a + " should be equal to " + b );
assertEquals( Equality.TRUE, a.ternaryEquals( b ), a + " should be equal to " + b );
assertEquals( Equality.TRUE, b.ternaryEquals( a ), a + " should be equal to " + b );
}
public static void assertNotEqual( AnyValue a, AnyValue b )
{
assertNotEquals( a, b, a + " should not be equivalent to " + b );
assertNotEquals( b, a, b + " should not be equivalent to " + a );
assertNotEquals( a.ternaryEquals( b ), Equality.TRUE, a + " should not equal " + b );
assertNotEquals( b.ternaryEquals( a ), Equality.TRUE, b + " should not equal " + a );
assertEquals( Equality.FALSE, a.ternaryEquals( b ), a + " should not equal " + b );
assertEquals( Equality.FALSE, b.ternaryEquals( a ), b + " should not equal " + a );
}
public static void assertIncomparable( AnyValue a, AnyValue b )
{
assertNotEquals( a, b, a + " should not be equivalent to " + b );
assertNotEquals( b, a, b + " should not be equivalent to " + a );
assertEquals( a.ternaryEquals( b ), Equality.UNDEFINED, a + " should be incomparable to " + b );
assertEquals( b.ternaryEquals( a ), Equality.UNDEFINED, b + " should be incomparable to " + a );
assertEquals( Equality.UNDEFINED, a.ternaryEquals( b ), a + " should be incomparable to " + b );
assertEquals( Equality.UNDEFINED, b.ternaryEquals( a ), b + " should be incomparable to " + a );
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment