Skip to content

Commit 0cf158a

Browse files
authored
Fix filtering behavior within non-existent @optional blocks. (#697)
* Add test data that reproduces incorrect query evaluation. * Add filtering fix and update test cases.
1 parent 5e6585a commit 0cf158a

File tree

32 files changed

+3035
-2
lines changed

32 files changed

+3035
-2
lines changed

trustfall_core/src/interpreter/filtering.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ fn apply_unary_filter<
233233
) -> ContextIterator<'query, Vertex> {
234234
Box::new(iterator.filter_map(move |mut context| {
235235
let last_value = context.values.pop().expect("no value present");
236-
filter_op(&last_value).then_some(context)
236+
(context.within_nonexistent_optional() || filter_op(&last_value)).then_some(context)
237237
}))
238238
}
239239

@@ -391,7 +391,7 @@ fn apply_filter_op<
391391
left: &FieldValue,
392392
right: &RightValue,
393393
) -> Option<DataContext<Vertex>> {
394-
filter_op(left, right).then_some(ctx)
394+
(ctx.within_nonexistent_optional() || filter_op(left, right)).then_some(ctx)
395395
}
396396

397397
fn apply_filter_with_static_argument_value<'query, Vertex: Debug + Clone + 'query>(

trustfall_core/src/interpreter/mod.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,20 @@ impl<Vertex> DataContext<Vertex> {
9999
self.active_vertex.as_ref().and_then(AsVertex::as_vertex)
100100
}
101101

102+
/// Whether this context is currently inside an `@optional` block with no values.
103+
///
104+
/// Let's unpack that:
105+
/// - At this point in the query execution, we're within an `@optional` block of the query.
106+
/// - An `@optional` edge in the evaluation of this context did not exist. It might not be
107+
/// the innermost `@optional` one — it might be a prior one if we're several `@optional` deep.
108+
///
109+
/// This is relevant, for example, for situations where we have filters or type coercions
110+
/// to apply within the `@optional` block and need to know if there's anything to filter/coerce.
111+
#[inline]
112+
pub(crate) fn within_nonexistent_optional(&self) -> bool {
113+
self.active_vertex.is_none()
114+
}
115+
102116
/// Converts `DataContext<Vertex>` to `DataContext<Other>` by mapping each `Vertex` to `Other`.
103117
///
104118
/// If you are implementing an [`Adapter`] for a data source,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
Ok(TestParsedGraphQLQuery(
2+
schema_name: "numbers",
3+
query: Query(
4+
root_connection: FieldConnection(
5+
position: Pos(
6+
line: 3,
7+
column: 5,
8+
),
9+
name: "Zero",
10+
),
11+
root_field: FieldNode(
12+
position: Pos(
13+
line: 3,
14+
column: 5,
15+
),
16+
name: "Zero",
17+
connections: [
18+
(FieldConnection(
19+
position: Pos(
20+
line: 4,
21+
column: 9,
22+
),
23+
name: "value",
24+
alias: Some("zero"),
25+
), FieldNode(
26+
position: Pos(
27+
line: 4,
28+
column: 9,
29+
),
30+
name: "value",
31+
alias: Some("zero"),
32+
output: [
33+
OutputDirective(),
34+
],
35+
)),
36+
(FieldConnection(
37+
position: Pos(
38+
line: 6,
39+
column: 9,
40+
),
41+
name: "predecessor",
42+
optional: Some(OptionalDirective()),
43+
), FieldNode(
44+
position: Pos(
45+
line: 6,
46+
column: 9,
47+
),
48+
name: "predecessor",
49+
connections: [
50+
(FieldConnection(
51+
position: Pos(
52+
line: 7,
53+
column: 13,
54+
),
55+
name: "value",
56+
), FieldNode(
57+
position: Pos(
58+
line: 7,
59+
column: 13,
60+
),
61+
name: "value",
62+
filter: [
63+
FilterDirective(
64+
operation: LessThan((), VariableRef("zero")),
65+
),
66+
],
67+
output: [
68+
OutputDirective(),
69+
],
70+
)),
71+
],
72+
)),
73+
],
74+
),
75+
),
76+
arguments: {
77+
"zero": Int64(0),
78+
},
79+
))
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
TestGraphQLQuery (
2+
schema_name: "numbers",
3+
query: r#"
4+
{
5+
Zero {
6+
zero: value @output
7+
8+
predecessor @optional {
9+
value @output @filter(op: "<", value: ["$zero"])
10+
}
11+
}
12+
}"#,
13+
arguments: {
14+
"zero": Int64(0),
15+
},
16+
)
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
Ok(TestIRQuery(
2+
schema_name: "numbers",
3+
ir_query: IRQuery(
4+
root_name: "Zero",
5+
root_component: IRQueryComponent(
6+
root: Vid(1),
7+
vertices: {
8+
Vid(1): IRVertex(
9+
vid: Vid(1),
10+
type_name: "Number",
11+
),
12+
Vid(2): IRVertex(
13+
vid: Vid(2),
14+
type_name: "Number",
15+
filters: [
16+
LessThan(LocalField(
17+
field_name: "value",
18+
field_type: "Int",
19+
), Variable(VariableRef(
20+
variable_name: "zero",
21+
variable_type: "Int!",
22+
))),
23+
],
24+
),
25+
},
26+
edges: {
27+
Eid(1): IREdge(
28+
eid: Eid(1),
29+
from_vid: Vid(1),
30+
to_vid: Vid(2),
31+
edge_name: "predecessor",
32+
optional: true,
33+
),
34+
},
35+
outputs: {
36+
"value": ContextField(
37+
vertex_id: Vid(2),
38+
field_name: "value",
39+
field_type: "Int",
40+
),
41+
"zero": ContextField(
42+
vertex_id: Vid(1),
43+
field_name: "value",
44+
field_type: "Int",
45+
),
46+
},
47+
),
48+
variables: {
49+
"zero": "Int!",
50+
},
51+
),
52+
arguments: {
53+
"zero": Int64(0),
54+
},
55+
))
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
TestInterpreterOutputData(
2+
schema_name: "numbers",
3+
outputs: {
4+
"value": Output(
5+
name: "value",
6+
value_type: "Int",
7+
vid: Vid(2),
8+
),
9+
"zero": Output(
10+
name: "zero",
11+
value_type: "Int",
12+
vid: Vid(1),
13+
),
14+
},
15+
results: [
16+
{
17+
"value": Null,
18+
"zero": Int64(0),
19+
},
20+
],
21+
)

0 commit comments

Comments
 (0)