Support nullable and non-nullable types with same name in the same template

fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/typing/TypeRepository.java

@@ -23,8 +23,6 @@
 import com.apple.foundationdb.record.TupleFieldsProto;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Verify;
-import com.google.common.collect.BiMap;
-import com.google.common.collect.HashBiMap;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 import com.google.protobuf.DescriptorProtos;
@@ -435,13 +433,15 @@ private void addEnumType(@Nonnull final EnumDescriptor enumType,
     public static class Builder {
         private @Nonnull final FileDescriptorProto.Builder fileDescProtoBuilder;
         private @Nonnull final FileDescriptorSet.Builder fileDescSetBuilder;
-        private @Nonnull final BiMap<Type, String> typeToNameMap;
+        private @Nonnull final Map<Type, String> typeToNameMap;
+        private @Nonnull final Map<String, Type> nameToCanonicalTypeMap;
 
         private Builder() {
             fileDescProtoBuilder = FileDescriptorProto.newBuilder();
             fileDescProtoBuilder.addAllDependency(DEPENDENCIES.stream().map(FileDescriptor::getFullName).collect(Collectors.toList()));
             fileDescSetBuilder = FileDescriptorSet.newBuilder();
-            typeToNameMap = HashBiMap.create();
+            typeToNameMap = new HashMap<>();
+            nameToCanonicalTypeMap = new HashMap<>();
         }
 
         @Nonnull
@@ -471,19 +471,46 @@ public Builder setPackage(@Nonnull final String name) {
         @Nonnull
         public Builder addTypeIfNeeded(@Nonnull final Type type) {
             if (!typeToNameMap.containsKey(type)) {
+                // Check if we have a structurally identical type with different nullability already registered
+                Type canonicalType = findCanonicalTypeForStructure(type);
+                if (canonicalType != null) {
+                    // Use the same protobuf name as the canonical type
+                    String existingProtoName = typeToNameMap.get(canonicalType);
+                    if (existingProtoName != null) {
+                        typeToNameMap.put(type, existingProtoName);
+                        return this;
+                    }
+                }
+
+                // Standard case: define the protobuf type
                 type.defineProtoType(this);
             }
             return this;
         }
 
+        /**
+         * Finds a type in typeToNameMap that has the same structure as the given type but different nullability.
+         * Returns null if no such type exists.
+         */
+        @Nullable
+        private Type findCanonicalTypeForStructure(@Nonnull final Type type) {
+            for (Map.Entry<Type, String> entry : typeToNameMap.entrySet()) {
+                Type existingType = entry.getKey();
+                if (differsOnlyInNullability(type, existingType)) {
+                    return existingType;
+                }
+            }
+            return null;
+        }
+
         @Nonnull
         public Optional<String> getTypeName(@Nonnull final Type type) {
             return Optional.ofNullable(typeToNameMap.get(type));
         }
 
         @Nonnull
         public Optional<Type> getTypeByName(@Nonnull final String name) {
-            return Optional.ofNullable(typeToNameMap.inverse().get(name));
+            return Optional.ofNullable(nameToCanonicalTypeMap.get(name));
         }
 
         @Nonnull
@@ -500,11 +527,56 @@ public Builder addEnumType(@Nonnull final DescriptorProtos.EnumDescriptorProto e
 
         @Nonnull
         public Builder registerTypeToTypeNameMapping(@Nonnull final Type type, @Nonnull final String protoTypeName) {
-            Verify.verify(!typeToNameMap.containsKey(type));
+            final String existingTypeName = typeToNameMap.get(type);
+            if (existingTypeName != null) {
+                // Type already registered, verify same protobuf name
+                Verify.verify(existingTypeName.equals(protoTypeName), "Type %s is already registered with name %s, cannot register with different name %s", type, existingTypeName, protoTypeName);
+                return this;
+            }
+
+            // Check if a type with same structure but different nullability is already registered
+            final Type existingTypeForName = nameToCanonicalTypeMap.get(protoTypeName);
+            if (existingTypeForName != null) {
+                Verify.verify(differsOnlyInNullability(type, existingTypeForName), "Name %s is already registered with a different type, cannot register different types with same name", existingTypeName);
+                // Allow both nullable and non-nullable variants to map to the same protobuf type
+                // Don't update nameToCanonicalTypeMap - keep the first registered type as canonical
+                typeToNameMap.put(type, protoTypeName);
+                return this;
+            }
+
+            // Standard case: new type with new name
             typeToNameMap.put(type, protoTypeName);
+            nameToCanonicalTypeMap.put(protoTypeName, type);
             return this;
         }
 
+        /**
+         * Checks if two types differ only in their nullability setting.
+         * This is used to allow both nullable and non-nullable variants of the same structural type
+         * to map to the same protobuf type name.
+         */
+        private boolean differsOnlyInNullability(@Nonnull final Type type1, @Nonnull final Type type2) {
+            if (type1.equals(type2)) {
+                return false; // Same type, doesn't differ
+            }
+
+            // Handle Type.Null specially - it can only be nullable, so it can't have a non-nullable variant
+            if (type1 instanceof Type.Null || type2 instanceof Type.Null) {
+                return false; // Type.Null can't have non-nullable variants
+            }
+
+            // Check if they have different nullability
+            if (type1.isNullable() == type2.isNullable()) {
+                return false; // Same nullability, so they differ in structure
+            }
+
+            // Create non-nullable versions to compare structure
+            final Type nonNullable1 = type1.isNullable() ? type1.notNullable() : type1;
+            final Type nonNullable2 = type2.isNullable() ? type2.notNullable() : type2;
+
+            return nonNullable1.equals(nonNullable2);
+        }
+
         @Nonnull
         public Builder addAllTypes(@Nonnull final Collection<Type> types) {
             types.forEach(this::addTypeIfNeeded);

fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/query/StandardQueryTests.java

@@ -1187,7 +1187,7 @@ void testNamingStructsDifferentTypesThrows() throws Exception {
             try (var statement = ddl.setSchemaAndGetConnection().createStatement()) {
                 statement.executeUpdate("insert into t1 values (42, 100, 500, 101)");
                 final var message = Assertions.assertThrows(SQLException.class, () -> statement.execute("select struct asd (a, 42, struct def (b, c), struct def(b, c, a)) as X from t1")).getMessage();
-                Assertions.assertTrue(message.contains("value already present: DEF")); // we could improve this error message.
+                Assertions.assertTrue(message.contains("cannot register different types with same name")); // we could improve this error message.
             }
         }
     }

yaml-tests/src/test/java/YamlIntegrationTests.java

@@ -226,6 +226,11 @@ void arrays(YamlTest.Runner runner) throws Exception {
         runner.runYamsql("arrays.yamsql");
     }
 
+    @TestTemplate
+    public void structTypeVariants(YamlTest.Runner runner) throws Exception {
+        runner.runYamsql("struct-type-nullability-variants.yamsql");
+    }
+
     @TestTemplate
     public void insertEnum(YamlTest.Runner runner) throws Exception {
         runner.runYamsql("insert-enum.yamsql");