fix: unnested array columns don't expand into rows and crash on cross-filter click #1065
Conversation
jheer
left a comment
There was a problem hiding this comment.
Overall, this is looking good to me.
@derekperkins if you can take a quick look as well to confirm changes to clauseList behavior, that would be great! 🙏
There was a problem hiding this comment.
Pull request overview
Fixes vgplot cross-filtering and query generation for array columns when a source is configured with unnest, ensuring arrays expand into rows (via UNNEST(...)) and that interactive filtering uses properly-quoted DuckDB list predicates (list_has_any / list_has_all) instead of invalid IN (...) SQL.
Changes:
- Wrap unnested channel fields in
UNNEST()during mark query construction and tightenisUnnested()to only match real column references / names. - Update
ToggleandNearestinteractors to generate list-membership predicates for unnested fields and thread through alistMatch: 'any' | 'all'option. - Fix
clauseList()to correctly handle array values (quote as list literals) and treat empty arrays as “clear selection”, plus add regression tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/vgplot/plot/src/marks/Mark.js | Ensures unnest-declared fields are selected as UNNEST(...) to expand arrays into rows; adds isUnnested() helper for interactors. |
| packages/vgplot/plot/src/interactors/Toggle.js | Uses clauseList() for single-field unnested selections and supports listMatch. |
| packages/vgplot/plot/src/interactors/Nearest.js | Uses clauseList() for single-field unnested selections and supports listMatch. |
| packages/mosaic/core/src/SelectionClause.ts | Correctly builds list predicates from array values and clears selection on empty arrays. |
| packages/mosaic/core/test/selection-clause.test.ts | Adds regression coverage for quoting/spacing and empty-array clearing behavior in clauseList(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
this is a good fix |
|
Thanks for the reviews @jheer @derekperkins @domoritz! Just pulled in the latest main and sorted out the conflict, so this should be all clear now. |
Description
I've been exploring Mosaic (love that it can push millions of rows straight to DuckDB and still feel snappy) and ran into this bug while building a chart on an array column. Turns out there was already an attempt at a fix in #978, closed by the author because of the time issue, so I picked it up, brought it up to date with current main, fixed a couple of things.
Summary of changes:
Mark.js: Wraps unnested fields inUNNEST()inquery(), so arrays expand into individual rows instead of one stringified bar.isUnnested()only matches real column references, not just anything with a.columnproperty.Toggle.js&Nearest.js: Unnested fields now useclauseList()instead ofclausePoints(), generatinglist_has_any("tags", ['Roads'])instead of an unquotedIN. Both take alistMatchoption ('any' | 'all'), same asMenu.SelectionClause.ts:clauseList()now passes arrays straight tolistHasAny/listHasAllinstead of throughliteral(), which was coercing them to unquoted strings. Also fixed empty-array clears filtering out every row.selection-clause.test.ts(new): Tests for all of the above, including the exact values from the issue that used to crash.Testing
Ran the new tests, they pass. Also checked it by hand against the repro from the issue: tag bars now render individually instead of as one combined array, and clicking a bar cross-filters cleanly with no console errors.
Before.online-video-cutter.com.mp4
After.online-video-cutter.com.mp4
Closes #977.
cc @derekperkins for review.