Deduplication of deferred fields #65
Replies: 9 comments 24 replies
-
I think query {
data: {
a: {
b: {
c: {
d: "d-data"
}
}
},
hasNext: true,
} {
incremental: [
{
path: [],
completed: false,
data: {
a: {
b: {
e: {
f: "f-data"
}
}
}
}
],
hasNext: true,
} {
incremental: [
{
path: ['a', 'b'],
completed: true,
data: {
potentiallySlowFieldA: "potentiallySlowFieldA-data"
}
},
{
path: [],
completed: true,
data: {
g: { h: "h-data" },
potentiallySlowFieldB: "potentiallySlowFieldB-data"
}
}
],
"hasNext": false
} So basically:
|
Beta Was this translation helpful? Give feedback.
-
I don't think we need to go for maximal deduplication. The example query B, in my opinion, should stay effectively as-is, the two {
a {
b {
c {
d
}
... @defer {
e {
f
}
potentiallySlowFieldA
}
}
}
... @defer {
a {
b {
+ c {
+ d
+ }
e {
f
}
}
}
g {
h
}
potentiallySlowFieldB
}
} |
Beta Was this translation helpful? Give feedback.
-
At the Jan 30 2023 defer/stream wg meeting we discussed an option where there is both no deduplication and one incremental payload per path. This can be achieved by merging deferred fields under different deferred paths together to optimize the result. The trade-off with this approach is that a deferred fragments at one path could be blocked by fields in a deferred fragment on a different path. This tradeoff was deemed unacceptable and this option is no longer being considered. |
Beta Was this translation helpful? Give feedback.
-
Apologize for being late to the conversation. Wondering if we've considered reliability aspect of dedup. Wondering if we may potentially introduce ambiguous incremental payload that client is unsure how to merge due to partial errors, now that we are trimming the data. |
Beta Was this translation helpful? Give feedback.
-
In @yaacovCR's implementation only leaf fields were deduplicated. So for example this query: Example A query ExampleA {
hero {
nestedObject {
id
}
... @defer {
nestedObject {
id
}
}
}
} would return the following result [
{
"data": { "hero": { "nestedObject": { "id": "1" } } },
"hasNext": true
},
{
"incremental": [{ "data": { "nestedObject": {} }, "path": ["hero"] }],
"hasNext": false
}
] I've iterated on this to add deduplication of non-leaf fields. Since there is no pre-execution analysis in either graphql-js or the spec algorithms, my approach was to return a unique symbol as fields are resolved indicating to the parent object that they have been deduplicated. If all subfields were deduped, the parent object can also be deduped. My implementation is here: https://github.com/robrichard/graphql-js/compare/mask-parent-base...robrichard:graphql-js:mask-parent?expand=1 With this update, the ExampleA query above would return the following result: {
"data": { "hero": { "nestedObject": { "id": "1" } } },
"hasNext": false
} But if [
{
"data": { "hero": { "nestedObject": null } },
"hasNext": true
},
{
"incremental": [{ "data": { "nestedObject": null }, "path": ["hero"] }],
"hasNext": false
}
] Another option is we treat Example B query ExampleB {
hero {
nestedObject {
id
}
... @defer {
nestedObject {
name
}
}
}
}
Result: {
"data": { "hero": { "nestedObject": null } },
"hasNext": false
} In either case, the result is determined by the data returned by the resolver and would not be the same as an algorithm that statically rewrites the query. This same scenario would also exist for list fields that are null or empty. |
Beta Was this translation helpful? Give feedback.
-
I have become less and less convinced that deduplication of any fields is "worth it." graphql states that it "gives clients the power to ask for exactly what they need and nothing more" but that can be read as a reference to no extraneous data, not necessarily deduplication. Advanced clients can rewrite queries to avoid deduplication, which may be a better solution. I would even continue for advocate both for no inlining and for no deduplication, empowering incremental delivery without a cache.... |
Beta Was this translation helpful? Give feedback.
This comment has been hidden.
This comment has been hidden.
-
Here's a really really early version of my latest proposal - the "no overlapping branches" proposal: There are many more details within the PR, but highlights: No overlapping branching. Example A {
f2 { a b c { d e f { h i } } }
... @defer {
f2 { a b c { d e f { h j } } }
}
} Example Result: [
{
"data": {
"f2": {
"a": "a",
"b": "b",
"c": {
"d": "d",
"e": "e",
"f": { "h": "h", "i": "i" }
}
}
},
"pending": [{ "path": ["f2", "c", "f"], "id": "0" }],
"hasNext": true
},
{
"incremental": [
{ "path": ["f2", "c", "f"], "data": { "j": "j" } }
],
"completed": [{ "id": "0" }],
"hasNext": false
}
] Example A2 {
f2 { a b c { d e f { h i } } }
... @defer {
MyFragment: __typename
f2 { a b c { d e f { h j } } }
}
} Example Result: [
{
"data": {
"f2": {
"a": "a",
"b": "b",
"c": {
"d": "d",
"e": "e",
"f": { "h": "h", "i": "i" }
}
}
},
"pending": [{ "path": [], "id": "0" }],
"hasNext": true
},
{
"incremental": [
{ "path": [], "data": { "MyFragment": "Query" } }
{ "path": ["f2", "c", "f"], "data": { "j": "j" } }
],
"completed": [{ "id": "0" }],
"hasNext": false
}
] Example B {
a {
b {
c {
d
}
... @defer {
e {
f
}
potentiallySlowFieldA
}
}
}
... @defer {
a {
b {
e {
f
}
}
}
g {
h
}
potentiallySlowFieldB
}
} Example Result: [
{
"data": {
"a": { "b": { "c": { "d": "d" } } }
},
"pending": [
{ "path": [], "id": "0" },
],
"hasNext": true
},
{
"incremental": [
{
"path": [],
"data": {
"g": { "h": "h" },
"potentiallySlowFieldB": "potentiallySlowFieldB"
}
}
{
"path": ["a", "b"],
"data": {
"e": { "f": "f" },
"potentiallySlowFieldA": "potentiallySlowFieldA"
}
}
],
"completed": [{ "id": "0" }],
"hasNext": false
}
] Example C {
firstObject {
secondObject {
... @defer {
thirdObject {
fourthObject {
fieldA
fieldB
}
}
}
}
... @defer {
secondObject {
thirdObject {
fourthObject {
fieldA
}
... @defer {
fourthObject {
fieldA
fieldC
}
}
}
}
}
}
} Example Result [
{
"data": {
"firstObject": {
"secondObject": {}
}
},
"pending": [
{ "path": ["firstObject", "secondObject"], "id": "0" }
],
"hasNext": true
},
{
"incremental": [
{
"path": ["firstObject", "secondObject"],
"data": {
"thirdObject": {
"fourthObject": { "fieldA": "fieldA", "fieldB": "fieldB" }
}
}
}
],
"completed": [{ "id": "0" }],
"pending": [{ "path": ["firstObject", "secondObject", "thirdObject", "fourthObject"], "id": "1" }],
"hasNext": true
},
{
"incremental": [
{
"path": ["firstObject", "secondObject", "thirdObject", "fourthObject"],
"data": {
"fieldC": "fieldC"
}
}
],
"completed": [{ "id": "1" }],
"hasNext": false
}
] |
Beta Was this translation helpful? Give feedback.
-
This has been addressed by the new response format described in #69 Deferred fields are no longer duplicated in this response format |
Beta Was this translation helpful? Give feedback.
-
Context
depends on #64
This discussion is taking place with the assumption that we are moving forward with the proposal outlined in #64
Should fields that appear in both a deferred fragment and non-deferred fragment be duplicated?
Example A: Nested objects with overlapping deferred and non-deferred fields
Can we make this equivalent to this?
Example B: Optimally deduplicating fields could depend on the order which data is resolved
This example shows that optimal deduplication cannot be done statically. If the first defer is resolved first, a.b.e.f can be removed from the second defer. If the second defer is resolved first, it cannot be removed, and the second defer can be removed entirely.
Options being considered
Some options conform to the constraints that are currently in place in this proposal and some recommend adjusting these constraints to better allow deduplication.
The current constraints are:
Additionally the following requirements must also be met
Options that satisfy these constraints
Option 1 - No Deduplication
Option 2 - Spec algo doesn't deduplicate, deduplication optional
Option 3: Spec algo deduplicates only from initial payload (and/or parent payloads), further deduplication optional
Option 4: Full deduplication
Options that adjust the above constraints:
Option 5: (@IvanGoncharov's proposal: details in comment #65 (comment))
Option 6: (@benjie's proposal - details in spec edits graphql/graphql-spec@d0a2f95)
Beta Was this translation helpful? Give feedback.
All reactions