Fix setting inner object type names in TypeSignature#createType.

common/src/main/java/io/crate/types/ObjectParameterTypeSignature.java

@@ -0,0 +1,38 @@
+/*
+ * Licensed to 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;
+
+final class ObjectParameterTypeSignature extends TypeSignature {
+
+    private final String parameterName;
+
+    public ObjectParameterTypeSignature(String parameterName,
+                                        TypeSignature typeSignature) {
+        super(typeSignature.getBaseTypeName(), typeSignature.getParameters());
+        this.parameterName = parameterName;
+    }
+
+    public String parameterName() {
+        return parameterName;
+    }
+}

common/src/main/java/io/crate/types/ObjectType.java

@@ -261,10 +261,13 @@ public void writeTo(StreamOutput out) throws IOException {
     @Override
     public TypeSignature getTypeSignature() {
         ArrayList<TypeSignature> parameters = new ArrayList<>(innerTypes.size() * 2);
-        for (var type : innerTypes.values()) {
+        for (var innerTypeKeyValue : innerTypes.entrySet()) {
             // all keys are of type 'text'
             parameters.add(StringType.INSTANCE.getTypeSignature());
-            parameters.add(type.getTypeSignature());
+
+            var innerType = innerTypeKeyValue.getValue();
+            parameters.add(
+                new ObjectParameterTypeSignature(innerTypeKeyValue.getKey(), innerType.getTypeSignature()));
         }
         return new TypeSignature(NAME, parameters);
     }

common/src/main/java/io/crate/types/TypeSignature.java

@@ -132,7 +132,10 @@ public DataType<?> createType() {
             var builder = ObjectType.builder();
             for (int i = 0; i < parameters.size() - 1;) {
                 var valTypeSignature = parameters.get(i + 1);
-                builder.setInnerType(String.valueOf(i), valTypeSignature.createType());
+                assert valTypeSignature instanceof ObjectParameterTypeSignature
+                    : "the inner type signature must be named (must have ObjectParameterTypeSignature type)";
+                var innerTypeName = ((ObjectParameterTypeSignature) valTypeSignature).parameterName();
+                builder.setInnerType(innerTypeName, valTypeSignature.createType());
                 i += 2;
             }
             return builder.build();
@@ -160,7 +163,8 @@ public boolean equals(Object o) {
         if (this == o) {
             return true;
         }
-        if (o == null || getClass() != o.getClass()) {
+        if (o == null ||
+            !(getClass() == o.getClass() || getClass() == ObjectParameterTypeSignature.class)) {
             return false;
         }
         TypeSignature that = (TypeSignature) o;

common/src/test/java/io/crate/types/ObjectTypeTest.java

@@ -22,22 +22,21 @@
 
 package io.crate.types;
 
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.nullValue;
+import io.crate.test.integration.CrateUnitTest;
+import org.elasticsearch.common.collect.MapBuilder;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.hamcrest.Matchers;
+import org.junit.Test;
 
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 
-import org.elasticsearch.common.collect.MapBuilder;
-import org.elasticsearch.common.io.stream.BytesStreamOutput;
-import org.elasticsearch.common.io.stream.StreamInput;
-import org.hamcrest.Matchers;
-import org.junit.Test;
-
-import io.crate.test.integration.CrateUnitTest;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
 
 public class ObjectTypeTest extends CrateUnitTest {
 
@@ -178,4 +177,12 @@ public void testResolveInnerType() {
 
         assertThat(type.resolveInnerType(List.of("s", "inner", "i")), is(DataTypes.INTEGER));
     }
+
+    @Test
+    public void test_object_type_to_signature_to_object_type_round_trip() {
+        var objectType = ObjectType.builder()
+            .setInnerType("inner", DataTypes.STRING)
+            .build();
+        assertThat(objectType.getTypeSignature().createType(), is(objectType));
+    }
 }

common/src/test/java/io/crate/types/TypeSignatureTest.java

@@ -25,7 +25,10 @@
 import io.crate.test.integration.CrateUnitTest;
 import org.junit.Test;
 
+import java.util.List;
+
 import static io.crate.types.TypeSignature.parseTypeSignature;
+import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.is;
 
 public class TypeSignatureTest extends CrateUnitTest {
@@ -45,17 +48,26 @@ public void testParsingOfArray() {
 
     @Test
     public void testParsingOfObject() {
-        ObjectType objectType = ObjectType.builder().setInnerType("V", IntegerType.INSTANCE).build();
-        assertThat(parseTypeSignature("object(text, integer)"), is(objectType.getTypeSignature()));
+        var signature = parseTypeSignature("object(text, integer)");
+        assertThat(signature.getBaseTypeName(), is(ObjectType.NAME));
+        assertThat(
+            signature.getParameters(),
+            contains(
+                new TypeSignature(DataTypes.STRING.getName()),
+                new TypeSignature(DataTypes.INTEGER.getName())));
     }
 
     @Test
     public void testParsingOfNestedArray() {
-        var type = new ArrayType<>(
-            ObjectType.builder()
-                .setInnerType("x", new ArrayType<>(IntegerType.INSTANCE))
-            .build()
-        );
-        assertThat(parseTypeSignature("array(object(text, array(integer)))"), is(type.getTypeSignature()));
+        var signature = parseTypeSignature("array(object(text, array(integer)))");
+        assertThat(signature.getBaseTypeName(), is(ArrayType.NAME));
+
+        var innerObjectTypeSignature = signature.getParameters().get(0);
+        assertThat(innerObjectTypeSignature.getBaseTypeName(), is(ObjectType.NAME));
+        assertThat(
+            innerObjectTypeSignature.getParameters(),
+            contains(
+                new TypeSignature(DataTypes.STRING.getName()),
+                new TypeSignature(ArrayType.NAME, List.of(new TypeSignature(DataTypes.INTEGER.getName())))));
     }
 }

sql/src/test/java/io/crate/metadata/functions/SignatureBinderTest.java

@@ -24,6 +24,8 @@
 
 import io.crate.test.integration.CrateUnitTest;
 import io.crate.types.DataType;
+import io.crate.types.DataTypes;
+import io.crate.types.ObjectType;
 import io.crate.types.TypeSignature;
 import org.junit.Test;
 
@@ -239,15 +241,21 @@ public void testMap() {
             .build();
 
         assertThat(getValueFunction)
-            .boundTo("object(text, bigint)", "text")
+            .boundTo(
+                ObjectType.builder()
+                    .setInnerType("V", DataTypes.LONG).build(),
+                DataTypes.STRING)
             .produces(new BoundVariables(
                 Map.of(
                     "K", type("text"),
                     "V", type("bigint"))
             ));
 
         assertThat(getValueFunction)
-            .boundTo("object(text, bigint)", "bigint")
+            .boundTo(
+                ObjectType.builder()
+                    .setInnerType("V", DataTypes.LONG).build(),
+                DataTypes.LONG)
             .withoutCoercion()
             .fails();
     }
@@ -446,17 +454,17 @@ public BindSignatureAssertion boundTo(Object... arguments) {
         public BindSignatureAssertion boundTo(List<Object> arguments) {
             ArrayList<TypeSignature> builder = new ArrayList<>(arguments.size());
             for (Object argument : arguments) {
-                if (argument instanceof String) {
+                if (argument instanceof DataType<?>) {
+                    builder.add(((DataType<?>) argument).getTypeSignature());
+                } else if (argument instanceof String) {
                     builder.add(TypeSignature.parseTypeSignature((String) argument));
-                    continue;
-                }
-                if (argument instanceof TypeSignature) {
+                } else if (argument instanceof TypeSignature) {
                     builder.add((TypeSignature) argument);
-                    continue;
+                } else {
+                    throw new IllegalArgumentException(format(
+                        "argument is of type %s. It should be DataType, String or TypeSignature",
+                        argument.getClass()));
                 }
-                throw new IllegalArgumentException(format(
-                    "argument is of type %s. It should be String or TypeSignature",
-                    argument.getClass()));
             }
             this.argumentTypes = Collections.unmodifiableList(builder);
             return this;