Compare commits

..

6 Commits

Author SHA1 Message Date
Evan
91a56e0a25 test(sql): strengthen sanitize_clause comment assertions and add type hints
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-23 21:53:27 -07:00
Evan
0228ff4f33 fix(sql): normalize comment clauses with base dialect to keep #36113 fix
The comment-handling branch of sanitize_clause re-rendered through the
engine dialect, which re-applied the Postgres ROUND/CAST rewrite for any
clause containing a comment, reintroducing #36113. Re-render with the
base dialect instead so comments are normalized without engine-specific
semantic rewrites, and add a parametrized regression test for the
comment path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-23 19:40:08 -07:00
Evan
401d218992 fix(sql): strip trailing statement terminator in sanitize_clause
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-23 19:40:08 -07:00
Evan
1a998e61a8 test(sql): update integration assertions for verbatim sanitize_clause output
sanitize_clause no longer re-renders user SQL through the dialect generator,
so two existing integration assertions expecting the reformatted (uppercased)
output now need to match the verbatim user input.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-23 19:40:08 -07:00
Claude Code
f5a841b320 fix(sql): stop sanitize_clause from rewriting user SQL semantics (#36113)
`sanitize_clause` round-tripped the user's clause through SQLGlot's dialect
generator and returned the re-rendered SQL. SQLGlot's Postgres dialect (borrowed
by engines such as Redshift, CockroachDB, Netezza and SAP HANA) rewrites
`ROUND(AVG(x), n)` into `ROUND(CAST(AVG(x) AS DECIMAL), n)`. On engines whose
unqualified DECIMAL defaults to scale 0, that injected cast rounds the aggregate
to an integer before the explicit ROUND, so `ROUND(AVG(0.949583), 4)` returns
`1` instead of `0.9496`.

This regressed when validation moved from sqlparse to SQLGlot; the legacy
implementation only validated the clause and returned it unchanged.

Validate by parsing as before, but return the original clause verbatim. Clauses
that contain comments are still re-rendered, since a trailing line comment can
comment out surrounding SQL once the clause is embedded into a larger query.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-23 19:40:08 -07:00
Claude Code
ee0a93771f test(sql): reproduce sanitize_clause mutating user aggregation SQL (#36113)
Add a failing regression test for #36113. `sanitize_clause` round-trips a
user-authored clause through SQLGlot's dialect generator and returns the
re-rendered SQL. The Postgres dialect (borrowed by several engines) rewrites
`ROUND(AVG(x), n)` to `ROUND(CAST(AVG(x) AS DECIMAL), n)` at generation time;
on engines whose unqualified DECIMAL defaults to scale 0 the injected cast
rounds the aggregate before the explicit ROUND, producing wrong values.

The test asserts the clause is returned unchanged and currently fails for
postgresql/cockroachdb/netezza/hana.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-23 19:40:08 -07:00
50 changed files with 248 additions and 839 deletions

View File

@@ -31,7 +31,7 @@ jobs:
checks: write
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: true
ref: master
@@ -40,7 +40,7 @@ jobs:
uses: ./.github/actions/setup-supersetbot/
- name: Set up Python ${{ inputs.python-version }}
uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0
uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6
with:
python-version: "3.10"

View File

@@ -22,7 +22,7 @@ jobs:
runs-on: ubuntu-22.04
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive

View File

@@ -25,7 +25,7 @@ jobs:
pull-requests: write
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
- name: Check and notify

View File

@@ -26,7 +26,7 @@ jobs:
frontend: ${{ steps.check.outputs.frontend }}
steps:
- name: Checkout
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
- name: Check for file changes
@@ -58,7 +58,7 @@ jobs:
steps:
- name: Checkout repository
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false

View File

@@ -27,7 +27,7 @@ jobs:
runs-on: ubuntu-24.04
steps:
- name: "Checkout Repository"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
- name: "Dependency Review"
@@ -51,7 +51,7 @@ jobs:
runs-on: ubuntu-22.04
steps:
- name: "Checkout Repository"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false

View File

@@ -30,7 +30,7 @@ jobs:
docker: ${{ steps.check.outputs.docker }}
steps:
- name: Checkout
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
- name: Check for file changes
@@ -71,7 +71,7 @@ jobs:
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
@@ -177,7 +177,7 @@ jobs:
timeout-minutes: 30
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
- name: Free up disk space

View File

@@ -23,7 +23,7 @@ jobs:
run:
working-directory: superset-embedded-sdk
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
# Note: registry-url is intentionally omitted. When set, actions/setup-node

View File

@@ -21,7 +21,7 @@ jobs:
run:
working-directory: superset-embedded-sdk
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
- uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6

View File

@@ -32,7 +32,7 @@ jobs:
runs-on: ubuntu-24.04
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive

View File

@@ -27,7 +27,7 @@ jobs:
security-events: write
steps:
- name: Checkout Repository
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false

View File

@@ -16,7 +16,7 @@ jobs:
issues: write
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false

View File

@@ -12,7 +12,7 @@ jobs:
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive

View File

@@ -18,7 +18,7 @@ jobs:
runs-on: ubuntu-24.04
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive

View File

@@ -21,7 +21,7 @@ jobs:
pull-requests: write
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive

View File

@@ -28,7 +28,7 @@ jobs:
python-version: ${{ github.event_name == 'pull_request' && fromJSON('["current"]') || fromJSON('["current", "previous", "next"]') }}
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive

View File

@@ -33,7 +33,7 @@ jobs:
permissions:
contents: write
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
# pulls all commits (needed for lerna / semantic release to correctly version)

View File

@@ -152,7 +152,7 @@ jobs:
- name: Checkout PR code (only if build needed)
if: steps.auth.outputs.authorized == 'true' && steps.check.outputs.build_needed == 'true'
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
ref: ${{ steps.check.outputs.target_sha }}
persist-credentials: false

View File

@@ -41,7 +41,7 @@ jobs:
- 16379:6379
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive

View File

@@ -60,7 +60,7 @@ jobs:
runs-on: ubuntu-24.04
steps:
- name: "Checkout ${{ github.event.workflow_run.head_sha || github.sha }}"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
ref: ${{ github.event.workflow_run.head_sha || github.sha }}
persist-credentials: false

View File

@@ -28,7 +28,7 @@ jobs:
name: Link Checking
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
# Do not bump this linkinator-action version without opening
@@ -73,7 +73,7 @@ jobs:
working-directory: docs
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive
@@ -112,7 +112,7 @@ jobs:
working-directory: docs
steps:
- name: "Checkout PR head: ${{ github.event.workflow_run.head_sha }}"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
ref: ${{ github.event.workflow_run.head_sha }}
persist-credentials: false

View File

@@ -38,7 +38,7 @@ jobs:
frontend: ${{ steps.check.outputs.frontend }}
steps:
- name: Checkout
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
- name: Check for file changes
@@ -97,21 +97,21 @@ jobs:
# Conditional checkout based on context
- name: Checkout for push or pull_request event
if: github.event_name == 'push' || github.event_name == 'pull_request'
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}
- name: Checkout using ref (workflow_dispatch)
if: github.event_name == 'workflow_dispatch' && github.event.inputs.ref != ''
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
ref: ${{ github.event.inputs.ref }}
submodules: recursive
- name: Checkout using PR ID (workflow_dispatch)
if: github.event_name == 'workflow_dispatch' && github.event.inputs.pr_id != ''
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
ref: refs/pull/${{ github.event.inputs.pr_id }}/merge
@@ -207,21 +207,21 @@ jobs:
# Conditional checkout based on context (same as Cypress workflow)
- name: Checkout for push or pull_request event
if: github.event_name == 'push' || github.event_name == 'pull_request'
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}
- name: Checkout using ref (workflow_dispatch)
if: github.event_name == 'workflow_dispatch' && github.event.inputs.ref != ''
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
ref: ${{ github.event.inputs.ref }}
submodules: recursive
- name: Checkout using PR ID (workflow_dispatch)
if: github.event_name == 'workflow_dispatch' && github.event.inputs.pr_id != ''
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
ref: refs/pull/${{ github.event.inputs.pr_id }}/merge

