Skip to content

fix(deckgl): emit usable cross-filter values from polygon and geojson clicks#39906

Open
alex-poor wants to merge 1 commit intoapache:masterfrom
alex-poor:fix/deck-polygon-cross-filter-column
Open

fix(deckgl): emit usable cross-filter values from polygon and geojson clicks#39906
alex-poor wants to merge 1 commit intoapache:masterfrom
alex-poor:fix/deck-polygon-cross-filter-column

Conversation

@alex-poor
Copy link
Copy Markdown
Contributor

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:

REPLACE(<geometry_column>, ' ', '') == '<JSON.stringify(picked path)>'

…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:

sa3_name == 'Christchurch West'

…which other charts on the dashboard match by column name and renders as a readable chip.

Resolution paths for the dimension value:

  • deck_polygon: read from data.object[col] (groupby/columns are spread onto the picked feature by addPropertiesToFeature), falling back to data.object.extraProps[col] for js_columns overlap. The control is added to the chart's query in buildQuery.ts so the column actually comes back in records.
  • deck_geojson: read from data.object.properties[col] — the standard non-spatial location on a GeoJSON Feature.

When cross_filter_column is unset (or its value is missing on the picked feature), behavior falls back to the legacy filter so existing dashboards keep working. A console.warn flags 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 commonLayerProps was checking event.leftButton / event.rightButton, which deck.gl v9 no longer provides:

if (event.leftButton && setDataMask !== undefined && crossFilters) { ... }
else if (event.rightButton && onContextMenu !== undefined) { ... }

Both flags are undefined in v9; the actual event shape is { type, offsetCenter, srcEvent, tapCount }. Dispatch now uses event.type and srcEvent.button (standard MouseEvent semantics — button === 0 is left, === 2 is 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

  1. Build a deck.gl Polygon chart with Polygon Column = geojson (any dataset whose geometry column stores GeoJSON Features will do).
  2. Add it to a dashboard alongside another chart whose dataset shares a non-spatial dimension (e.g. sa3_name).
  3. Enable cross-filtering on the dashboard and on the chart.
  4. Without the fix: clicking a polygon does nothing visible (left-click event not gated correctly) or, if the click handler ran, would emit a polygon-JSON chip and "No data" in the receiving chart.
  5. With the fix, cross_filter_column unset: clicking emits the legacy broken filter (chip = polygon JSON), which is what older charts expect. Backwards-compatible.
  6. With the fix, 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 emits sa3_name == '<region>'. Chip reads sa3_name: <region>. Receiving chart filters correctly.
  7. Right-click on a polygon should now open the standard context menu (Drill / Filter) — same fix as Query component: limit_spec is not valid for query type: groupBy.The list of valid #4.
  8. Same flow works for deck.gl Geojson, where the dimension column must exist inside each Feature's properties object.

Automated tests

  • crossFiltersDataMask.test.ts extended 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.
  • All 256 tests in preset-chart-deckgl pass; tsc and prettier clean.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

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).

… 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>
@dosubot dosubot Bot added dashboard:cross-filters Related to the Dashboard cross filters plugins viz:charts:deck.gl Related to deck.gl charts labels May 6, 2026
{
label: t('Advanced'),
controlSetRows: [
[crossFilterColumn],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
👍 | 👎

Comment on lines +482 to +493
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;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
👍 | 👎

@bito-code-review
Copy link
Copy Markdown
Contributor

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

export interface DeckGeojsonFormData extends SqlaFormData {
  // ... existing fields
  cross_filter_column?: string | null;
}

export default function buildQuery(formData: DeckGeojsonFormData) {
  const {
    // ... existing destructuring
    cross_filter_column,
  } = formData;
  // ... existing code
  if (cross_filter_column && !columns.includes(cross_filter_column)) {
    columns.push(cross_filter_column);
  }
  // ... rest
}

Copy link
Copy Markdown
Contributor

@bito-code-review bito-code-review Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

AI Code Review powered by Bito Logo

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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CWE-1321: Object injection vulnerability via dynamic property access

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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CWE-1321: Object injection vulnerability in GeoJSON properties access

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dashboard:cross-filters Related to the Dashboard cross filters plugins size/L viz:charts:deck.gl Related to deck.gl charts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant