fix(deckgl): emit usable cross-filter values from polygon and geojson clicks#39906
fix(deckgl): emit usable cross-filter values from polygon and geojson clicks#39906alex-poor wants to merge 1 commit intoapache:masterfrom
Conversation
… clicks Adds a new optional `cross_filter_column` control to the deck_polygon and deck_geojson chart types. When set, clicking a feature emits a `<dimension> == <value>` filter (e.g. `sa3_name == 'Christchurch West'`) that other charts on the dashboard can match against by column name. Previously the emitted filter was a `REPLACE(<geometry_col>, ' ', '') == <JSON.stringify(path)>` SQL clause comparing a bare coordinate path string against the full stored GeoJSON Feature, which could never match in practice. Receiving charts saw "No data" and the dashboard chip showed a wall of polygon JSON. Polygon resolves the dimension value from the picked feature's top-level fields (groupby/columns spread by addPropertiesToFeature), falling back to extraProps for js_columns. Geojson reads from the picked Feature's properties object (the standard non-spatial location). The legacy behaviour is preserved as a fallback when cross_filter_column is unset so existing dashboards keep working. Also fixes a related click-handling regression: deck.gl v9 dropped event.leftButton/event.rightButton from the picking event. The commonLayerProps wiring was checking those undefined fields, so left clicks silently emitted nothing and right clicks never opened the context menu. Dispatch now uses event.type and srcEvent.button. Supersedes apache#28262. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| { | ||
| label: t('Advanced'), | ||
| controlSetRows: [ | ||
| [crossFilterColumn], |
There was a problem hiding this comment.
Suggestion: Adding the cross_filter_column control to GeoJSON without wiring that field into the GeoJSON query/transform path creates a contract mismatch: users can select a dataset column, but click handling reads only data.object.properties[col] from the parsed GeoJSON blob. When that property is not embedded in the GeoJSON payload, cross-filtering silently falls back to the legacy geometry filter and still produces non-matching filters. Update the GeoJSON query/transform flow to ensure the selected cross-filter dimension is present on picked features before exposing this control. [logic error]
Severity Level: Major ⚠️
- ❌ Deck.gl GeoJSON cross-filters fail for some dataset schemas.
- ⚠️ Cross-filter column UI may not match GeoJSON properties.
- ⚠️ Users may see legacy geometry filters despite configuration.Steps of Reproduction ✅
1. Configure a Deck.gl GeoJSON chart in the UI that uses a dataset whose table has a
`geojson` column containing GeoJSON features (without a `sa3_name` property) and a
separate SQL column `sa3_name`. In the frontend this chart is backed by `transformProps`
at
`superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/transformProps.ts:24-49`,
which parses only the `formData.geojson` column value into GeoJSON features and does not
merge other SQL columns into `feature.properties`.
2. In the chart's Advanced section, use the newly added `cross_filter_column` control
(`[crossFilterColumn]` wired in `controlPanel.ts:20-27`, corresponding to diff line 371)
to select the dataset column `sa3_name`. This control is defined via `crossFilterColumn`
in `Shared_DeckGL.tsx:185-27`, which reuses `sharedControls.groupby` so it offers dataset
columns, but `DeckGeoJson`'s `buildQuery.ts:39-65` only ensures the `geojson` column (and
js/tooltip columns) are queried and is unaware of `cross_filter_column`.
3. Add this chart to a dashboard alongside another chart filtered on the same logical
dimension (e.g., a bar chart grouped by `sa3_name`). Enable cross-filtering for the
GeoJSON chart so clicks go through `commonLayerProps` (`common.tsx:50-55`), which on click
calls `getCrossFilterDataMask({ data, filterState, formData })` at `common.tsx:118-122`.
4. Click a region on the GeoJSON chart. `getCrossFilterDataMask` dispatches to
`getGeojsonFilters` in `crossFiltersDataMask.ts:15-76`, which looks up
`formData.cross_filter_column` in `data.object.properties[col]`. Because `transformProps`
never attached the SQL `sa3_name` column and the GeoJSON payload lacks a `sa3_name`
property, `dimensionVal` is `null`, triggering the console warning at
`crossFiltersDataMask.ts:48-52` and falling back to the legacy geometry-based LIKE filter
(`REPLACE(formData.geojson, ' ', '') LIKE '%<coords>%'`). The resulting filter cannot
match the other chart's `sa3_name` dimension, so the receiving chart shows "No data"
despite the user having selected a `cross_filter_column`. Note that this failure only
occurs for columns not present in `feature.properties`; when the GeoJSON payload includes
the property (as in the test `deck_geojson with cross_filter_column emits filter from
feature.properties` in `crossFiltersDataMask.test.ts:587-67`), the dimension filter works
as intended, so the suggestion's claim that selected dimensions can "never" be emitted is
overstated.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/controlPanel.ts
**Line:** 371:371
**Comment:**
*Logic Error: Adding the `cross_filter_column` control to GeoJSON without wiring that field into the GeoJSON query/transform path creates a contract mismatch: users can select a dataset column, but click handling reads only `data.object.properties[col]` from the parsed GeoJSON blob. When that property is not embedded in the GeoJSON payload, cross-filtering silently falls back to the legacy geometry filter and still produces non-matching filters. Update the GeoJSON query/transform flow to ensure the selected cross-filter dimension is present on picked features before exposing this control.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| if (formData.cross_filter_column) { | ||
| const col = formData.cross_filter_column; | ||
| const properties = (data.object?.properties ?? {}) as Record< | ||
| string, | ||
| unknown | ||
| >; | ||
| const dimensionVal = properties[col] as | ||
| | string | ||
| | number | ||
| | boolean | ||
| | null | ||
| | undefined; |
There was a problem hiding this comment.
Suggestion: This new GeoJSON cross-filter path assumes the selected column is present under feature.properties, but the GeoJSON query pipeline does not fetch or hydrate cross_filter_column into clicked features for normal dataset columns. As a result, many valid column selections will always miss here and silently fall back to the legacy geometry filter (still unmatchable). Wire cross_filter_column through GeoJSON query/transform (or read from where row-level columns are actually stored) so selected columns consistently produce dimension filters. [logic error]
Severity Level: Major ⚠️
- ❌ deck_geojson cross-filters ignore selected dimension columns.
- ⚠️ GeoJSON-originated filters rarely match other dashboard charts.Steps of Reproduction ✅
1. Configure a `deck_geojson` chart in the UI using a dataset that has a GeoJSON column
plus a separate dimension column (for example, `geojson` and `sa3_name`). In the control
panel, set the "Cross-filter column" control (defined in
`superset-frontend/plugins/preset-chart-deckgl/src/utilities/Shared_DeckGL.tsx:184-197` as
`cross_filter_column`) to the dataset column `sa3_name`.
2. Render the chart. The front-end query for `deck_geojson` is built in
`superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/buildQuery.ts:39-85`.
This query only ensures the GeoJSON column (and any `js_columns` / tooltip columns) are
included; it does not specially hydrate `cross_filter_column` into the GeoJSON payload. In
the corresponding transform step
(`superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/transformProps.ts:24-49`),
each row's `geojson` string is parsed and only that JSON is returned as a feature; the
rest of the row, including `sa3_name`, is discarded and never attached to the feature's
`properties` or `extraProps`.
3. On the client, the `GeoJsonLayer` is created in
`superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/Geojson.tsx:255-84`
using `processedFeatures` as its `data`. When a user clicks a feature on the map, deck.gl
emits a `PickingInfo` whose `object` is that `ProcessedFeature` (with `properties` coming
from the parsed GeoJSON only). Because `transformProps.ts` never copied the row-level
`sa3_name` into that feature, `data.object.properties.sa3_name` is `undefined` at click
time.
4. The click handler in `commonLayerProps` (imported into `Geojson.tsx` around lines
41-42) calls `getCrossFilterDataMask` in
`superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts:138-162`.
Since `formData.geojson` is set, it routes to `getGeojsonFilters` (defined at lines
474-135 in the same file). Inside the new logic (lines 482-493), `const properties =
(data.object?.properties ?? {})` and `const dimensionVal = properties[col]` are evaluated
with `col === 'sa3_name'`, but `dimensionVal` is `undefined` because that key was never
hydrated into `properties`. The code logs the warning and falls through to the legacy
geometry filter (lines 115-135), emitting a `LIKE '%<coordinates>%'` filter on the GeoJSON
column instead of `sa3_name == 'Christchurch West'`. Any receiving chart expecting to be
filtered on column `sa3_name` therefore cannot match this filter and will display "No
data", meaning the new `cross_filter_column` setting on `deck_geojson` silently fails for
normal dataset columns.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts
**Line:** 482:493
**Comment:**
*Logic Error: This new GeoJSON cross-filter path assumes the selected column is present under `feature.properties`, but the GeoJSON query pipeline does not fetch or hydrate `cross_filter_column` into clicked features for normal dataset columns. As a result, many valid column selections will always miss here and silently fall back to the legacy geometry filter (still unmatchable). Wire `cross_filter_column` through GeoJSON query/transform (or read from where row-level columns are actually stored) so selected columns consistently produce dimension filters.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix|
The flagged issue is correct: adding the cross_filter_column control to GeoJSON without updating the query to include the selected column causes a mismatch, leading to fallback filters. To resolve, update Geojson/buildQuery.ts to include cross_filter_column in queried columns, similar to Polygon layer. No other comments found in the PR. superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/buildQuery.ts |
There was a problem hiding this comment.
Code Review Agent Run #7bad6c
Actionable Suggestions - 2
-
superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts - 2
- CWE-1321: Object injection vulnerability via dynamic property access · Line 424-424
- CWE-1321: Object injection vulnerability in GeoJSON properties access · Line 488-488
Review Details
-
Files reviewed - 7 · Commit Range:
2d7bdfb..2d7bdfb- superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/controlPanel.ts
- superset-frontend/plugins/preset-chart-deckgl/src/layers/Polygon/buildQuery.ts
- superset-frontend/plugins/preset-chart-deckgl/src/layers/Polygon/controlPanel.ts
- superset-frontend/plugins/preset-chart-deckgl/src/layers/common.tsx
- superset-frontend/plugins/preset-chart-deckgl/src/utilities/Shared_DeckGL.tsx
- superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.test.ts
- superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
| const col = formData.cross_filter_column; | ||
| const obj = data.object ?? {}; | ||
| const extraProps = (obj.extraProps ?? {}) as Record<string, unknown>; | ||
| const dimensionVal = (obj[col] ?? extraProps[col]) as |
There was a problem hiding this comment.
Dynamic property access on obj and extraProps using user-controlled col variable (lines 424) creates a Generic Object Injection vulnerability (CWE-1321). Similar issues exist at line 488. Validate or sanitize the cross_filter_column value before using it as an object key.
Code suggestion
Check the AI-generated fix before applying
- const dimensionVal = (obj[col] ?? extraProps[col]) as
+ // Validate column name to prevent object injection
+ if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(col)) {
+ console.warn(`Invalid column name: ${col}`);
+ // Fall through to legacy path
+ } else {
+ const dimensionVal = (obj[col] ?? extraProps[col]) as
| string
| number
| boolean
| null
| undefined;
+ }
}
Code Review Run #7bad6c
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
| string, | ||
| unknown | ||
| >; | ||
| const dimensionVal = properties[col] as |
There was a problem hiding this comment.
Dynamic property access on properties object using col variable (line 488) creates a Generic Object Injection vulnerability (CWE-1321). This is a similar issue to line 424. Validate the cross_filter_column value before using it as an object key to prevent prototype pollution attacks.
Code suggestion
Check the AI-generated fix before applying
- const dimensionVal = properties[col] as
+ // Validate column name to prevent object injection
+ if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(col)) {
+ console.warn(`Invalid column name: ${col}`);
+ // Fall through to legacy path
+ } else {
+ const dimensionVal = properties[col] as
| string
| number
| boolean
| null
| undefined;
+ }
}
Code Review Run #7bad6c
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
SUMMARY
deck.gl Polygon and Geojson charts emit cross-filters whose values cannot match anything in receiving charts. Clicking a region produces a chip with a long polygon-coordinate JSON string, and any receiving chart filtered by it shows "No data". The emitted SQL clause is:
…which compares a bare coordinate path to a column that typically stores a full GeoJSON Feature. It can never match.
This PR adds a new optional Cross-filter column control (
cross_filter_column) to deck_polygon and deck_geojson. When set, clicking a feature emits a clean dimension filter:…which other charts on the dashboard match by column name and renders as a readable chip.
Resolution paths for the dimension value:
data.object[col](groupby/columns are spread onto the picked feature byaddPropertiesToFeature), falling back todata.object.extraProps[col]forjs_columnsoverlap. The control is added to the chart's query inbuildQuery.tsso the column actually comes back in records.data.object.properties[col]— the standard non-spatial location on a GeoJSON Feature.When
cross_filter_columnis unset (or its value is missing on the picked feature), behavior falls back to the legacy filter so existing dashboards keep working. Aconsole.warnflags the fallback case so users know to re-save.Bonus fix: deck.gl v9 event-shape regression
While testing locally I found that left-click on any deck.gl chart with cross-filtering enabled has been silently emitting nothing since the deck.gl v9 upgrade. The handler in
commonLayerPropswas checkingevent.leftButton/event.rightButton, which deck.gl v9 no longer provides:Both flags are
undefinedin v9; the actual event shape is{ type, offsetCenter, srcEvent, tapCount }. Dispatch now usesevent.typeandsrcEvent.button(standard MouseEvent semantics —button === 0is left,=== 2is right). This restores left-click filter emission and right-click context menus across all deck.gl chart types, not just the polygon/geojson cases above.This fix is included in scope because the cross-filter feature is unobservably broken without it — even verifying the original bug locally requires the click handler to fire at all.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — text-only diff. Symptom is described in detail above.
TESTING INSTRUCTIONS
Polygon Column = geojson(any dataset whose geometry column stores GeoJSON Features will do).sa3_name).cross_filter_columnunset: clicking emits the legacy broken filter (chip = polygon JSON), which is what older charts expect. Backwards-compatible.cross_filter_column = 'sa3_name'(set under Customize → Advanced → Cross-filter column, then re-run the chart so the column lands in records, then save): clicking emitssa3_name == '<region>'. Chip readssa3_name: <region>. Receiving chart filters correctly.propertiesobject.Automated tests
crossFiltersDataMask.test.tsextended with 6 new cases: polygon w/ top-level dimension, polygon w/ extraProps fallback, polygon w/ legacy fallback when value missing (validates the warn), polygon legacy path (no cross_filter_column), geojson w/ properties, geojson legacy path.preset-chart-deckglpass; tsc and prettier clean.ADDITIONAL INFORMATION
This supersedes #28262 (open since April 2024, marked stale by maintainers, targets the now-removed
legacy-preset-chart-deckgl/path, predates the merged cross-filter feature in #33789).