fixed offset handling in ByteRef usage

CHANGES.txt

@@ -5,6 +5,12 @@ Changes for Crate
 Unreleased
 ==========
 
+ - Fix: insert into a partitioned primary key column of type long
+   could fail under some circumstances
+
+ - Fix: missing characters when concatinating string under some
+   rare circumstances
+
  - Fix: UPDATE and INSERT ON DUPLICATE KEY assignment expression did
    not work if a used reference was also updated.
 

core/src/main/java/io/crate/types/IpType.java

@@ -83,7 +83,7 @@ static public boolean isValid(BytesRef ip) {
         short symbolsInOctet = 0;
         short numberOfDots = 0;
         int segmentValue = 0;
-        for (int i = 0; i < ip.length; i++) {
+        for (int i = ip.offset; i < ip.length + ip.offset; i++) {
             int sym = ip.bytes[i] & 0xff;
             if (sym < 46 || sym > 57 || sym == 47) {  // digits and dot symbol range a slash in a symbol range
                 return false;
@@ -92,7 +92,7 @@ static public boolean isValid(BytesRef ip) {
                 precededByZero = (sym == 48 && symbolsInOctet == 0);
                 segmentValue = segmentValue * 10 + (sym - '0');
                 symbolsInOctet++;
-            } else if (sym == 46 && i < ip.length - 1) {
+            } else if (sym == 46 && i < ip.length + ip.offset - 1) {
                 numberOfDots++;
                 if (numberOfDots > 3) {
                     return false;

core/src/main/java/io/crate/types/LongType.java

@@ -78,7 +78,7 @@ private long parseLong(BytesRef value) {
         boolean negative = false;
         long result = 0;
 
-        int i = 0;
+        int i = value.offset;
         int len = value.length;
         int radix = 10;
         long limit = -Long.MAX_VALUE;
@@ -106,7 +106,7 @@ private long parseLong(BytesRef value) {
             i++;;
         }
         multmin = limit / radix;
-        while (i < len) {
+        while (i < len + value.offset) {
             digit = Character.digit((char) bytes[i], radix);
             i++;
             if (digit < 0) {

core/src/test/java/io/crate/types/IpTypeTest.java

@@ -43,6 +43,7 @@ public void testValidation() throws Exception {
         };
         for (BytesRef ip : validIps) {
             assertEquals(true, IpType.isValid(ip));
+            assertEquals(true, IpType.isValid(TypeTestUtils.addOffset(ip)));
         }
         BytesRef[] invalidIps = {
                 new BytesRef("192.168.0.2555"),
@@ -72,6 +73,7 @@ public void testValidation() throws Exception {
         };
         for (BytesRef ip : invalidIps) {
             assertEquals(false, IpType.isValid(ip));
+            assertEquals(false, IpType.isValid(TypeTestUtils.addOffset(ip)));
         }
     }
 

core/src/test/java/io/crate/types/LongTypeTest.java

@@ -71,6 +71,8 @@ public void testNumberThatIsGreaterThanMaxValue() throws Exception {
     }
 
     private void assertBytesRefParsing(String s, long l) {
-        assertThat(LongType.INSTANCE.value(new BytesRef(s)), is(l));
+        BytesRef bytesRef = new BytesRef(s);
+        assertThat(LongType.INSTANCE.value(bytesRef), is(l));
+        assertThat(LongType.INSTANCE.value(TypeTestUtils.addOffset(bytesRef)), is(l));
     }
 }

core/src/test/java/io/crate/types/TypeTestUtils.java

@@ -0,0 +1,34 @@
+/*
+ * Licensed to CRATE Technology GmbH ("Crate") under one or more contributor
+ * license agreements.  See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.  Crate licenses
+ * this file to you under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.  You may
+ * obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ * However, if you have executed another commercial license agreement
+ * with Crate these terms will supersede the license and you may use the
+ * software solely pursuant to the terms of the relevant commercial agreement.
+ */
+
+package io.crate.types;
+
+import org.apache.lucene.util.BytesRef;
+
+public class TypeTestUtils {
+
+    public static BytesRef addOffset(BytesRef bytesRef) {
+        byte[] result = new byte[bytesRef.length + 2];
+        System.arraycopy(new byte[]{0, 1}, 0, result, 0, 2); // OFFSET
+        System.arraycopy(bytesRef.bytes, 0, result, 2, bytesRef.length);
+        return new BytesRef(result, 2, bytesRef.length);
+    }
+}

sql/src/main/java/io/crate/operation/scalar/ConcatFunction.java

@@ -87,8 +87,8 @@ public BytesRef evaluate(Input[] args) {
                 return firstArg;
             }
             byte[] bytes = new byte[firstArg.length + secondArg.length];
-            System.arraycopy(firstArg.bytes, 0, bytes, 0, firstArg.length);
-            System.arraycopy(secondArg.bytes, 0, bytes, firstArg.length, secondArg.length);
+            System.arraycopy(firstArg.bytes, firstArg.offset, bytes, 0, firstArg.length);
+            System.arraycopy(secondArg.bytes, secondArg.offset, bytes, firstArg.length, secondArg.length);
             return new BytesRef(bytes);
         }
     }
@@ -116,7 +116,7 @@ public BytesRef evaluate(Input[] args) {
             byte[] bytes = new byte[numBytes];
             int numWritten = 0;
             for (BytesRef bytesRef : bytesRefs) {
-                System.arraycopy(bytesRef.bytes, 0, bytes, numWritten, bytesRef.length);
+                System.arraycopy(bytesRef.bytes, bytesRef.offset, bytes, numWritten, bytesRef.length);
                 numWritten += bytesRef.length;
             }
             return new BytesRef(bytes);

sql/src/test/java/io/crate/integrationtests/InsertIntoIntegrationTest.java

@@ -729,4 +729,11 @@ public void testBulkInsert() throws Exception {
         execute("select sum(lashes), date from giveittome group by date");
         assertThat(response.rowCount(), is((long)bulkSize));
     }
+
+    @Test
+    public void testInsertIntoLongPartitionedBy() throws Exception {
+        execute("create table import (col1 int, col2 long primary key) partitioned by (col2)");
+        ensureYellow();
+        execute("insert into import (col1, col2) values (1, 1)");
+    }
 }

sql/src/test/java/io/crate/operation/scalar/ConcatFunctionTest.java

@@ -28,6 +28,7 @@
 import io.crate.metadata.FunctionIdent;
 import io.crate.metadata.Scalar;
 import io.crate.operation.Input;
+import io.crate.testing.TestingHelpers;
 import io.crate.types.ArrayType;
 import io.crate.types.DataType;
 import io.crate.types.DataTypes;
@@ -126,6 +127,8 @@ public void testTwoStringsNullCombinations() throws Exception {
     @Test
     public void testTwoStrings() throws Exception {
         assertEval("foobar", "foo", "bar");
+        assertEval("foobar", TestingHelpers.addOffset(new BytesRef("foo")),
+                             TestingHelpers.addOffset(new BytesRef("bar")));
     }
 
     @Test
@@ -148,6 +151,7 @@ public void testNumberAndNull() throws Exception {
     public void testStringAndNumber() throws Exception {
         assertEval("foo3", new BytesRef("foo"), 3);
         assertEval("foo3", new BytesRef("foo"), 3L);
+        assertEval("foo3", TestingHelpers.addOffset(new BytesRef("foo")), 3L);
         assertEval("foo3", new BytesRef("foo"), (short)3);
     }
 

sql/src/test/java/io/crate/operation/scalar/SubstrFunctionTest.java

@@ -29,6 +29,7 @@
 import io.crate.metadata.FunctionInfo;
 import io.crate.metadata.Scalar;
 import io.crate.operation.Input;
+import io.crate.testing.TestingHelpers;
 import io.crate.types.DataType;
 import io.crate.types.DataTypes;
 import org.apache.lucene.util.BytesRef;
@@ -129,6 +130,13 @@ public void testNullLiteralFromCount() throws Exception {
         assertThat(result, isLiteral(null, DataTypes.UNDEFINED));
     }
 
+    @Test
+    public void testSubstring() throws Exception {
+        assertThat(SubstrFunction.substring(new BytesRef("cratedata"), 2, 5), is(new BytesRef("ate")));
+        assertThat(SubstrFunction.substring(TestingHelpers.addOffset(new BytesRef("cratedata")), 2, 5),
+                                            is(new BytesRef("ate")));
+    }
+
     @Test
     @SuppressWarnings("unchecked")
     public void testEvaluate() throws Exception {

sql/src/test/java/io/crate/testing/TestingHelpers.java

@@ -687,4 +687,11 @@ public static Map<String, Object> jsonMap(String json) {
             throw new RuntimeException(e);
         }
     }
+
+    public static BytesRef addOffset(BytesRef bytesRef) {
+        byte[] result = new byte[bytesRef.length + 2];
+        System.arraycopy(new byte[]{0, 1}, 0, result, 0, 2); // OFFSET
+        System.arraycopy(bytesRef.bytes, 0, result, 2, bytesRef.length);
+        return new BytesRef(result, 2, bytesRef.length);
+    }
 }