Skip to content

Commit d218cbb

Browse files
g31pranjalg31pranjal
authored
Rollback UUID support in SQL Layer (#3293)
This PR reverts the SQL layer support for UUID that was introduced in #3198. However, the Record Layer support for UUID stays as it is. This makes this compatible with releases < `4.2.3.0` which was required to maintain plan continuation compatibility for plans serialized with non-UUID Type. Also adds a yaml-test for explicitly defining the RecordMetadata using proto, and using the `TupleField.UUID` message in it. Owing to difference in behavior in how this and versions (4.2.3.0 and 4.2.4.0) interprets `TupleField.UUID`, this PR also blocklists these versions from yaml-testing. fixes: #3285
1 parent ccbbc76 commit d218cbb

File tree

22 files changed

+205
-468
lines changed

22 files changed

+205
-468
lines changed

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -421,8 +421,6 @@ private static Type fromProtoType(@Nullable Descriptors.GenericDescriptor descri
421421
final var elementField = messageDescriptor.findFieldByName(NullableArrayTypeUtils.getRepeatedFieldName());
422422
final var elementTypeCode = TypeCode.fromProtobufType(elementField.getType());
423423
return fromProtoTypeToArray(descriptor, protoType, elementTypeCode, true);
424-
} else if (TupleFieldsProto.UUID.getDescriptor().equals(messageDescriptor)) {
425-
return Type.uuidType(isNullable);
426424
} else {
427425
return Record.fromFieldDescriptorsMap(isNullable, Record.toFieldDescriptorMap(messageDescriptor.getFields()));
428426
}

‎fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/TypeTest.java

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,16 @@ public Stream<? extends Arguments> provideArguments(final ExtensionContext conte
128128
.setStartDate(random.nextLong())
129129
.setSchoolName("randomString" + random.nextInt()).build()
130130
).build()
131-
),
132-
Arguments.of(
133-
"TestRecordsUuidProto.UuidRecord", TestRecordsUuidProto.UuidRecord.newBuilder()
134-
.setPkey(TupleFieldsProto.UUID.newBuilder()
135-
.setMostSignificantBits(98452560)
136-
.setLeastSignificantBits(30900234)
137-
.build())
138-
.build()
139131
)
132+
// This does not work currently owing to https://github.com/FoundationDB/fdb-record-layer/issues/3295
133+
// Arguments.of(
134+
// "TestRecordsUuidProto.UuidRecord", TestRecordsUuidProto.UuidRecord.newBuilder()
135+
// .setPkey(TupleFieldsProto.UUID.newBuilder()
136+
// .setMostSignificantBits(98452560)
137+
// .setLeastSignificantBits(30900234)
138+
// .build())
139+
// .build()
140+
// )
140141
);
141142
}
142143
}
@@ -307,4 +308,28 @@ void testAnyRecordSerialization() {
307308
Type.AnyRecord r1 = new Type.AnyRecord(false);
308309
Assertions.assertEquals(r1, Type.AnyRecord.fromProto(serializationContext, r1.toProto(serializationContext)));
309310
}
311+
312+
@Test
313+
void testUUIDInterpretedAsRecordType() {
314+
final TestRecordsUuidProto.UuidRecord uuidRecord = TestRecordsUuidProto.UuidRecord.newBuilder()
315+
.setName("testUuidRecord")
316+
.setPkey(TupleFieldsProto.UUID.newBuilder()
317+
.setMostSignificantBits(1)
318+
.setLeastSignificantBits(2))
319+
.build();
320+
final Type.Record recordType = Type.Record.fromDescriptor(uuidRecord.getDescriptorForType());
321+
final Map<String, Type.Record.Field> fieldsMaps = recordType.getFieldNameFieldMap();
322+
checkIsUuidRecordType(fieldsMaps, "pkey");
323+
checkIsUuidRecordType(fieldsMaps, "secondary");
324+
checkIsUuidRecordType(fieldsMaps, "unique");
325+
}
326+
327+
private static void checkIsUuidRecordType(@Nonnull Map<String, Type.Record.Field> fieldsMap, @Nonnull String fieldName) {
328+
Assertions.assertTrue(fieldsMap.containsKey(fieldName));
329+
Assertions.assertInstanceOf(Type.Record.class, fieldsMap.get(fieldName).getFieldType());
330+
final Type.Record uuidRecord = (Type.Record) fieldsMap.get(fieldName).getFieldType();
331+
Assertions.assertEquals(2, uuidRecord.getFields().size());
332+
Assertions.assertEquals(Type.TypeCode.LONG, uuidRecord.getFields().get(0).getFieldType().getTypeCode());
333+
Assertions.assertEquals(Type.TypeCode.LONG, uuidRecord.getFields().get(1).getFieldType().getTypeCode());
334+
}
310335
}

