New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixing bug when inferring doubly-nested dynamic list field types #3900
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release/v0.23.1 #3900 +/- ##
================================================
Coverage 15.86% 15.86%
================================================
Files 731 731
Lines 81544 81544
Branches 1093 1093
================================================
Hits 12936 12936
Misses 68608 68608
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
if is_list_field and not _path.endswith("[]"): | ||
_path += "[]" | ||
|
||
# Cache the manual unwind in case we need it recursively | ||
# Insert rather than append so that nested paths are found | ||
# first when using this cache | ||
_clean_path = _path.replace("[]", "") | ||
unwind_cache.insert(0, (_clean_path, _path)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not entirely understand what's going on here so just making sure it's intentional that this logic only applies when _path.endswith("[]")
is False
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this is correct as written. The only way that []
gets into the path is inside this if
, which is the time when we need to cache an undeclared list field that we've found.
Fixes a very subtle issue when inferring dynamic schemas for doubly-nested list fields. The new unit test fails on
develop
but passes on this branch.