Skip to content

Commit 4873b51

Browse files
authored
Reintroduce translation of identifiers to Protobuf compliant names (#3736)
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 #3696 and #3706. In particular, it adds additional tests around enums and view and function definitions with non-compliant names, which now work.
1 parent 8f4d50a commit 4873b51

File tree

47 files changed

+3753
-235
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+3753
-235
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import com.apple.foundationdb.record.query.plan.cascades.values.EmptyValue;
4141
import com.apple.foundationdb.record.query.plan.cascades.values.FieldValue;
4242
import com.apple.foundationdb.record.query.plan.cascades.values.Value;
43+
import com.apple.foundationdb.record.util.ProtoUtils;
4344
import com.google.common.base.Verify;
4445
import com.google.common.collect.ImmutableList;
4546
import com.google.common.collect.ImmutableSet;
@@ -119,7 +120,7 @@ public GraphExpansion visitExpression(@Nonnull final EmptyKeyExpression emptyKey
119120
@Nonnull
120121
@Override
121122
public GraphExpansion visitExpression(@Nonnull FieldKeyExpression fieldKeyExpression) {
122-
final String fieldName = fieldKeyExpression.getFieldName();
123+
final String fieldName = ProtoUtils.toUserIdentifier(fieldKeyExpression.getFieldName());
123124
final KeyExpression.FanType fanType = fieldKeyExpression.getFanType();
124125
final VisitorState state = getCurrentState();
125126
final List<String> fieldNamePrefix = state.getFieldNamePrefix();

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import com.apple.foundationdb.record.query.plan.cascades.values.FieldValue;
3636
import com.apple.foundationdb.record.query.plan.cascades.values.QuantifiedObjectValue;
3737
import com.apple.foundationdb.record.query.plan.cascades.values.Value;
38+
import com.apple.foundationdb.record.util.ProtoUtils;
3839
import com.google.common.collect.ImmutableList;
3940
import com.google.common.collect.Iterables;
4041

@@ -124,7 +125,7 @@ public Value visitExpression(@Nonnull FieldKeyExpression fieldKeyExpression) {
124125
}
125126

126127
final ScalarVisitorState state = getCurrentState();
127-
final String fieldName = fieldKeyExpression.getFieldName();
128+
final String fieldName = ProtoUtils.toUserIdentifier(fieldKeyExpression.getFieldName());
128129
final List<String> fieldNamePrefix = state.getFieldNamePrefix();
129130
final List<String> fieldNames = ImmutableList.<String>builder()
130131
.addAll(fieldNamePrefix)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ private static List<FieldAccessTrieNodeBuilder> computeFieldAccessForDerivation(
275275
Verify.verify(type.isRecord());
276276
final var field = ((Type.Record)type).getField(fieldAccessor.getOrdinal());
277277
currentTrieBuilder =
278-
currentTrieBuilder.compute(FieldValue.ResolvedAccessor.of(field.getFieldName(), fieldAccessor.getOrdinal(), fieldAccessor.getType()),
278+
currentTrieBuilder.compute(FieldValue.ResolvedAccessor.of(field, fieldAccessor.getOrdinal()),
279279
(resolvedAccessor, oldTrieBuilder) -> {
280280
if (oldTrieBuilder == null) {
281281
return new FieldAccessTrieNodeBuilder(null, null, field.getFieldType());

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

Lines changed: 121 additions & 35 deletions
Large diffs are not rendered by default.

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

Lines changed: 43 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
import java.util.ArrayList;
6363
import java.util.Comparator;
6464
import java.util.List;
65+
import java.util.Map;
6566
import java.util.Objects;
6667
import java.util.Optional;
6768
import java.util.UUID;
@@ -284,7 +285,7 @@ public static FieldPath resolveFieldPath(@Nonnull final Type inputType, @Nonnull
284285
ordinal = accessor.getOrdinal();
285286
}
286287
currentType = field.getFieldType();
287-
accessorPathBuilder.add(ResolvedAccessor.of(field.getFieldNameOptional().orElse(null), ordinal, currentType));
288+
accessorPathBuilder.add(ResolvedAccessor.of(field, ordinal));
288289
}
289290
return new FieldPath(accessorPathBuilder.build());
290291
}
@@ -531,7 +532,7 @@ public static FieldPath empty() {
531532
@Nonnull
532533
private static List<Optional<String>> computeFieldNames(@Nonnull final List<ResolvedAccessor> fieldAccessors) {
533534
return fieldAccessors.stream()
534-
.map(accessor -> Optional.ofNullable(accessor.getName()))
535+
.map(accessor -> accessor.getField().getFieldStorageNameOptional())
535536
.collect(ImmutableList.toImmutableList());
536537
}
537538

@@ -550,8 +551,8 @@ private static List<Type> computeFieldTypes(@Nonnull final List<ResolvedAccessor
550551
}
551552

552553
@Nonnull
553-
public static FieldPath ofSingle(@Nullable final String fieldName, @Nonnull final Type fieldType, @Nonnull final Integer fieldOrdinal) {
554-
return new FieldPath(ImmutableList.of(ResolvedAccessor.of(fieldName, fieldOrdinal, fieldType)));
554+
public static FieldPath ofSingle(@Nonnull Field field, @Nonnull final Integer fieldOrdinal) {
555+
return new FieldPath(ImmutableList.of(ResolvedAccessor.of(field, fieldOrdinal)));
555556
}
556557

557558
@Nonnull
@@ -633,33 +634,33 @@ public int hashCode() {
633634
* A resolved {@link Accessor} that now also holds the resolved {@link Type}.
634635
*/
635636
public static class ResolvedAccessor implements PlanSerializable {
636-
@Nullable
637-
final String name;
638-
637+
@Nonnull
638+
final Field field;
639639
final int ordinal;
640640

641-
@Nullable
642-
private final Type type;
643-
644-
protected ResolvedAccessor(@Nullable final String name, final int ordinal, @Nullable final Type type) {
641+
protected ResolvedAccessor(@Nonnull Field field, int ordinal) {
645642
Preconditions.checkArgument(ordinal >= 0);
646-
this.name = name;
643+
this.field = field;
647644
this.ordinal = ordinal;
648-
this.type = type;
649645
}
650646

651647
@Nullable
652648
public String getName() {
653-
return name;
649+
return field.getFieldNameOptional().orElse(null);
654650
}
655651

656652
public int getOrdinal() {
657653
return ordinal;
658654
}
659655

656+
@Nonnull
657+
public Field getField() {
658+
return field;
659+
}
660+
660661
@Nonnull
661662
public Type getType() {
662-
return Objects.requireNonNull(type);
663+
return Objects.requireNonNull(field.getFieldType());
663664
}
664665

665666
@Override
@@ -682,18 +683,21 @@ public int hashCode() {
682683
@Nonnull
683684
@Override
684685
public String toString() {
685-
return name + ';' + ordinal + ';' + type;
686+
return getName() + ';' + ordinal + ';' + getType();
686687
}
687688

688689
@Nonnull
689690
@Override
690691
public PResolvedAccessor toProto(@Nonnull final PlanSerializationContext serializationContext) {
691692
PResolvedAccessor.Builder builder = PResolvedAccessor.newBuilder();
692-
builder.setName(name);
693+
// Older serialization: write out the name, ordinal, and type manually
694+
builder.setName(field.getFieldName());
693695
builder.setOrdinal(ordinal);
694-
if (type != null) {
695-
builder.setType(type.toTypeProto(serializationContext));
696+
if (field.getFieldType() != null) {
697+
builder.setType(getType().toTypeProto(serializationContext));
696698
}
699+
// Newer serialization: write that information in a nested field
700+
builder.setField(field.toProto(serializationContext));
697701
return builder.build();
698702
}
699703

@@ -706,22 +710,37 @@ public static ResolvedAccessor fromProto(@Nonnull PlanSerializationContext seria
706710
} else {
707711
type = null;
708712
}
709-
return new ResolvedAccessor(resolvedAccessorProto.getName(), resolvedAccessorProto.getOrdinal(), type);
713+
714+
final Field field;
715+
if (resolvedAccessorProto.hasField()) {
716+
// Newer serialization: use a single nested field. If both are set, we need to deserialize
717+
// the field after reading the type information, as the type will be cached in the
718+
// serialization context
719+
field = Field.fromProto(serializationContext, resolvedAccessorProto.getField());
720+
} else {
721+
// Older serialization: get the name and type information from separate fields
722+
field = Field.of(Objects.requireNonNull(type), Optional.of(resolvedAccessorProto.getName()));
723+
}
724+
return new ResolvedAccessor(field, resolvedAccessorProto.getOrdinal());
710725
}
711726

712727
@Nonnull
713728
public static ResolvedAccessor of(@Nonnull final Field field, final int ordinal) {
714-
return of(field.getFieldNameOptional().orElse(null), ordinal, field.getFieldType());
729+
return new ResolvedAccessor(field, ordinal);
715730
}
716731

717732
@Nonnull
718733
public static ResolvedAccessor of(@Nullable final String fieldName, final int ordinalFieldNumber, @Nonnull final Type type) {
719-
return new ResolvedAccessor(fieldName, ordinalFieldNumber, type);
734+
final Field field = Field.of(type, Optional.ofNullable(fieldName));
735+
return new ResolvedAccessor(field, ordinalFieldNumber);
720736
}
721737

722738
@Nonnull
723-
public static ResolvedAccessor of(@Nullable final String fieldName, final int ordinalFieldNumber) {
724-
return new ResolvedAccessor(fieldName, ordinalFieldNumber, null);
739+
public static ResolvedAccessor of(@Nonnull final Type.Record recordType, @Nonnull final String fieldName, final int ordinalFieldNumber) {
740+
final Map<String, Field> fieldNameMap = recordType.getFieldNameFieldMap();
741+
Field field = fieldNameMap.get(fieldName);
742+
SemanticException.check(field != null, SemanticException.ErrorCode.RECORD_DOES_NOT_CONTAIN_FIELD);
743+
return new ResolvedAccessor(field, ordinalFieldNumber);
725744
}
726745
}
727746

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import com.apple.foundationdb.record.query.plan.cascades.typing.Type;
4242
import com.apple.foundationdb.record.query.plan.cascades.values.MessageHelpers.CoercionTrieNode;
4343
import com.apple.foundationdb.record.query.plan.serialization.PlanSerialization;
44+
import com.apple.foundationdb.record.util.ProtoUtils;
4445
import com.apple.foundationdb.record.util.pair.NonnullPair;
4546
import com.google.auto.service.AutoService;
4647
import com.google.common.base.Suppliers;
@@ -144,7 +145,7 @@ public static PhysicalOperator fromProto(@Nonnull final PlanSerializationContext
144145

145146
@Nonnull
146147
public static Descriptors.EnumValueDescriptor stringToEnumValue(Descriptors.EnumDescriptor enumDescriptor, String value) {
147-
final var maybeValue = enumDescriptor.findValueByName(value);
148+
final var maybeValue = enumDescriptor.findValueByName(ProtoUtils.toProtoBufCompliantName(value));
148149
SemanticException.check(maybeValue != null, SemanticException.ErrorCode.INVALID_ENUM_VALUE, value);
149150
return maybeValue;
150151
}

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,7 @@ public static List<Value> primitiveAccessorsForType(@Nonnull final Type type,
111111
for (int i = 0; i < fields.size(); i++) {
112112
final var field = fields.get(i);
113113
final var singleStepPath =
114-
FieldValue.FieldPath.ofSingle(FieldValue.ResolvedAccessor.of(
115-
field.getFieldNameOptional().orElse(null), i, field.getFieldType()));
114+
FieldValue.FieldPath.ofSingle(FieldValue.ResolvedAccessor.of(field, i));
116115
primitiveAccessorsForType(field.getFieldType(),
117116
() -> FieldValue.ofFieldsAndFuseIfPossible(baseValueSupplier.get(), singleStepPath))
118117
.forEach(orderingValuesBuilder::add);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public void onMatch(@Nonnull final ValueComputationRuleCall<Iterable<? extends V
8181
final var argumentValueCompensation = childValueEntry.getValue();
8282
final var field = column.getField();
8383
resultingMatchedValuesMap.putIfAbsent(argumentValue,
84-
new FieldValueCompensation(FieldValue.FieldPath.ofSingle(field.getFieldNameOptional().orElse(null), field.getFieldType(), i), argumentValueCompensation));
84+
new FieldValueCompensation(FieldValue.FieldPath.ofSingle(field, i), argumentValueCompensation));
8585
}
8686
}
8787
}

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,65 @@
2121
package com.apple.foundationdb.record.util;
2222

2323
import com.apple.foundationdb.record.PlanHashable;
24+
import com.apple.foundationdb.record.metadata.MetaDataException;
2425
import com.google.protobuf.Internal;
2526

2627
import javax.annotation.Nonnull;
28+
import javax.annotation.Nullable;
29+
import java.util.List;
2730
import java.util.Objects;
2831
import java.util.UUID;
32+
import java.util.regex.Pattern;
2933

3034
/**
3135
* Utility functions for interacting with protobuf.
3236
*/
3337
public class ProtoUtils {
3438

39+
private static final String DOUBLE_UNDERSCORE_ESCAPE = "__0";
40+
private static final String DOLLAR_ESCAPE = "__1";
41+
private static final String DOT_ESCAPE = "__2";
42+
43+
private static final List<String> INVALID_START_SEQUENCES = List.of(".", "$", DOUBLE_UNDERSCORE_ESCAPE, DOLLAR_ESCAPE, DOT_ESCAPE);
44+
45+
private static final Pattern VALID_PROTOBUF_COMPLIANT_NAME_PATTERN = Pattern.compile("^[A-Za-z_][A-Za-z0-9_]*$");
46+
3547
private ProtoUtils() {
3648
}
3749

50+
@Nonnull
51+
public static String toProtoBufCompliantName(final String name) {
52+
if (INVALID_START_SEQUENCES.stream().anyMatch(name::startsWith)) {
53+
throw new InvalidNameException("name cannot start with " + INVALID_START_SEQUENCES);
54+
}
55+
String translated;
56+
if (name.startsWith("__")) {
57+
translated = "__" + translateSpecialCharacters(name.substring(2));
58+
} else {
59+
if (name.isEmpty()) {
60+
throw new InvalidNameException("name cannot be empty string");
61+
}
62+
translated = translateSpecialCharacters(name);
63+
}
64+
checkValidProtoBufCompliantName(translated);
65+
return translated;
66+
}
67+
68+
public static void checkValidProtoBufCompliantName(String name) {
69+
if (!VALID_PROTOBUF_COMPLIANT_NAME_PATTERN.matcher(name).matches()) {
70+
throw new InvalidNameException(name + " it not a valid protobuf identifier");
71+
}
72+
}
73+
74+
@Nonnull
75+
private static String translateSpecialCharacters(final String userIdentifier) {
76+
return userIdentifier.replace("__", DOUBLE_UNDERSCORE_ESCAPE).replace("$", DOLLAR_ESCAPE).replace(".", DOT_ESCAPE);
77+
}
78+
79+
public static String toUserIdentifier(String protoIdentifier) {
80+
return protoIdentifier.replace(DOT_ESCAPE, ".").replace(DOLLAR_ESCAPE, "$").replace(DOUBLE_UNDERSCORE_ESCAPE, "__");
81+
}
82+
3883
/**
3984
* Generates a JVM-wide unique type name.
4085
* @return a unique type name.
@@ -103,4 +148,11 @@ public String toString() {
103148
return getName();
104149
}
105150
}
151+
152+
@SuppressWarnings("serial")
153+
public static class InvalidNameException extends MetaDataException {
154+
public InvalidNameException(@Nonnull final String msg, @Nullable final Object... keyValues) {
155+
super(msg, keyValues);
156+
}
157+
}
106158
}

fdb-record-layer-core/src/main/proto/record_query_plan.proto

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,24 +90,28 @@ message PType {
9090
message PEnumValue {
9191
optional string name = 1;
9292
optional int32 number = 2;
93+
optional string storage_name = 3;
9394
}
9495

9596
optional bool is_nullable = 1;
9697
repeated PEnumValue enum_values = 2;
9798
optional string name = 3; // referential name -- not used in the planner
99+
optional string storage_name = 4;
98100
}
99101

100102
message PRecordType {
101103
message PField {
102104
optional PType field_type = 1;
103105
optional string field_name = 2;
104106
optional int32 field_index = 3;
107+
optional string field_storage_name = 4;
105108
}
106109

107110
optional int32 reference_id = 1;
108111
optional string name = 2; // referential name -- not used in the planner
109112
optional bool is_nullable = 3;
110113
repeated PField fields = 4;
114+
optional string storage_name = 5;
111115
}
112116

113117
message PRelationType {
@@ -482,6 +486,7 @@ message PFieldPath {
482486
optional string name = 1;
483487
optional int32 ordinal = 2;
484488
optional PType type = 3;
489+
optional PType.PRecordType.PField field = 4;
485490
}
486491
repeated PResolvedAccessor field_accessors = 1;
487492
}

0 commit comments

Comments
 (0)