Skip to content

Commit e2b8adf

Browse files
danieljharveyhasura-bot
authored andcommittedMar 11, 2025·
Remove always true expressions in OpenDD planning (#1719)
<!-- The PR description should answer 2 important questions: --> ### What All of the execution plan mismatches had this in common - they passed an `{}` to a filter. In OpenDD this leaves empty `And []` around the place, where we used to erase them. ### How Wherever we were using `Some(ResolvedFilterExpression::mk_and(exprs))` we do `ResolvedFilterExpression::mk_and(exprs).remove_always_true_expression()` like old pipeline. V3_GIT_ORIGIN_REV_ID: 18cbf20c88ac8a94c6ec78f8153c094351d52e60
1 parent 1abb4d7 commit e2b8adf

File tree

5 files changed

+32
-8
lines changed

5 files changed

+32
-8
lines changed
 

‎v3/changelog.md

+3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88

99
### Changed
1010

11+
- Empty filter expressions are consistent between old and OpenDD execution
12+
pipelines
13+
1114
### Fixed
1215

1316
## [v2025.03.10]

‎v3/crates/engine/tests/execute/models/select_many/where/null_input/expected.json

+16
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,16 @@
2020
"author_id": 2,
2121
"first_name": "John"
2222
}
23+
],
24+
"AuthorManyWhereEmpty": [
25+
{
26+
"author_id": 1,
27+
"first_name": "Peter"
28+
},
29+
{
30+
"author_id": 2,
31+
"first_name": "John"
32+
}
2333
]
2434
}
2535
},
@@ -36,6 +46,12 @@
3646
"author_id": 2,
3747
"first_name": "John"
3848
}
49+
],
50+
"AuthorManyWhereEmpty": [
51+
{
52+
"author_id": 2,
53+
"first_name": "John"
54+
}
3955
]
4056
}
4157
}

‎v3/crates/engine/tests/execute/models/select_many/where/null_input/request.gql

+4
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,8 @@ query ($where_input: AuthorFilter) {
77
author_id
88
first_name
99
}
10+
AuthorManyWhereEmpty: AuthorMany(where: {}) {
11+
author_id
12+
first_name
13+
}
1014
}

‎v3/crates/plan/src/query/field_selection.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -492,9 +492,9 @@ fn from_model_relationship(
492492

493493
// combine existing filter with new ones
494494
if !relationship_join_filter_expressions.is_empty() {
495-
query_execution.query_node.predicate = Some(ResolvedFilterExpression::mk_and(
496-
relationship_join_filter_expressions.into(),
497-
));
495+
query_execution.query_node.predicate =
496+
ResolvedFilterExpression::mk_and(relationship_join_filter_expressions.into())
497+
.remove_always_true_expression();
498498
}
499499

500500
// add the new arguments
@@ -978,10 +978,10 @@ fn from_relationship_aggregate_selection(
978978

979979
// combine existing filter with new ones
980980
if !relationship_join_filter_expressions.is_empty() {
981-
query_execution.query_node.predicate =
982-
Some(ResolvedFilterExpression::mk_and(
983-
relationship_join_filter_expressions.into(),
984-
));
981+
query_execution.query_node.predicate = ResolvedFilterExpression::mk_and(
982+
relationship_join_filter_expressions.into(),
983+
)
984+
.remove_always_true_expression();
985985
}
986986

987987
// add the new arguments

‎v3/crates/plan/src/query/model_target.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,8 @@ pub fn model_target_to_ndc_query(
9999
permission_filter,
100100
filter,
101101
])),
102-
};
102+
}
103+
.and_then(ResolvedFilterExpression::remove_always_true_expression);
103104

104105
let order_by_elements = model_target
105106
.order_by

0 commit comments

Comments
 (0)