fix(2892): consider entity resolvers in n + 1 check #2978
Conversation
github-actions
bot
added
the
type: fix
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2978 +/- ##
==========================================
+ Coverage 87.22% 87.28% +0.06%
==========================================
Files 280 280
Lines 27425 27471 +46
==========================================
+ Hits 23921 23978 +57
+ Misses 3504 3493 -11 ☔ View full report in Codecov by Sentry. |
| query { t2 { t2 { n } } } | ||
| query { t2_ls { t2 } } | ||
| query { t3 { t3 { n } } } | ||
| query { t3_ls { t3 { n } } } |
There was a problem hiding this comment.
on previous version it was
query { t2 { t2 { n } } }
query { t2 { t2 { n } } }
query { t2_ls { t2 } }
query { t2 { t2 { n } } }
because of caching the whole paths
| @@ -2,5 +2,4 @@ | |||
| source: src/core/config/npo/tracker.rs | |||
| expression: actual | |||
| --- | |||
| query { f1 { f1 { f2 } } } | |||
| query { f1 { f2 } } | |||
There was a problem hiding this comment.
here one path is dropped, but it was basically the same path so if batching is added it'd resolve both branches. I don't think there is difference if we show both
| } else { | ||
| let mut visited = visited.clone(); |
There was a problem hiding this comment.
visited has been cloned every time
| @@ -2,5 +2,5 @@ | |||
| source: src/core/config/npo/tracker.rs | |||
| expression: actual | |||
| --- | |||
| query { f1 { f2 } } | |||
| query { f1 { f3 } } | |||
There was a problem hiding this comment.
just the order change, I'm not sure why it's inconsistent for different test cases
|
Action required: PR inactive for 5 days. |
github-actions
bot
added
the
state: inactive
github-actions
bot
removed
the
state: inactive
| @link(type: Config, src: "nested/a.graphql") | ||
| @link(type: Config, src: "nested/b.graphql") | ||
| @link(type: Config, src: "nested/c.graphql") |
There was a problem hiding this comment.
ah, those are unused links, I removed them.
I've tried first with external files, but the include_config doesn't resolve links so I decided just merge it manually for the test without links
|
Action required: PR inactive for 5 days. |
github-actions
bot
added
the
state: inactive
meskill
removed
the
state: inactive
Summary:
Briefly describe the changes made in this PR.
Issue Reference(s):
Fixes #2892, #3008
/claim #2892
Build & Testing:
cargo testsuccessfully../lint.sh --mode=fixto fix all linting issues raised by./lint.sh --mode=check.Checklist:
<type>(<optional scope>): <title>