View File

@@ -31,7 +31,7 @@ jobs:
working-directory: superset-extensions-cli
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive

View File

@@ -27,7 +27,7 @@ jobs:
should-run: ${{ steps.check.outputs.frontend }}
steps:
- name: Checkout Code
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
fetch-depth: 0
@@ -110,7 +110,7 @@ jobs:
id-token: write
steps:
- name: Checkout Code
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
fetch-depth: 0

View File

@@ -19,7 +19,7 @@ jobs:
runs-on: ubuntu-24.04
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive

View File

@@ -29,7 +29,7 @@ jobs:
steps:
- name: Checkout code
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
ref: ${{ inputs.ref || github.ref_name }}
persist-credentials: true

View File

@@ -34,7 +34,7 @@ jobs:
frontend: ${{ steps.check.outputs.frontend }}
steps:
- name: Checkout
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
- name: Check for file changes
@@ -83,21 +83,21 @@ jobs:
# Conditional checkout based on context (same as Cypress workflow)
- name: Checkout for push or pull_request event
if: github.event_name == 'push' || github.event_name == 'pull_request'
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}
- name: Checkout using ref (workflow_dispatch)
if: github.event_name == 'workflow_dispatch' && github.event.inputs.ref != ''
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
ref: ${{ github.event.inputs.ref }}
submodules: recursive
- name: Checkout using PR ID (workflow_dispatch)
if: github.event_name == 'workflow_dispatch' && github.event.inputs.pr_id != ''
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
ref: refs/pull/${{ github.event.inputs.pr_id }}/merge

View File

@@ -29,7 +29,7 @@ jobs:
python: ${{ steps.check.outputs.python }}
steps:
- name: Checkout
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
- name: Check for file changes
@@ -72,7 +72,7 @@ jobs:
- 16379:6379
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive
@@ -157,7 +157,7 @@ jobs:
- 16379:6379
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive
@@ -207,7 +207,7 @@ jobs:
- 16379:6379
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive

View File

@@ -25,7 +25,7 @@ jobs:
python: ${{ steps.check.outputs.python }}
steps:
- name: Checkout
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
- name: Check for file changes
@@ -72,7 +72,7 @@ jobs:
- 16379:6379
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive
@@ -127,7 +127,7 @@ jobs:
- 16379:6379
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive

View File

@@ -30,7 +30,7 @@ jobs:
python: ${{ steps.check.outputs.python }}
steps:
- name: Checkout
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
- name: Check for file changes
@@ -55,7 +55,7 @@ jobs:
PYTHONPATH: ${{ github.workspace }}
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive

View File

@@ -25,7 +25,7 @@ jobs:
pull-requests: read
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive
@@ -61,7 +61,7 @@ jobs:
pull-requests: read
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
submodules: recursive

View File

@@ -25,7 +25,7 @@ jobs:
timeout-minutes: 20
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
- name: Install dependencies

View File

@@ -38,7 +38,7 @@ jobs:
});
- name: "Checkout ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false

View File

@@ -27,7 +27,7 @@ jobs:
# zizmor: ignore[artipacked] - required persisted credentials to push synced requirement changes back to remote
- name: Checkout source code
if: ${{ steps.dependabot-metadata.outputs.package-ecosystem == 'pip' }}
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
ref: ${{ github.event.pull_request.head.sha }}
persist-credentials: true

View File

@@ -54,7 +54,7 @@ jobs:
fail-fast: false
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
fetch-depth: 0
@@ -120,7 +120,7 @@ jobs:
pull-requests: write
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false
fetch-depth: 0

View File

@@ -32,7 +32,7 @@ jobs:
name: Generate Reports
steps:
- name: Checkout Repository
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
persist-credentials: false

View File

