Reintroduce translation of identifiers to Protobuf compliant names

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/values/FieldValue.java

@@ -62,6 +62,7 @@
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.UUID;
@@ -284,7 +285,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 +532,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().getFieldStorageNameOptional())
                     .collect(ImmutableList.toImmutableList());
         }
 
@@ -550,8 +551,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 +634,33 @@ public int hashCode() {
      * A resolved {@link Accessor} that now also holds the resolved {@link Type}.
      */
     public static class ResolvedAccessor implements PlanSerializable {
-        @Nullable
-        final String name;
-
+        @Nonnull
+        final Field field;
         final int ordinal;
 
-        @Nullable
-        private final Type type;
-
-        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.field = field;
             this.ordinal = ordinal;
-            this.type = type;
         }
 
         @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,18 +683,21 @@ 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);
+            // Older serialization: write out the name, ordinal, and type manually
+            builder.setName(field.getFieldName());
             builder.setOrdinal(ordinal);
-            if (type != null) {
-                builder.setType(type.toTypeProto(serializationContext));
+            if (field.getFieldType() != null) {
+                builder.setType(getType().toTypeProto(serializationContext));
             }
+            // Newer serialization: write that information in a nested field
+            builder.setField(field.toProto(serializationContext));
             return builder.build();
         }
 
@@ -706,22 +710,37 @@ public static ResolvedAccessor fromProto(@Nonnull PlanSerializationContext seria
             } else {
                 type = null;
             }
-            return new ResolvedAccessor(resolvedAccessorProto.getName(), resolvedAccessorProto.getOrdinal(), type);
+
+            final Field field;
+            if (resolvedAccessorProto.hasField()) {
+                // Newer serialization: use a single nested field. If both are set, we need to deserialize
+                // the field after reading the type information, as the type will be cached in the
+                // serialization context
+                field = Field.fromProto(serializationContext, resolvedAccessorProto.getField());
+            } else {
+                // Older serialization: get the name and type information from separate fields
+                field = Field.of(Objects.requireNonNull(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);
+        public static ResolvedAccessor of(@Nonnull final Type.Record recordType, @Nonnull final String fieldName, final int ordinalFieldNumber) {
+            final Map<String, Field> fieldNameMap = recordType.getFieldNameFieldMap();
+            Field field = fieldNameMap.get(fieldName);
+            SemanticException.check(field != null, SemanticException.ErrorCode.RECORD_DOES_NOT_CONTAIN_FIELD);
+            return new ResolvedAccessor(field, ordinalFieldNumber);
         }
     }
 

fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/PromoteValue.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/Values.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/simplification/CompensateRecordConstructorRule.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/util/ProtoUtils.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 @@
             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/proto/record_query_plan.proto

@@ -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 field_storage_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 {
@@ -482,6 +486,7 @@ message PFieldPath {
     optional string name = 1;
     optional int32 ordinal = 2;
     optional PType type = 3;
+    optional PType.PRecordType.PField field = 4;
   }
   repeated PResolvedAccessor field_accessors = 1;
 }

fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/query/FDBSimpleQueryGraphTest.java

@@ -828,24 +828,24 @@ void testMediumJoinTypeEvolutionIdentical() {
         //
         var childrenMap = fieldAccessesRestaurantRecord.getChildrenMap();
         Assertions.assertNotNull(childrenMap);
-        var childFieldAccesses = childrenMap.get(FieldValue.ResolvedAccessor.of("name", 1));
+        var childFieldAccesses = childrenMap.get(FieldValue.ResolvedAccessor.of(restaurantReviewerType, "name", 1));
         Assertions.assertNotNull(childFieldAccesses);
         var leafType = childFieldAccesses.getValue();
         Assertions.assertNotNull(leafType);
         Assertions.assertEquals(leafType, Type.primitiveType(Type.TypeCode.STRING, true));
 
-        childFieldAccesses = childrenMap.get(FieldValue.ResolvedAccessor.of("reviews", 2));
+        childFieldAccesses = childrenMap.get(FieldValue.ResolvedAccessor.of(restaurantRecordType, "reviews", 2));
         Assertions.assertNotNull(childFieldAccesses);
         childrenMap = fieldAccessesRestaurantRecord.getChildrenMap();
         Assertions.assertNotNull(childrenMap);
-        childFieldAccesses = childrenMap.get(FieldValue.ResolvedAccessor.of("reviewer", 0));
+        childFieldAccesses = childrenMap.get(FieldValue.ResolvedAccessor.of(reviewsType, "reviewer", 0));
         Assertions.assertNotNull(childFieldAccesses);
         leafType = childFieldAccesses.getValue();
         Assertions.assertNotNull(leafType);
         Assertions.assertEquals(leafType, Type.primitiveType(Type.TypeCode.LONG, false));
 
         childrenMap = fieldAccessesRestaurantRecord.getChildrenMap();
-        childFieldAccesses = childrenMap.get(FieldValue.ResolvedAccessor.of("rest_no", 0));
+        childFieldAccesses = childrenMap.get(FieldValue.ResolvedAccessor.of(restaurantRecordType, "rest_no", 0));
         Assertions.assertNotNull(childFieldAccesses);
         leafType = childFieldAccesses.getValue();
         Assertions.assertNotNull(leafType);
@@ -861,13 +861,13 @@ void testMediumJoinTypeEvolutionIdentical() {
         //
         childrenMap = fieldAccessesRestaurantReviewer.getChildrenMap();
         Assertions.assertNotNull(childrenMap);
-        childFieldAccesses = childrenMap.get(FieldValue.ResolvedAccessor.of("name", 1));
+        childFieldAccesses = childrenMap.get(FieldValue.ResolvedAccessor.of(restaurantReviewerType, "name", 1));
         Assertions.assertNotNull(childFieldAccesses);
         leafType = childFieldAccesses.getValue();
         Assertions.assertNotNull(leafType);
         Assertions.assertEquals(leafType, Type.primitiveType(Type.TypeCode.STRING, false));
 
-        childFieldAccesses = childrenMap.get(FieldValue.ResolvedAccessor.of("id", 0));
+        childFieldAccesses = childrenMap.get(FieldValue.ResolvedAccessor.of(restaurantReviewerType, "id", 0));
         Assertions.assertNotNull(childFieldAccesses);
         leafType = childFieldAccesses.getValue();
         Assertions.assertNotNull(leafType);