Remove recursive type name preservation when parsing meta-data objects

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

@@ -2183,7 +2183,12 @@ public Record withNullability(final boolean newIsNullable) {
 
         @Nonnull
         public Record withName(@Nonnull final String name) {
-            return new Record(name, ProtoUtils.toProtoBufCompliantName(name), isNullable, fields);
+            return withNameAndStorageName(name, ProtoUtils.toProtoBufCompliantName(name));
+        }
+
+        @Nonnull
+        public Record withNameAndStorageName(@Nonnull final String name, @Nonnull final String storageName) {
+            return new Record(name, storageName, isNullable, fields);
         }
 
         @Nullable

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/metadata/serde/RecordMetadataDeserializer.java

@@ -40,14 +40,14 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
+import com.google.common.collect.ImmutableList;
 import com.google.protobuf.Descriptors;
 
 import javax.annotation.Nonnull;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
-import java.util.Objects;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
@@ -133,16 +133,31 @@
     private RecordLayerTable.Builder generateTableBuilder(@Nonnull final String userName, @Nonnull final String storageName) {
         final RecordType recordType = recordMetaData.getRecordType(storageName);
 
         // todo (yhatem) we rely on the record type for deserialization from ProtoBuf for now, later on
         //      we will avoid this step by having our own deserializers.
+        var recordLayerType = Type.Record.fromDescriptor(recordType.getDescriptor()).withNameAndStorageName(userName, storageName);
         // todo (yhatem) this is hacky and must be cleaned up. We need to understand the actually field types so we can take decisions
-        //      on higher level based on these types (wave3).
-        final var recordLayerType = Type.Record.fromDescriptorPreservingName(recordType.getDescriptor());
+        // on higher level based on these types (wave3).
+        if (recordLayerType.getFields().stream().anyMatch(f -> f.getFieldType().isRecord())) {
+            ImmutableList.Builder<Type.Record.Field> newFields = ImmutableList.builder();
+            for (int i = 0; i < recordLayerType.getFields().size(); i++) {
+                final var protoField = recordType.getDescriptor().getFields().get(i);
+                final var field = recordLayerType.getField(i);
+                final Type fieldtype = field.getFieldType();
+                if (fieldtype.isRecord()) {
+                    final String fieldProtoType = protoField.getMessageType().getName();
+                    Type.Record r = ((Type.Record)fieldtype).withNameAndStorageName(ProtoUtils.toProtoBufCompliantName(fieldProtoType), fieldProtoType);
+                    newFields.add(Type.Record.Field.of(r, field.getFieldNameOptional(), field.getFieldIndexOptional()));
+                } else {
+                    newFields.add(field);
+                }
+            }
+            recordLayerType = Type.Record.fromFields(recordLayerType.isNullable(), newFields.build()).withNameAndStorageName(userName, storageName);
+        }
         return RecordLayerTable.Builder
                 .from(recordLayerType)
-                .setName(userName)
                 .setPrimaryKey(recordType.getPrimaryKey())
-                .addIndexes(recordType.getIndexes().stream().map(index -> RecordLayerIndex.from(Objects.requireNonNull(recordLayerType.getName()), Objects.requireNonNull(recordLayerType.getStorageName()), index)).collect(Collectors.toSet()));
+                .addIndexes(recordType.getIndexes().stream().map(index -> RecordLayerIndex.from(userName, storageName, index)).collect(Collectors.toSet()));
     }
 
     @Nonnull

yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/command/CommandUtil.java

@@ -26,7 +26,6 @@
 import com.apple.foundationdb.relational.api.metadata.SchemaTemplate;
 import com.apple.foundationdb.relational.recordlayer.metadata.RecordLayerSchemaTemplate;
 import com.apple.foundationdb.relational.yamltests.generated.schemainstance.SchemaInstanceOuterClass;
-
 import com.google.gson.JsonArray;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
@@ -47,8 +46,11 @@
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.StringTokenizer;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -102,24 +104,50 @@
 
     private static RecordMetaData loadRecordMetaDataFromJson(String jsonFileName) {
         RecordMetaDataProto.MetaData.Builder builder = RecordMetaDataProto.MetaData.newBuilder();
-        List<String> deps = new ArrayList<>();
+        Set<String> neededDependencies = new LinkedHashSet<>();
+        Set<String> includedDependencies = new HashSet<>();
+
+        // These dependencies are automatically added, so we can treat them like they are bundled with the file dependencies
+        includedDependencies.add("record_metadata.proto");
+        includedDependencies.add("record_metadata_options.proto");
+        includedDependencies.add("tuple_fields.proto");
+
         try {
             String jsonStr = Files.readString(Paths.get(jsonFileName), StandardCharsets.UTF_8);
+
+            // Load the definition into the meta-data proto
             JsonFormat.parser().ignoringUnknownFields().merge(jsonStr, builder);
+
+            // Find the list of dependencies of the top-level file
             JsonObject obj = JsonParser.parseString(jsonStr).getAsJsonObject();
             JsonArray dependencyArray = obj.getAsJsonObject("records").getAsJsonArray("dependency");
             for (JsonElement element : dependencyArray) {
                 String curDep = element.getAsString();
-                if (!"record_metadata.proto".equals(curDep) && !"record_metadata_options.proto".equals(curDep)) {
-                    deps.add(curDep);
+                neededDependencies.add(curDep);
+            }
+
+            // Some dependencies may be included in the JSON descriptor itself and do not need to be
+            // provided from the environment
+            JsonArray includedDependencyDefinitions = obj.getAsJsonArray("dependencies");
+            if (includedDependencyDefinitions != null) {
+                for (JsonElement element : includedDependencyDefinitions) {
+                    JsonObject definition = element.getAsJsonObject();
+                    includedDependencies.add(definition.get("name").getAsString());
+                    if (definition.has("dependency")) {
+                        definition.getAsJsonArray("dependency")
+                                .forEach(dep -> neededDependencies.add(dep.getAsString()));
+                    }
                 }
             }
         } catch (IOException e) {
             throw new RuntimeException(e);
         }
 
         List<Descriptors.FileDescriptor> fileDescriptors = new ArrayList<>();
-        for (String dep: deps) {
+        for (String dep: neededDependencies) {
+            if (includedDependencies.contains(dep)) {
+                continue;
+            }
             try {
                 String fullClassName = getFullClassName(dep);
                 Class<?> act = Class.forName(fullClassName);

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

@@ -212,6 +212,11 @@ public void nested(YamlTest.Runner runner) throws Exception {
         runner.runYamsql("nested-tests.yamsql");
     }
 
+    @TestTemplate
+    public void nestedTypeNameClash(YamlTest.Runner runner) throws Exception {
+        runner.runYamsql("nested-type-name-clash.yamsql");
+    }
+
     @TestTemplate
     public void nestedWithNulls(YamlTest.Runner runner) throws Exception {
         runner.runYamsql("nested-with-nulls.yamsql");

yaml-tests/src/test/proto/nested_type_name_clash.proto

@@ -0,0 +1,135 @@
+/*
+ * nested_type_name_clash.proto
+ *
+ * This source file is part of the FoundationDB open source project
+ *
+ * Copyright 2021-2026 Apple Inc. and the FoundationDB project authors
+ *
+ * Licensed 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.
+ */
+
+syntax = "proto3";
+
+// make sure to use this package naming convention:
+// com.apple.foundationdb.relational.yamltests.generated.<name_of_test_suite>
+// adding "generated" is important to exclude the generated Java file from Jacoco reports.
+// suffixing the namespace with your test name is important for segregating similarly-named
+// generated-Java-classes (such as RecordTypeUnion) into separate packages, otherwise you
+// get an error from `protoc`.
+package com.apple.foundationdb.relational.yamltests.generated.nestedtypenameclash;
+
+option java_outer_classname = "NestedTypeNameClashProto";
+
+import "record_metadata_options.proto";
+
+message OuterRecord {
+  optional int64 id = 1 [(com.apple.foundationdb.record.field).primary_key = true];
+  optional Middle middle = 2;
+  optional Inner inner = 3;
+}
+
+message Middle {
+  enum Type {
+    // Same name as Inner.Type, but with different values
+    A = 0;
+    B = 1;
+    C = 2;
+  }
+  enum Category {
+    // Same name as Inner.Category
+    ONE = 0;
+    TWO = 1;
+    THREE = 2;
+    FOUR = 3;
+  }
+  enum Grouping {
+    // Structurally equal to Inner.Sign, but different name
+    ALFA = 0;
+    BRAVO = 1;
+    CHARLIE = 2;
+    DELTA = 3;
+  }
+
+  message Blah {
+    // Same name as Inner.Blah, but different fields
+    optional int64 blah = 1;
+  }
+  message Whatever {
+    // Identical to Inner.Whatever
+    optional string whatever = 1;
+  }
+  message Anything {
+    // Structurally equal to Inner.Payload
+    optional bool datum = 1;
+  }
+
+  optional string middle_id = 1;
+  optional Inner inner = 2;
+
+  optional Type middle_type = 3;
+  optional Category middle_category = 4;
+  optional Grouping middle_grouping = 5;
+  optional Blah middle_blah = 6;
+  optional Whatever middle_whatever = 7;
+  optional Anything middle_anything = 8;
+}
+
+message Inner {
+  enum Type {
+    // Same name as Middle.Type, but with different values
+    A = 0;
+    B = 1;
+    C = 2;
+    D = 3;
+  }
+  enum Category {
+    // Same name as Middle.Category
+    ONE = 0;
+    TWO = 1;
+    THREE = 2;
+    FOUR = 3;
+  }
+  enum Sign {
+    // Structurally equal to Middle.Grouping, but different name
+    ALFA = 0;
+    BRAVO = 1;
+    CHARLIE = 2;
+    DELTA = 3;
+  }
+
+  message Blah {
+    // Same name as Middle.Blah, but different fields
+    optional int64 blah_blah = 1;
+  }
+  message Whatever {
+    // Identical to Outer.Whatever
+    optional string whatever = 1;
+  }
+  message Payload {
+    // Structurally equal to Middle.Anything
+    optional bool datum = 1;
+  }
+
+  optional int64 inner_id = 1;
+
+  optional Type inner_type = 2;
+  optional Category inner_category = 3;
+  optional Sign inner_sign = 4;
+  optional Blah inner_blah = 5;
+  optional Whatever inner_whatever = 6;
+  optional Payload inner_payload = 7;
+}
+
+message RecordTypeUnion {
+  OuterRecord _OuterRecord = 1;
+}