‎fdb-relational-api/src/main/java/com/apple/foundationdb/relational/api/metadata/DataType.java

Lines changed: 1 addition & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ public abstract class DataType {
6666
typeCodeJdbcTypeMap.put(Code.DOUBLE, Types.DOUBLE);
6767
typeCodeJdbcTypeMap.put(Code.STRING, Types.VARCHAR);
6868
typeCodeJdbcTypeMap.put(Code.ENUM, Types.JAVA_OBJECT); // TODO (Rethink Relational Enum mapping to SQL type)
69-
typeCodeJdbcTypeMap.put(Code.UUID, Types.OTHER); // TODO (Rethink Relational Enum mapping to SQL type)
7069
typeCodeJdbcTypeMap.put(Code.BYTES, Types.BINARY);
7170
typeCodeJdbcTypeMap.put(Code.STRUCT, Types.STRUCT);
7271
typeCodeJdbcTypeMap.put(Code.ARRAY, Types.ARRAY);
@@ -752,79 +751,6 @@ public String toString() {
752751
}
753752
}
754753

755-
public static final class UuidType extends DataType {
756-
@Nonnull
757-
private static final UuidType NOT_NULLABLE_INSTANCE = new UuidType(false);
758-
759-
@Nonnull
760-
private static final UuidType NULLABLE_INSTANCE = new UuidType(true);
761-
762-
@Nonnull
763-
private final Supplier<Integer> hashCodeSupplier = Suppliers.memoize(this::computeHashCode);
764-
765-
private UuidType(boolean isNullable) {
766-
super(isNullable, true, Code.UUID);
767-
}
768-
769-
@Override
770-
@Nonnull
771-
public DataType withNullable(boolean isNullable) {
772-
if (isNullable) {
773-
return Primitives.NULLABLE_UUID.type();
774-
} else {
775-
return Primitives.UUID.type();
776-
}
777-
}
778-
779-
@Override
780-
public boolean isResolved() {
781-
return true;
782-
}
783-
784-
@Nonnull
785-
@Override
786-
public DataType resolve(@Nonnull Map<String, Named> resolutionMap) {
787-
return this;
788-
}
789-
790-
@Nonnull
791-
public static UuidType nullable() {
792-
return NULLABLE_INSTANCE;
793-
}
794-
795-
@Nonnull
796-
public static UuidType notNullable() {
797-
return NOT_NULLABLE_INSTANCE;
798-
}
799-
800-
private int computeHashCode() {
801-
return Objects.hash(getCode(), isNullable());
802-
}
803-
804-
@Override
805-
public int hashCode() {
806-
return hashCodeSupplier.get();
807-
}
808-
809-
@Override
810-
public boolean equals(Object other) {
811-
if (this == other) {
812-
return true;
813-
}
814-
815-
if (!(other instanceof UuidType)) {
816-
return false;
817-
}
818-
final var otherUuidType = (UuidType) other;
819-
return this.isNullable() == otherUuidType.isNullable();
820-
}
821-
822-
@Override
823-
public String toString() {
824-
return "uuid" + (isNullable() ? " ∪ ∅" : "");
825-
}
826-
}
827-
828754
public static final class EnumType extends DataType implements Named {
829755
@Nonnull
830756
private final Supplier<Integer> hashCodeSupplier = Suppliers.memoize(this::computeHashCode);
@@ -1331,7 +1257,6 @@ public enum Code {
13311257
BYTES,
13321258
VERSION,
13331259
ENUM,
1334-
UUID,
13351260
STRUCT,
13361261
ARRAY,
13371262
UNKNOWN
@@ -1348,16 +1273,14 @@ public enum Primitives {
13481273
STRING(StringType.notNullable()),
13491274
BYTES(BytesType.notNullable()),
13501275
VERSION(VersionType.notNullable()),
1351-
UUID(UuidType.notNullable()),
13521276
NULLABLE_BOOLEAN(BooleanType.nullable()),
13531277
NULLABLE_LONG(LongType.nullable()),
13541278
NULLABLE_INTEGER(IntegerType.nullable()),
13551279
NULLABLE_FLOAT(FloatType.nullable()),
13561280
NULLABLE_DOUBLE(DoubleType.nullable()),
13571281
NULLABLE_STRING(StringType.nullable()),
13581282
NULLABLE_BYTES(BytesType.nullable()),
1359-
NULLABLE_VERSION(VersionType.nullable()),
1360-
NULLABLE_UUID(UuidType.nullable())
1283+
NULLABLE_VERSION(VersionType.nullable())
13611284
;
13621285

13631286
@Nonnull

‎fdb-relational-core/src/main/antlr/RelationalParser.g4

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ columnType
131131
: primitiveType | customType=uid;
132132

133133
primitiveType
134-
: BOOLEAN | INTEGER | BIGINT | FLOAT | DOUBLE | STRING | BYTES | UUID;
134+
: BOOLEAN | INTEGER | BIGINT | FLOAT | DOUBLE | STRING | BYTES;
135135

136136
columnConstraint
137137
: nullNotnull #nullColumnConstraint
@@ -1229,7 +1229,7 @@ functionNameBase
12291229
| TIMESTAMPADD | TIMESTAMPDIFF | TIME_FORMAT | TIME_TO_SEC
12301230
| TOUCHES | TO_BASE64 | TO_DAYS | TO_SECONDS | UCASE
12311231
| UNCOMPRESS | UNCOMPRESSED_LENGTH | UNHEX | UNIX_TIMESTAMP
1232-
| UPDATEXML | UPPER
1232+
| UPDATEXML | UPPER | UUID | UUID_SHORT
12331233
| VALIDATE_PASSWORD_STRENGTH | VERSION | VISIBLE
12341234
| WAIT_UNTIL_SQL_THREAD_AFTER_GTIDS | WEEK | WEEKDAY
12351235
| WEEKOFYEAR | WEIGHT_STRING | WITHIN | YEAR | YEARWEEK

‎fdb-relational-core/src/main/java/com/apple/foundationdb/relational/api/RowStruct.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
package com.apple.foundationdb.relational.api;
2222

23+
import com.apple.foundationdb.record.metadata.expressions.TupleFieldsHelper;
2324
import com.apple.foundationdb.relational.api.exceptions.ErrorCode;
2425
import com.apple.foundationdb.relational.api.exceptions.InvalidColumnReferenceException;
2526
import com.apple.foundationdb.relational.api.exceptions.RelationalException;
@@ -29,6 +30,7 @@
2930
import com.apple.foundationdb.relational.util.NullableArrayUtils;
3031
import com.google.protobuf.ByteString;
3132
import com.google.protobuf.Descriptors;
33+
import com.google.protobuf.DynamicMessage;
3234
import com.google.protobuf.Message;
3335

3436
import java.net.URI;
@@ -188,6 +190,21 @@ public Object getObject(int oneBasedPosition) throws SQLException {
188190
return getArray(oneBasedPosition);
189191
case Types.BINARY:
190192
return getBytes(oneBasedPosition);
193+
case Types.OTHER:
194+
final var object = getObjectInternal(getZeroBasedPosition(oneBasedPosition));
195+
// This is a temporary workaround to support UUID as a primitive type. The fix essentially involves
196+
// delaying the conversion of a message field (of type UUID) in the messageTuple until now when we can
197+
// (little bit) predict that the field is actually a pre-defined UUID message. If the UUID message
198+
// field is of sql.Types.OTHER (rather than sql.Types.STRUCT), we can expect that the plan is baked with
199+
// the future-supported UUID type, and hence the result expects a JAVA UUID object. This can be
200+
// removed (and pushed to MessageTuple) once we have proper and complete support for UUID.
201+
if (object instanceof DynamicMessage) {
202+
final var msg = (DynamicMessage) object;
203+
if (TupleFieldsHelper.isTupleField(msg.getDescriptorForType())) {
204+
return TupleFieldsHelper.fromProto(msg, msg.getDescriptorForType());
205+
}
206+
}
207+
return object;
191208
default:
192209
return getObjectInternal(getZeroBasedPosition(oneBasedPosition));
193210
}

‎fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/MessageTuple.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,9 @@
2121
package com.apple.foundationdb.relational.recordlayer;
2222

2323
import com.apple.foundationdb.annotation.API;
24-
import com.apple.foundationdb.record.TupleFieldsProto;
2524
import com.apple.foundationdb.relational.api.exceptions.InvalidColumnReferenceException;
2625
import com.google.protobuf.Descriptors;
2726
import com.google.protobuf.Message;
28-
import com.google.protobuf.MessageOrBuilder;
29-
30-
import java.util.UUID;
3127

3228
@API(API.Status.EXPERIMENTAL)
3329
public class MessageTuple extends AbstractRow {
@@ -52,10 +48,6 @@ public Object getObject(int position) throws InvalidColumnReferenceException {
5248
final var field = message.getField(message.getDescriptorForType().getFields().get(position));
5349
if (fieldDescriptor.getType() == Descriptors.FieldDescriptor.Type.ENUM) {
5450
return ((Descriptors.EnumValueDescriptor) field).getName();
55-
} else if (fieldDescriptor.getType() == Descriptors.FieldDescriptor.Type.MESSAGE && fieldDescriptor.getMessageType().equals(TupleFieldsProto.UUID.getDescriptor())) {
56-
final var dynamicMsg = (MessageOrBuilder) field;
57-
return new UUID((Long) dynamicMsg.getField(dynamicMsg.getDescriptorForType().findFieldByName("most_significant_bits")),
58-
(Long) dynamicMsg.getField(dynamicMsg.getDescriptorForType().findFieldByName("least_significant_bits")));
5951
}
6052
return field;
6153
} else {

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,6 @@ public static Type toRecordLayerType(@Nonnull final DataType type) {
148148
primitivesMap.put(DataType.Primitives.BYTES.type(), Type.primitiveType(Type.TypeCode.BYTES, false));
149149
primitivesMap.put(DataType.Primitives.STRING.type(), Type.primitiveType(Type.TypeCode.STRING, false));
150150
primitivesMap.put(DataType.Primitives.VERSION.type(), Type.primitiveType(Type.TypeCode.VERSION, false));
151-
primitivesMap.put(DataType.Primitives.UUID.type(), Type.uuidType(false));
152151

153152
primitivesMap.put(DataType.Primitives.NULLABLE_BOOLEAN.type(), Type.primitiveType(Type.TypeCode.BOOLEAN, true));
154153
primitivesMap.put(DataType.Primitives.NULLABLE_INTEGER.type(), Type.primitiveType(Type.TypeCode.INT, true));
@@ -158,6 +157,5 @@ public static Type toRecordLayerType(@Nonnull final DataType type) {
158157
primitivesMap.put(DataType.Primitives.NULLABLE_BYTES.type(), Type.primitiveType(Type.TypeCode.BYTES, true));
159158
primitivesMap.put(DataType.Primitives.NULLABLE_STRING.type(), Type.primitiveType(Type.TypeCode.STRING, true));
160159
primitivesMap.put(DataType.Primitives.NULLABLE_VERSION.type(), Type.primitiveType(Type.TypeCode.VERSION, true));
161-
primitivesMap.put(DataType.Primitives.NULLABLE_UUID.type(), Type.uuidType(true));
162160
}
163161
}

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@
2121
package com.apple.foundationdb.relational.recordlayer.metadata;
2222

2323
import com.apple.foundationdb.annotation.API;
24+
2425
import com.apple.foundationdb.record.RecordMetaData;
2526
import com.apple.foundationdb.record.RecordMetaDataProto;
2627
import com.apple.foundationdb.record.metadata.Key;
2728
import com.apple.foundationdb.record.query.combinatorics.TopologicalSort;
28-
import com.apple.foundationdb.record.query.plan.cascades.typing.TypeRepository;
2929
import com.apple.foundationdb.relational.api.exceptions.ErrorCode;
3030
import com.apple.foundationdb.relational.api.exceptions.RelationalException;
3131
import com.apple.foundationdb.relational.api.metadata.DataType;
@@ -37,6 +37,7 @@
3737
import com.apple.foundationdb.relational.recordlayer.metadata.serde.RecordMetadataDeserializer;
3838
import com.apple.foundationdb.relational.recordlayer.metadata.serde.RecordMetadataSerializer;
3939
import com.apple.foundationdb.relational.util.Assert;
40+
4041
import com.google.common.annotations.VisibleForTesting;
4142
import com.google.common.base.Supplier;
4243
import com.google.common.base.Suppliers;
@@ -47,7 +48,6 @@
4748
import com.google.protobuf.Descriptors;
4849

4950
import javax.annotation.Nonnull;
50-
import java.util.ArrayList;
5151
import java.util.BitSet;
5252
import java.util.Collection;
5353
import java.util.HashSet;
@@ -157,11 +157,10 @@ private RecordMetaData buildRecordMetadata() {
157157
final var fileDescriptorProtoSerializer = new FileDescriptorSerializer();
158158
accept(fileDescriptorProtoSerializer);
159159
final Descriptors.FileDescriptor fileDescriptor;
160-
final var dependencies = new ArrayList<>(TypeRepository.DEPENDENCIES);
161-
dependencies.add(RecordMetaDataProto.getDescriptor());
162160
try {
163161
fileDescriptor = Descriptors.FileDescriptor.buildFrom(
164-
fileDescriptorProtoSerializer.getFileBuilder().build(), dependencies.toArray(new Descriptors.FileDescriptor[0]));
162+
fileDescriptorProtoSerializer.getFileBuilder().build(),
163+
new Descriptors.FileDescriptor[]{RecordMetaDataProto.getDescriptor()});
165164
} catch (Descriptors.DescriptorValidationException e) {
166165
throw new RelationalException(ErrorCode.SERIALIZATION_FAILURE, e).toUncheckedWrappedException();
167166
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
package com.apple.foundationdb.relational.recordlayer.metadata.serde;
2222

2323
import com.apple.foundationdb.annotation.API;
24+
2425
import com.apple.foundationdb.record.RecordMetaDataOptionsProto;
2526
import com.apple.foundationdb.record.query.plan.cascades.typing.Type;
2627
import com.apple.foundationdb.record.query.plan.cascades.typing.TypeRepository;
@@ -30,15 +31,14 @@
3031
import com.apple.foundationdb.relational.recordlayer.metadata.RecordLayerTable;
3132
import com.apple.foundationdb.relational.recordlayer.metadata.SkeletonVisitor;
3233
import com.apple.foundationdb.relational.util.Assert;
34+
3335
import com.google.protobuf.DescriptorProtos;
34-
import com.google.protobuf.Descriptors;
3536

3637
import javax.annotation.Nonnull;
3738
import java.util.LinkedHashSet;
3839
import java.util.Locale;
3940
import java.util.Map;
4041
import java.util.Set;
41-
import java.util.stream.Collectors;
4242

4343
@API(API.Status.EXPERIMENTAL)
4444
public class FileDescriptorSerializer extends SkeletonVisitor {
@@ -73,7 +73,6 @@ public FileDescriptorSerializer() {
7373

7474
public FileDescriptorSerializer(@Nonnull DescriptorProtos.FileDescriptorProto.Builder fileBuilder) {
7575
this.fileBuilder = fileBuilder;
76-
this.fileBuilder.addAllDependency(TypeRepository.DEPENDENCIES.stream().map(Descriptors.FileDescriptor::getFullName).collect(Collectors.toList()));
7776
this.unionDescriptorBuilder = DescriptorProtos.DescriptorProto.newBuilder().setName("RecordTypeUnion");
7877
final RecordMetaDataOptionsProto.RecordTypeOptions options = RecordMetaDataOptionsProto.RecordTypeOptions.newBuilder().setUsage(RecordMetaDataOptionsProto.RecordTypeOptions.Usage.UNION).build();
7978
unionDescriptorBuilder.getOptionsBuilder().setExtension(RecordMetaDataOptionsProto.record, options);

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -501,9 +501,6 @@ public DataType lookupType(@Nonnull Identifier typeIdentifier, boolean isNullabl
501501
case "FLOAT":
502502
type = isNullable ? DataType.Primitives.NULLABLE_FLOAT.type() : DataType.Primitives.FLOAT.type();
503503
break;
504-
case "UUID":
505-
type = isNullable ? DataType.Primitives.NULLABLE_UUID.type() : DataType.Primitives.UUID.type();
506-
break;
507504
default:
508505
Assert.notNullUnchecked(metadataCatalog);
509506
// assume it is a custom type, will fail in upper layers if the type can not be resolved.

0 commit comments

Comments
 (0)