From add0ca2fe5dc4f9912482369865de377e60ad3ee Mon Sep 17 00:00:00 2001 From: Anton Persson Date: Tue, 21 Aug 2018 12:14:04 +0200 Subject: [PATCH] Address PR comments worstCaseLength for TextArray Don't serialize TextValues from TextArray when checking worst lenght. Instead go directly to underlying String. worstCaseLength for other SequenceValue Simply muliply length with BIGGEST_STATIC_SIZE instead of recursively call in loop. Use eclipse collection for prettier code when removing values from array. --- .../impl/schema/GenericIndexValidationIT.java | 16 ++++++++------ .../schema/GenericIndexKeyValidator.java | 22 ++++++++++++++----- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/community/community-it/index-it/src/test/java/org/neo4j/kernel/api/impl/schema/GenericIndexValidationIT.java b/community/community-it/index-it/src/test/java/org/neo4j/kernel/api/impl/schema/GenericIndexValidationIT.java index 4e17f04f5b05..70c8d76a7485 100644 --- a/community/community-it/index-it/src/test/java/org/neo4j/kernel/api/impl/schema/GenericIndexValidationIT.java +++ b/community/community-it/index-it/src/test/java/org/neo4j/kernel/api/impl/schema/GenericIndexValidationIT.java @@ -19,8 +19,9 @@ */ package org.neo4j.kernel.api.impl.schema; -import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.RandomStringUtils; +import org.eclipse.collections.api.list.MutableList; +import org.eclipse.collections.impl.factory.Iterables; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -96,13 +97,14 @@ public class GenericIndexValidationIT @Before public void setup() { - allValidNonArrayTypes = Types.values(); - allValidNonArrayTypes = ArrayUtils.removeElement( allValidNonArrayTypes, Types.ARRAY ); + MutableList listOfValidValues = Iterables.mList( Types.values() ) + .without( Types.ARRAY ) + .without( Types.CARTESIAN_POINT ) + .without( Types.CARTESIAN_POINT_3D ) + .without( Types.GEOGRAPHIC_POINT ) + .without( Types.GEOGRAPHIC_POINT_3D ); // todo include points when NATIVE_BTREE10 support spatial - allValidNonArrayTypes = ArrayUtils.removeElement( allValidNonArrayTypes, Types.CARTESIAN_POINT ); - allValidNonArrayTypes = ArrayUtils.removeElement( allValidNonArrayTypes, Types.CARTESIAN_POINT_3D ); - allValidNonArrayTypes = ArrayUtils.removeElement( allValidNonArrayTypes, Types.GEOGRAPHIC_POINT ); - allValidNonArrayTypes = ArrayUtils.removeElement( allValidNonArrayTypes, Types.GEOGRAPHIC_POINT_3D ); + allValidNonArrayTypes = listOfValidValues.toArray( new Types[listOfValidValues.size()] ); } @Test diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/GenericIndexKeyValidator.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/GenericIndexKeyValidator.java index 993f7800e9cb..4e6c0a36e38b 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/GenericIndexKeyValidator.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/GenericIndexKeyValidator.java @@ -26,6 +26,7 @@ import org.neo4j.kernel.impl.util.Validator; import org.neo4j.values.AnyValue; import org.neo4j.values.SequenceValue; +import org.neo4j.values.storable.TextArray; import org.neo4j.values.storable.TextValue; import org.neo4j.values.storable.Value; @@ -85,16 +86,20 @@ private static int worstCaseLength( Value[] values ) private static int worstCaseLength( AnyValue value ) { - // todo we can simply multiply length with BIGGEST_STATIC_SIZE if ( value.isSequenceValue() ) { SequenceValue sequenceValue = (SequenceValue) value; - int length = 0; - for ( int i = 0; i < sequenceValue.length(); i++ ) + if ( sequenceValue instanceof TextArray ) { - length += worstCaseLength( sequenceValue.value( i ) ); + TextArray textArray = (TextArray) sequenceValue; + int length = 0; + for ( int i = 0; i < textArray.length(); i++ ) + { + length += stringWorstCaseLength( textArray.stringValue( i ).length() ); + } + return length; } - return length; + return sequenceValue.length() * BIGGEST_STATIC_SIZE; } else { @@ -102,7 +107,7 @@ private static int worstCaseLength( AnyValue value ) { case TEXT: // For text, which is very dynamic in its nature do a worst-case off of number of characters in it - return ((TextValue) value).length() * 4; + return stringWorstCaseLength( ((TextValue) value).length() ); default: // For all else then use the biggest possible value for a non-dynamic, non-array value a state can occupy return BIGGEST_STATIC_SIZE; @@ -110,6 +115,11 @@ private static int worstCaseLength( AnyValue value ) } } + private static int stringWorstCaseLength( int stringLength ) + { + return stringLength * 4; + } + private int actualLength( Value[] values ) { CompositeGenericKey key = layout.newKey();