Skip to content

Commit 9d21654

Browse files
authored
Fix Incorrect Plan Constraints Generated for Join Predicates (#3971)
Plan constraints are used to ensure that cached query plans can be safely reused when parameter values change. These constraints validate that a cached plan remains valid for a given set of parameter bindings and are designed to work with constant-bound comparands where a field is compared against a constant or parameter (e.g., `field = ?`, `field > 5`). However, when a query contains both join predicates with non-constant comparands (e.g., `game.score_id = score.id`) and static predicates without constant-bound comparands (e.g., `game.score_id IS NOT NULL`), the plan constraint generation logic could incorrectly attempt to create constraints for the join predicate. This occurs because the static IS NOT NULL predicate acts as a filter that allows the query to proceed, but has no constant literals to capture in the constraint. Without proper filtering, the planner might try to create a plan constraint for the join predicate, which is semantically invalid since join predicates establish correlations between fields rather than between fields and constants. This fixes #3973.
1 parent 0116b7c commit 9d21654

7 files changed

Lines changed: 344 additions & 271 deletions

File tree

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,12 @@ private QueryPlanConstraint captureConstraint(@Nonnull final PredicateWithValueA
531531
.stream()
532532
.filter(comparison -> comparison instanceof Comparisons.ValueComparison)
533533
.map(valueComparison -> ((Comparisons.ValueComparison)valueComparison).getComparandValue())
534+
// Plan constraints are only defined for constant-bound comparands (e.g., field = 5).
535+
// Non-constant comparands (such as join predicates like field1 = field2) are filtered out here.
536+
// This filtering is essential when queries combine join predicates with static predicates
537+
// like IS NOT NULL. Since neither predicate has constant literals to bind, we must avoid
538+
// incorrectly generating constraints for the join predicate, which would be semantically invalid.
539+
.filter(comparandValue -> comparandValue instanceof Value.RangeMatchableValue)
534540
.map(constant -> PredicateWithValueAndRanges.ofRanges(constant, candidateRanges))
535541
.collect(Collectors.toList())));
536542
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ public ExplainTokensWithPrecedence explain() {
297297
}
298298

299299
if (!deferredRanges.isEmpty()) {
300-
resultExplainTokens.addSequence(() -> new ExplainTokens().addWhitespace()
300+
resultExplainTokens.addWhitespace().addKeyword("AND").addWhitespace().addSequence(() -> new ExplainTokens().addWhitespace()
301301
.addKeyword("AND").addWhitespace(),
302302
() -> deferredRanges.stream().map(Comparisons.Comparison::explain)
303303
.map(Precedence.AND::parenthesizeChild).iterator());

fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/query/cache/ConstraintValidityTests.java

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public class ConstraintValidityTests {
7575

7676
@RegisterExtension
7777
@Order(2)
78-
public final SimpleDatabaseRule database = new SimpleDatabaseRule(RelationalPlanCacheTests.class, TestSchemas.score());
78+
public final SimpleDatabaseRule database = new SimpleDatabaseRule(ConstraintValidityTests.class, TestSchemas.score());
7979

8080
@RegisterExtension
8181
@Order(3)
@@ -94,6 +94,9 @@ public class ConstraintValidityTests {
9494
@Nonnull
9595
private static final String BitAndScore4 = "BITANDSCORE4";
9696

97+
@Nonnull
98+
private static final String GameIdx = "GAMEIDX";
99+
97100
@Nonnull
98101
private static final String Scan = "SCAN";
99102

@@ -156,7 +159,9 @@ private static String inferScanType(@Nonnull final Plan<?> plan) {
156159
Assertions.assertInstanceOf(QueryPlan.PhysicalQueryPlan.class, plan);
157160
final var physicalPlan = (QueryPlan.PhysicalQueryPlan) plan;
158161
final var desc = physicalPlan.getRecordQueryPlan().toString(); // very ad-hoc :/
159-
if (desc.contains(MaxScoreByGame10)) {
162+
if (desc.contains(GameIdx)) { // not the best plan interpreter, gameIdx should be used in join queries only
163+
return GameIdx;
164+
} else if (desc.contains(MaxScoreByGame10)) {
160165
return MaxScoreByGame10;
161166
} else if (desc.contains(MaxScoreByGame20)) {
162167
return MaxScoreByGame20;
@@ -341,4 +346,13 @@ void cachingQueryWithComplexGroupByExpressionSubsumingIndexExpressionBehavesCorr
341346
ppe(cons(and(and(c17Equals2, c17Equals2), and(c17Int, c8Int, c10Int, c8Equalsc15, c17IntNotNull, c8IntNotNull, c10IntNotNull)))), BitAndScore2,
342347
ppe(cons(and(and(c17Equals4, c17Equals4), and(c17Int, c8Int, c10Int, c8Equalsc15, c17IntNotNull, c8IntNotNull, c10IntNotNull)))), BitAndScore4)));
343348
}
349+
350+
@Test
351+
void cachingQueryWithIsNotNullFilterWorksAsExpected() throws Exception {
352+
final var ticker = new FakeTicker();
353+
final var cache = getCache(ticker);
354+
planQuery(cache, "SELECT game.name from score, game use index (gameIdx) where game.score_id is not null and game.score_id = score.id", GameIdx);
355+
cacheShouldBe(cache, Map.of("SELECT \"GAME\" . \"NAME\" from \"SCORE\" , \"GAME\" use index ( \"GAMEIDX\" ) " +
356+
"where \"GAME\" . \"SCORE_ID\" is not null and \"GAME\" . \"SCORE_ID\" = \"SCORE\" . \"ID\" ", Map.of(ppe(QueryPlanConstraint.noConstraint()), GameIdx)));
357+
}
344358
}

fdb-relational-core/src/testFixtures/java/com/apple/foundationdb/relational/utils/TestSchemas.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,12 @@ public static String books() {
8888
@Nonnull
8989
private static final String SCORE_SCHEMA =
9090
"CREATE TABLE score(id bigint, player string, game bigint, score bigint, playDuration bigint, primary key(id))" +
91+
"CREATE TABLE game(id bigint, score_id bigint, name string, primary key(id))" +
9192
"CREATE INDEX maxScoreByGame10 AS SELECT MAX(score), game + 10 FROM score GROUP BY game + 10" +
9293
"CREATE INDEX maxScoreByGame20 AS SELECT MAX(score), game + 20 FROM score GROUP BY game + 20" +
9394
"CREATE INDEX bitAndScore2 AS SELECT game & 2, MAX(score) FROM score GROUP BY game & 2" +
94-
"CREATE INDEX bitAndScore4 AS SELECT game & 4, MAX(score) FROM score GROUP BY game & 4";
95+
"CREATE INDEX bitAndScore4 AS SELECT game & 4, MAX(score) FROM score GROUP BY game & 4" +
96+
"CREATE INDEX gameIdx AS SELECT score_id, id, name FROM game WHERE score_id IS NOT NULL ORDER BY score_id, id, name";
9597

9698
@Nonnull
9799
public static String score() {

0 commit comments

Comments
 (0)