Skip to content

Commit 29cfcd0

Browse files
hatyohatyo
authored
Simplification rules for constant coalesce and promote expressions (#3425)
This change introduces two new `Value` simplification rules: 1. A rule to simplify constant `coalesce` expressions where arguments are constant as they are evaluated from left to right. 2. A rule to simplify constant `promote` expressions that attempt to promote a boolean value (or a null value) to a boolean type (or any type, in the case of a null value). Additionally, this change corrects an issue in the encapsulation logic of variadic functions, which previously prevented the correct promotion of arguments containing a `null` literal. This fix is now exposed through simplification, enabling the creation of `null` literals within `coalesce` expressions. Finally, the predicate testing framework has been extended to include random `coalesce` and `promote` values, improving test coverage. This fixes #3418 and #3417.
1 parent 29f2f1f commit 29cfcd0

File tree

14 files changed

+464
-48
lines changed

14 files changed

+464
-48
lines changed

‎fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/matching/structure/ValueMatchers.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,13 @@
2828
import com.apple.foundationdb.record.query.plan.cascades.values.LiteralValue;
2929
import com.apple.foundationdb.record.query.plan.cascades.values.NullValue;
3030
import com.apple.foundationdb.record.query.plan.cascades.values.NumericAggregationValue;
31+
import com.apple.foundationdb.record.query.plan.cascades.values.PromoteValue;
3132
import com.apple.foundationdb.record.query.plan.cascades.values.QuantifiedObjectValue;
3233
import com.apple.foundationdb.record.query.plan.cascades.values.RecordConstructorValue;
3334
import com.apple.foundationdb.record.query.plan.cascades.values.StreamableAggregateValue;
3435
import com.apple.foundationdb.record.query.plan.cascades.values.ToOrderedBytesValue;
3536
import com.apple.foundationdb.record.query.plan.cascades.values.Value;
37+
import com.apple.foundationdb.record.query.plan.cascades.values.VariadicFunctionValue;
3638
import com.apple.foundationdb.record.query.plan.cascades.values.VersionValue;
3739
import com.apple.foundationdb.tuple.TupleOrdering;
3840
import com.google.common.collect.ImmutableList;
@@ -71,6 +73,28 @@ public static BindingMatcher<ConstantObjectValue> anyConstantObjectValue() {
7173
return typed(ConstantObjectValue.class);
7274
}
7375

76+
@Nonnull
77+
public static BindingMatcher<PromoteValue> anyPromoteValue() {
78+
return typed(PromoteValue.class);
79+
}
80+
81+
@Nonnull
82+
public static BindingMatcher<VariadicFunctionValue> anyVariadicFunction() {
83+
return typed(VariadicFunctionValue.class);
84+
}
85+
86+
@Nonnull
87+
public static BindingMatcher<VariadicFunctionValue> variadicFunction(
88+
@Nonnull final VariadicFunctionValue.ComparisonFunction comparisonFunction) {
89+
return typedMatcherWithPredicate(VariadicFunctionValue.class,
90+
variadicFunctionValue -> variadicFunctionValue.getComparisonFunction() == comparisonFunction);
91+
}
92+
93+
@Nonnull
94+
public static BindingMatcher<VariadicFunctionValue> coalesceFunction() {
95+
return variadicFunction(VariadicFunctionValue.ComparisonFunction.COALESCE);
96+
}
97+
7498
@SuppressWarnings("unchecked")
7599
@Nonnull
76100
public static BindingMatcher<LiteralValue<Boolean>> anyBooleanLiteralValue() {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@
5555
* <li>{@code TRUE = [FALSE, FALSE] (AND [FALSE, FALSE])+ → FALSE}</li>
5656
* <li>{@code FALSE = [TRUE, TRUE] (AND [TRUE, TRUE])+ → FALSE}</li>
5757
* <li>{@code FALSE = [FALSE, FALSE] (AND [FALSE, FALSE])+ → TRUE}</li>
58+
* <li>{@code NULL = <ANYTHING> → NULL}</li>
59+
* <li>{@code <ANYTHING> = NULL → NULL}</li>
5860
* <li>{@code NULL ≠ [NULL, NULL] (AND [NULL, NULL])+ → NULL}</li>
5961
* <li>{@code NULL ≠ [TRUE, TRUE] (AND [TRUE, TRUE])+ → NULL}</li>
6062
* <li>{@code NULL ≠ [FALSE, FALSE] (AND [FALSE, FALSE])+ → NULL}</li>

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,8 @@
4040
* The following simplifications are performed:
4141
*
4242
* <ul>
43-
* <li>{@code NULL = [NULL, NULL] → NULL}</li>
44-
* <li>{@code NULL = [TRUE, TRUE] → NULL}</li>
45-
* <li>{@code NULL = [FALSE, FALSE] → NULL}</li>
43+
* <li>{@code NULL = [<ANYTHING>] → NULL}</li>
44+
* <li>{@code [<ANYTHING>] = NULL → NULL}</li>
4645
* <li>{@code TRUE = [NULL, NULL] → NULL}</li>
4746
* <li>{@code TRUE = [TRUE, TRUE] → TRUE}</li>
4847
* <li>{@code TRUE = [FALSE, FALSE] → FALSE}</li>

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

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,25 +36,24 @@
3636
* Simplifies a specific form of constant {@link ValuePredicate}. The following simplifications are performed:
3737
*
3838
* <ul>
39-
* <li>{@code NULL = NULL → NULL}</li>
40-
* <li>{@code NULL = TRUE → NULL}</li>
41-
* <li>{@code NULL = FALSE → NULL}</li>
42-
* <li>{@code TRUE = NULL → NULL}</li>
43-
* <li>{@code TRUE = TRUE → TRUE}</li>
39+
* <li>{@code NULL = <ANYTHING> → NULL}</li>
40+
* <li>{@code <ANYTHING> = NULL → NULL}</li>
41+
* <li>{@code TRUE = NULL → NULL}</li>
42+
* <li>{@code TRUE = TRUE → TRUE}</li>
4443
* <li>{@code TRUE = FALSE → FALSE}</li>
4544
* <li>{@code FALSE = NULL → NULL}</li>
46-
* <li>{@code FALSE = TRUE → FALSE}</li>
45+
* <li>{@code FALSE = TRUE → FALSE}</li>
4746
* <li>{@code FALSE = FALSE → TRUE}</li>
48-
* <li>{@code NULL ≠ NULL → NULL}</li>
49-
* <li>{@code NULL ≠ TRUE → NULL}</li>
47+
* <li>{@code NULL ≠ NULL → NULL}</li>
48+
* <li>{@code NULL ≠ TRUE → NULL}</li>
5049
* <li>{@code NULL ≠ FALSE → NULL}</li>
51-
* <li>{@code TRUE ≠ NULL → NULL}</li>
52-
* <li>{@code TRUE ≠ TRUE → FALSE}</li>
50+
* <li>{@code TRUE ≠ NULL → NULL}</li>
51+
* <li>{@code TRUE ≠ TRUE → FALSE}</li>
5352
* <li>{@code TRUE ≠ FALSE → TRUE}</li>
5453
* <li>{@code FALSE ≠ NULL → NULL}</li>
55-
* <li>{@code FALSE ≠ TRUE → TRUE}</li>
54+
* <li>{@code FALSE ≠ TRUE → TRUE}</li>
5655
* <li>{@code FALSE ≠ FALSE → FALSE}</li>
57-
* <li>{@code NOT_NULL IS NULL → FALSE}</li>
56+
* <li>{@code NOT_NULL IS NULL → FALSE}</li>
5857
* <li>{@code TRUE IS NULL → FALSE}</li>
5958
* <li>{@code FALSE IS NULL → FALSE}</li>
6059
* <li>{@code NULL IS NULL → TRUE}</li>

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

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,14 @@
3030
import com.apple.foundationdb.record.query.plan.cascades.values.Value;
3131

3232
import javax.annotation.Nonnull;
33+
import javax.annotation.Nullable;
3334
import java.util.Optional;
3435

3536
/**
3637
* Trait that facilitates folding a constant {@link QueryPredicate}.
3738
*/
3839
@API(API.Status.EXPERIMENTAL)
39-
final class ConstantPredicateFoldingUtil {
40+
public final class ConstantPredicateFoldingUtil {
4041

4142
/**
4243
* Analyzes the provided operand and comparison to produce a simplified, semantically equivalent {@link QueryPredicate}.
@@ -79,31 +80,44 @@ public static Optional<QueryPredicate> foldComparisonMaybe(@Nonnull final Value
7980
}
8081
}
8182

82-
if (!(comparison instanceof Comparisons.ValueComparison)) {
83+
EffectiveConstant rhsOperand;
84+
85+
if (comparison instanceof Comparisons.ValueComparison) {
86+
final var valueComparison = (Comparisons.ValueComparison)comparison;
87+
final var rhsValue = valueComparison.getValue();
88+
rhsOperand = EffectiveConstant.from(rhsValue);
89+
} else if (comparison instanceof Comparisons.SimpleComparison) {
90+
final var simpleComparison = (Comparisons.SimpleComparison)comparison;
91+
final var rhsValue = simpleComparison.getComparand();
92+
rhsOperand = EffectiveConstant.from(rhsValue);
93+
} else {
8394
return Optional.empty();
8495
}
8596

86-
final var valueComparison = (Comparisons.ValueComparison)comparison;
87-
final var rhsValue = valueComparison.getValue();
88-
final var rhsOperand = EffectiveConstant.from(rhsValue);
97+
switch (comparisonType) {
98+
case EQUALS: // fallthrough
99+
case NOT_EQUALS:
100+
if (lhsOperand == EffectiveConstant.NULL || rhsOperand == EffectiveConstant.NULL) {
101+
return Optional.of(ConstantPredicate.NULL);
102+
}
103+
break;
104+
default:
105+
return Optional.empty();
106+
}
89107

90108
if (rhsOperand.isUnknownLiteral() || lhsOperand.isUnknownLiteral()) {
91109
return Optional.empty();
92110
}
93111

94112
switch (comparisonType) {
95113
case EQUALS:
96-
if (lhsOperand == EffectiveConstant.NULL || rhsOperand == EffectiveConstant.NULL) {
97-
return Optional.of(ConstantPredicate.NULL);
98-
} else if (lhsOperand.equals(rhsOperand)) {
114+
if (lhsOperand.equals(rhsOperand)) {
99115
return Optional.of(ConstantPredicate.TRUE);
100116
} else {
101117
return Optional.of(ConstantPredicate.FALSE);
102118
}
103119
case NOT_EQUALS:
104-
if (lhsOperand == EffectiveConstant.NULL || rhsOperand == EffectiveConstant.NULL) {
105-
return Optional.of(ConstantPredicate.NULL);
106-
} else if (!lhsOperand.equals(rhsOperand)) {
120+
if (!lhsOperand.equals(rhsOperand)) {
107121
return Optional.of(ConstantPredicate.TRUE);
108122
} else {
109123
return Optional.of(ConstantPredicate.FALSE);
@@ -113,7 +127,7 @@ public static Optional<QueryPredicate> foldComparisonMaybe(@Nonnull final Value
113127
}
114128
}
115129

116-
enum EffectiveConstant {
130+
public enum EffectiveConstant {
117131
TRUE(true),
118132
FALSE(true),
119133
NULL(true),
@@ -130,6 +144,25 @@ boolean isUnknownLiteral() {
130144
return !isKnownLiteral;
131145
}
132146

147+
public static EffectiveConstant from(@Nullable Object value) {
148+
if (value == null) {
149+
return EffectiveConstant.NULL;
150+
}
151+
152+
if (value instanceof Value) {
153+
return from((Value)value);
154+
}
155+
156+
if (value instanceof Boolean) {
157+
if ((Boolean)value) {
158+
return EffectiveConstant.TRUE;
159+
}
160+
return EffectiveConstant.FALSE;
161+
}
162+
163+
return EffectiveConstant.NOT_NULL;
164+
}
165+
133166
public static EffectiveConstant from(@Nonnull final Value value) {
134167
if (value instanceof NullValue) {
135168
return EffectiveConstant.NULL;

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,10 @@ private static Descriptors.GenericDescriptor getTypeSpecificDescriptor(@Nonnull
514514
@Nullable
515515
@SuppressWarnings("PMD.CompareObjectsWithEquals")
516516
static Type maximumType(@Nonnull final Type t1, @Nonnull final Type t2) {
517+
if (t1.getTypeCode() == TypeCode.NULL && t2.getTypeCode() == TypeCode.NULL) {
518+
return Type.nullType();
519+
}
520+
517521
if (t1.getTypeCode() == TypeCode.NULL && PromoteValue.isPromotable(t1, t2)) {
518522
return t2.withNullability(true);
519523
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public T getLiteralValue() {
9494

9595
@Override
9696
public boolean canResultInType(@Nonnull final Type type) {
97-
return resultType.isNullable() && resultType.equals(type.nullable());
97+
return type.isNullable() && resultType.equals(type.nullable());
9898
}
9999

100100
@Nonnull

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,11 @@ public Type getResultType() {
101101
return children.get(0).getResultType();
102102
}
103103

104+
@Nonnull
105+
public ComparisonFunction getComparisonFunction() {
106+
return operator.getComparisonFunction();
107+
}
108+
104109
@Nonnull
105110
@Override
106111
protected Iterable<? extends Value> computeChildren() {
@@ -118,7 +123,7 @@ public VariadicFunctionValue withChildren(final Iterable<? extends Value> newChi
118123
public int hashCodeWithoutChildren() {
119124
return PlanHashable.objectsPlanHash(PlanHashable.CURRENT_FOR_CONTINUATION, BASE_HASH, operator);
120125
}
121-
126+
122127
@Override
123128
public int planHash(@Nonnull final PlanHashMode mode) {
124129
return PlanHashable.objectsPlanHash(mode, BASE_HASH, operator, children);
@@ -181,15 +186,21 @@ private static Map<NonnullPair<ComparisonFunction, TypeCode>, PhysicalOperator>
181186
}
182187

183188
@Nonnull
189+
@SuppressWarnings("PMD.CompareObjectsWithEquals")
184190
private static Value encapsulate(@Nonnull BuiltInFunction<Value> builtInFunction,
185191
@Nonnull final List<? extends Typed> arguments) {
186192
Verify.verify(arguments.size() >= 2);
187193
Type resultType = null;
188194
for (final var arg : arguments) {
189195
Type argType = arg.getResultType();
190-
if (resultType == null || resultType.isUnresolved()) {
196+
// This logic allows one (but not all) of the arguments to be of `NullType`
197+
// (e.g., a 'null' literal). In such cases, `Type.maximumType` will correctly
198+
// determine the appropriate concrete type *and* set its nullability to `true`,
199+
// due to the presence of the `NullType` argument.
200+
Verify.verify(argType == Type.NULL || !argType.isUnresolved());
201+
if (resultType == null) {
191202
resultType = argType;
192-
} else if (!argType.isUnresolved()) {
203+
} else {
193204
resultType = Type.maximumType(resultType, argType);
194205
SemanticException.check(resultType != null, SemanticException.ErrorCode.INCOMPATIBLE_TYPE);
195206
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,18 @@ public class DereferenceConstantObjectValueRuleSet extends AbstractValueRuleSet<
4141
@Nonnull
4242
protected static final ValueSimplificationRule<? extends Value> dereferenceConstantObjectValueRule = new DereferenceConstantObjectValueRule();
4343

44+
@Nonnull
45+
protected static final ValueSimplificationRule<? extends Value> evaluateConstantPromotionRule = new EvaluateConstantPromotionRule();
46+
47+
@Nonnull
48+
protected static final ValueSimplificationRule<? extends Value> evaluateConstantCoalesceRule = new EvaluateConstantCoalesceRule();
49+
4450
private static final Set<ValueSimplificationRule<? extends Value>> DEREFERENCE_CONSTANT_OBJECT_VALUE_RULES =
4551
ImmutableSet.<ValueSimplificationRule<? extends Value>>builder()
4652
.addAll(DefaultValueSimplificationRuleSet.SIMPLIFICATION_RULES)
4753
.add(dereferenceConstantObjectValueRule)
54+
.add(evaluateConstantPromotionRule)
55+
.add(evaluateConstantCoalesceRule)
4856
.build();
4957

5058
private static final SetMultimap<ValueSimplificationRule<? extends Value>, ValueSimplificationRule<? extends Value>> DEREFERENCE_CONSTANT_OBJECT_VALUE_DEPENDS_ON;
@@ -55,6 +63,10 @@ public class DereferenceConstantObjectValueRuleSet extends AbstractValueRuleSet<
5563
simplificationDependsOnBuilder.putAll(DefaultValueSimplificationRuleSet.SIMPLIFICATION_DEPENDS_ON);
5664

5765
DefaultValueSimplificationRuleSet.SIMPLIFICATION_RULES.forEach(existingRule -> simplificationDependsOnBuilder.put(existingRule, dereferenceConstantObjectValueRule));
66+
DefaultValueSimplificationRuleSet.SIMPLIFICATION_RULES.forEach(existingRule -> simplificationDependsOnBuilder.put(existingRule, evaluateConstantPromotionRule));
67+
DefaultValueSimplificationRuleSet.SIMPLIFICATION_RULES.forEach(existingRule -> simplificationDependsOnBuilder.put(existingRule, evaluateConstantCoalesceRule));
68+
simplificationDependsOnBuilder.put(evaluateConstantPromotionRule, dereferenceConstantObjectValueRule);
69+
simplificationDependsOnBuilder.put(evaluateConstantCoalesceRule, dereferenceConstantObjectValueRule);
5870
DEREFERENCE_CONSTANT_OBJECT_VALUE_DEPENDS_ON = simplificationDependsOnBuilder.build();
5971
}
6072

0 commit comments

Comments
 (0)