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.
neo4j · Aug 23, 2018 · add0ca2 · add0ca2
1 parent 8b067bd
commit add0ca2
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 13 deletions.
diff --git a/...-it/index-it/src/test/java/org/neo4j/kernel/api/impl/schema/GenericIndexValidationIT.java b/...-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<Types> 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/...ity/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/GenericIndexKeyValidator.java b/...ity/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,31 +86,40 @@ 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
         {
             switch ( ((Value) value).valueGroup().category() )
             {
             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;
             }
         }
     }
 
+    private static int stringWorstCaseLength( int stringLength )
+    {
+        return stringLength * 4;
+    }
+
     private int actualLength( Value[] values )
     {
         CompositeGenericKey key = layout.newKey();