Skip to content

Add support for Nullable primitive types (via TupleFields) in SQL #3281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add support for Nullable primitive types (via TupleFields) in SQL #3281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions ...e/src/main/java/com/apple/foundationdb/record/metadata/expressions/TupleFieldsHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import com.apple.foundationdb.annotation.API;
import com.apple.foundationdb.record.RecordCoreArgumentException;
import com.apple.foundationdb.record.TupleFieldsProto;
import com.apple.foundationdb.record.query.plan.cascades.SemanticException;
import com.apple.foundationdb.record.query.plan.cascades.typing.Type;
import com.google.common.collect.ImmutableSet;
import com.google.protobuf.ByteString;
import com.google.protobuf.Descriptors;
Expand Down Expand Up @@ -89,6 +91,48 @@ public static Object fromProto(@Nonnull Message value, @Nonnull Descriptors.Desc
}
}

public static Descriptors.Descriptor getNullableWrapperDescriptorForTypeCode(Type.TypeCode typeCode) {
switch (typeCode) {
case INT:
return TupleFieldsProto.NullableInt32.getDescriptor();
case LONG:
return TupleFieldsProto.NullableInt64.getDescriptor();
case DOUBLE:
return TupleFieldsProto.NullableDouble.getDescriptor();
case FLOAT:
return TupleFieldsProto.NullableFloat.getDescriptor();
case BYTES:
return TupleFieldsProto.NullableBytes.getDescriptor();
case BOOLEAN:
return TupleFieldsProto.NullableBool.getDescriptor();
default:
throw new SemanticException(SemanticException.ErrorCode.UNSUPPORTED, "nullable for type " + typeCode.name() + "is not supported (yet).", null);
}
}

public static Type getTypeForNullableWrapper(@Nonnull Descriptors.Descriptor descriptor) {
if (descriptor == TupleFieldsProto.UUID.getDescriptor()) {
// just for simplicity, lets just say that nullable UUID do exist.
return Type.uuidType(false);
} else if (descriptor == TupleFieldsProto.NullableDouble.getDescriptor()) {
return Type.primitiveType(Type.TypeCode.DOUBLE);
} else if (descriptor == TupleFieldsProto.NullableFloat.getDescriptor()) {
return Type.primitiveType(Type.TypeCode.FLOAT);
} else if (descriptor == TupleFieldsProto.NullableInt32.getDescriptor()) {
return Type.primitiveType(Type.TypeCode.INT);
} else if (descriptor == TupleFieldsProto.NullableInt64.getDescriptor()) {
return Type.primitiveType(Type.TypeCode.LONG);
} else if (descriptor == TupleFieldsProto.NullableBool.getDescriptor()) {
return Type.primitiveType(Type.TypeCode.BOOLEAN);
} else if (descriptor == TupleFieldsProto.NullableString.getDescriptor()) {
return Type.primitiveType(Type.TypeCode.STRING);
} else if (descriptor == TupleFieldsProto.NullableBytes.getDescriptor()) {
return Type.primitiveType(Type.TypeCode.BYTES);
} else {
throw new RecordCoreArgumentException("value is not of a known message type");
}
}

