fix: Fix incorrect lazy schema for aggregations #19753
Conversation
github-actions
bot
added
fix
| *agg_list = false; | ||
| let mut field = arena | ||
| .get(*expr) | ||
| .to_field_impl(schema, ctx, arena, &mut false)?; |
There was a problem hiding this comment.
We previously set agg_list to false (*nested = nested.saturating_sub(1)), but then we passed it to to_field_impl, so it would get incorrectly overridden by the upper node - this is the cause of the linked issue.
| // Indicates whether we should auto-implode the result. This is initialized to true if we are | ||
| // in an aggregation context, so functions that return scalars should explicitly set this | ||
| // to false in `to_field_impl`. | ||
| let mut agg_list = matches!(ctx, Context::Aggregation); |
There was a problem hiding this comment.
Refactored to use a bool indicator - I think we only need to keep track of 2 states
| run: | | ||
| curl -LsSf https://astral.sh/uv/install.sh | sh | ||
| echo "$HOME/.local/bin" >> "$GITHUB_PATH" |
There was a problem hiding this comment.
drive-by fix CI, ~/.local/bin needs to be in $PATH to use uv - we only seem to need this on macOS runners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #19753 +/- ##
==========================================
- Coverage 79.73% 79.72% -0.02%
==========================================
Files 1542 1543 +1
Lines 212223 212715 +492
Branches 2449 2443 -6
==========================================
+ Hits 169216 169583 +367
- Misses 42453 42582 +129
+ Partials 554 550 -4 ☔ View full report in Codecov by Sentry. |
nameexhaustion
requested review from
ritchie46,
c-peters,
alexander-beedie,
MarcoGorelli,
reswqa and
orlp
as code owners
Fixes #19752
Also adjusts a CI workflow file to fix code coverage