@@ -36,7 +36,7 @@
"@great-expectations/jsonforms-antd-renderers": "^2.2.10",
"@jsonforms/core": "^3.7.0",
"@jsonforms/react": "^3.7.0",
"@jsonforms/vanilla-renderers": "^3.8.0",
"@jsonforms/vanilla-renderers": "^3.7.0",
"@luma.gl/constants": "~9.2.5",
"@luma.gl/core": "~9.2.5",
"@luma.gl/engine": "~9.2.5",
@@ -187,7 +187,7 @@
"@storybook/test-runner": "0.24.4",
"@svgr/webpack": "^8.1.0",
"@swc/core": "^1.15.41",
"@swc/plugin-emotion": "^14.13.0",
"@swc/plugin-emotion": "^14.12.0",
"@swc/plugin-transform-imports": "^12.5.0",
"@testing-library/dom": "^9.3.4",
"@testing-library/jest-dom": "^6.9.1",
@@ -272,7 +272,7 @@
"source-map": "^0.7.6",
"source-map-support": "^0.5.21",
"speed-measure-webpack-plugin": "^1.6.0",
"storybook": "10.4.6",
"storybook": "10.4.5",
"style-loader": "^4.0.0",
"swc-loader": "^0.2.7",
"terser-webpack-plugin": "^5.6.1",
@@ -5387,41 +5387,41 @@
}
},
"node_modules/@jsonforms/core": {
"version": "3.8.0",
"resolved": "https://registry.npmjs.org/@jsonforms/core/-/core-3.8.0.tgz",
"integrity": "sha512-XSvaZuQSs/MceG5nDDcrE879onPHkGBy0xEuLeZMUkSM/M8wc1dEUrJtMOZVNSITocm9YXFY1qQ5gnsPP38zAg==",
"version": "3.7.0",
"resolved": "https://registry.npmjs.org/@jsonforms/core/-/core-3.7.0.tgz",
"integrity": "sha512-CE9viWtwi9QWLqlWLeOul1/R1GRAyOA9y6OoUpsCc0FhyR+g5p29F3k0fUExHWxL0Sf4KHcXYkfhtqfRBPS8ww==",
"license": "MIT",
"dependencies": {
"@types/json-schema": "^7.0.3",
"ajv": "^8.18.0",
"ajv": "^8.6.1",
"ajv-formats": "^2.1.0",
"lodash": "^4.17.21"
}
},
"node_modules/@jsonforms/react": {
"version": "3.8.0",
"resolved": "https://registry.npmjs.org/@jsonforms/react/-/react-3.8.0.tgz",
"integrity": "sha512-k81+yWLpCQl+3XizS1bLjXoBwYhW1OAkjSXFA8W5qNtfPZjSOXDgtiuMOGYDv4b60tu2e9RB8h2P2O7QhfkhiA==",
"version": "3.7.0",
"resolved": "https://registry.npmjs.org/@jsonforms/react/-/react-3.7.0.tgz",
"integrity": "sha512-HkY7qAx8vW97wPEgZ7GxCB3iiXG1c95GuObxtcDHGPBJWMwnxWBnVYJmv5h7nthrInKsQKHZL5OusnC/sj/1GQ==",
"license": "MIT",
"dependencies": {
"lodash": "^4.17.21"
},
"peerDependencies": {
"@jsonforms/core": "3.8.0",
"@jsonforms/core": "3.7.0",
"react": "^16.12.0 || ^17.0.0 || ^18.0.0 || ^19.0.0"
}
},
"node_modules/@jsonforms/vanilla-renderers": {
"version": "3.8.0",
"resolved": "https://registry.npmjs.org/@jsonforms/vanilla-renderers/-/vanilla-renderers-3.8.0.tgz",
"integrity": "sha512-s75TG4hSYgYLN9IRVhYtGjijqyhVXijgDhb2WnMqY+Ki7MQkLn9U7yg/l89NEpwzWS1sv0DxKUxriqVUq382og==",
"version": "3.7.0",
"resolved": "https://registry.npmjs.org/@jsonforms/vanilla-renderers/-/vanilla-renderers-3.7.0.tgz",
"integrity": "sha512-RdXQGsheARUJVbaTe6SqGw9W4/yrm0BgUok6OKUj8krp1NF4fqXc5UbYGHFksMR/p7LCuoYHCtQzKLXEfxJbDw==",
"license": "MIT",
"dependencies": {
"lodash": "^4.17.21"
},
"peerDependencies": {
"@jsonforms/core": "3.8.0",
"@jsonforms/react": "3.8.0",
"@jsonforms/core": "3.7.0",
"@jsonforms/react": "3.7.0",
"react": "^16.12.0 || ^17.0.0 || ^18.0.0 || ^19.0.0"
}
},
@@ -10819,9 +10819,9 @@
}
},
"node_modules/@swc/plugin-emotion": {
"version": "14.13.0",
"resolved": "https://registry.npmjs.org/@swc/plugin-emotion/-/plugin-emotion-14.13.0.tgz",
"integrity": "sha512-UT1l9tr934HjnktUiMGbw1rWrIXUhAByTB0DwZJwHmS8KWox+wNBIK4ZkJ2tKVU/PnQZRni+R9e6xklFkmgSYg==",
"version": "14.12.0",
"resolved": "https://registry.npmjs.org/@swc/plugin-emotion/-/plugin-emotion-14.12.0.tgz",
"integrity": "sha512-lyAQgTeDkowq/4+8JYaviVOL4jXSdObz+uuk84DjM0z4qoiMpI6xoDVp7/tjWeVjmLc2U6Qp3hDuwWMZ5xe88Q==",
"dev": true,
"license": "Apache-2.0",
"dependencies": {
@@ -39527,9 +39527,9 @@
}
},
"node_modules/storybook": {
"version": "10.4.6",
"resolved": "https://registry.npmjs.org/storybook/-/storybook-10.4.6.tgz",
"integrity": "sha512-6wkA6LxfDSSilloITsrFOJfsnw0mDUP2h8Ls+lRt8oRsudtz2RWFhLv+Toiwg6NW7hUpdTDc2hzR7DztJid6+A==",
"version": "10.4.5",
"resolved": "https://registry.npmjs.org/storybook/-/storybook-10.4.5.tgz",
"integrity": "sha512-QZuv1gS9Tf9RMCjDw5JOfv1XSB5IhU0uhSKQNS7l/N9zDpmSydirCspkCNT9e0zkFfPkZ9vmQUTzHY/BA07saA==",
"dev": true,
"license": "MIT",
"dependencies": {
@@ -39540,7 +39540,7 @@
"@vitest/expect": "3.2.4",
"@vitest/spy": "3.2.4",
"@webcontainer/env": "^1.1.1",
"esbuild": "^0.18.0 || ^0.19.0 || ^0.20.0 || ^0.21.0 || ^0.22.0 || ^0.23.0 || ^0.24.0 || ^0.25.0 || ^0.26.0 || ^0.27.0 || ^0.28.0",
"esbuild": "^0.18.0 || ^0.19.0 || ^0.20.0 || ^0.21.0 || ^0.22.0 || ^0.23.0 || ^0.24.0 || ^0.25.0 || ^0.26.0 || ^0.27.0",
"open": "^10.2.0",
"oxc-parser": "^0.127.0",
"oxc-resolver": "^11.19.1",

View File

@@ -119,7 +119,7 @@
"@great-expectations/jsonforms-antd-renderers": "^2.2.10",
"@jsonforms/core": "^3.7.0",
"@jsonforms/react": "^3.7.0",
"@jsonforms/vanilla-renderers": "^3.8.0",
"@jsonforms/vanilla-renderers": "^3.7.0",
"@luma.gl/constants": "~9.2.5",
"@luma.gl/core": "~9.2.5",
"@luma.gl/engine": "~9.2.5",
@@ -270,7 +270,7 @@
"@storybook/test-runner": "0.24.4",
"@svgr/webpack": "^8.1.0",
"@swc/core": "^1.15.41",
"@swc/plugin-emotion": "^14.13.0",
"@swc/plugin-emotion": "^14.12.0",
"@swc/plugin-transform-imports": "^12.5.0",
"@testing-library/dom": "^9.3.4",
"@testing-library/jest-dom": "^6.9.1",
@@ -355,7 +355,7 @@
"source-map": "^0.7.6",
"source-map-support": "^0.5.21",
"speed-measure-webpack-plugin": "^1.6.0",
"storybook": "10.4.6",
"storybook": "10.4.5",
"style-loader": "^4.0.0",
"swc-loader": "^0.2.7",
"terser-webpack-plugin": "^5.6.1",

View File

@@ -21,12 +21,10 @@
import d3 from 'd3';
import { extent as d3Extent } from 'd3-array';
import {
BinaryQueryObjectFilterClause,
CategoricalColorNamespace,
ContextMenuFilters,
DataMask,
ValueFormatter,
getNumberFormatter,
getSequentialSchemeRegistry,
CategoricalColorNamespace,
} from '@superset-ui/core';
import countries, { countryOptions } from './countries';
@@ -67,28 +65,9 @@ interface CountryMapProps {
formatter: ValueFormatter;
colorScheme: string;
sliceId: number;
onContextMenu?: (
clientX: number,
clientY: number,
data: ContextMenuFilters,
) => void;
emitCrossFilters?: boolean;
setDataMask?: (dataMask: DataMask) => void;
filterState?: {
selectedValues?: string[];
extraFormData?: {
filters?: BinaryQueryObjectFilterClause[];
};
};
entity?: string;
}
const maps: Record<string, GeoData> = {};
// Store zoom state per chart instance using element as key to enable garbage collection
const zoomStates = new WeakMap<
HTMLElement,
{ scale: number; translate: [number, number] }
>();
function CountryMap(element: HTMLElement, props: CountryMapProps) {
const {
@@ -96,15 +75,10 @@ function CountryMap(element: HTMLElement, props: CountryMapProps) {
width,
height,
country,
entity,
linearColorScheme,
formatter,
colorScheme,
sliceId,
filterState,
emitCrossFilters,
onContextMenu,
setDataMask,
} = props;
const container = element;
@@ -125,15 +99,7 @@ function CountryMap(element: HTMLElement, props: CountryMapProps) {
? colorScale(d.country_id, sliceId)
: (linearColorScale(d.metric) ?? '');
});
const colorFn = (feature: GeoFeature): string => {
if (!feature?.properties) return '#d9d9d9';
const iso = feature.properties.ISO;
return colorMap[iso] || '#d9d9d9';
};
// Check if dashboard is in edit mode
const isEditMode = container.closest('.dashboard--editing') !== null;
const colorFn = (d: GeoFeature) => colorMap[d.properties.ISO] || 'none';
const path = d3.geo.path();
const div = d3.select(container);
@@ -146,11 +112,6 @@ function CountryMap(element: HTMLElement, props: CountryMapProps) {
.attr('width', width)
.attr('height', height)
.attr('preserveAspectRatio', 'xMidYMid meet');
// Only set grab cursor if not in edit mode
if (!isEditMode) {
svg.style('cursor', 'grab');
}
const backgroundRect = svg
.append('rect')
.attr('class', 'background')
@@ -158,65 +119,40 @@ function CountryMap(element: HTMLElement, props: CountryMapProps) {
.attr('height', height);
const g = svg.append('g');
const mapLayer = g.append('g').classed('map-layer', true);
// Add hover popup for tooltip
const hoverPopup = div.append('div').attr('class', 'hover-popup');
// Track mouse position to distinguish clicks from drags
let mousedownPos: { x: number; y: number } | null = null;
let centered: GeoFeature | null;
// Cross-filter support
const getCrossFilterDataMask = (
source: GeoFeature,
): { dataMask: DataMask; isCurrentValueSelected: boolean } | undefined => {
if (!entity) return undefined;
const clicked = function clicked(d: GeoFeature) {
const hasCenter = d && centered !== d;
let x: number;
let y: number;
let k: number;
const halfWidth = width / 2;
const halfHeight = height / 2;
const selected = filterState?.selectedValues || [];
const iso = source?.properties?.ISO;
if (!iso) return undefined;
const isSelected = selected.includes(iso);
const values = isSelected ? [] : [iso];
return {
dataMask: {
extraFormData: {
filters: values.length
? [{ col: entity, op: 'IN', val: values }]
: [],
},
filterState: {
value: values.length ? values : null,
selectedValues: values.length ? values : null,
},
},
isCurrentValueSelected: isSelected,
};
};
// Handle right-click context menu
const handleContextMenu = (feature: GeoFeature): void => {
const pointerEvent = d3.event;
if (typeof onContextMenu === 'function') {
pointerEvent?.preventDefault();
if (hasCenter) {
const centroid = path.centroid(d);
[x, y] = centroid;
k = 4;
centered = d;
} else {
x = halfWidth;
y = halfHeight;
k = 1;
centered = null;
}
const iso = feature?.properties?.ISO;
if (!iso || typeof onContextMenu !== 'function' || !entity) return;
const drillVal = iso;
const drillToDetailFilters = [
{ col: entity, op: '==', val: drillVal, formattedVal: drillVal },
];
const drillByFilters = [{ col: entity, op: '==', val: drillVal }];
onContextMenu(pointerEvent.clientX, pointerEvent.clientY, {
drillToDetail: drillToDetailFilters,
crossFilter: getCrossFilterDataMask(feature),
drillBy: { filters: drillByFilters, groupbyFieldName: 'entity' },
});
g.transition()
.duration(750)
.attr(
'transform',
`translate(${halfWidth},${halfHeight})scale(${k})translate(${-x},${-y})`,
);
};
backgroundRect.on('click', clicked);
const getNameOfRegion = function getNameOfRegion(
feature: GeoFeature,
): string {
@@ -229,7 +165,7 @@ function CountryMap(element: HTMLElement, props: CountryMapProps) {
return '';
};
const updatePopupPosition = (): void => {
const updatePopupPosition = () => {
const svgHeight = svg.node().getBoundingClientRect().height;
const [x, y] = d3.mouse(svg.node());
hoverPopup
@@ -239,135 +175,36 @@ function CountryMap(element: HTMLElement, props: CountryMapProps) {
.classed('popup-at-bottom', y > (svgHeight * 2) / 3);
};
const mouseenter = function mouseenter(
this: SVGPathElement,
d: GeoFeature,
): void {
const mouseenter = function mouseenter(this: SVGPathElement, d: GeoFeature) {
// Darken color
let c: string = colorFn(d);
if (c) {
if (c !== 'none') {
c = d3.rgb(c).darker().toString();
}
d3.select(this).style('fill', c);
// Display information popup
const result = data.filter(r => r.country_id === d?.properties?.ISO);
const regionName = escapeHtml(getNameOfRegion(d));
const metricValue =
result.length > 0 ? escapeHtml(String(formatter(result[0].metric))) : '';
const result = data.filter(
region => region.country_id === d.properties.ISO,
);
hoverPopup
.style('display', 'block')
.html(`<div><strong>${regionName}</strong><br>${metricValue}</div>`);
.html(
`<div><strong>${getNameOfRegion(d)}</strong><br>${result.length > 0 ? formatter(result[0].metric) : ''}</div>`,
);
updatePopupPosition();
};
// Mouse move handler to update tooltip position
const mousemove = function mousemove(): void {
const mousemove = function mousemove() {
updatePopupPosition();
};
const mouseout = function mouseout(this: SVGPathElement): void {
d3.select(this).style('fill', (d: GeoFeature) => colorFn(d));
const mouseout = function mouseout(this: SVGPathElement) {
d3.select(this).style('fill', colorFn);
hoverPopup.style('display', 'none');
};
// Only enable zoom if not in edit mode
if (!isEditMode) {
// Zoom with panning bounds
const zoom = d3.behavior
.zoom()
.scaleExtent([1, 4])
.on('zoomstart', () => {
svg.style('cursor', 'grabbing');
})
.on('zoom', () => {
const { translate, scale } = d3.event;
let [tx, ty] = translate;
const scaledW = width * scale;
const scaledH = height * scale;
const minX = Math.min(0, width - scaledW);
const maxX = 0;
const minY = Math.min(0, height - scaledH);
const maxY = 0;
tx = Math.max(Math.min(tx, maxX), minX);
ty = Math.max(Math.min(ty, maxY), minY);
// Sync D3's internal translate state with the clamped values so the
// next wheel/zoom event starts from the constrained position rather
// than the unclamped one (otherwise the view jumps).
zoom.translate([tx, ty]);
g.attr('transform', `translate(${tx}, ${ty}) scale(${scale})`);
const prev = zoomStates.get(element);
const changed =
!prev ||
prev.scale !== scale ||
prev.translate[0] !== tx ||
prev.translate[1] !== ty;
if (changed) {
zoomStates.set(element, { scale, translate: [tx, ty] });
}
})
.on('zoomend', () => {
svg.style('cursor', 'grab');
});
d3.select(svg.node()).call(zoom);
// Restore previous zoom state if it exists
const savedZoom = zoomStates.get(element);
if (savedZoom) {
const { scale, translate } = savedZoom;
zoom.scale(scale).translate(translate);
g.attr(
'transform',
`translate(${translate[0]}, ${translate[1]}) scale(${scale})`,
);
}
}
// Visual highlighting for selected regions
function highlightSelectedRegion(
selectedValues: string[] | null = null,
): void {
const selected = selectedValues || filterState?.selectedValues || [];
mapLayer
.selectAll('path.region')
.style('fill-opacity', (d: GeoFeature) => {
const iso = d?.properties?.ISO;
return selected.length === 0 || selected.includes(iso) ? 1 : 0.3;
})
.style('stroke', (d: GeoFeature) => {
const iso = d?.properties?.ISO;
return selected.includes(iso) ? '#222' : null;
})
.style('stroke-width', (d: GeoFeature) => {
const iso = d?.properties?.ISO;
return selected.includes(iso) ? '1.5px' : '0.5px';
});
}
// Click handler for cross-filters
const handleClick = (feature: GeoFeature): void => {
if (!entity || !emitCrossFilters || typeof setDataMask !== 'function') {
return;
}
const result = getCrossFilterDataMask(feature);
if (!result) return;
const { dataMask, isCurrentValueSelected } = result;
setDataMask(dataMask);
const iso = feature?.properties?.ISO;
const newSelection = isCurrentValueSelected || !iso ? [] : [iso];
highlightSelectedRegion(newSelection);
};
function drawMap(mapData: GeoData): void {
function drawMap(mapData: GeoData) {
const { features } = mapData;
const center = d3.geo.centroid(mapData);
const scale = 100;
@@ -378,11 +215,13 @@ function CountryMap(element: HTMLElement, props: CountryMapProps) {
.translate([width / 2, height / 2]);
path.projection(projection);
// Compute scale that fits container.
const bounds = path.bounds(mapData);
const hscale = (scale * width) / (bounds[1][0] - bounds[0][0]);
const vscale = (scale * height) / (bounds[1][1] - bounds[0][1]);
const newScale = Math.min(hscale, vscale);
const newScale = hscale < vscale ? hscale : vscale;
// Compute bounds and offset using the updated scale.
projection.scale(newScale);
const newBounds = path.bounds(mapData);
projection.translate([
@@ -390,45 +229,20 @@ function CountryMap(element: HTMLElement, props: CountryMapProps) {
height - (newBounds[0][1] + newBounds[1][1]) / 2,
]);
const sel = mapLayer.selectAll('path.region').data(features);
sel
// Draw each province as a path
mapLayer
.selectAll('path')
.data(features)
.enter()
.append('path')
.attr('class', 'region')
.attr('vector-effect', 'non-scaling-stroke');
// Apply attributes and event handlers to all elements (enter + update)
mapLayer
.selectAll('path.region')
.attr('d', path)
.attr('class', 'region')
.attr('vector-effect', 'non-scaling-stroke')
.style('fill', colorFn)
.on('mouseenter', mouseenter)
.on('mousemove', mousemove)
.on('mouseout', mouseout)
.on('contextmenu', handleContextMenu)
.on('mousedown', function mousedown() {
const pos = d3.mouse(svg.node());
mousedownPos = { x: pos[0], y: pos[1] };
})
.on('click', function click(feature: GeoFeature) {
if (mousedownPos) {
const pos = d3.mouse(svg.node());
const dx = Math.abs(pos[0] - mousedownPos.x);
const dy = Math.abs(pos[1] - mousedownPos.y);
const dragThreshold = 5;
if (dx < dragThreshold && dy < dragThreshold) {
handleClick(feature);
}
mousedownPos = null;
}
});
sel.exit().remove();
highlightSelectedRegion();
.on('click', clicked);
}
const map = maps[country];

View File

@@ -17,7 +17,7 @@
* under the License.
*/
import { t } from '@apache-superset/core/translation';
import { ChartMetadata, ChartPlugin, Behavior } from '@superset-ui/core';
import { ChartMetadata, ChartPlugin } from '@superset-ui/core';
import transformProps from './transformProps';
import exampleUsa from './images/exampleUsa.jpg';
import exampleUsaDark from './images/exampleUsa-dark.jpg';
@@ -49,11 +49,6 @@ const metadata = new ChartMetadata({
thumbnail,
thumbnailDark,
useLegacyApi: true,
behaviors: [
Behavior.InteractiveChart,
Behavior.DrillToDetail,
Behavior.DrillBy,
],
});
export default class CountryMapChartPlugin extends ChartPlugin {

View File

@@ -19,18 +19,8 @@
import { ChartProps, getValueFormatter } from '@superset-ui/core';
export default function transformProps(chartProps: ChartProps) {
const { width, height, formData, queriesData, datasource } = chartProps;
const {
width,
height,
formData,
queriesData,
datasource,
hooks = {},
filterState,
emitCrossFilters,
} = chartProps;
const {
entity,
linearColorScheme,
numberFormat,
currencyFormat,
@@ -59,8 +49,6 @@ export default function transformProps(chartProps: ChartProps) {
detectedCurrency,
);
const { onContextMenu, setDataMask } = hooks;
return {
width,
height,
@@ -71,10 +59,5 @@ export default function transformProps(chartProps: ChartProps) {
colorScheme,
sliceId,
formatter,
entity,
onContextMenu,
setDataMask,
emitCrossFilters,
filterState,
};
}

View File

@@ -133,11 +133,10 @@ describe('CountryMap (legacy d3)', () => {
expect(popup!).toHaveStyle({ display: 'none' });
});
test('emits a cross-filter data mask when a region is clicked', () => {
test('shows tooltip on mouseenter/mousemove/mouseout', async () => {
d3Any.json.mockImplementation((_url: string, cb: D3JsonCallback) =>
cb(null, mockMapData),
);
const setDataMask = jest.fn();
render(
<ReactCountryMap
@@ -148,101 +147,19 @@ describe('CountryMap (legacy d3)', () => {
linearColorScheme="bnbColors"
colorScheme=""
formatter={jest.fn().mockReturnValue('100')}
entity="country_code"
emitCrossFilters
setDataMask={setDataMask}
filterState={{ selectedValues: [] }}
/>,
);
const region = document.querySelector('path.region');
expect(region).not.toBeNull();
// A click is only treated as a selection when it follows a mousedown
// without dragging beyond the threshold (d3.mouse is mocked to a fixed
// position, so the down/up positions match).
fireEvent.mouseDown(region!);
fireEvent.click(region!);
const popup = document.querySelector('.hover-popup');
expect(popup).not.toBeNull();
expect(setDataMask).toHaveBeenCalledTimes(1);
expect(setDataMask).toHaveBeenCalledWith(
expect.objectContaining({
extraFormData: {
filters: [{ col: 'country_code', op: 'IN', val: ['CAN'] }],
},
filterState: expect.objectContaining({ value: ['CAN'] }),
}),
);
});
fireEvent.mouseEnter(region!);
expect(popup!).toHaveStyle({ display: 'block' });
test('does not emit a cross-filter when emitCrossFilters is disabled', () => {
d3Any.json.mockImplementation((_url: string, cb: D3JsonCallback) =>
cb(null, mockMapData),
);
const setDataMask = jest.fn();
render(
<ReactCountryMap
width={500}
height={300}
data={[{ country_id: 'CAN', metric: 100 }]}
country="canada"
linearColorScheme="bnbColors"
colorScheme=""
formatter={jest.fn().mockReturnValue('100')}
entity="country_code"
emitCrossFilters={false}
setDataMask={setDataMask}
filterState={{ selectedValues: [] }}
/>,
);
const region = document.querySelector('path.region');
fireEvent.mouseDown(region!);
fireEvent.click(region!);
expect(setDataMask).not.toHaveBeenCalled();
});
test('opens the context menu with drill-by keyed on the entity control', () => {
d3Any.json.mockImplementation((_url: string, cb: D3JsonCallback) =>
cb(null, mockMapData),
);
const onContextMenu = jest.fn();
render(
<ReactCountryMap
width={500}
height={300}
data={[{ country_id: 'CAN', metric: 100 }]}
country="canada"
linearColorScheme="bnbColors"
colorScheme=""
formatter={jest.fn().mockReturnValue('100')}
entity="country_code"
onContextMenu={onContextMenu}
filterState={{ selectedValues: [] }}
/>,
);
const region = document.querySelector('path.region');
expect(region).not.toBeNull();
fireEvent.contextMenu(region!, { clientX: 123, clientY: 45 });
expect(onContextMenu).toHaveBeenCalledTimes(1);
const [[clientX, clientY, payload]] = onContextMenu.mock.calls;
expect(clientX).toBe(123);
expect(clientY).toBe(45);
expect(payload.drillToDetail).toEqual([
{ col: 'country_code', op: '==', val: 'CAN', formattedVal: 'CAN' },
]);
// groupbyFieldName must be the form-data control key ('entity'), not the
// selected column value ('country_code'), so DrillByModal can map the
// selection back to the chart control.
expect(payload.drillBy).toEqual({
filters: [{ col: 'country_code', op: '==', val: 'CAN' }],
groupbyFieldName: 'entity',
});
fireEvent.mouseOut(region!);
expect(popup!).toHaveStyle({ display: 'none' });
});
});

View File

@@ -1,76 +0,0 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { ChartProps } from '@superset-ui/core';
import transformProps from '../src/transformProps';
const onContextMenu = jest.fn();
const setDataMask = jest.fn();
const createProps = (formDataOverrides = {}, chartPropsOverrides = {}) =>
({
width: 800,
height: 600,
formData: {
entity: 'country_code',
linearColorScheme: 'bnbColors',
numberFormat: '.2f',
selectCountry: 'France',
colorScheme: '',
sliceId: 1,
metric: 'count',
...formDataOverrides,
},
queriesData: [{ data: [{ country_id: 'FRA', metric: 10 }] }],
datasource: { currencyFormats: {}, columnFormats: {} },
hooks: { onContextMenu, setDataMask },
filterState: { selectedValues: ['FRA'] },
emitCrossFilters: true,
...chartPropsOverrides,
}) as unknown as ChartProps;
test('forwards cross-filter hooks and state to the chart', () => {
const transformed = transformProps(createProps());
expect(transformed).toMatchObject({
width: 800,
height: 600,
entity: 'country_code',
onContextMenu,
setDataMask,
emitCrossFilters: true,
filterState: { selectedValues: ['FRA'] },
data: [{ country_id: 'FRA', metric: 10 }],
});
});
test('lowercases the selected country for map lookup', () => {
const transformed = transformProps(createProps());
expect(transformed.country).toBe('france');
});
test('passes a null country when none is selected', () => {
const transformed = transformProps(createProps({ selectCountry: undefined }));
expect(transformed.country).toBeNull();
});
test('defaults hooks to an empty object when none are provided', () => {
const transformed = transformProps(createProps({}, { hooks: undefined }));
expect(transformed.onContextMenu).toBeUndefined();
expect(transformed.setDataMask).toBeUndefined();
});

View File

@@ -31,12 +31,7 @@ from superset.exceptions import SupersetSecurityException
from superset.extensions import cache_manager
from superset.superset_typing import FlaskResponse
from superset.utils import json
from superset.utils.core import (
apply_max_row_limit,
DatasourceType,
parse_boolean_string,
SqlExpressionType,
)
from superset.utils.core import apply_max_row_limit, DatasourceType, SqlExpressionType
from superset.views.base_api import BaseSupersetApi, statsd_metrics
logger = logging.getLogger(__name__)
@@ -130,63 +125,13 @@ class DatasourceRestApi(BaseSupersetApi):
row_limit = apply_max_row_limit(app.config["FILTER_SELECT_ROW_LIMIT"])
denormalize_column = not datasource.normalize_columns
# Cache distinct column-value results so a dashboard with many filters
# backed by the same (often heavy) virtual dataset doesn't re-execute
# the wrapping query per filter (#39342).
#
# Key fields:
# - ``rls`` — full RLS fingerprint via
# ``security_manager.get_rls_cache_key`` (the canonical helper used
# by viz.py and query_context_processor.py). This is the sole
# security-isolation field — two users with identical effective
# RLS share a cache entry (intentional: they would see identical
# filtered values anyway), while users with different RLS, guest
# sessions with different guest-token RLS, and anonymous sessions
# with no RLS each get their own partition. We deliberately do
# NOT include the raw user id; doing so would defeat the
# intended cross-user cache sharing without adding any real
# security boundary beyond what the RLS fingerprint already
# provides.
# - ``changed_on`` — auto-busts cached entries when the dataset's
# underlying SQL is edited.
# - ``uid`` / ``col`` / ``limit`` / ``denorm`` — basic query-shape
# isolation so different inputs never collide.
force = parse_boolean_string(request.args.get("force"))
cache_key = (
"col_values:"
+ hashlib.sha256(
json.dumps(
{
"uid": datasource.uid,
"col": column_name,
"limit": row_limit,
"denorm": denormalize_column,
"rls": security_manager.get_rls_cache_key(datasource),
"changed_on": str(getattr(datasource, "changed_on", "")),
},
sort_keys=True,
).encode()
).hexdigest()
)
if (
not force
and (cached := cache_manager.data_cache.get(cache_key)) is not None
):
logger.debug(
"column-values cache HIT: uid=%s col=%s", datasource.uid, column_name
)
response = self.response(200, result=cached)
response.headers["X-Cache-Status"] = "HIT"
return response
try:
payload = datasource.values_for_column(
column_name=column_name,
limit=row_limit,
denormalize_column=denormalize_column,
)
return self.response(200, result=payload)
except KeyError:
return self.response(
400, message=f"Column name {column_name} does not exist"
@@ -200,31 +145,6 @@ class DatasourceRestApi(BaseSupersetApi):
),
)
# Warn before caching very large payloads (high-cardinality columns)
# so operators can spot cache-memory pressure before Redis OOMs.
# Threshold is operator-tunable; defaults to 100k rows.
warn_threshold = app.config.get("FILTER_VALUES_CACHE_WARN_THRESHOLD", 100_000)
if (payload_size := len(payload)) > warn_threshold:
logger.warning(
"column-values payload exceeds cache-warn threshold: "
"uid=%s col=%s rows=%d threshold=%d",
datasource.uid,
column_name,
payload_size,
warn_threshold,
)
timeout = datasource.cache_timeout or app.config.get(
"CACHE_DEFAULT_TIMEOUT", 300
)
cache_manager.data_cache.set(cache_key, payload, timeout=timeout)
logger.debug(
"column-values cache MISS: uid=%s col=%s", datasource.uid, column_name
)
response = self.response(200, result=payload)
response.headers["X-Cache-Status"] = "MISS"
return response
@expose(
"/<datasource_type>/<int:datasource_id>/validate_expression/",
methods=("POST",),

View File

@@ -73,21 +73,9 @@ def stringify_values(array: NDArray[Any]) -> NDArray[Any]:
obj[na_obj] = None
else:
try:
val = obj.item()
if isinstance(val, (dict, list)):
try:
# Use json.dumps for valid double-quoted JSON.
# str() gives single-quoted repr like {'a': 1}
# which breaks the frontend cell viewer.
obj[...] = stringify(val)
except TypeError:
# Non-JSON-serializable value (e.g. bytes, custom
# objects): fall back to str() to avoid crashing.
obj[...] = str(val)
else:
# for simple string conversions
# this handles odd character types better
obj[...] = obj.astype(str)
# for simple string conversions
# this handles odd character types better
obj[...] = obj.astype(str)
except ValueError:
obj[...] = stringify(obj)

View File

@@ -1843,16 +1843,39 @@ def process_jinja_sql(
def sanitize_clause(clause: str, engine: str) -> str:
"""
Make sure the SQL clause is valid.
Validate a SQL clause and return it unchanged.
The clause is parsed to ensure it is a single, well-formed statement. We
intentionally return the *original* text rather than a re-rendered version:
round-tripping user SQL through SQLGlot's dialect generator can silently
alter semantics. For example, the Postgres dialect (borrowed by several
engines) rewrites ``ROUND(AVG(x), n)`` to ``ROUND(CAST(AVG(x) AS DECIMAL),
n)``, which rounds the value to an integer before the explicit ``ROUND`` on
engines whose unqualified ``DECIMAL`` defaults to scale 0 (see #36113).
Comments are the one exception: a trailing line comment can comment out
surrounding SQL once the clause is embedded into a larger query (e.g.
wrapped in parentheses), so any clause that contains comments is re-rendered
to normalize them into a safe form. That re-rendering uses the *base* dialect
rather than the engine dialect, so it normalizes comments without re-applying
the engine-specific rewrites (e.g. the Postgres ``ROUND``/``CAST`` rewrite
from #36113) that we deliberately avoid above. A trailing statement
terminator is likewise stripped, since callers embed the clause inside a
larger fragment (``WHERE (...)``) where a stray ``;`` would produce invalid
SQL.
"""
try:
statement = SQLStatement(clause, engine)
parsed = statement._parsed # pylint: disable=protected-access
if not any(node.comments for node in parsed.walk()):
return clause.rstrip().rstrip(";").rstrip()
return _normalized_generator(
SQLGLOT_DIALECTS.get(engine),
None,
pretty=False,
comments=True,
).generate(
statement._parsed, # pylint: disable=protected-access
parsed,
copy=True,
)
except SupersetParseError as ex:

View File

@@ -15,7 +15,6 @@
# specific language governing permissions and limitations
# under the License.
from datetime import datetime
from unittest.mock import ANY, patch
import pytest
@@ -24,21 +23,12 @@ from sqlalchemy.sql.elements import TextClause
from superset import db, security_manager
from superset.connectors.sqla.models import SqlaTable
from superset.daos.exceptions import DatasourceTypeNotSupportedError
from superset.extensions import cache_manager
from superset.utils import json
from tests.integration_tests.base_tests import SupersetTestCase
from tests.integration_tests.constants import ADMIN_USERNAME, GAMMA_USERNAME
class TestDatasourceApi(SupersetTestCase):
def setUp(self):
# Clear the column-values cache before every test so that
# ``get_column_values`` always re-runs ``values_for_column`` rather
# than returning a payload populated by a previous test. Prevents
# order-dependent flakes now that the endpoint caches its result.
super().setUp()
cache_manager.data_cache.clear()
def get_virtual_dataset(self):
return (
db.session.query(SqlaTable)
@@ -215,123 +205,6 @@ class TestDatasourceApi(SupersetTestCase):
response = json.loads(rv.data.decode("utf-8"))
assert response["result"] == []
@pytest.mark.usefixtures("app_context", "virtual_dataset")
@patch("superset.models.helpers.ExploreMixin.values_for_column")
def test_get_column_values_cache_hit_skips_query(self, values_for_column_mock):
"""Regression test for #39342.
Two identical requests for the same column on the same datasource
should hit ``values_for_column`` exactly once — the second request
returns the cached payload.
"""
cache_manager.data_cache.clear()
values_for_column_mock.return_value = ["a", "b", "c"]
self.login(ADMIN_USERNAME)
table = self.get_virtual_dataset()
url = f"api/v1/datasource/table/{table.id}/column/col2/values/"
rv1 = self.client.get(url)
rv2 = self.client.get(url)
assert rv1.status_code == 200
assert rv2.status_code == 200
assert json.loads(rv2.data.decode("utf-8"))["result"] == ["a", "b", "c"]
assert values_for_column_mock.call_count == 1
@pytest.mark.usefixtures("app_context", "virtual_dataset")
@patch("superset.models.helpers.ExploreMixin.values_for_column")
def test_get_column_values_force_bypasses_cache(self, values_for_column_mock):
"""``?force=true`` should bypass the cache and re-query the source."""
cache_manager.data_cache.clear()
values_for_column_mock.return_value = ["a", "b"]
self.login(ADMIN_USERNAME)
table = self.get_virtual_dataset()
url = f"api/v1/datasource/table/{table.id}/column/col2/values/"
self.client.get(url)
self.client.get(f"{url}?force=true")
assert values_for_column_mock.call_count == 2
@pytest.mark.usefixtures("app_context", "virtual_dataset")
@patch("superset.models.helpers.ExploreMixin.values_for_column")
def test_get_column_values_cache_isolated_per_column(self, values_for_column_mock):
"""Different columns on the same datasource must not share a cache
entry — otherwise filter values would be silently swapped."""
cache_manager.data_cache.clear()
values_for_column_mock.return_value = ["x"]
self.login(ADMIN_USERNAME)
table = self.get_virtual_dataset()
self.client.get(f"api/v1/datasource/table/{table.id}/column/col1/values/")
self.client.get(f"api/v1/datasource/table/{table.id}/column/col2/values/")
assert values_for_column_mock.call_count == 2
@pytest.mark.usefixtures("app_context", "virtual_dataset")
@patch("superset.models.helpers.ExploreMixin.values_for_column")
def test_get_column_values_cache_busts_on_changed_on(self, values_for_column_mock):
"""Editing the underlying virtual dataset SQL bumps ``changed_on``,
which is part of the cache key — so the next request must miss the
cache and re-run the query."""
cache_manager.data_cache.clear()
values_for_column_mock.return_value = ["v"]
self.login(ADMIN_USERNAME)
table = self.get_virtual_dataset()
url = f"api/v1/datasource/table/{table.id}/column/col2/values/"
self.client.get(url)
# Simulate an edit to the dataset; ``changed_on`` is what the cache
# key reads, so any new value forces a miss.
table.changed_on = datetime(2030, 1, 1)
db.session.flush()
self.client.get(url)
assert values_for_column_mock.call_count == 2
@pytest.mark.usefixtures("app_context", "virtual_dataset")
@patch("superset.datasource.api.security_manager.get_rls_cache_key")
@patch("superset.models.helpers.ExploreMixin.values_for_column")
def test_get_column_values_cache_isolated_per_rls_context(
self, values_for_column_mock, get_rls_cache_key_mock
):
"""RLS safety for guest/embedded sessions. ``get_user_id()`` returns
``None`` for guest users, so two embedded dashboards with different
guest-token RLS would otherwise collide on ``user=None``. Including
the RLS fingerprint in the cache key keeps them separate."""
cache_manager.data_cache.clear()
values_for_column_mock.return_value = ["v"]
self.login(ADMIN_USERNAME)
table = self.get_virtual_dataset()
url = f"api/v1/datasource/table/{table.id}/column/col2/values/"
get_rls_cache_key_mock.return_value = ["dept='A'"]
self.client.get(url)
get_rls_cache_key_mock.return_value = ["dept='B'"]
self.client.get(url)
assert values_for_column_mock.call_count == 2
@pytest.mark.usefixtures("app_context", "virtual_dataset")
@patch("superset.models.helpers.ExploreMixin.values_for_column")
def test_get_column_values_response_advertises_cache_status(
self, values_for_column_mock
):
"""The ``X-Cache-Status`` response header should advertise MISS on
the populating request and HIT on the next identical request, so
operators can debug cache behavior from logs or browser devtools."""
cache_manager.data_cache.clear()
values_for_column_mock.return_value = ["v"]
self.login(ADMIN_USERNAME)
table = self.get_virtual_dataset()
url = f"api/v1/datasource/table/{table.id}/column/col2/values/"
rv_miss = self.client.get(url)
rv_hit = self.client.get(url)
assert rv_miss.headers.get("X-Cache-Status") == "MISS"
assert rv_hit.headers.get("X-Cache-Status") == "HIT"
@patch("superset.datasource.api.security_manager.can_access")
@patch("superset.datasource.api.GetCombinedDatasourceListCommand.run")
def test_combined_list_invalid_order_column(

View File

@@ -804,7 +804,7 @@ def test_get_samples_with_multiple_filters(
assert "2000-01-02" in rv.json["result"]["query"]
assert "2000-01-04" in rv.json["result"]["query"]
assert "col3 = 1.2" in rv.json["result"]["query"]
assert "col4 IS NULL" in rv.json["result"]["query"]
assert "col4 is null" in rv.json["result"]["query"]
assert "col2 = 'c'" in rv.json["result"]["query"]

View File

@@ -21,7 +21,7 @@ import re
from datetime import datetime
from typing import Any, cast, Literal, NamedTuple, Optional, Union
from re import Pattern
from unittest.mock import Mock, patch
from unittest.mock import MagicMock, Mock, patch
import pytest
import numpy as np
@@ -138,8 +138,8 @@ class TestDatabaseModel(SupersetTestCase):
assert col.is_temporal
@patch("superset.jinja_context.get_username", return_value="abc")
def test_jinja_metrics_and_calc_columns(self, mock_username):
base_query_obj = {
def test_jinja_metrics_and_calc_columns(self, mock_username: MagicMock) -> None:
base_query_obj: dict[str, Any] = {
"granularity": None,
"from_dttm": None,
"to_dttm": None,
@@ -199,8 +199,8 @@ class TestDatabaseModel(SupersetTestCase):
assert "'foo_P1D'" in query
# assert dataset saved metric
assert "count('bar_P1D')" in query
# assert adhoc metric
assert "SUM(CASE WHEN user = 'user_abc' THEN 1 ELSE 0 END)" in query
# assert adhoc metric (sanitize_clause preserves the user's SQL verbatim)
assert "SUM(case when user = 'user_abc' then 1 else 0 end)" in query
# Cleanup
db.session.delete(table)
db.session.commit()

View File

@@ -450,93 +450,3 @@ def test_stringify_extension_columns() -> None:
# plain binary BLOBs and other types are left untouched
assert pa.types.is_binary(result.schema.field("blob").type)
assert pa.types.is_integer(result.schema.field("n").type)
def test_stringify_values_dict_and_list_produce_valid_json() -> None:
"""
ClickHouse native JSON and Map types return Python dicts. When stringified for
Arrow array storage they must produce valid double-quoted JSON strings, not
Python's single-quoted repr. Single-quoted strings pass the cheap '{' prefix
check in the frontend's safeJsonObjectParse but then fail JSONbig.parse(),
so the SQL Lab cell viewer never activates.
"""
data = np.array(
[
{"key": "value", "nested": {"a": 1}},
# str() gives ['a', 'b'] (single-quoted, invalid JSON);
# json.dumps gives ["a", "b"] (double-quoted, valid JSON).
["a", "b"],
{"items": [1, 2, 3], "d": "Hello, World!"},
None,
],
dtype=object,
)
result = stringify_values(data)
# Must be valid JSON strings (double-quoted), not Python repr (single-quoted)
assert result[0] == '{"key": "value", "nested": {"a": 1}}'
assert result[1] == '["a", "b"]'
assert result[2] == '{"items": [1, 2, 3], "d": "Hello, World!"}'
assert result[3] is None
# Parseable by a JSON parser — confirms the frontend's JSON.parse would succeed
parsed = superset_json.loads(result[0])
assert parsed == {"key": "value", "nested": {"a": 1}}
parsed = superset_json.loads(result[1])
assert parsed == ["a", "b"]
def test_clickhouse_json_column_in_pa_table_is_valid_json() -> None:
"""
Verify that ClickHouse-style heterogeneous dict columns produce valid JSON
strings in the Arrow table used by the msgpack serialization path.
When clickhouse-connect returns Python dicts for JSON/Map type columns,
SupersetResultSet must serialize them with json.dumps (not str()) so that
the SQL Lab grid's cell viewer can call JSON.parse on the value.
"""
data = [
(1, {"a": {"b": 42}, "c": [1, 2, 3], "d": "Hello, World!"}),
(2, {"e": 5}),
(3, None),
]
description = [
("id", 3, None, None, None, None, None),
("json_col", None, None, None, None, None, None),
]
result_set = SupersetResultSet(data, description, BaseEngineSpec) # type: ignore
df_from_pa = SupersetResultSet.convert_table_to_df(result_set.pa_table)
val0 = df_from_pa["json_col"].iloc[0]
val1 = df_from_pa["json_col"].iloc[1]
# Values in pa_table must be valid JSON strings (parseable by JSON.parse)
assert isinstance(val0, str)
assert isinstance(val1, str)
# Double-quoted JSON, not single-quoted Python repr
parsed0 = superset_json.loads(val0)
assert parsed0 == {"a": {"b": 42}, "c": [1, 2, 3], "d": "Hello, World!"}
parsed1 = superset_json.loads(val1)
assert parsed1 == {"e": 5}
def test_stringify_values_non_serializable_dict_falls_back_to_str() -> None:
"""
When a dict/list contains a value that json.dumps cannot serialize (e.g. bytes),
stringify_values must fall back to str() rather than raising TypeError and crashing
the result-set construction path.
"""
class _Unserializable:
def __repr__(self) -> str:
return "unserializable"
data = np.array(
[{"key": _Unserializable()}],
dtype=object,
)
# Must not raise — falls back to str()
result = stringify_values(data)
assert result[0] == str({"key": _Unserializable()})

View File

@@ -3123,7 +3123,8 @@ def test_is_valid_cvas(sql: str, engine: str, expected: bool) -> None:
"sql, expected, engine",
[
("col = 1", "col = 1", "base"),
("1=\t\n1", "1 = 1", "base"),
# Comment-free clauses are returned verbatim (no semantic round-trip).
("1=\t\n1", "1=\t\n1", "base"),
("(col = 1)", "(col = 1)", "base"), # Compact format without newlines
(
"(col1 = 1) AND (col2 = 2)",
@@ -3147,6 +3148,10 @@ def test_is_valid_cvas(sql: str, engine: str, expected: bool) -> None:
), # Block comments preserved
("col = 'col1 = 1) AND (col2 = 2'", "col = 'col1 = 1) AND (col2 = 2'", "base"),
("col = 'select 1; select 2'", "col = 'select 1; select 2'", "base"),
# Trailing statement terminators are stripped so the clause stays valid
# once embedded inside a larger fragment (e.g. ``WHERE (...)``).
("col = 1;", "col = 1", "base"),
("col = 1 ; ", "col = 1", "base"),
("col = 'abc -- comment'", "col = 'abc -- comment'", "base"),
("col1 = 1) AND (col2 = 2)", QueryClauseValidationException, "base"),
("(col1 = 1) AND (col2 = 2", QueryClauseValidationException, "base"),
@@ -3208,6 +3213,63 @@ def test_sanitize_clause(sql: str, expected: str | Exception, engine: str) -> No
sanitize_clause(sql, engine)
@pytest.mark.parametrize(
"engine",
["postgresql", "redshift", "cockroachdb", "netezza", "hana", "base", "mysql"],
)
def test_sanitize_clause_preserves_aggregation_semantics(engine: str) -> None:
"""
Regression test for https://github.com/apache/superset/issues/36113.
`sanitize_clause` must not silently rewrite a user-authored expression. The
Postgres SQLGlot dialect (which several engines borrow) rewrites
``ROUND(AVG(x), n)`` to ``ROUND(CAST(AVG(x) AS DECIMAL), n)`` at generation
time. On engines whose unqualified ``DECIMAL`` defaults to scale 0 (e.g.
Redshift, Netezza) the injected cast rounds the aggregate to an integer
*before* the explicit ``ROUND``, producing wrong results.
The clause must be returned unchanged regardless of the engine dialect.
"""
clause = "ROUND(AVG(col), 4)"
sanitized = sanitize_clause(clause, engine)
assert "CAST" not in sanitized.upper(), (
f"sanitize_clause injected a cast for engine {engine!r}: {sanitized!r}"
)
assert sanitized == clause
@pytest.mark.parametrize(
"engine",
["postgresql", "redshift", "cockroachdb", "netezza", "hana", "base", "mysql"],
)
def test_sanitize_clause_preserves_aggregation_semantics_with_comment(
engine: str,
) -> None:
"""
Regression test for https://github.com/apache/superset/issues/36113.
A clause that contains a comment takes the re-rendering branch of
``sanitize_clause``. That branch must normalize comments using the *base*
dialect rather than the engine dialect, so it must not re-apply the Postgres
``ROUND(AVG(x), n)`` -> ``ROUND(CAST(AVG(x) AS DECIMAL), n)`` rewrite that
truncates results on engines where ``DECIMAL`` defaults to scale 0.
"""
clause = "ROUND(AVG(col), 4) /* precise_count_distinct=true */"
sanitized = sanitize_clause(clause, engine)
assert "CAST" not in sanitized.upper(), (
f"sanitize_clause injected a cast for engine {engine!r}: {sanitized!r}"
)
# The comment-handling branch must preserve the user-authored expression and
# comment payload, not just avoid the cast (otherwise dropping the comment or
# rewriting the clause entirely would still pass the assertion above).
assert "ROUND(AVG(col), 4)" in sanitized, (
f"sanitize_clause rewrote the clause for engine {engine!r}: {sanitized!r}"
)
assert "precise_count_distinct=true" in sanitized, (
f"sanitize_clause dropped the comment for engine {engine!r}: {sanitized!r}"
)
@pytest.mark.parametrize(
"engine",
[