Reintroduce translation of identifiers to Protobuf compliant names

This reintroduces the work that was reverted (namely #3696 and #3706; see also #3726). It mainly copies the original PR for the translation work itself, though it takes a different approach to actually performing the translation.

Perhaps most notably, this purposefully omits translation of function, view, and index names--essentially, any item that is not serialized into a Protobuf identifier is skipped. That means that we only worry about type (including table, struct, and enum names), enum values, and struct/table fields.

From an implementation this perspective, this moves the translation of the names into the areas that are designed to interface between the `Type` system of the planner and the Protobuf-based system of Record Layer core. This is then stored in the `Type` information that is associated with the plan. At the moment, we cheat in a few places and require that the type's fields are always the result of applying the translation function to the display name, or vice versa. However, it allows us to think about allowing custom field names in the future. In a world where we only use Protobuf for storage but have a different mechanism for keeping track of fields in the runtime, we'd need a similar mechanism, though it may take the form of something like a field name to ordinal position mapping, say.

This borrows and expands the tests from #3969 and #3706. In particular, it adds additional tests around enums and view and function definitions with non-compliant names, which now work.

Author: alecgrieser

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

@@ -40,6 +40,7 @@
 import com.apple.foundationdb.record.query.plan.cascades.values.EmptyValue;
 import com.apple.foundationdb.record.query.plan.cascades.values.FieldValue;
 import com.apple.foundationdb.record.query.plan.cascades.values.Value;
+import com.apple.foundationdb.record.util.ProtoUtils;
 import com.google.common.base.Verify;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
@@ -119,7 +120,7 @@ public GraphExpansion visitExpression(@Nonnull final EmptyKeyExpression emptyKey
     @Nonnull
     @Override
     public GraphExpansion visitExpression(@Nonnull FieldKeyExpression fieldKeyExpression) {
-        final String fieldName = fieldKeyExpression.getFieldName();
+        final String fieldName = ProtoUtils.toUserIdentifier(fieldKeyExpression.getFieldName());
         final KeyExpression.FanType fanType = fieldKeyExpression.getFanType();
         final VisitorState state = getCurrentState();
         final List<String> fieldNamePrefix = state.getFieldNamePrefix();

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

@@ -35,6 +35,7 @@
 import com.apple.foundationdb.record.query.plan.cascades.values.FieldValue;
 import com.apple.foundationdb.record.query.plan.cascades.values.QuantifiedObjectValue;
 import com.apple.foundationdb.record.query.plan.cascades.values.Value;
+import com.apple.foundationdb.record.util.ProtoUtils;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 
@@ -124,7 +125,7 @@ public Value visitExpression(@Nonnull FieldKeyExpression fieldKeyExpression) {
         }
 
         final ScalarVisitorState state = getCurrentState();
-        final String fieldName = fieldKeyExpression.getFieldName();
+        final String fieldName = ProtoUtils.toUserIdentifier(fieldKeyExpression.getFieldName());
         final List<String> fieldNamePrefix = state.getFieldNamePrefix();
         final List<String> fieldNames = ImmutableList.<String>builder()
                 .addAll(fieldNamePrefix)

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

@@ -275,7 +275,7 @@ private static List<FieldAccessTrieNodeBuilder> computeFieldAccessForDerivation(
                     Verify.verify(type.isRecord());
                     final var field = ((Type.Record)type).getField(fieldAccessor.getOrdinal());
                     currentTrieBuilder =
-                            currentTrieBuilder.compute(FieldValue.ResolvedAccessor.of(field.getFieldName(), fieldAccessor.getOrdinal(), fieldAccessor.getType()),
+                            currentTrieBuilder.compute(FieldValue.ResolvedAccessor.of(field, fieldAccessor.getOrdinal()),
                                     (resolvedAccessor, oldTrieBuilder) -> {
                                         if (oldTrieBuilder == null) {
                                             return new FieldAccessTrieNodeBuilder(null, null, field.getFieldType());

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

@@ -284,7 +284,7 @@ public static FieldPath resolveFieldPath(@Nonnull final Type inputType, @Nonnull
                 ordinal = accessor.getOrdinal();
             }
             currentType = field.getFieldType();
-            accessorPathBuilder.add(ResolvedAccessor.of(field.getFieldNameOptional().orElse(null), ordinal, currentType));
+            accessorPathBuilder.add(ResolvedAccessor.of(field, ordinal));
         }
         return new FieldPath(accessorPathBuilder.build());
     }
@@ -531,7 +531,7 @@ public static FieldPath empty() {
         @Nonnull
         private static List<Optional<String>> computeFieldNames(@Nonnull final List<ResolvedAccessor> fieldAccessors) {
             return fieldAccessors.stream()
-                    .map(accessor -> Optional.ofNullable(accessor.getName()))
+                    .map(accessor -> accessor.getField().getStorageFieldNameOptional())
                     .collect(ImmutableList.toImmutableList());
         }
 
@@ -550,8 +550,8 @@ private static List<Type> computeFieldTypes(@Nonnull final List<ResolvedAccessor
         }
 
         @Nonnull
-        public static FieldPath ofSingle(@Nullable final String fieldName, @Nonnull final Type fieldType, @Nonnull final Integer fieldOrdinal) {
-            return new FieldPath(ImmutableList.of(ResolvedAccessor.of(fieldName, fieldOrdinal, fieldType)));
+        public static FieldPath ofSingle(@Nonnull Field field, @Nonnull final Integer fieldOrdinal) {
+            return new FieldPath(ImmutableList.of(ResolvedAccessor.of(field, fieldOrdinal)));
         }
 
         @Nonnull
@@ -633,33 +633,47 @@ public int hashCode() {
      * A resolved {@link Accessor} that now also holds the resolved {@link Type}.
      */
     public static class ResolvedAccessor implements PlanSerializable {
+        @Nonnull
+        final Field field;
+
+        /*
         @Nullable
         final String name;
 
-        final int ordinal;
-
         @Nullable
         private final Type type;
+         */
+
+        final int ordinal;
 
-        protected ResolvedAccessor(@Nullable final String name, final int ordinal, @Nullable final Type type) {
+        protected ResolvedAccessor(@Nonnull Field field, int ordinal) {
             Preconditions.checkArgument(ordinal >= 0);
+            /*
             this.name = name;
             this.ordinal = ordinal;
             this.type = type;
+             */
+            this.field = field;
+            this.ordinal = ordinal;
         }
 
         @Nullable
         public String getName() {
-            return name;
+            return field.getFieldNameOptional().orElse(null);
         }
 
         public int getOrdinal() {
             return ordinal;
         }
 
+        @Nonnull
+        public Field getField() {
+            return field;
+        }
+
         @Nonnull
         public Type getType() {
-            return Objects.requireNonNull(type);
+            return Objects.requireNonNull(field.getFieldType());
         }
 
         @Override
@@ -682,17 +696,17 @@ public int hashCode() {
         @Nonnull
         @Override
         public String toString() {
-            return name + ';' + ordinal + ';' + type;
+            return getName() + ';' + ordinal + ';' + getType();
         }
 
         @Nonnull
         @Override
         public PResolvedAccessor toProto(@Nonnull final PlanSerializationContext serializationContext) {
             PResolvedAccessor.Builder builder = PResolvedAccessor.newBuilder();
-            builder.setName(name);
+            builder.setName(field.getFieldName());
             builder.setOrdinal(ordinal);
-            if (type != null) {
-                builder.setType(type.toTypeProto(serializationContext));
+            if (field.getFieldType() != null) {
+                builder.setType(getType().toTypeProto(serializationContext));
             }
             return builder.build();
         }
@@ -706,22 +720,25 @@ public static ResolvedAccessor fromProto(@Nonnull PlanSerializationContext seria
             } else {
                 type = null;
             }
-            return new ResolvedAccessor(resolvedAccessorProto.getName(), resolvedAccessorProto.getOrdinal(), type);
+            final Field field = Field.of(type, Optional.of(resolvedAccessorProto.getName()));
+            return new ResolvedAccessor(field, resolvedAccessorProto.getOrdinal());
         }
 
         @Nonnull
         public static ResolvedAccessor of(@Nonnull final Field field, final int ordinal) {
-            return of(field.getFieldNameOptional().orElse(null), ordinal, field.getFieldType());
+            return new ResolvedAccessor(field, ordinal);
         }
 
         @Nonnull
         public static ResolvedAccessor of(@Nullable final String fieldName, final int ordinalFieldNumber, @Nonnull final Type type) {
-            return new ResolvedAccessor(fieldName, ordinalFieldNumber, type);
+            final Field field = Field.of(type, Optional.ofNullable(fieldName));
+            return new ResolvedAccessor(field, ordinalFieldNumber);
         }
 
         @Nonnull
         public static ResolvedAccessor of(@Nullable final String fieldName, final int ordinalFieldNumber) {
-            return new ResolvedAccessor(fieldName, ordinalFieldNumber, null);
+            final Field field = Field.of(null, Optional.ofNullable(fieldName));
+            return new ResolvedAccessor(field, ordinalFieldNumber);
         }
     }
 

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

@@ -41,6 +41,7 @@
 import com.apple.foundationdb.record.query.plan.cascades.typing.Type;
 import com.apple.foundationdb.record.query.plan.cascades.values.MessageHelpers.CoercionTrieNode;
 import com.apple.foundationdb.record.query.plan.serialization.PlanSerialization;
+import com.apple.foundationdb.record.util.ProtoUtils;
 import com.apple.foundationdb.record.util.pair.NonnullPair;
 import com.google.auto.service.AutoService;
 import com.google.common.base.Suppliers;
@@ -144,7 +145,7 @@ public static PhysicalOperator fromProto(@Nonnull final PlanSerializationContext
 
         @Nonnull
         public static Descriptors.EnumValueDescriptor stringToEnumValue(Descriptors.EnumDescriptor enumDescriptor, String value) {
-            final var maybeValue = enumDescriptor.findValueByName(value);
+            final var maybeValue = enumDescriptor.findValueByName(ProtoUtils.toProtoBufCompliantName(value));
             SemanticException.check(maybeValue != null, SemanticException.ErrorCode.INVALID_ENUM_VALUE, value);
             return maybeValue;
         }

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

@@ -111,8 +111,7 @@ public static List<Value> primitiveAccessorsForType(@Nonnull final Type type,
         for (int i = 0; i < fields.size(); i++) {
             final var field = fields.get(i);
             final var singleStepPath =
-                    FieldValue.FieldPath.ofSingle(FieldValue.ResolvedAccessor.of(
-                            field.getFieldNameOptional().orElse(null), i, field.getFieldType()));
+                    FieldValue.FieldPath.ofSingle(FieldValue.ResolvedAccessor.of(field, i));
             primitiveAccessorsForType(field.getFieldType(),
                     () -> FieldValue.ofFieldsAndFuseIfPossible(baseValueSupplier.get(), singleStepPath))
                     .forEach(orderingValuesBuilder::add);

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

@@ -81,7 +81,7 @@ public void onMatch(@Nonnull final ValueComputationRuleCall<Iterable<? extends V
                     final var argumentValueCompensation = childValueEntry.getValue();
                     final var field = column.getField();
                     resultingMatchedValuesMap.putIfAbsent(argumentValue,
-                            new FieldValueCompensation(FieldValue.FieldPath.ofSingle(field.getFieldNameOptional().orElse(null), field.getFieldType(), i), argumentValueCompensation));
+                            new FieldValueCompensation(FieldValue.FieldPath.ofSingle(field, i), argumentValueCompensation));
                 }
             }
         }

fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/simplification/CompensateRecordConstructorRule.java

@@ -21,20 +21,65 @@
 package com.apple.foundationdb.record.util;
 
 import com.apple.foundationdb.record.PlanHashable;
+import com.apple.foundationdb.record.metadata.MetaDataException;
 import com.google.protobuf.Internal;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import java.util.List;
 import java.util.Objects;
 import java.util.UUID;
+import java.util.regex.Pattern;
 
 /**
  * Utility functions for interacting with protobuf.
  */
 public class ProtoUtils {
 
+    private static final String DOUBLE_UNDERSCORE_ESCAPE = "__0";
+    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_]*$");
+
     private ProtoUtils() {
     }
 
+    @Nonnull
+    public static String toProtoBufCompliantName(final String name) {
+        if (INVALID_START_SEQUENCES.stream().anyMatch(name::startsWith)) {
+            throw new InvalidNameException("name cannot start with " + INVALID_START_SEQUENCES);
+        }
+        String translated;
+        if (name.startsWith("__")) {
+            translated = "__" + translateSpecialCharacters(name.substring(2));
+        } else {
+            if (name.isEmpty()) {
+                throw new InvalidNameException("name cannot be empty string");
+            }
+            translated = translateSpecialCharacters(name);
+        }
+        checkValidProtoBufCompliantName(translated);
+        return translated;
+    }
+
+    public static void checkValidProtoBufCompliantName(String name) {
+        if (!VALID_PROTOBUF_COMPLIANT_NAME_PATTERN.matcher(name).matches()) {
+            throw new InvalidNameException(name + " it not a valid protobuf identifier");
+        }
+    }
+
+    @Nonnull
+    private static String translateSpecialCharacters(final String userIdentifier) {
+        return userIdentifier.replace("__", DOUBLE_UNDERSCORE_ESCAPE).replace("$", DOLLAR_ESCAPE).replace(".", DOT_ESCAPE);
+    }
+
+    public static String toUserIdentifier(String protoIdentifier) {
+        return protoIdentifier.replace(DOT_ESCAPE, ".").replace(DOLLAR_ESCAPE, "$").replace(DOUBLE_UNDERSCORE_ESCAPE, "__");
+    }
+
     /**
      * Generates a JVM-wide unique type name.
      * @return a unique type name.
@@ -103,4 +148,11 @@ public String toString() {
             return getName();
         }
     }
+
+    @SuppressWarnings("serial")
+    public static class InvalidNameException extends MetaDataException {
+        public InvalidNameException(@Nonnull final String msg, @Nullable final Object... keyValues) {
+            super(msg, keyValues);
+        }
+    }
 }

fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/util/ProtoUtils.java

@@ -90,24 +90,28 @@ message PType {
     message PEnumValue {
       optional string name = 1;
       optional int32 number = 2;
+      optional string storage_name = 3;
     }
 
     optional bool is_nullable = 1;
     repeated PEnumValue enum_values = 2;
     optional string name = 3; // referential name -- not used in the planner
+    optional string storage_name = 4;
   }
 
   message PRecordType {
     message PField {
       optional PType field_type = 1;
       optional string field_name = 2;
       optional int32 field_index = 3;
+      optional string storage_field_name = 4;
     }
 
     optional int32 reference_id = 1;
     optional string name = 2;  // referential name -- not used in the planner
     optional bool is_nullable = 3;
     repeated PField fields = 4;
+    optional string storage_name = 5;
   }
 
   message PRelationType {