/**
* Convert a Protobuf {@code UUID} to a Java {@link UUID}.
* @param proto the value of a Protobuf {@code UUID} field
Expand Down
54 changes: 34 additions & 20 deletions ...yer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/typing/Type.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.apple.foundationdb.record.RecordCoreException;
import com.apple.foundationdb.record.TupleFieldsProto;
import com.apple.foundationdb.record.logging.LogMessageKeys;
import com.apple.foundationdb.record.metadata.expressions.TupleFieldsHelper;
import com.apple.foundationdb.record.planprotos.PType;
import com.apple.foundationdb.record.planprotos.PType.PAnyRecordType;
import com.apple.foundationdb.record.planprotos.PType.PAnyType;
Expand Down Expand Up @@ -77,6 +78,8 @@
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import static com.apple.foundationdb.record.metadata.expressions.TupleFieldsHelper.getNullableWrapperDescriptorForTypeCode;

/**
* Provides type information about the output of an expression such as {@link Value} in a QGM.
* <br>
Expand Down Expand Up @@ -404,6 +407,20 @@ private static Type fromProtoType(@Nullable Descriptors.GenericDescriptor descri
@Nonnull Descriptors.FieldDescriptor.Type protoType,
@Nonnull FieldDescriptorProto.Label protoLabel,
boolean isNullable) {
// A MESSAGE field type can be descriptive of types other than the nested type. Hence, first check for those.
if (protoType == Descriptors.FieldDescriptor.Type.MESSAGE) {
Objects.requireNonNull(descriptor);
final var messageDescriptor = (Descriptors.Descriptor)descriptor;
if (TupleFieldsHelper.isTupleField((Descriptors.Descriptor) descriptor)) {
return TupleFieldsHelper.getTypeForNullableWrapper((Descriptors.Descriptor) descriptor);
}
if (NullableArrayTypeUtils.describesWrappedArray(messageDescriptor)) {
// find TypeCode of array elements
final var elementField = messageDescriptor.findFieldByName(NullableArrayTypeUtils.getRepeatedFieldName());
final var elementTypeCode = TypeCode.fromProtobufType(elementField.getType());
return fromProtoTypeToArray(descriptor, protoType, elementTypeCode, true);
}
}
final var typeCode = TypeCode.fromProtobufType(protoType);
if (protoLabel == FieldDescriptorProto.Label.LABEL_REPEATED) {
// collection type
Expand All @@ -414,18 +431,7 @@ private static Type fromProtoType(@Nullable Descriptors.GenericDescriptor descri
final var enumDescriptor = (Descriptors.EnumDescriptor)Objects.requireNonNull(descriptor);
return Enum.fromProtoValues(isNullable, enumDescriptor.getValues());
} else if (typeCode == TypeCode.RECORD) {
Objects.requireNonNull(descriptor);
final var messageDescriptor = (Descriptors.Descriptor)descriptor;
if (NullableArrayTypeUtils.describesWrappedArray(messageDescriptor)) {
// find TypeCode of array elements
final var elementField = messageDescriptor.findFieldByName(NullableArrayTypeUtils.getRepeatedFieldName());
final var elementTypeCode = TypeCode.fromProtobufType(elementField.getType());
return fromProtoTypeToArray(descriptor, protoType, elementTypeCode, true);
} else if (TupleFieldsProto.UUID.getDescriptor().equals(messageDescriptor)) {
return Type.uuidType(isNullable);
} else {
return Record.fromFieldDescriptorsMap(isNullable, Record.toFieldDescriptorMap(messageDescriptor.getFields()));
}
return Record.fromFieldDescriptorsMap(isNullable, Record.toFieldDescriptorMap(((Descriptors.Descriptor) descriptor).getFields()));
}

throw new IllegalStateException("unable to translate protobuf descriptor to type");
Expand Down Expand Up @@ -970,12 +976,22 @@ public void addProtoField(@Nonnull final TypeRepository.Builder typeRepositoryBu
@Nonnull final Optional<String> ignored,
@Nonnull final FieldDescriptorProto.Label label) {
final var protoType = Objects.requireNonNull(getTypeCode().getProtoType());
descriptorBuilder.addField(FieldDescriptorProto.newBuilder()
.setNumber(fieldNumber)
.setName(fieldName)
.setType(protoType)
.setLabel(label)
.build());
if (isNullable) {
final var nullableWrapperDescriptor = getNullableWrapperDescriptorForTypeCode(typeCode);
descriptorBuilder.addField(FieldDescriptorProto.newBuilder()
.setNumber(fieldNumber)
.setName(fieldName)
.setTypeName(nullableWrapperDescriptor.getFullName())
.setLabel(label)
.build());
} else {
descriptorBuilder.addField(FieldDescriptorProto.newBuilder()
.setNumber(fieldNumber)
.setName(fieldName)
.setType(protoType)
.setLabel(label)
.build());
}
}

@Override
Expand Down Expand Up @@ -2944,8 +2960,6 @@ public Array fromProto(@Nonnull final PlanSerializationContext serializationCont

class Uuid implements Type {

public static final String MESSAGE_NAME = TupleFieldsProto.UUID.getDescriptor().getName();

private final boolean isNullable;

private Uuid(boolean isNullable) {
Expand Down
13 changes: 5 additions & 8 deletions ...java/com/apple/foundationdb/record/query/plan/cascades/values/RecordConstructorValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import com.apple.foundationdb.record.PlanDeserializer;
import com.apple.foundationdb.record.PlanHashable;
import com.apple.foundationdb.record.PlanSerializationContext;
import com.apple.foundationdb.record.TupleFieldsProto;
import com.apple.foundationdb.record.metadata.expressions.TupleFieldsHelper;
import com.apple.foundationdb.record.planprotos.PRecordConstructorValue;
import com.apple.foundationdb.record.planprotos.PValue;
import com.apple.foundationdb.record.provider.foundationdb.FDBRecordStoreBase;
Expand Down Expand Up @@ -173,12 +173,7 @@ public static Object deepCopyIfNeeded(@Nonnull TypeRepository typeRepository,
}

if (fieldType.isUuid()) {
Verify.verify(field instanceof UUID);
final var uuidObject = (UUID) field;
return TupleFieldsProto.UUID.newBuilder()
.setMostSignificantBits(uuidObject.getMostSignificantBits())
.setLeastSignificantBits(uuidObject.getLeastSignificantBits())
.build();
return TupleFieldsHelper.toProto((UUID) field);
}

if (fieldType instanceof Type.Array) {
Expand Down Expand Up @@ -215,7 +210,9 @@ public static Object deepCopyIfNeeded(@Nonnull TypeRepository typeRepository,
}

private static Object protoObjectForPrimitive(@Nonnull Type type, @Nonnull Object field) {
if (type.getTypeCode() == Type.TypeCode.BYTES) {
if (type.isNullable()) {
return TupleFieldsHelper.toProto(field, TupleFieldsHelper.getNullableWrapperDescriptorForTypeCode(type.getTypeCode()));
} else if (type.getTypeCode() == Type.TypeCode.BYTES) {
if (field instanceof byte[]) {
// todo: we're a little inconsistent about whether the field should be byte[] or ByteString for BYTES fields
return ZeroCopyByteString.wrap((byte[]) field);
Expand Down
30 changes: 10 additions & 20 deletions ...relational-api/src/main/java/com/apple/foundationdb/relational/api/metadata/DataType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,12 @@
import com.apple.foundationdb.relational.api.exceptions.RelationalException;
import com.apple.foundationdb.relational.util.Assert;
import com.apple.foundationdb.relational.util.SpotBugsSuppressWarnings;

import com.google.common.base.Suppliers;
import com.google.common.collect.BiMap;
import com.google.common.collect.HashBiMap;
import com.google.common.collect.ImmutableList;

import javax.annotation.Nonnull;
import java.sql.Types;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand All @@ -54,19 +52,19 @@
*/
public abstract class DataType {
@Nonnull
private static final BiMap<Code, Integer> typeCodeJdbcTypeMap;
private static final Map<Code, Integer> typeCodeJdbcTypeMap;

static {
typeCodeJdbcTypeMap = HashBiMap.create();
typeCodeJdbcTypeMap = new HashMap<>();

typeCodeJdbcTypeMap.put(Code.BOOLEAN, Types.BOOLEAN);
typeCodeJdbcTypeMap.put(Code.LONG, Types.BIGINT);
typeCodeJdbcTypeMap.put(Code.INTEGER, Types.INTEGER);
typeCodeJdbcTypeMap.put(Code.FLOAT, Types.FLOAT);
typeCodeJdbcTypeMap.put(Code.DOUBLE, Types.DOUBLE);
typeCodeJdbcTypeMap.put(Code.STRING, Types.VARCHAR);
typeCodeJdbcTypeMap.put(Code.ENUM, Types.JAVA_OBJECT); // TODO (Rethink Relational Enum mapping to SQL type)
typeCodeJdbcTypeMap.put(Code.UUID, Types.OTHER); // TODO (Rethink Relational Enum mapping to SQL type)
typeCodeJdbcTypeMap.put(Code.ENUM, Types.OTHER); // TODO (Rethink Relational Enum mapping to SQL type)
typeCodeJdbcTypeMap.put(Code.UUID, Types.OTHER); // TODO (Rethink Relational UUID mapping to SQL type)
typeCodeJdbcTypeMap.put(Code.BYTES, Types.BINARY);
typeCodeJdbcTypeMap.put(Code.STRUCT, Types.STRUCT);
typeCodeJdbcTypeMap.put(Code.ARRAY, Types.ARRAY);
Expand Down Expand Up @@ -696,11 +694,8 @@ private VersionType(boolean isNullable) {
@Override
@Nonnull
public DataType withNullable(boolean isNullable) {
if (isNullable) {
return Primitives.NULLABLE_VERSION.type();
} else {
return Primitives.VERSION.type();
}
Assert.thatUnchecked(!isNullable, ErrorCode.UNSUPPORTED_OPERATION, "Nullable VersionType not supported");
return Primitives.VERSION.type();
}

@Override
Expand Down Expand Up @@ -769,11 +764,8 @@ private UuidType(boolean isNullable) {
@Override
@Nonnull
public DataType withNullable(boolean isNullable) {
if (isNullable) {
return Primitives.NULLABLE_UUID.type();
} else {
return Primitives.UUID.type();
}
Assert.thatUnchecked(!isNullable, ErrorCode.UNSUPPORTED_OPERATION, "Nullable UUID not supported");
return Primitives.UUID.type();
}

@Override
Expand Down Expand Up @@ -1355,9 +1347,7 @@ public enum Primitives {
NULLABLE_FLOAT(FloatType.nullable()),
NULLABLE_DOUBLE(DoubleType.nullable()),
NULLABLE_STRING(StringType.nullable()),
NULLABLE_BYTES(BytesType.nullable()),
NULLABLE_VERSION(VersionType.nullable()),
NULLABLE_UUID(UuidType.nullable())
NULLABLE_BYTES(BytesType.nullable())
;

@Nonnull
Expand Down
17 changes: 9 additions & 8 deletions ...tional-core/src/main/java/com/apple/foundationdb/relational/recordlayer/MessageTuple.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,10 @@
package com.apple.foundationdb.relational.recordlayer;

import com.apple.foundationdb.annotation.API;
import com.apple.foundationdb.record.TupleFieldsProto;
import com.apple.foundationdb.record.metadata.expressions.TupleFieldsHelper;
import com.apple.foundationdb.relational.api.exceptions.InvalidColumnReferenceException;
import com.google.protobuf.Descriptors;
import com.google.protobuf.Message;
import com.google.protobuf.MessageOrBuilder;

import java.util.UUID;

@API(API.Status.EXPERIMENTAL)
public class MessageTuple extends AbstractRow {
Expand All @@ -52,10 +49,14 @@ public Object getObject(int position) throws InvalidColumnReferenceException {
final var field = message.getField(message.getDescriptorForType().getFields().get(position));
if (fieldDescriptor.getType() == Descriptors.FieldDescriptor.Type.ENUM) {
return ((Descriptors.EnumValueDescriptor) field).getName();
} else if (fieldDescriptor.getType() == Descriptors.FieldDescriptor.Type.MESSAGE && fieldDescriptor.getMessageType().equals(TupleFieldsProto.UUID.getDescriptor())) {
final var dynamicMsg = (MessageOrBuilder) field;
return new UUID((Long) dynamicMsg.getField(dynamicMsg.getDescriptorForType().findFieldByName("most_significant_bits")),
(Long) dynamicMsg.getField(dynamicMsg.getDescriptorForType().findFieldByName("least_significant_bits")));
} else if (fieldDescriptor.getType() == Descriptors.FieldDescriptor.Type.MESSAGE) {
if (TupleFieldsHelper.isTupleField(fieldDescriptor.getMessageType())) {
if (field == null) {
return null;
} else {
return TupleFieldsHelper.fromProto((Message) field, fieldDescriptor.getMessageType());
}
}
}
return field;
} else {
Expand Down
2 changes: 0 additions & 2 deletions ...e/src/main/java/com/apple/foundationdb/relational/recordlayer/metadata/DataTypeUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,5 @@ public static Type toRecordLayerType(@Nonnull final DataType type) {
primitivesMap.put(DataType.Primitives.NULLABLE_FLOAT.type(), Type.primitiveType(Type.TypeCode.FLOAT, true));
primitivesMap.put(DataType.Primitives.NULLABLE_BYTES.type(), Type.primitiveType(Type.TypeCode.BYTES, true));
primitivesMap.put(DataType.Primitives.NULLABLE_STRING.type(), Type.primitiveType(Type.TypeCode.STRING, true));
primitivesMap.put(DataType.Primitives.NULLABLE_VERSION.type(), Type.primitiveType(Type.TypeCode.VERSION, true));
primitivesMap.put(DataType.Primitives.NULLABLE_UUID.type(), Type.uuidType(true));
}
}
3 changes: 2 additions & 1 deletion ...e/src/main/java/com/apple/foundationdb/relational/recordlayer/query/SemanticAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,8 @@ public DataType lookupType(@Nonnull Identifier typeIdentifier, boolean isNullabl
type = isNullable ? DataType.Primitives.NULLABLE_FLOAT.type() : DataType.Primitives.FLOAT.type();
break;
case "UUID":
type = isNullable ? DataType.Primitives.NULLABLE_UUID.type() : DataType.Primitives.UUID.type();
Assert.thatUnchecked(!isNullable, ErrorCode.UNSUPPORTED_OPERATION, "Nullable UUID not supported");
type = DataType.Primitives.UUID.type();
break;
default:
Assert.notNullUnchecked(metadataCatalog);
Expand Down
5 changes: 0 additions & 5 deletions ...rc/main/java/com/apple/foundationdb/relational/recordlayer/query/visitors/DdlVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,6 @@ public RecordLayerColumn visitColumnDefinition(@Nonnull RelationalParser.ColumnD
final var columnId = visitUid(ctx.colName);
final var isRepeated = ctx.ARRAY() != null;
final var isNullable = ctx.columnConstraint() != null ? (Boolean) ctx.columnConstraint().accept(this) : true;
// TODO: We currently do not support NOT NULL for any type other than ARRAY. This is because there is no way to
// specify not "nullability" at the RecordMetaData level. For ARRAY, specifying that is actually possible
// by means of NullableArrayWrapper. In essence, we don't actually need a wrapper per se for non-array types,
// but a way to represent it in RecordMetadata.
Assert.thatUnchecked(isRepeated || isNullable, ErrorCode.UNSUPPORTED_OPERATION, "NOT NULL is only allowed for ARRAY column type");
containsNullableArray = containsNullableArray || (isRepeated && isNullable);
final var columnTypeId = ctx.columnType().customType != null ? visitUid(ctx.columnType().customType) : Identifier.of(ctx.columnType().getText());
final var semanticAnalyzer = getDelegate().getSemanticAnalyzer();
Expand Down
5 changes: 5 additions & 0 deletions yaml-tests/src/test/java/YamlIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,4 +258,9 @@ public void enumTest(YamlTest.Runner runner) throws Exception {
public void uuidTest(YamlTest.Runner runner) throws Exception {
runner.runYamsql("uuid.yamsql");
}

@TestTemplate
public void nullColumnConstraintTest(YamlTest.Runner runner) throws Exception {
runner.runYamsql("null-column-constraint.yamsql");
}
}
Loading
Loading