Support equality/inequality and NULL comparisons for ARRAYs

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

@@ -52,6 +52,7 @@ public enum ErrorCode {
         INVALID_ENUM_VALUE(12, "Invalid enum value for the enum type"),
         INVALID_UUID_VALUE(13, "Invalid UUID value for the UUID type"),
         INVALID_CAST(14, "Invalid cast operation"),
+        COMPARISON_OF_INCOMPATIBLE_TYPES(15, "The operands of a comparison operator are not compatible."),
 
         // insert, update, deletes
         UPDATE_TRANSFORM_AMBIGUOUS(1_000, "The transformations used in an UPDATE statement are ambiguous."),

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

@@ -288,8 +288,7 @@ default Optional<Type.Enum> narrowEnumMaybe() {
     }
 
     /**
-     * Checks whether a {@link Type} is numeric.
-     * @return <code>true</code> if the {@link Type} is numeric, otherwise <code>false</code>.
+     * Whether this is a numeric type.
      */
     default boolean isNumeric() {
         return getTypeCode().isNumeric();
@@ -300,6 +299,20 @@ default boolean isUnresolved() {
         return typeCode == TypeCode.UNKNOWN;
     }
 
+    /**
+     * Whether this is the {@code NULL} type.
+     */
+    default boolean isNull() {
+        return getTypeCode() == TypeCode.NULL;
+    }
+
+    /**
+     * Whether this is the {@code NONE} type, i.e., the type of the untyped empty array {@code []}.
+     */
+    default boolean isNone() {
+        return getTypeCode() == TypeCode.NONE;
+    }
+
     @Nonnull
     ExplainTokens describe();
 
@@ -569,17 +582,26 @@ private static Descriptors.GenericDescriptor getTypeSpecificDescriptor(@Nonnull
     @Nullable
     @SuppressWarnings("PMD.CompareObjectsWithEquals")
     static Type maximumType(@Nonnull final Type t1, @Nonnull final Type t2) {
-        if (t1.getTypeCode() == TypeCode.NULL && t2.getTypeCode() == TypeCode.NULL) {
+        // NULL case
+        if (t1.isNull() && t2.isNull()) {
             return Type.nullType();
         }
-
-        if (t1.getTypeCode() == TypeCode.NULL && PromoteValue.isPromotable(t1, t2)) {
+        if (t1.isNull() && PromoteValue.isPromotable(t1, t2)) {
             return t2.withNullability(true);
         }
-        if (t2.getTypeCode() == TypeCode.NULL && PromoteValue.isPromotable(t2, t1)) {
+        if (t2.isNull() && PromoteValue.isPromotable(t2, t1)) {
             return t1.withNullability(true);
         }
 
+        // NONE case: The untyped empty array [] identity-promotes to any ARRAY type; so the maximum type is simply the
+        // other side (no nullability change, since NONE is non-nullable).
+        if (t1.isNone() && PromoteValue.isPromotable(t1, t2)) {
+            return t2;
+        }
+        if (t2.isNone() && PromoteValue.isPromotable(t2, t1)) {
+            return t1;
+        }
+
         Verify.verify(!t1.isUnresolved());
         Verify.verify(!t2.isUnresolved());
 
@@ -1389,14 +1411,17 @@ public boolean equals(final Object o) {
     }
 
     /**
-     * The none type is an unresolved type meaning that an entity returning a none type should resolve the
-     * type to a regular type as the runtime does not support a none-typed data producer. Only the empty array constant
-     * is actually of type {@code none}, however, that type is changed to an actual type during type resolution (to an
-     * array of some regular type).
-     * It is correct to say that the none type (just as {@link Null} type) are types that have no instances.
-     * It is still useful use this type for modelling purposes. Just as in Scala, the none-type is implicitly, a
-     * subtype of every other type in a sense that the substitution principle holds, e.g. {@code none} can be substituted
-     * for any value of an array type.
+     * The none type.
+     *
+     * <p>The none type is the type of the untyped empty array. It is an <em>unresolved</em> type. An entity returning
+     * a none type must resolve the type to a regular type, as the runtime does not support a none-typed data producer.
+     * For the empty array constant {@code []}, the type {@code None} is changed to an actual type during type
+     * resolution (to an array of some regular type).
+     *
+     * <p>It is correct to say that the none type (just as the {@link Null} type) is a type that has no instances.
+     * It is still useful to have this type for modeling purposes. Just as in Scala, the none type is implicitly a
+     * subtype of every other array type in a sense that the substitution principle holds; e.g., {@code none} can be
+     * substituted for any value of an array type.
      */
     class None implements Type {
         @Override

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

@@ -290,7 +290,7 @@ public Set<String> getEnumTypes() {
      */
     @Nonnull
     private static Type canonicalizeNullability(@Nonnull final Type type) {
-        if (type.getTypeCode() == Type.TypeCode.RELATION || type.getTypeCode() == Type.TypeCode.NONE) {
+        if (type.getTypeCode() == Type.TypeCode.RELATION || type.isNone()) {
             return type.notNullable();
         } else {
             return type.nullable();

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

@@ -27,18 +27,17 @@
 import com.apple.foundationdb.record.PlanDeserializer;
 import com.apple.foundationdb.record.PlanHashable;
 import com.apple.foundationdb.record.PlanSerializationContext;
-import com.apple.foundationdb.record.RecordCoreException;
 import com.apple.foundationdb.record.planprotos.PAbstractArrayConstructorValue;
 import com.apple.foundationdb.record.planprotos.PLightArrayConstructorValue;
 import com.apple.foundationdb.record.planprotos.PValue;
 import com.apple.foundationdb.record.provider.foundationdb.FDBRecordStoreBase;
 import com.apple.foundationdb.record.query.plan.cascades.AliasMap;
 import com.apple.foundationdb.record.query.plan.cascades.BuiltInFunction;
-import com.apple.foundationdb.record.query.plan.explain.ExplainTokens;
-import com.apple.foundationdb.record.query.plan.explain.ExplainTokensWithPrecedence;
 import com.apple.foundationdb.record.query.plan.cascades.SemanticException;
 import com.apple.foundationdb.record.query.plan.cascades.typing.Type;
 import com.apple.foundationdb.record.query.plan.cascades.typing.Typed;
+import com.apple.foundationdb.record.query.plan.explain.ExplainTokens;
+import com.apple.foundationdb.record.query.plan.explain.ExplainTokensWithPrecedence;
 import com.google.auto.service.AutoService;
 import com.google.common.base.Verify;
 import com.google.common.collect.ImmutableList;
@@ -234,14 +233,19 @@
             if (!Iterables.isEmpty(getChildren())) {
                 return false;
             }
-            return type.isUnresolved();
+            // An untyped empty array (NONE result type) can be promoted to any concrete array type.
+            return type.isUnresolved() || (getResultType().isNone() && type.isArray());
         }
 
         @Nonnull
         @Override
         public Value with(@Nonnull final Type type) {
             Verify.verify(Iterables.isEmpty(getChildren()));
-            return emptyArray(type); // only empty arrays are currently promotable
+            Verify.verify(type.isArray());
+            // `type` is the desired array type; extract its element type.
+            final Type elementType = Verify.verifyNotNull(((Type.Array)type).getElementType());
+            // Note: Only empty arrays are currently promotable.
+            return emptyArray(elementType);
         }
 
         @Nonnull
@@ -305,10 +309,21 @@
                     return Type.noneType();
                 }
 
+                /**
+                 * Evaluate the array.
+                 *
+                 * <p>This returns an empty immutable list of `Object`.
+                 *
+                 * <p>We don’t generally want {@code []} to be evaluated at runtime, because {@link Type.None} is an
+                 * unresolved type that the semantic analysis is supposed to eliminate via promotion to a concrete array
+                 * type. However, it is useful to simplify some places in semantic analysis, which otherwise would
+                 * require special cases in the code for {@code []}. For example, we can then keep `[] IS NULL` as is in
+                 * the value tree and eliminate the untyped empty array later via the usual constant folding mechanism.
+                 */
                 @Nullable
                 @Override
                 public <M extends Message> Object eval(@Nullable final FDBRecordStoreBase<M> store, @Nonnull final EvaluationContext context) {
-                    throw new RecordCoreException("invalid evaluation attempt");
+                    return ImmutableList.of();
                 }
             };
         }

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

@@ -115,8 +115,7 @@ public ConstrainedBoolean equalsWithoutChildren(@Nonnull final Value other) {
 
     @Override
     public boolean canResultInType(@Nonnull final Type type) {
-        return resultType.getTypeCode() == Type.TypeCode.NULL ||
-                (resultType.isNullable() && resultType.equals(type.nullable()));
+        return resultType.isNull() || (resultType.isNullable() && resultType.equals(type.nullable()));
     }
 
     @Nonnull

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

@@ -452,13 +452,12 @@ public static Object coerceObject(@Nullable final CoercionTrieNode coercionsTrie
             return Verify.verifyNotNull(coercionFunction.apply(targetDescriptor, current));
         }
 
-        //
-        // This juggles with a change in nullability for arrays. If we were nullable before, but now we are not or
-        // vice versa, we need to change the wrapping in protobuf.
-        //
+        // This juggles with a change in nullability for arrays. If the array was nullable before, but now is not or
+        // vice versa, we need to change the wrapping in protobuf. This case also covers the promotion of `[]` (of type
+        // `None`) to an array (via the coercion function NONE_TO_ARRAY, which just returns the empty list unchanged).
         if (targetType.isArray()) {
-            Verify.verify(currentType.isArray());
-            final var coercionFunction = Verify.verifyNotNull(coercionsTrie.getValue());
+            Verify.verify(currentType.isArray() || currentType.isNone());
+            final CoercionBiFunction coercionFunction = Verify.verifyNotNull(coercionsTrie.getValue());
             return Verify.verifyNotNull(coercionFunction.apply(targetDescriptor, current));
         }
 
@@ -471,13 +470,16 @@ public static Object coerceObject(@Nullable final CoercionTrieNode coercionsTrie
     }
 
     /**
-     * Method to coerce an array.
-     * This juggles with a change in nullability for arrays. If we were nullable before, but now we are not or
-     * vice versa, we need to change the wrapping in protobuf.
+     * Coerce the given array {@code current}.
+     *
+     * <p>This juggles with a change in nullability for arrays. If the array was nullable before, but now is not or
+     * vice versa, we need to change the wrapping in protobuf. Note though that the protobuf wrapping path is only taken
+     * when a real {@link Descriptors.Descriptor} is provided (i.e., during storage-layer serialization as opposed to
+     * in-memory evaluation, where {@code targetDescriptor} would be {@code null}).
      *
      * @param targetArrayType target array type
      * @param currentArrayType current array type
-     * @param targetDescriptor target protobuf descriptor
+     * @param targetDescriptor target protobuf descriptor, if available; else {@code null}
      * @param elementsTrie a trie describing the coercions of the elements data structures
      * @param current the current object
      * @return a coerced array adjusted for nullability-differences of current versus target
@@ -492,7 +494,7 @@ public static Object coerceArray(@Nonnull final Type.Array targetArrayType,
         final var currentElementType = Verify.verifyNotNull(currentArrayType.getElementType());
 
         final Descriptors.FieldDescriptor targetElementFieldDescriptor;
-        if (targetArrayType.isNullable()) {
+        if (targetDescriptor != null && targetArrayType.isNullable()) {
             Verify.verify(targetDescriptor instanceof Descriptors.Descriptor);
             targetElementFieldDescriptor = Verify.verifyNotNull((Descriptors.Descriptor)targetDescriptor).findFieldByName(NullableArrayTypeUtils.getRepeatedFieldName());
         } else {
@@ -518,17 +520,29 @@ public static Object coerceArray(@Nonnull final Type.Array targetArrayType,
                             currentObject));
             coercedObjectsBuilder.add(coercedObject);
         }
-        final var coercedArray = coercedObjectsBuilder.build();
+        final ImmutableList<Object> coercedArray = coercedObjectsBuilder.build();
 
-        if (targetArrayType.isNullable()) {
-            // the target descriptor is the wrapping holder
-            final var wrapperBuilder = DynamicMessage.newBuilder(Verify.verifyNotNull((Descriptors.Descriptor)targetDescriptor));
-            wrapperBuilder.setField(Verify.verifyNotNull(targetElementFieldDescriptor), coercedArray);
-            return wrapperBuilder.build();
+        if (targetDescriptor != null && targetArrayType.isNullable()) {
+            Verify.verifyNotNull(targetElementFieldDescriptor);
+            return wrapNullableArray((Descriptors.Descriptor)targetDescriptor, targetElementFieldDescriptor, coercedArray);
         }
         return coercedArray;
     }
 
+    /**
+     * Wrap the given {@code array} into a message.
+     *
+     * @param targetDescriptor Descriptor for the wrapper message holding the array.
+     */
+    @Nonnull
+    private static DynamicMessage wrapNullableArray(@Nonnull final Descriptors.Descriptor targetDescriptor,
+                                                    @Nonnull final Descriptors.FieldDescriptor targetElementFieldDescriptor,
+                                                    final List<? extends Object> array) {
+        final var builder = DynamicMessage.newBuilder(targetDescriptor);
+        builder.setField(targetElementFieldDescriptor, array);
+        return builder.build();
+    }
+
     @Nonnull
     public static Message coerceMessage(@Nullable final CoercionTrieNode coercionsTrie,
                                         @Nonnull final Type targetType,

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

@@ -372,6 +372,14 @@ public static CoercionTrieNode computePromotionsTrie(@Nonnull final Type targetT
             return new CoercionTrieNode(new PrimitiveCoercionBiFunction(physicalOperator), null);
         }
 
+        // NONE is the type of the untyped empty array `[]`. Like a primitive, this is a leaf case that handles the
+        // promotion via a single physical operator (NONE_TO_ARRAY).
+        if (currentType.isNone()) {
+            final var physicalOperator = resolvePhysicalOperator(currentType, targetType);
+            SemanticException.check(physicalOperator != null, SemanticException.ErrorCode.INCOMPATIBLE_TYPE);
+            return new CoercionTrieNode(new PrimitiveCoercionBiFunction(physicalOperator), null);
+        }
+
         Verify.verify(targetType.getTypeCode() == currentType.getTypeCode());
 
         if (currentType.isUuid()) {
@@ -421,6 +429,11 @@ public static CoercionTrieNode computePromotionsTrie(@Nonnull final Type targetT
         return childrenMap.isEmpty() ? null : new CoercionTrieNode(null, childrenMap);
     }
 
+    /**
+     * Wrap a {@link PromoteValue} instance around {@code value} if necessary.
+     *
+     * <p>{@code inject()} is idempotent and does not modify the value if its result is already the desired type.
+     */
     @Nonnull
     public static Value inject(@Nonnull final Value inValue, @Nonnull final Type promoteToType) {
         final var inType = inValue.getResultType();
@@ -430,7 +443,7 @@ public static Value inject(@Nonnull final Value inValue, @Nonnull final Type pro
         if (inValue.canResultInType(promoteToType)) {
             return inValue.with(promoteToType);
         }
-        final var promotionTrie = computePromotionsTrie(promoteToType, inType, null);
+        final CoercionTrieNode promotionTrie = computePromotionsTrie(promoteToType, inType, null);
         return new PromoteValue(inValue, promoteToType, promotionTrie);
     }
 
@@ -447,10 +460,10 @@ public static boolean isPromotionNeeded(@Nonnull final Type inType, @Nonnull fin
         if (promoteToType.getTypeCode() == Type.TypeCode.ANY) {
             return false;
         }
-        if (inType.getTypeCode() == Type.TypeCode.NULL) {
+        if (inType.isNull()) {
             return true;
         }
-        if (inType.getTypeCode() == Type.TypeCode.NONE) {
+        if (inType.isNone()) {
             return true;
         }
         if (inType.isArray() && promoteToType.isArray()) {