Skip to content

fix(scatter): Fix ad-hoc metric for pointsize#37669

Draft
alexandrusoare wants to merge 1 commit intomasterfrom
alexandrusoare/fix/scatterplot-point-size
Draft

fix(scatter): Fix ad-hoc metric for pointsize#37669
alexandrusoare wants to merge 1 commit intomasterfrom
alexandrusoare/fix/scatterplot-point-size

Conversation

@alexandrusoare
Copy link
Contributor

@alexandrusoare alexandrusoare commented Feb 4, 2026

SUMMARY

When creating a deck.gl Scatterplot chart, it's possible to choose between a fixed point size, or a dynamic one (calculated with a metric). The metric popover allows you to either select an existing metric from the dataset, or create a new one via custom SQL. Charts using a custom SQL metric are showing errors.

Root Cause

This is a regression from a previous fix that addressed fixed point size not working. That fix introduced extractMetricKey() which converts adhoc metric objects to plain strings, losing the expressionType property that the backend needs to identify custom SQL metrics.

What happens:

  1. User creates a custom SQL metric like count(*) * 1.1
  2. The UI correctly stores it as an object with expressionType: "SQL" and sqlExpression
  3. extractMetricKey() strips it down to just the string "count(*) * 1.1"
  4. Backend receives the string, can't identify it as an adhoc metric (missing expressionType), and it's not a saved metric either
  5. Error: "Metric does not exist"

Fix

Modified buildQuery.ts to preserve the full metric object instead of extracting only the label string. Used getMetricLabel() only where a string representation is actually needed (for the orderby clause).

Alternative Considered

Fix extractMetricKey() to optionally return full objects - This would be a more "correct" fix at the utility level, but touches shared code used by other components, increasing risk of unintended side effects. The minimal fix in buildQuery.ts is safer and achieves the same result.

Tests Added

5 new tests to prevent future regressions:

  • Adhoc SQL metrics (count(*) * 1.1)
  • Adhoc SIMPLE metrics (AVG(column))
  • Deduplication of adhoc metrics
  • Fixed type with string values
  • Undefined metric values

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE

Screen.Recording.2026-02-04.at.15.28.12.mov

AFTER

Screen.Recording.2026-02-04.at.15.25.04.mov

TESTING INSTRUCTIONS

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant