Omit __ prefixed identifiers from protobuf translation

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

@@ -34,6 +34,7 @@
 import java.util.Locale;
 import java.util.Optional;
 import java.util.UUID;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 @API(API.Status.EXPERIMENTAL)
@@ -43,6 +44,10 @@ public class DataTypeUtils {
     private static final String DOLLAR_ESCAPE = "__1";
     private static final String DOT_ESCAPE = "__2";
 
+    private static final List<String> INVALID_START_SEQUENCES = List.of(".", "$", DOUBLE_UNDERSCORE_ESCAPE, DOLLAR_ESCAPE, DOT_ESCAPE);
+
+    private static final Pattern VALID_PROTOBUF_COMPLIANT_NAME_PATTERN = Pattern.compile("^[A-Za-z_][A-Za-z0-9_]*$");
+
     @Nonnull
     private static final BiMap<DataType, Type> primitivesMap;
 
@@ -116,10 +121,29 @@ private static String getUniqueName() {
         return "id" + modified;
     }
 
-    public static String toProtoBufCompliantName(String userIdentifier) {
+    @Nonnull
+    public static String toProtoBufCompliantName(final String name) {
+        Assert.thatUnchecked(INVALID_START_SEQUENCES.stream().noneMatch(name::startsWith), ErrorCode.INVALID_NAME, "name cannot start with %s", INVALID_START_SEQUENCES);
+        String translated;
+        if (name.startsWith("__")) {
+            translated = "__" + translateSpecialCharacters(name.substring(2));
+        } else {
+            Assert.thatUnchecked(!name.isEmpty(), ErrorCode.INVALID_NAME, "name cannot be empty String.");
+            translated = translateSpecialCharacters(name);
+        }
+        checkValidProtoBufCompliantName(translated);
+        return translated;
+    }
+
+    @Nonnull
+    private static String translateSpecialCharacters(final String userIdentifier) {
         return userIdentifier.replace("__", DOUBLE_UNDERSCORE_ESCAPE).replace("$", DOLLAR_ESCAPE).replace(".", DOT_ESCAPE);
     }
 
+    public static void checkValidProtoBufCompliantName(String name) {
+        Assert.thatUnchecked(VALID_PROTOBUF_COMPLIANT_NAME_PATTERN.matcher(name).matches(), ErrorCode.INVALID_NAME, name + " is not a valid name!");
+    }
+
     public static String toUserIdentifier(String protoIdentifier) {
         return protoIdentifier.replace(DOT_ESCAPE, ".").replace(DOLLAR_ESCAPE, "$").replace(DOUBLE_UNDERSCORE_ESCAPE, "__");
     }

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/Identifier.java

@@ -147,8 +147,7 @@ public boolean qualifiedWith(@Nonnull Identifier identifier) {
     @Nonnull
     public static Identifier toProtobufCompliant(@Nonnull final Identifier identifier) {
         final var qualifier = identifier.getQualifier().stream().map(DataTypeUtils::toProtoBufCompliantName).collect(Collectors.toList());
-        final var name = DataTypeUtils.toProtoBufCompliantName(identifier.getName());
-        return Identifier.of(name, qualifier);
+        return Identifier.of(DataTypeUtils.toProtoBufCompliantName(identifier.getName()), qualifier);
     }
 
     @Override

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/PseudoColumn.java

@@ -73,13 +73,4 @@ public static Optional<Expression> mapToExpressionMaybe(@Nonnull LogicalOperator
         }
         return Optional.empty();
     }
-
-    public static boolean isPseudoColumn(@Nonnull String name) {
-        for (PseudoColumn pseudo : PseudoColumn.values()) {
-            if (name.equals(pseudo.getColumnName())) {
-                return true;
-            }
-        }
-        return false;
-    }
 }

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/visitors/DdlVisitor.java

@@ -376,7 +376,7 @@ private RecordLayerView getViewMetadata(@Nonnull final RelationalParser.ViewDefi
         getDelegate().replaceSchemaTemplate(ddlCatalog);
 
         // 1. get the view name.
-        final var viewName = visitFullId(viewCtx.viewName).toString();
+        final var viewName = Identifier.toProtobufCompliant(visitFullId(viewCtx.viewName)).getName();
 
         // 2. get the view SQL definition string.
         final var queryString = getDelegate().getPlanGenerationContext().getQuery();

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/visitors/ExpressionVisitor.java

@@ -48,7 +48,6 @@
 import com.apple.foundationdb.relational.recordlayer.query.LogicalPlanFragment;
 import com.apple.foundationdb.relational.recordlayer.query.OrderByExpression;
 import com.apple.foundationdb.relational.recordlayer.query.ParseHelpers;
-import com.apple.foundationdb.relational.recordlayer.query.PseudoColumn;
 import com.apple.foundationdb.relational.recordlayer.query.SemanticAnalyzer;
 import com.apple.foundationdb.relational.recordlayer.query.StringTrieNode;
 import com.apple.foundationdb.relational.recordlayer.query.TautologicalValue;
@@ -181,11 +180,7 @@ public Expression visitSelectExpressionElement(@Nonnull RelationalParser.SelectE
     @Override
     public Expression visitFullColumnName(@Nonnull RelationalParser.FullColumnNameContext fullColumnNameContext) {
         final var id = visitFullId(fullColumnNameContext.fullId());
-        if (!PseudoColumn.isPseudoColumn(id.getName())) {
-            return getDelegate().getSemanticAnalyzer().resolveIdentifier(Identifier.toProtobufCompliant(id), getDelegate().getCurrentPlanFragment());
-        } else {
-            return getDelegate().getSemanticAnalyzer().resolveIdentifier(id.replaceQualifier(q -> q.stream().map(DataTypeUtils::toProtoBufCompliantName).collect(Collectors.toList())), getDelegate().getCurrentPlanFragment());
-        }
+        return getDelegate().getSemanticAnalyzer().resolveIdentifier(Identifier.toProtobufCompliant(id), getDelegate().getCurrentPlanFragment());
     }
 
     @Nonnull

fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/metadata/DataTypeUtilsTest.java

@@ -0,0 +1,83 @@
+/*
+ * DataTypeUtilsTest.java
+ *
+ * This source file is part of the FoundationDB open source project
+ *
+ * Copyright 2015-2025 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.
+ */
+
+package com.apple.foundationdb.relational.recordlayer.metadata;
+
+import com.apple.foundationdb.relational.api.exceptions.UncheckedRelationalException;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import java.util.stream.Stream;
+
+public class DataTypeUtilsTest {
+
+
+    static Stream<Arguments> protobufCompliantNameTestArguments() {
+        return Stream.of(
+                Arguments.of("__", "__"),
+                Arguments.of("_", "_"),
+                Arguments.of("$", null),
+                Arguments.of(".", null),
+                Arguments.of("__hello", "__hello"),
+                Arguments.of("__$hello", "____1hello"),
+                Arguments.of("__$hello", "____1hello"),
+                Arguments.of("__.hello", "____2hello"),
+                Arguments.of("____hello", "____0hello"),
+                Arguments.of("__$h$e$l$l$o", "____1h__1e__1l__1l__1o"),
+                Arguments.of("__0", null),
+                Arguments.of("__0hello", null),
+                Arguments.of("__1", null),
+                Arguments.of("__1hello", null),
+                Arguments.of("__2", null),
+                Arguments.of("__2hello", null),
+                Arguments.of("__4", "__4"),
+                Arguments.of("__$$$$", "____1__1__1__1"),
+                Arguments.of("______", "____0__0"),
+                Arguments.of("__4hello", "__4hello"),
+                Arguments.of("$", null),
+                Arguments.of("$hello", null),
+                Arguments.of(".", null),
+                Arguments.of(".hello", null),
+                Arguments.of("h__e__l__l__o", "h__0e__0l__0l__0o"),
+                Arguments.of("h.e.l.l.o", "h__2e__2l__2l__2o"),
+                Arguments.of("h$e$l$l$o", "h__1e__1l__1l__1o"),
+                Arguments.of("1hello", null),
+                Arguments.of("डेटाबेस", null)
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("protobufCompliantNameTestArguments")
+    void protobufCompliantNameTest(@Nonnull String userIdentifier, @Nullable String protobufIdentifier) {
+        if (protobufIdentifier != null) {
+            final var actual = DataTypeUtils.toProtoBufCompliantName(userIdentifier);
+            Assertions.assertThat(actual).isEqualTo(protobufIdentifier);
+            final var reTranslated = DataTypeUtils.toUserIdentifier(actual);
+            Assertions.assertThat(reTranslated).isEqualTo(userIdentifier);
+        } else {
+            Assertions.assertThatThrownBy(() -> DataTypeUtils.toProtoBufCompliantName(userIdentifier))
+                    .isInstanceOf(UncheckedRelationalException.class);
+        }
+    }
+}

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

@@ -0,0 +1,70 @@
+/*
+ * identifiers.proto
+ *
+ * This source file is part of the FoundationDB open source project
+ *
+ * Copyright 2021-2024 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.identifierstests;
+
+option java_outer_classname = "IdentifiersTestProto";
+
+import "record_metadata_options.proto";
+
+message T1 {
+  int64 ID = 1 [(com.apple.foundationdb.record.field).primary_key = true];
+  int64 T1__2COL1 = 2;
+  int64 T1__2COL2 = 3;
+}
+
+message T2 {
+  int64 ID = 1 [(com.apple.foundationdb.record.field).primary_key = true];
+  int64 T2__1COL1 = 2;
+  int64 T2__1COL2 = 3;
+}
+
+message __T3 {
+  int64 ID = 1 [(com.apple.foundationdb.record.field).primary_key = true];
+  int64 __T3__1COL1 = 2;
+  int64 __T3__1COL2 = 3;
+}
+
+message internal {
+  int64 a = 1;
+  int64 b = 2;
+}
+
+message T4 {
+  int64 ID = 1 [(com.apple.foundationdb.record.field).primary_key = true];
+  internal ___hidden = 2;
+  int64 T4__2COL1 = 3;
+  int64 T4__2COL2 = 4;
+}
+
+message RecordTypeUnion {
+  T1 _T1 = 1;
+  T2 _T2 = 2;
+  __T3 ___T3 = 3;
+  T4 _T4 = 4;
+}