fix(chromatic): emit webpack stats so TurboSnap can trace story deps

Second live-test failure on #44111: after fixing the E2BIG git-log crash, the publish step got further but then failed with "Failed to retrieve dependent story files" / "Could not retrieve dependent story files." TurboSnap (onlyChanged) needs a webpack stats.json to build the module dependency graph it uses to trace which stories a changed file affects. Normally chromaui/action builds Storybook itself and can inject stats collection; since we hand it an already-built storybookBuildDir instead (to reuse the shared Docker image rather than installing Node on the bare runner), that stats file has to already exist in the build output. Add --webpack-stats-json to the build-storybook invocation so it does. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix(chromatic): bound fetch-depth to avoid E2BIG on a dormant project
2026-09-10 01:04:25 +00:00 · 2026-09-09 12:16:39 -07:00 · 2026-09-09 12:06:38 -07:00 · 2026-09-09 11:17:15 -07:00 · 2026-09-09 11:15:06 -07:00 · 2026-09-09 10:56:59 -07:00
25 changed files with 2751 additions and 4708 deletions
@@ -0,0 +1,127 @@
+# 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.
+
+# Publishes superset-frontend's Storybook to Chromatic for visual
+# regression testing. See https://www.chromatic.com/docs/github-actions
+#
+# Runs on pushes to master (keeps the Chromatic baseline in sync with
+# mainline) and on pull requests that touch superset-frontend. Fork PRs
+# don't receive CHROMATIC_PROJECT_TOKEN -- GitHub withholds repository
+# secrets from pull_request runs triggered by a fork -- so the publish
+# steps below no-op for them (via the CHROMATIC_PROJECT_TOKEN != '' guard)
+# rather than failing.
+#
+# Non-blocking for now (exitZeroOnChanges: true): visual changes are
+# surfaced as a PR check/comment for review, not enforced as a merge gate.
+# A prior Chromatic setup here (#21095) was removed in #27232 for being
+# unmaintained and overlapping with Applitools (since also discontinued).
+# Keep this one simple and watch it before considering a required check.
+name: Chromatic
+
+on:
+  push:
+    branches:
+      - master
+    paths:
+      - "superset-frontend/**"
+  pull_request:
+    types: [synchronize, opened, reopened, ready_for_review]
+    paths:
+      - "superset-frontend/**"
+  workflow_dispatch: {}
+
+# cancel previous workflow jobs for PRs
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
+  cancel-in-progress: true
+
+env:
+  TAG: apache/superset:chromatic-${{ github.run_id }}
+  CHROMATIC_PROJECT_TOKEN: ${{ secrets.CHROMATIC_PROJECT_TOKEN }}
+
+permissions:
+  contents: read
+
+jobs:
+  chromatic:
+    runs-on: ubuntu-26.04
+    timeout-minutes: 30
+    # pull-requests: write lets chromaui/action post its check and PR
+    # comment. Withheld automatically by GitHub for fork-triggered
+    # pull_request runs, same as CHROMATIC_PROJECT_TOKEN above.
+    permissions:
+      contents: read
+      pull-requests: write
+    steps:
+      - name: Checkout Code
+        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          persist-credentials: false
+          # TurboSnap (onlyChanged below) needs history to diff against the
+          # baseline commit, but NOT fetch-depth: 0. This Chromatic project
+          # was dormant for ~2 years (Chromatic here shipped in #21095,
+          # removed in #27232) before this workflow, so its last known
+          # baseline predates thousands of commits on a very active repo.
+          # With full history, Chromatic's CLI tries to `git log` every
+          # commit back to that ancient baseline as individual CLI args and
+          # hits the OS ARG_MAX limit (E2BIG) -- see
+          # https://github.com/chromaui/chromatic-cli/issues/432, where the
+          # Chromatic team's own recommended workaround is exactly this: a
+          # bounded depth, since a multi-year-old baseline isn't useful
+          # anyway. 500 is far more than any realistic PR needs once a
+          # recent baseline exists (i.e. after this workflow's own first
+          # successful run on master).
+          fetch-depth: 500
+          ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}
+
+      - name: Build Docker Image
+        if: ${{ env.CHROMATIC_PROJECT_TOKEN != '' }}
+        run: |
+          docker buildx build \
+            -t $TAG \
+            --cache-from=type=registry,ref=apache/superset-cache:3.11-slim-trixie \
+            --target superset-node-ci \
+            .
+
+      # --webpack-stats-json writes preview-stats.json into storybook-static.
+      # TurboSnap (onlyChanged below) needs it to trace which stories a
+      # changed file affects; without it, chromaui/action fails with "Could
+      # not retrieve dependent story files" since storybookBuildDir points
+      # at an already-built Storybook it can't inject its own stats
+      # collection into.
+      - name: Build Storybook
+        if: ${{ env.CHROMATIC_PROJECT_TOKEN != '' }}
+        run: |
+          mkdir -p ${{ github.workspace }}/superset-frontend/storybook-static
+          docker run \
+          -v ${{ github.workspace }}/superset-frontend/storybook-static:/app/superset-frontend/storybook-static \
+          --rm $TAG \
+          bash -c "npm i && npm run build-storybook -- --webpack-stats-json"
+
+      - name: Publish to Chromatic
+        if: ${{ env.CHROMATIC_PROJECT_TOKEN != '' }}
+        uses: chromaui/action@6b31c4307e3f5a150ab5345b051bb40a62923a5f # v18.7.3
+        with:
+          projectToken: ${{ env.CHROMATIC_PROJECT_TOKEN }}
+          token: ${{ secrets.GITHUB_TOKEN }}
+          workingDir: superset-frontend
+          storybookBuildDir: storybook-static
+          # TurboSnap: only re-snapshot stories affected by files changed
+          # since the baseline build.
+          onlyChanged: true
+          exitZeroOnChanges: true
+          zip: true
@@ -93,7 +93,7 @@ Look through the GitHub issues. Issues tagged with

 Superset could always use better documentation,
 whether as part of the official Superset docs,
-in docstrings, `docs/*.rst` or even on the web as blog posts or
+in docstrings, Markdown files in `docs/`, or even on the web as blog posts or
 articles. See [Documentation](./howtos.md#contributing-to-documentation) for more details.

 ### Add Translations
@@ -25,6 +25,7 @@ import {
  AxisType,
  buildCustomFormatters,
  CategoricalColorNamespace,
+  ComparisonType,
  CurrencyFormatter,
  DataRecordValue,
  DTTM_ALIAS,
@@ -419,6 +420,11 @@ export default function transformProps(

  const refs: Refs = {};
  const groupBy = ensureIsArray(groupby);
+  // Series whose `label_map` entry led with a time offset, recorded before the shift
+  // below drops it. That leading column is the only structural marker distinguishing a
+  // derived comparison row from a base row whose dimension value happens to read like
+  // the offset, and it is gone from `labelMap` by the time the formatters run.
+  const derivedComparisonSeries = new Set<string>();
  const labelMap: { [key: string]: string[] } = Object.entries(
    label_map,
  ).reduce((acc, entry) => {
@@ -427,6 +433,7 @@ export default function transformProps(
      Array.isArray(timeCompare) &&
      timeCompare.includes(entry[1][0])
    ) {
+      derivedComparisonSeries.add(entry[0]);
      entry[1].shift();
    }
    return { ...acc, [entry[0]]: entry[1] };
@@ -681,6 +688,51 @@ export default function transformProps(
  const array = ensureIsArray(chartProps.rawFormData?.time_compare);
  const inverted = invert(verboseMap);

+  // A Percentage or Ratio time comparison replaces the derived series' values with a
+  // dimensionless number, so that row is no longer in the source metric's units and
+  // must not inherit its currency/D3 format.
+  //
+  // `label_map` carries the structured identity behind a rendered series name, and
+  // `renameOperator` puts the offset at the front of a derived row's entry:
+  //
+  //   derived  '1 week ago, East'  -> ['1 week ago', 'East']
+  //   derived  'count, 1 year ago' -> ['1 year ago', 'count']
+  //   base     'sum__num, East'    -> ['sum__num', 'East']
+  //
+  // so the leading column says which it is. Matching the rendered name instead would
+  // misread a base series whose dimension value happens to equal the offset — a region
+  // literally named "1 week ago" gives 'sum__num, 1 week ago', which reads as derived.
+  const isDerivedComparisonSeries = (seriesKey: string) => {
+    // Recorded above, before the offset was shifted off the `label_map` entry.
+    if (derivedComparisonSeries.has(seriesKey)) {
+      return true;
+    }
+    const columns = labelMap?.[seriesKey];
+    // The shift only runs when `timeCompare` is populated; otherwise the entry still
+    // leads with the offset and can be read directly.
+    return columns?.length
+      ? array.includes(columns[0])
+      : array.includes(seriesKey);
+  };
+
+  // Percentage yields `(s - c) / c`, which reads as a percentage. Ratio yields `s / c`,
+  // a plain multiplier, so it takes a unitless number format rather than a percent one.
+  const ratioFormatter = getNumberFormatter(NumberFormats.SMART_NUMBER);
+
+  const getComparisonFormatter = (seriesKey: string) => {
+    if (!isDerivedComparisonSeries(seriesKey)) {
+      return undefined;
+    }
+    switch (chartProps.rawFormData?.comparison_type) {
+      case ComparisonType.Percentage:
+        return percentFormatter;
+      case ComparisonType.Ratio:
+        return ratioFormatter;
+      default:
+        return undefined;
+    }
+  };
+
  // With the "full range" time-shift option, offset series are outer-joined onto
  // the main series, which inserts null rows into the main series wherever the
  // comparison period has data the current period lacks. Connect nulls so the
@@ -1522,6 +1574,31 @@ export default function transformProps(
            value.forecastTrend || value.forecastLower || value.forecastUpper,
        );

+        // Resolve the value formatter per series so each metric keeps its own
+        // D3/currency format, matching how the series labels are formatted.
+        // Without the series key, `getCustomFormatter` returns undefined for
+        // multi-metric charts and every row falls back to `defaultFormatter`,
+        // rendering the y-axis/currency format for all metrics.
+        //
+        // The tooltip key is the rendered series name, so resolve it through
+        // `labelMap`, whose values lead with the raw metric label. Series
+        // renamed by a verbose_name are absent from that map, so fall back to
+        // the verbose-name inversion, as MixedTimeseries does. A Percentage or
+        // Ratio comparison row is dimensionless rather than a value in the
+        // metric's units, so it takes its own formatter instead of the metric's.
+        const getSeriesFormatter = (seriesKey: string) =>
+          forcePercentFormatter
+            ? percentFormatter
+            : (getComparisonFormatter(seriesKey) ??
+              getCustomFormatter(
+                customFormatters,
+                metrics,
+                labelMap?.[seriesKey]?.[0] ?? inverted[seriesKey],
+              ) ??
+              defaultFormatter);
+
+        // The total row aggregates every series, so it keeps the chart-level
+        // formatter rather than any single metric's format.
        const formatter = forcePercentFormatter
          ? percentFormatter
          : (getCustomFormatter(customFormatters, metrics) ?? defaultFormatter);
@@ -1552,7 +1629,7 @@ export default function transformProps(
            const row = formatForecastTooltipSeries({
              ...value,
              seriesName: key,
-              formatter,
+              formatter: getSeriesFormatter(key),
              marker,
              truncation: tooltipTruncation,
            });
@@ -3576,3 +3576,582 @@ test('boundary label alignment is dropped when the orientation moves the time ax
  expect(horizontal.axisLabel.showMinLabel).toBe(true);
  expect(horizontal.axisLabel.showMaxLabel).toBe(true);
 });
+
+test('tooltip formats each series with its own metric format instead of the default formatter', () => {
+  // Two saved metrics with different formats: `pct_change` carries a percentage
+  // D3 format, `count` carries a currency format. The series labels already
+  // honor each metric's format; the tooltip must do the same.
+  const chartProps = createTestChartProps({
+    formData: {
+      metrics: ['count', 'pct_change'],
+      richTooltip: true,
+    },
+    queriesData: [
+      createTestQueryData(
+        [{ count: 1000, pct_change: 0.1234, __timestamp: BASE_TIMESTAMP }],
+        { label_map: { count: ['count'], pct_change: ['pct_change'] } },
+      ),
+    ],
+    datasource: {
+      verboseMap: {},
+      columnFormats: { pct_change: '.2%' },
+      currencyFormats: { count: { symbol: 'USD', symbolPosition: 'prefix' } },
+    },
+  });
+
+  const { echartOptions } = transformProps(chartProps);
+  const { tooltip } = echartOptions as unknown as TooltipFormatterOptions;
+
+  const result = tooltip.formatter([
+    { seriesId: 'count', seriesName: 'count', value: [BASE_TIMESTAMP, 1000] },
+    {
+      seriesId: 'pct_change',
+      seriesName: 'pct_change',
+      value: [BASE_TIMESTAMP, 0.1234],
+    },
+  ]);
+
+  expect(result).toContain('12.34%');
+  expect(result).toContain('$');
+});
+
+test('tooltip resolves per-metric formats for series renamed by verbose_name', () => {
+  // With a verbose_name configured, the rendered series name (and so the
+  // tooltip key) is the verbose label, while `label_map` stays keyed by the
+  // raw metric label. The formatter lookup has to bridge that gap.
+  const chartProps = createTestChartProps({
+    formData: {
+      metrics: ['count', 'pct_change'],
+      richTooltip: true,
+    },
+    queriesData: [
+      createTestQueryData(
+        [{ count: 1000, pct_change: 0.1234, __timestamp: BASE_TIMESTAMP }],
+        { label_map: { count: ['count'], pct_change: ['pct_change'] } },
+      ),
+    ],
+    datasource: {
+      verboseMap: { count: 'Total Count', pct_change: 'Percent Change' },
+      columnFormats: { pct_change: '.2%' },
+      currencyFormats: { count: { symbol: 'USD', symbolPosition: 'prefix' } },
+    },
+  });
+
+  const { echartOptions } = transformProps(chartProps);
+  const { tooltip } = echartOptions as unknown as TooltipFormatterOptions;
+
+  const result = tooltip.formatter([
+    {
+      seriesId: 'Total Count',
+      seriesName: 'Total Count',
+      value: [BASE_TIMESTAMP, 1000],
+    },
+    {
+      seriesId: 'Percent Change',
+      seriesName: 'Percent Change',
+      value: [BASE_TIMESTAMP, 0.1234],
+    },
+  ]);
+
+  expect(result).toContain('12.34%');
+  expect(result).toContain('$');
+});
+
+test('tooltip keeps per-metric formats on time-comparison (time-shifted) series', () => {
+  // A time-shifted series renders under a name carrying the offset, and its
+  // `label_map` entry leads with that offset rather than the metric. The
+  // formatter lookup has to land on the underlying metric so the shifted row is
+  // formatted like the series it is compared against.
+  const chartProps = createTestChartProps({
+    formData: {
+      metrics: ['count', 'pct_change'],
+      richTooltip: true,
+      timeCompare: ['1 year ago'],
+    },
+    queriesData: [
+      createTestQueryData(
+        [
+          {
+            count: 1000,
+            pct_change: 0.1234,
+            'count, 1 year ago': 900,
+            __timestamp: BASE_TIMESTAMP,
+          },
+        ],
+        {
+          label_map: {
+            count: ['count'],
+            pct_change: ['pct_change'],
+            'count, 1 year ago': ['1 year ago', 'count'],
+          },
+        },
+      ),
+    ],
+    datasource: {
+      verboseMap: {},
+      columnFormats: { pct_change: '.2%' },
+      currencyFormats: { count: { symbol: 'USD', symbolPosition: 'prefix' } },
+    },
+  });
+
+  const { echartOptions } = transformProps(chartProps);
+  const { tooltip } = echartOptions as unknown as TooltipFormatterOptions;
+
+  const result = tooltip.formatter([
+    { seriesId: 'count', seriesName: 'count', value: [BASE_TIMESTAMP, 1000] },
+    {
+      seriesId: 'count, 1 year ago',
+      seriesName: 'count, 1 year ago',
+      value: [BASE_TIMESTAMP, 900],
+    },
+  ]);
+
+  // The base series and its time-shifted counterpart keep the currency format.
+  expect(result).toContain('$ 1k');
+  expect(result).toContain('$ 900');
+});
+
+test('tooltip does not apply a metric currency format to a Percentage time comparison', () => {
+  // Reported on #33757: a Time Comparison set to Percentage change on a
+  // currency metric kept rendering the derived row in dollars. That row holds a
+  // ratio rather than a value in the metric's units, so it must not inherit the
+  // metric's saved CurrencyFormatter.
+  const chartProps = createTestChartProps({
+    formData: {
+      metric: 'sum__num',
+      metrics: ['sum__num'],
+      richTooltip: true,
+      time_compare: ['1 week ago'],
+      comparison_type: ComparisonType.Percentage,
+    },
+    queriesData: [
+      createTestQueryData(
+        [{ sum__num: 100, '1 week ago': 0.25, __timestamp: BASE_TIMESTAMP }],
+        { label_map: { sum__num: ['sum__num'], '1 week ago': ['1 week ago'] } },
+      ),
+    ],
+    datasource: {
+      verboseMap: {},
+      columnFormats: {},
+      currencyFormats: {
+        sum__num: { symbol: 'USD', symbolPosition: 'prefix' },
+      },
+    },
+  });
+
+  const { echartOptions } = transformProps(chartProps);
+  const { tooltip } = echartOptions as unknown as TooltipFormatterOptions;
+
+  const result = tooltip.formatter([
+    {
+      seriesId: 'sum__num',
+      seriesName: 'sum__num',
+      value: [BASE_TIMESTAMP, 100],
+    },
+    {
+      seriesId: '1 week ago',
+      seriesName: '1 week ago',
+      value: [BASE_TIMESTAMP, 0.25],
+    },
+  ]);
+
+  // The source metric keeps its currency; the percentage-change row does not.
+  expect(result).toContain('$ 100');
+  expect(result).toContain('25.00%');
+  expect(result).not.toContain('$ 0.25');
+});
+
+test('tooltip does not apply a metric currency format to a grouped Percentage time comparison', () => {
+  // A groupby appends the dimension values to the derived series name
+  // ("1 week ago, East"), so matching the dimensionless names alone left the
+  // grouped rows resolving back to the source metric's CurrencyFormatter.
+  const chartProps = createTestChartProps({
+    formData: {
+      metric: 'sum__num',
+      metrics: ['sum__num'],
+      groupby: ['region'],
+      richTooltip: true,
+      time_compare: ['1 week ago'],
+      comparison_type: ComparisonType.Percentage,
+    },
+    queriesData: [
+      createTestQueryData(
+        [
+          {
+            'sum__num, East': 100,
+            '1 week ago, East': 0.25,
+            __timestamp: BASE_TIMESTAMP,
+          },
+        ],
+        {
+          label_map: {
+            'sum__num, East': ['sum__num', 'East'],
+            '1 week ago, East': ['1 week ago', 'East'],
+          },
+        },
+      ),
+    ],
+    datasource: {
+      verboseMap: {},
+      columnFormats: {},
+      currencyFormats: {
+        sum__num: { symbol: 'USD', symbolPosition: 'prefix' },
+      },
+    },
+  });
+
+  const { echartOptions } = transformProps(chartProps);
+  const { tooltip } = echartOptions as unknown as TooltipFormatterOptions;
+
+  const result = tooltip.formatter([
+    {
+      seriesId: 'sum__num, East',
+      seriesName: 'sum__num, East',
+      value: [BASE_TIMESTAMP, 100],
+    },
+    {
+      seriesId: '1 week ago, East',
+      seriesName: '1 week ago, East',
+      value: [BASE_TIMESTAMP, 0.25],
+    },
+  ]);
+
+  expect(result).toContain('$ 100');
+  expect(result).toContain('25.00%');
+  expect(result).not.toContain('$ 0.25');
+});
+
+test('tooltip does not apply a metric currency format to a Ratio time comparison', () => {
+  // A Ratio comparison is `source / compare`, a plain multiplier, so the derived row is
+  // no more in the metric's currency than a Percentage one is — but it is not a
+  // percentage either, so it takes a unitless number format rather than the percent one.
+  const chartProps = createTestChartProps({
+    formData: {
+      metric: 'sum__num',
+      metrics: ['sum__num'],
+      richTooltip: true,
+      time_compare: ['1 week ago'],
+      comparison_type: ComparisonType.Ratio,
+    },
+    queriesData: [
+      createTestQueryData(
+        [{ sum__num: 100, '1 week ago': 1.25, __timestamp: BASE_TIMESTAMP }],
+        { label_map: { sum__num: ['sum__num'], '1 week ago': ['1 week ago'] } },
+      ),
+    ],
+    datasource: {
+      verboseMap: {},
+      columnFormats: {},
+      currencyFormats: {
+        sum__num: { symbol: 'USD', symbolPosition: 'prefix' },
+      },
+    },
+  });
+
+  const { echartOptions } = transformProps(chartProps);
+  const { tooltip } = echartOptions as unknown as TooltipFormatterOptions;
+
+  const result = tooltip.formatter([
+    {
+      seriesId: 'sum__num',
+      seriesName: 'sum__num',
+      value: [BASE_TIMESTAMP, 100],
+    },
+    {
+      seriesId: '1 week ago',
+      seriesName: '1 week ago',
+      value: [BASE_TIMESTAMP, 1.25],
+    },
+  ]);
+
+  // The source metric keeps its currency; the ratio row renders as a plain number.
+  expect(result).toContain('$ 100');
+  expect(result).toContain('1.25');
+  expect(result).not.toContain('$ 1.25');
+});
+
+test('tooltip does not apply a metric currency format to a grouped Ratio time comparison', () => {
+  const chartProps = createTestChartProps({
+    formData: {
+      metric: 'sum__num',
+      metrics: ['sum__num'],
+      groupby: ['region'],
+      richTooltip: true,
+      time_compare: ['1 week ago'],
+      comparison_type: ComparisonType.Ratio,
+    },
+    queriesData: [
+      createTestQueryData(
+        [
+          {
+            'sum__num, East': 100,
+            '1 week ago, East': 1.25,
+            __timestamp: BASE_TIMESTAMP,
+          },
+        ],
+        {
+          label_map: {
+            'sum__num, East': ['sum__num', 'East'],
+            '1 week ago, East': ['1 week ago', 'East'],
+          },
+        },
+      ),
+    ],
+    datasource: {
+      verboseMap: {},
+      columnFormats: {},
+      currencyFormats: {
+        sum__num: { symbol: 'USD', symbolPosition: 'prefix' },
+      },
+    },
+  });
+
+  const { echartOptions } = transformProps(chartProps);
+  const { tooltip } = echartOptions as unknown as TooltipFormatterOptions;
+
+  const result = tooltip.formatter([
+    {
+      seriesId: 'sum__num, East',
+      seriesName: 'sum__num, East',
+      value: [BASE_TIMESTAMP, 100],
+    },
+    {
+      seriesId: '1 week ago, East',
+      seriesName: '1 week ago, East',
+      value: [BASE_TIMESTAMP, 1.25],
+    },
+  ]);
+
+  expect(result).toContain('$ 100');
+  expect(result).toContain('1.25');
+  expect(result).not.toContain('$ 1.25');
+});
+
+test('tooltip formats derived rows when timeCompare normalization strips the offset', () => {
+  // With `timeCompare` populated, `labelMap` has its leading offset shifted off before
+  // the formatters run, so the derived identity has to be captured during that pass —
+  // reading `labelMap[key][0]` afterwards sees the dimension value instead.
+  const chartProps = createTestChartProps({
+    formData: {
+      metric: 'sum__num',
+      metrics: ['sum__num'],
+      groupby: ['region'],
+      richTooltip: true,
+      timeCompare: ['1 week ago'],
+      time_compare: ['1 week ago'],
+      comparison_type: ComparisonType.Percentage,
+    },
+    queriesData: [
+      createTestQueryData(
+        [
+          {
+            'sum__num, East': 100,
+            '1 week ago, East': 0.25,
+            __timestamp: BASE_TIMESTAMP,
+          },
+        ],
+        {
+          label_map: {
+            'sum__num, East': ['sum__num', 'East'],
+            '1 week ago, East': ['1 week ago', 'East'],
+          },
+        },
+      ),
+    ],
+    datasource: {
+      verboseMap: {},
+      columnFormats: {},
+      currencyFormats: {
+        sum__num: { symbol: 'USD', symbolPosition: 'prefix' },
+      },
+    },
+  });
+
+  const { echartOptions } = transformProps(chartProps);
+  const { tooltip } = echartOptions as unknown as TooltipFormatterOptions;
+
+  const result = tooltip.formatter([
+    {
+      seriesId: 'sum__num, East',
+      seriesName: 'sum__num, East',
+      value: [BASE_TIMESTAMP, 100],
+    },
+    {
+      seriesId: '1 week ago, East',
+      seriesName: '1 week ago, East',
+      value: [BASE_TIMESTAMP, 0.25],
+    },
+  ]);
+
+  expect(result).toContain('$ 100');
+  expect(result).toContain('25.00%');
+  expect(result).not.toContain('$ 0.25');
+});
+
+test('tooltip gives a Ratio row a unitless format when timeCompare is set', () => {
+  const chartProps = createTestChartProps({
+    formData: {
+      metric: 'sum__num',
+      metrics: ['sum__num'],
+      groupby: ['region'],
+      richTooltip: true,
+      timeCompare: ['1 week ago'],
+      time_compare: ['1 week ago'],
+      comparison_type: ComparisonType.Ratio,
+    },
+    queriesData: [
+      createTestQueryData(
+        [
+          {
+            'sum__num, East': 100,
+            '1 week ago, East': 1.25,
+            __timestamp: BASE_TIMESTAMP,
+          },
+        ],
+        {
+          label_map: {
+            'sum__num, East': ['sum__num', 'East'],
+            '1 week ago, East': ['1 week ago', 'East'],
+          },
+        },
+      ),
+    ],
+    datasource: {
+      verboseMap: {},
+      columnFormats: {},
+      currencyFormats: {
+        sum__num: { symbol: 'USD', symbolPosition: 'prefix' },
+      },
+    },
+  });
+
+  const { echartOptions } = transformProps(chartProps);
+  const { tooltip } = echartOptions as unknown as TooltipFormatterOptions;
+
+  const result = tooltip.formatter([
+    {
+      seriesId: 'sum__num, East',
+      seriesName: 'sum__num, East',
+      value: [BASE_TIMESTAMP, 100],
+    },
+    {
+      seriesId: '1 week ago, East',
+      seriesName: '1 week ago, East',
+      value: [BASE_TIMESTAMP, 1.25],
+    },
+  ]);
+
+  expect(result).toContain('$ 100');
+  expect(result).toContain('1.25');
+  expect(result).not.toContain('$ 1.25');
+});
+
+test('tooltip keeps the metric format when a dimension value equals the offset', () => {
+  // A groupby value can legitimately read like the configured offset, giving a *base*
+  // series called `sum__num, 1 week ago`. Matching the rendered name would classify it
+  // as derived and strip its currency; `label_map` leads with the metric, not the
+  // offset, so it stays a base row.
+  const chartProps = createTestChartProps({
+    formData: {
+      metric: 'sum__num',
+      metrics: ['sum__num'],
+      groupby: ['region'],
+      richTooltip: true,
+      time_compare: ['1 week ago'],
+      comparison_type: ComparisonType.Percentage,
+    },
+    queriesData: [
+      createTestQueryData(
+        [
+          {
+            'sum__num, 1 week ago': 100,
+            '1 week ago, 1 week ago': 0.25,
+            __timestamp: BASE_TIMESTAMP,
+          },
+        ],
+        {
+          label_map: {
+            // The region is named "1 week ago"; the metric still leads the base entry.
+            'sum__num, 1 week ago': ['sum__num', '1 week ago'],
+            // Its derived counterpart leads with the offset.
+            '1 week ago, 1 week ago': ['1 week ago', '1 week ago'],
+          },
+        },
+      ),
+    ],
+    datasource: {
+      verboseMap: {},
+      columnFormats: {},
+      currencyFormats: {
+        sum__num: { symbol: 'USD', symbolPosition: 'prefix' },
+      },
+    },
+  });
+
+  const { echartOptions } = transformProps(chartProps);
+  const { tooltip } = echartOptions as unknown as TooltipFormatterOptions;
+
+  const result = tooltip.formatter([
+    {
+      seriesId: 'sum__num, 1 week ago',
+      seriesName: 'sum__num, 1 week ago',
+      value: [BASE_TIMESTAMP, 100],
+    },
+    {
+      seriesId: '1 week ago, 1 week ago',
+      seriesName: '1 week ago, 1 week ago',
+      value: [BASE_TIMESTAMP, 0.25],
+    },
+  ]);
+
+  // The base row keeps its currency even though its name ends in the offset, and the
+  // genuinely derived row is still formatted as a percentage.
+  expect(result).toContain('$ 100');
+  expect(result).toContain('25.00%');
+});
+
+test('tooltip keeps the metric format on a Difference time comparison', () => {
+  // Difference is `source - compare`, which stays in the metric's units, so unlike
+  // Percentage and Ratio it must keep the currency format.
+  const chartProps = createTestChartProps({
+    formData: {
+      metric: 'sum__num',
+      metrics: ['sum__num'],
+      richTooltip: true,
+      time_compare: ['1 week ago'],
+      comparison_type: ComparisonType.Difference,
+    },
+    queriesData: [
+      createTestQueryData(
+        [{ sum__num: 100, '1 week ago': 25, __timestamp: BASE_TIMESTAMP }],
+        { label_map: { sum__num: ['sum__num'], '1 week ago': ['1 week ago'] } },
+      ),
+    ],
+    datasource: {
+      verboseMap: {},
+      columnFormats: {},
+      currencyFormats: {
+        sum__num: { symbol: 'USD', symbolPosition: 'prefix' },
+      },
+    },
+  });
+
+  const { echartOptions } = transformProps(chartProps);
+  const { tooltip } = echartOptions as unknown as TooltipFormatterOptions;
+
+  const result = tooltip.formatter([
+    {
+      seriesId: 'sum__num',
+      seriesName: 'sum__num',
+      value: [BASE_TIMESTAMP, 100],
+    },
+    {
+      seriesId: '1 week ago',
+      seriesName: '1 week ago',
+      value: [BASE_TIMESTAMP, 25],
+    },
+  ]);
+
+  expect(result).toContain('$ 100');
+  expect(result).toContain('$ 25');
+});
@@ -16,6 +16,8 @@
 * specific language governing permissions and limitations
 * under the License.
 */
+
+/** @jsxImportSource @emotion/react */
 import {
  Children,
  cloneElement,
@@ -286,9 +288,28 @@ function StickyWrap({
      </colgroup>
    );

-    const headerContainerWidth = hasVerticalScroll
-      ? maxWidth - scrollBarSize
-      : maxWidth;
+    // Below, `width: maxWidth` is applied unconditionally (never reduced by
+    // subtracting a separately-measured scrollbar width, unlike this file's
+    // previous `maxWidth - scrollBarSize`). That's the load-bearing part of
+    // this fix: the shared colgroup (computed from the sizer below, whose
+    // own clientWidth can only ever be <= maxWidth) can never need more
+    // width than that, so a header/footer wrapper that's never narrowed
+    // below maxWidth can never clip it, regardless of whether any
+    // JS-measured scrollbar size agrees with what the sizer/body actually
+    // reserve in a given browser.
+    //
+    // `scrollbarGutter`/`scrollBarStyles` below are a separate, secondary
+    // measure -- matching an actual clip boundary is not what they're for
+    // (an `overflow: hidden` box's clip boundary sits at its real
+    // border-box edge regardless of `scrollbar-gutter`, which only affects
+    // what `clientWidth` reports). They keep header/footer's reported
+    // `clientWidth` consistent with body's so that, when both a vertical
+    // and a horizontal scrollbar are present, the horizontal `scrollLeft`
+    // synced from body (see `onScroll` below) reveals the same slice of the
+    // row in header/footer as is actually visible in body.
+    const headerFooterGutter: CSSProperties = {
+      scrollbarGutter: hasVerticalScroll ? 'stable' : undefined,
+    };

    headerTable = (
      <div
@@ -296,9 +317,11 @@ function StickyWrap({
        ref={scrollHeaderRef}
        style={{
          overflow: 'hidden',
-          width: headerContainerWidth,
+          width: maxWidth,
          boxSizing: 'border-box',
+          ...headerFooterGutter,
        }}
+        css={scrollBarStyles}
        role="presentation"
      >
        {cloneElement(
@@ -317,9 +340,11 @@ function StickyWrap({
        ref={scrollFooterRef}
        style={{
          overflow: 'hidden',
-          width: headerContainerWidth,
+          width: maxWidth,
          boxSizing: 'border-box',
+          ...headerFooterGutter,
        }}
+        css={scrollBarStyles}
        role="presentation"
      >
        {cloneElement(
@@ -0,0 +1,205 @@
+/**
+ * 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 { useCallback } from 'react';
+import { useTable, Column } from 'react-table';
+import { render } from '@superset-ui/core/spec';
+import useSticky from '../../../src/DataTable/hooks/useSticky';
+
+// A value distinguishable from any real scrollbar width, so the width
+// assertions below can detect whether header/footer's wrapper width was
+// computed by subtracting this JS-measured probe from `maxWidth` (the old,
+// removed `maxWidth - scrollBarSize` behavior) rather than always being the
+// unconditional `maxWidth` the fix uses. If that subtraction is ever
+// reintroduced, header/footer's `style.width` would read
+// `${MAX_WIDTH - MOCKED_SCROLLBAR_PROBE_SIZE}px`, an unmistakably wrong
+// value given how large this mock is.
+const MOCKED_SCROLLBAR_PROBE_SIZE = 42;
+
+jest.mock('../../../src/DataTable/utils/getScrollBarSize', () => ({
+  __esModule: true,
+  CUSTOM_SCROLLBAR_SIZE: 8,
+  default: () => 0,
+  getCustomScrollBarSize: () => MOCKED_SCROLLBAR_PROBE_SIZE,
+}));
+
+const MAX_WIDTH = 300;
+const MAX_HEIGHT = 120; // small enough that the mocked content forces a vertical scroll
+
+const TOTAL_HEADER_HEIGHT = 30;
+const TOTAL_FOOTER_HEIGHT = 30;
+// Larger than `MAX_HEIGHT - TOTAL_HEADER_HEIGHT - TOTAL_FOOTER_HEIGHT`, so the
+// sticky layout effect computes `hasVerticalScroll: true`.
+const FULL_TABLE_HEIGHT = 400;
+
+function mockMeasurements() {
+  jest
+    .spyOn(HTMLElement.prototype, 'clientHeight', 'get')
+    .mockImplementation(function mockClientHeight(this: HTMLElement) {
+      if (this.tagName === 'THEAD') return TOTAL_HEADER_HEIGHT;
+      if (this.tagName === 'TFOOT') return TOTAL_FOOTER_HEIGHT;
+      if (this.tagName === 'TABLE') return FULL_TABLE_HEIGHT;
+      return 0;
+    });
+  jest
+    .spyOn(HTMLElement.prototype, 'getBoundingClientRect')
+    .mockImplementation(function mockRect(this: HTMLElement) {
+      const width = this.tagName === 'TH' ? 60 : 0;
+      return {
+        width,
+        height: 0,
+        top: 0,
+        left: 0,
+        right: width,
+        bottom: 0,
+        x: 0,
+        y: 0,
+        toJSON: () => {},
+      } as DOMRect;
+    });
+}
+
+type Row = { category: string; amount: string };
+
+const columns: Column<Row>[] = [
+  { Header: 'Category', accessor: 'category' },
+  { Header: 'SUM(amount)', accessor: 'amount' },
+];
+
+const data: Row[] = Array.from({ length: 8 }, (_, i) => ({
+  category: `Category ${i}`,
+  amount: `${1234567.891234 + i}`,
+}));
+
+function StickyTableHarness() {
+  const getTableSize = useCallback(
+    () => ({ width: MAX_WIDTH, height: MAX_HEIGHT }),
+    [],
+  );
+  const { getTableProps, headerGroups, rows, prepareRow, wrapStickyTable } =
+    useTable<Row>(
+      {
+        columns,
+        data,
+        getTableSize,
+      },
+      useSticky,
+    );
+
+  const renderTable = () => (
+    <table {...getTableProps()}>
+      <thead>
+        {headerGroups.map(hg => (
+          <tr {...hg.getHeaderGroupProps()} key={hg.id}>
+            {hg.headers.map(col => (
+              <th {...col.getHeaderProps()} key={col.id}>
+                {col.render('Header')}
+              </th>
+            ))}
+          </tr>
+        ))}
+      </thead>
+      <tbody>
+        {rows.map(row => {
+          prepareRow(row);
+          return (
+            <tr {...row.getRowProps()} key={row.id}>
+              {row.cells.map(cell => (
+                <td {...cell.getCellProps()} key={cell.column.id}>
+                  {cell.render('Cell')}
+                </td>
+              ))}
+            </tr>
+          );
+        })}
+      </tbody>
+      <tfoot>
+        <tr key="footer">
+          <th>Summary</th>
+          <td>
+            <strong>14814904.694808</strong>
+          </td>
+        </tr>
+      </tfoot>
+    </table>
+  );
+
+  return <div data-test="sticky-root">{wrapStickyTable(renderTable)}</div>;
+}
+
+test('sticky header/footer width matches the body, independent of the scrollbar-size probe', () => {
+  mockMeasurements();
+
+  const { container } = render(<StickyTableHarness />);
+
+  const root = container.querySelector('[data-test="sticky-root"] > div');
+  expect(root).not.toBeNull();
+  const [headerDiv, bodyDiv, footerDiv] = Array.from(
+    root!.children,
+  ) as HTMLDivElement[];
+
+  expect(bodyDiv.style.width).toBe(`${MAX_WIDTH}px`);
+
+  // This is the load-bearing assertion for the reported bug. Before the fix
+  // these read `${MAX_WIDTH - MOCKED_SCROLLBAR_PROBE_SIZE}px` (258px) --
+  // genuinely narrower than the body, from a real CSS `width` subtraction
+  // (`maxWidth - scrollBarSize`), not just a smaller reported `clientWidth`.
+  // A wrapper that's actually narrower than the shared, fixed-layout
+  // colgroup it has to display gets genuinely clipped by its own
+  // `overflow: hidden` (verified with real hit-testing in a real browser --
+  // this is not true of the `scrollbarGutter` assertions below). The fix
+  // makes header/footer always exactly `maxWidth`, which the colgroup
+  // (bounded by the sizer's `clientWidth`, itself bounded by `maxWidth`)
+  // can never exceed.
+  expect(headerDiv.style.width).toBe(`${MAX_WIDTH}px`);
+  expect(footerDiv.style.width).toBe(`${MAX_WIDTH}px`);
+
+  // Secondary, not itself load-bearing for preventing clipping: real
+  // hit-testing shows `scrollbar-gutter` on an `overflow: hidden` box
+  // changes what `clientWidth` reports without moving where it actually
+  // clips, so this doesn't guard against the reported bug by itself. It's
+  // asserted because header/footer's reported `clientWidth` still needs to
+  // match body's `clientWidth` for their programmatically
+  // synced `scrollLeft` (see `onScroll` in `useSticky.tsx`) to reveal the
+  // same slice of the row body actually shows, when a horizontal scrollbar
+  // is present alongside a vertical one.
+  expect(headerDiv.style.scrollbarGutter).toBe(bodyDiv.style.scrollbarGutter);
+  expect(footerDiv.style.scrollbarGutter).toBe(bodyDiv.style.scrollbarGutter);
+  expect(bodyDiv.style.scrollbarGutter).toBe('stable');
+
+  // Pin the `css={scrollBarStyles}` addition to header/footer directly (part
+  // of the same secondary consistency measure as the `scrollbarGutter`
+  // assertions above, not the clipping fix). This component carries
+  // `/** @jsxImportSource @emotion/react */`, which makes
+  // Babel route its `css` prop through Emotion's jsx runtime instead of
+  // passing `css` straight through as an inert DOM attribute (the default in
+  // this repo's Jest/Babel setup, which -- unlike the webpack/SWC build --
+  // doesn't set `importSource: '@emotion/react'` globally). With the pragma
+  // in place, an applied `css` prop is observable as a real, non-empty
+  // className, so this assertion actually fails without the fix instead of
+  // passing regardless of whether `scrollBarStyles` is wired up.
+  //
+  // Before `css={scrollBarStyles}` was added to header/footer, they had no
+  // emotion-generated class at all (`className === ''`) while the body kept
+  // its own -- so this fails pre-fix and passes post-fix.
+  expect(headerDiv.className).not.toBe('');
+  expect(headerDiv.className).toBe(bodyDiv.className);
+  expect(footerDiv.className).toBe(bodyDiv.className);
+
+  jest.restoreAllMocks();
+});
@@ -45,8 +45,8 @@ test('getCustomScrollBarSize measures the probe using the shared custom scrollba
 });

 test('CUSTOM_SCROLLBAR_SIZE matches the custom scrollbar width rendered in the sticky table', () => {
-  // useSticky.tsx's scrollBarStyles must stay in sync with this constant so
-  // the sticky header's shrink amount always matches the body's real
-  // scrollbar width.
+  // useSticky.tsx's scrollBarStyles sets `::-webkit-scrollbar { width: ... }`
+  // from this constant, so it must stay in sync with it or the real
+  // scrollbar body/sizer render won't match what this constant claims.
  expect(CUSTOM_SCROLLBAR_SIZE).toBe(8);
 });
@@ -15,6 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 import logging
+import time
 from typing import Any, Callable

 import click
@@ -58,10 +59,18 @@ def _load_dataset(
    if "force" in sig.parameters:
        params["force"] = force

+    start = time.perf_counter()
    try:
        loader(**params)
    except Exception as e:
-        logger.warning("Failed to load %s: %s", dataset_name, e)
+        logger.warning(
+            "Failed to load %s after %.2fs: %s",
+            dataset_name,
+            time.perf_counter() - start,
+            e,
+        )
+    else:
+        logger.info("Finished [%s] in %.2fs", dataset_name, time.perf_counter() - start)


 def load_examples_run(
@@ -70,6 +79,7 @@ def load_examples_run(
    only_metadata: bool = False,
    force: bool = False,
 ) -> None:
+    run_start = time.perf_counter()
    if only_metadata:
        logger.info("Loading examples metadata")
    else:
@@ -94,7 +104,14 @@ def load_examples_run(
        _load_dataset(loader, loader_name, only_metadata, force)

    # Load examples that are stored as YAML config files
+    configs_start = time.perf_counter()
    examples.load_examples_from_configs(force, load_test_data)
+    logger.info(
+        "Finished [Examples From Configs] in %.2fs",
+        time.perf_counter() - configs_start,
+    )
+
+    logger.info("load_examples finished in %.2fs", time.perf_counter() - run_start)


@click.command()
@@ -22,6 +22,7 @@ from typing import Any, Optional, TypedDict
 import pandas as pd
 from flask import current_app
 from flask_babel import lazy_gettext as _
+from sqlalchemy import or_
 from werkzeug.datastructures import FileStorage

 from superset import db
@@ -168,12 +169,20 @@ class UploadCommand(BaseCommand):
            )
        )

+        catalog = self._model.get_default_catalog()
+
+        catalog_filter = (
+            or_(SqlaTable.catalog == catalog, SqlaTable.catalog.is_(None))
+            if catalog is not None
+            else SqlaTable.catalog.is_(None)
+        )
        sqla_table = (
            db.session.query(SqlaTable)
-            .filter_by(
-                table_name=self._table_name,
-                schema=self._schema,
-                database_id=self._model_id,
+            .filter(
+                SqlaTable.table_name == self._table_name,
+                SqlaTable.schema == self._schema,
+                SqlaTable.database_id == self._model_id,
+                catalog_filter,
            )
            .one_or_none()
        )
@@ -206,7 +215,7 @@ class UploadCommand(BaseCommand):
            )

            if soft_twin := DatasetDAO.find_soft_deleted_logical_duplicate(
-                self._model, Table(self._table_name, self._schema)
+                self._model, Table(self._table_name, self._schema, catalog)
            ):
                raise DatabaseUploadSoftDeletedDatasetExistsError(str(soft_twin.uuid))

@@ -217,12 +226,13 @@ class UploadCommand(BaseCommand):
                table_name=self._table_name,
                database=self._model,
                database_id=self._model_id,
+                catalog=catalog,
                editors=editors,
                schema=self._schema,
-                # Ensure catalog is set
-                catalog=self._model.get_default_catalog(),
            )
            db.session.add(sqla_table)
+        elif sqla_table.catalog is None and catalog is not None:
+            sqla_table.catalog = catalog

        sqla_table.fetch_metadata()

@@ -163,6 +163,14 @@ class ImportExamplesCommand(ImportModelsCommand):
        dataset_info: dict[str, dict[str, Any]] = {}
        for file_name, config in configs.items():
            if file_name.startswith("datasets/"):
+                # Some examples ship a dataset config for a table that another
+                # example already defines (same uuid, re-exported under a
+                # different folder). Import each uuid once per run --
+                # reimporting it just repeats the same column/metric sync
+                # against an identical config.
+                if config["uuid"] in dataset_info:
+                    continue
+
                # find the ID of the corresponding database
                if config["database_uuid"] not in database_ids:
                    raise Exception(  # pylint: disable=broad-exception-raised
@@ -42,6 +42,12 @@ NO_TIME_RANGE = "No filter"

 QUERY_CANCEL_KEY = "cancel_query"
 QUERY_EARLY_CANCEL_KEY = "early_cancel_query"
+# Set once execute_sql_statements() has opened a DB connection and asked the
+# engine spec for a cancel handle, regardless of whether one came back. Lets
+# cancel_query() tell "hasn't been dispatched to the engine yet" (safe to
+# fabricate a stop) apart from "this engine just has no cancel support"
+# (must fail honestly) when no cancel ID is on record.
+QUERY_DISPATCHED_KEY = "query_dispatched"

 LRU_CACHE_MAX_SIZE = 256

@@ -28,6 +28,7 @@ from superset.queries.filters import QueryFilter
 from superset.queries.saved_queries.filters import SavedQueryFilter
 from superset.utils.core import get_user_id
 from superset.utils.dates import now_as_float
+from superset.utils.decorators import transaction

 logger = logging.getLogger(__name__)

@@ -59,6 +60,7 @@ class QueryDAO(BaseDAO[Query]):
        )

    @staticmethod
+    @transaction()
    def stop_query(client_id: str) -> None:
        query = (
            db.session.query(Query)
@@ -81,6 +83,11 @@ class QueryDAO(BaseDAO[Query]):
        if not sql_lab.cancel_query(query):
            raise SupersetCancelQueryException("Could not cancel query")

+        # cancel_query() may have staged an early-cancel flag on query.extra
+        # without committing it (see its docstring/comments); the
+        # @transaction decorator commits it together with status=STOPPED
+        # below in one transaction, closing the window where another
+        # request could observe the flag set but the status still RUNNING.
        query.status = QueryStatus.STOPPED
        query.end_time = now_as_float()

@@ -1,348 +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.
-always_filter_main_dttm: false
-cache_timeout: null
-catalog: null
-columns:
- advanced_data_type: null
-  column_name: order_date
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: true
-  python_date_format: null
-  type: TIMESTAMP WITHOUT TIME ZONE
-  verbose_name: null
- advanced_data_type: null
-  column_name: price_each
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: DOUBLE PRECISION
-  verbose_name: null
- advanced_data_type: null
-  column_name: sales
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: DOUBLE PRECISION
-  verbose_name: null
- advanced_data_type: null
-  column_name: address_line1
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: TEXT
-  verbose_name: null
- advanced_data_type: null
-  column_name: address_line2
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: TEXT
-  verbose_name: null
- advanced_data_type: null
-  column_name: order_line_number
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: BIGINT
-  verbose_name: null
- advanced_data_type: null
-  column_name: quantity_ordered
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: BIGINT
-  verbose_name: null
- advanced_data_type: null
-  column_name: order_number
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: BIGINT
-  verbose_name: null
- advanced_data_type: null
-  column_name: quarter
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: BIGINT
-  verbose_name: null
- advanced_data_type: null
-  column_name: year
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: BIGINT
-  verbose_name: null
- advanced_data_type: null
-  column_name: month
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: BIGINT
-  verbose_name: null
- advanced_data_type: null
-  column_name: msrp
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: BIGINT
-  verbose_name: null
- advanced_data_type: null
-  column_name: contact_last_name
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: TEXT
-  verbose_name: null
- advanced_data_type: null
-  column_name: contact_first_name
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: TEXT
-  verbose_name: null
- advanced_data_type: null
-  column_name: postal_code
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: TEXT
-  verbose_name: null
- advanced_data_type: null
-  column_name: customer_name
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: TEXT
-  verbose_name: null
- advanced_data_type: null
-  column_name: deal_size
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: TEXT
-  verbose_name: null
- advanced_data_type: null
-  column_name: product_code
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: TEXT
-  verbose_name: null
- advanced_data_type: null
-  column_name: product_line
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: TEXT
-  verbose_name: null
- advanced_data_type: null
-  column_name: state
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: TEXT
-  verbose_name: null
- advanced_data_type: null
-  column_name: status
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: TEXT
-  verbose_name: null
- advanced_data_type: null
-  column_name: city
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: TEXT
-  verbose_name: null
- advanced_data_type: null
-  column_name: country
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: TEXT
-  verbose_name: null
- advanced_data_type: null
-  column_name: phone
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: TEXT
-  verbose_name: null
- advanced_data_type: null
-  column_name: territory
-  description: null
-  expression: null
-  extra: null
-  filterable: true
-  groupby: true
-  is_active: true
-  is_dttm: false
-  python_date_format: null
-  type: TEXT
-  verbose_name: null
-data_file: cleaned_sales_data.parquet
-database_uuid: a2dc77af-e654-49bb-b321-40f6b559a1ee
-default_endpoint: null
-description: null
-extra: null
-fetch_values_predicate: null
-filter_select_enabled: true
-folders: null
-main_dttm_col: order_date
-metrics:
- currency: null
-  d3format: null
-  description: null
-  expression: COUNT(*)
-  extra: null
-  metric_name: count
-  metric_type: count
-  verbose_name: COUNT(*)
-  warning_text: null
-normalize_columns: false
-offset: 0
-params: null
-schema: null
-sql: null
-table_name: cleaned_sales_data
-template_params: null
-uuid: e8623bb9-5e00-f531-506a-19607f5f8005
-version: 1.0.0
@@ -150,14 +150,23 @@ def load_parquet_table(  # noqa: C901
                except Exception as e:
                    logger.warning("Could not process column %s: %s", col, e)

-        # Write to target database
+        # Write to target database. Scale the row chunksize down for wide
+        # tables so a single batch's bound-parameter count (rows * columns)
+        # stays under stock SQLite's default SQLITE_MAX_VARIABLE_NUMBER of
+        # 32766 -- a flat chunksize=500 on a 328-column table generates
+        # ~164k params per batch, which only some builds raise the limit
+        # for (e.g. Debian's SQLite package). A right-sized batch also
+        # plans faster than an oversized one.
+        num_cols = max(len(pdf.columns), 1)
+        chunksize = max(50, min(500, 30_000 // num_cols))
+
        with database.get_sqla_engine() as engine:
            pdf.to_sql(
                table_name,
                engine,
                schema=schema,
                if_exists="replace",
-                chunksize=500,
+                chunksize=chunksize,
                method="multi",
                index=False,
            )
@@ -735,6 +735,66 @@ class BaseSQLStatement(Generic[InternalRepresentation]):
        return self.format()


+_SELECT_TRAILING_CLAUSES: tuple[str, ...] = (
+    "options",
+    "settings",
+    "format",
+    "locks",
+    "offset",
+    "limit",
+    "sort",
+    "cluster",
+    "distribute",
+    "order",
+    "windows",
+    "qualify",
+    "having",
+    "group",
+    "where",
+    "joins",
+    "laterals",
+    "from",
+    "into",
+    "expressions",
+)
+
+
+def _get_select_trailing_child(node: exp.Select) -> exp.Expression | None:
+    for clause_name in _SELECT_TRAILING_CLAUSES:
+        val = node.args.get(clause_name)
+        if isinstance(val, list) and val:
+            return _find_last_token_node(val[-1])
+        if isinstance(val, exp.Expression):
+            return _find_last_token_node(val)
+    return None
+
+
+def _find_last_token_node(node: exp.Expression) -> exp.Expression:
+    """
+    Find the last token/leaf node in SQL generation order to attach trailing comments.
+
+    Avoids optimizer hints (exp.Hint) and non-trailing subtrees to prevent injecting
+    trailing comments inside optimizer hint blocks (e.g. /*+ SET_VAR(...) */).
+    """
+    if isinstance(node, exp.Select):
+        if trailing := _get_select_trailing_child(node):
+            return trailing
+
+    children: list[exp.Expression] = []
+    for k, v in node.args.items():
+        if k in ("hint", "comments"):
+            continue
+        if isinstance(v, exp.Expression):
+            children.append(v)
+        elif isinstance(v, list):
+            children.extend(item for item in v if isinstance(item, exp.Expression))
+
+    if children:
+        return _find_last_token_node(children[-1])
+
+    return node
+
+
 class SQLStatement(BaseSQLStatement[exp.Expression]):
    """
    A SQL statement.
@@ -932,11 +992,7 @@ class SQLStatement(BaseSQLStatement[exp.Expression]):
        # statement; move them back to the last token in the last real statement
        if len(statements) > 1 and isinstance(statements[-1], exp.Semicolon):
            last_statement = statements.pop()
-            target = statements[-1]
-            for node in statements[-1].walk():
-                if hasattr(node, "comments"):  # pragma: no cover
-                    target = node
-
+            target = _find_last_token_node(statements[-1])
            target.comments = target.comments or []
            target.comments.extend(last_statement.comments)

@@ -39,7 +39,11 @@ from superset import (
    security_manager,
 )
 from superset.common.db_query_status import QueryStatus
-from superset.constants import QUERY_CANCEL_KEY, QUERY_EARLY_CANCEL_KEY
+from superset.constants import (
+    QUERY_CANCEL_KEY,
+    QUERY_DISPATCHED_KEY,
+    QUERY_EARLY_CANCEL_KEY,
+)
 from superset.dataframe import df_to_records
 from superset.db_engine_specs import BaseEngineSpec
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
@@ -99,6 +103,39 @@ def handle_query_error(
 ) -> dict[str, Any]:
    """Local method handling error while processing the SQL"""
    payload = payload or {}
+
+    # A stop request may have already committed STOPPED status while this
+    # exception was being raised/propagated -- this function is the general
+    # catch-all for failures anywhere in execute_sql_statements (connection
+    # setup, cancel-ID acquisition, parsing, or a per-block failure), not
+    # just ones caused by the stop itself. A terminal stop must stay
+    # terminal, so don't let an unrelated error overwrite it with FAILED.
+    #
+    # Deliberately NOT a flush()-then-refresh(query) here, unlike the other
+    # STOPPED-preservation checks in this module: the exception that got us
+    # here may itself have already set query.status (or other attributes)
+    # locally (e.g. SoftTimeLimitExceeded's own handler sets TIMED_OUT
+    # without committing). Flushing first would push that stale local state
+    # to the DB, clobbering a concurrently-committed STOPPED before this
+    # check ever gets to observe it.
+    #
+    # A targeted refresh(attribute_names=["status"]) alone isn't enough:
+    # verified empirically that even though it expires and reloads only the
+    # named attribute (so a dirty `status` itself is correctly discarded
+    # rather than written), the reload's own SELECT still triggers a normal
+    # autoflush of any OTHER dirty attribute on the session first -- e.g. a
+    # pending query.tmp_table_name or query.executed_sql set earlier would
+    # still get written before the status read. no_autoflush suppresses
+    # that: verified it emits only the targeted SELECT, with no UPDATE
+    # beforehand, and leaves other pending attributes exactly as dirty as
+    # they were (to be flushed normally by this function's own commit()
+    # below, once we're past the STOPPED check).
+    with db.session.no_autoflush:
+        db.session.refresh(query, attribute_names=["status"])
+    if query.status == QueryStatus.STOPPED:
+        payload.update({"status": query.status})
+        return payload
+
    msg = f"{prefix_message} {str(ex)}".strip()
    query.error_message = msg
    query.tmp_table_name = None
@@ -412,6 +449,21 @@ def execute_sql_statements(  # noqa: C901

    query = get_query(query_id=query_id)
    payload: dict[str, Any] = {"query_id": query_id}
+
+    # A stop request may have landed before this worker even started (e.g.
+    # the request was queued and the user clicked Stop before a worker
+    # picked it up). Honor it here, mirroring the per-block stopped-check
+    # further down, instead of unconditionally overwriting it back to
+    # RUNNING and dispatching the statement anyway.
+    #
+    # Same disclosed, unfixed TOCTOU residual as the other status checks in
+    # this function (see the longer comment above the pre-payload check
+    # further down): a stop committed strictly between this check and the
+    # `query.status = RUNNING` commit a few lines below is still missed.
+    if query.status == QueryStatus.STOPPED:
+        payload.update({"status": query.status})
+        return payload
+
    database = query.database
    db_engine_spec = database.db_engine_spec
    db_engine_spec.patch()
@@ -509,9 +561,14 @@ def execute_sql_statements(  # noqa: C901
        cursor = conn.cursor()

        cancel_query_id = db_engine_spec.get_cancel_query_id(cursor, query)
+        # Recorded unconditionally -- even when no cancel ID comes back --
+        # so cancel_query() can tell "hasn't reached the engine yet" (still
+        # safe to fabricate a stop) apart from "this engine has no cancel
+        # support" (must fail honestly) once we get here.
+        query.set_extra_json_key(QUERY_DISPATCHED_KEY, True)
        if cancel_query_id is not None:
            query.set_extra_json_key(QUERY_CANCEL_KEY, cancel_query_id)
-            db.session.commit()
+        db.session.commit()

        block_count = len(blocks)
        for i, block in enumerate(blocks):
@@ -564,6 +621,41 @@ def execute_sql_statements(  # noqa: C901
        if parsed_script.has_mutation() or query.select_as_cta:
            conn.commit()

+    # A stop request may have landed after the last per-block check but
+    # before the final statement finished (there's no next iteration to
+    # catch it on for the last block). Check again before building a SUCCESS
+    # payload or writing results to the backend -- both would otherwise
+    # disagree with the row. The results-backend-write-failure branch below
+    # has its own second check for the same reason (a stop landing while
+    # that specific write is in flight).
+    #
+    # KNOWN, DELIBERATELY UNFIXED RESIDUAL: this codebase has no DB-level
+    # locking, so every "check status, then later commit something based on
+    # what was read" pattern in this function -- this one, the
+    # results-backend-write-failure check below, the startup check before
+    # `query.status = RUNNING` is committed a few lines later, and
+    # cancel_query()'s own QUERY_DISPATCHED_KEY read/commit gap (see the
+    # disclosure comment there) -- has the same fundamental TOCTOU window: a
+    # stop committed strictly between the check and the later commit is
+    # still missed. Each check narrows its window as much as reasonably
+    # possible without locking; none of them claim to close it. Closing any
+    # of them for real needs real DB-level row locking (e.g.
+    # SELECT ... FOR UPDATE) or optimistic-concurrency versioning on the
+    # query row, neither of which is meaningfully verifiable against the
+    # sqlite backend this codebase tests against, and is deliberately not
+    # attempted here.
+    #
+    # flush() first: refresh() does NOT autoflush -- without this, any
+    # pending, uncommitted attribute set earlier in this iteration (e.g.
+    # query.executed_sql, set just before execute_query() ran) would be
+    # silently discarded and reloaded back to its previous committed value
+    # instead of surviving to the function's own later commits.
+    db.session.flush()
+    db.session.refresh(query)
+    if query.status == QueryStatus.STOPPED:
+        payload.update({"status": query.status})
+        return payload
+
    # Success, updating the query entry in database
    query.rows = result_set.size
    query.progress = 100
@@ -652,6 +744,36 @@ def execute_sql_statements(  # noqa: C901
                # For async queries (not returning results inline), mark as FAILED
                # because results are inaccessible to the user
                if not return_results:
+                    # A stop request may have landed and committed STOPPED
+                    # while this (potentially slow) results-backend write was
+                    # in flight. Refresh before marking FAILED -- a terminal
+                    # STOPPED must stay terminal, not be overwritten just
+                    # because the backend write also failed to complete
+                    # around the same time.
+                    #
+                    # flush() first: refresh() does NOT autoflush -- without
+                    # this, the result metadata already set earlier in this
+                    # function (rows, progress, extra "columns", select_sql,
+                    # end_time) plus the results_key = None set just above
+                    # would be silently discarded and reloaded back to their
+                    # previous (pre-execution) values instead of surviving to
+                    # this branch's own commit below.
+                    db.session.flush()
+                    db.session.refresh(query)
+                    if query.status == QueryStatus.STOPPED:
+                        # A fresh, minimal payload -- not `payload.update()`.
+                        # By this point `payload` already has the full
+                        # SUCCESS shape baked in from earlier (result data, a
+                        # nested query["state"] == SUCCESS, and a resultsKey
+                        # for a write that just failed), so patching only the
+                        # top-level "status" key would return a payload that
+                        # simultaneously claims STOPPED while still carrying
+                        # SUCCESS data and a resultsKey pointing at nothing
+                        # actually stored. Matches the shape the other
+                        # STOPPED-preservation return sites in this function
+                        # use (a plain {"query_id", "status"} pair).
+                        return {"query_id": query_id, "status": query.status}
+
                    query.status = QueryStatus.FAILED
                    query.error_message = (
                        "Failed to store query results in the results backend. "
@@ -676,8 +798,24 @@ def execute_sql_statements(  # noqa: C901
                    key,
                )

-    # Only set SUCCESS if we didn't already set FAILED above
-    if query.status != QueryStatus.FAILED:
+    # Only set SUCCESS if we didn't already set FAILED above, and don't
+    # clobber a STOPPED status a concurrent stop request may have committed
+    # since the check above -- a terminal stop must stay terminal. This is a
+    # backstop for the DB row specifically (the payload/results-write
+    # consistency check already happened above); it doesn't reopen or
+    # re-narrow the same disclosed race window from that check.
+    #
+    # flush() first: refresh() does NOT autoflush -- without this, every
+    # result field set on the success path above (rows, progress, extra
+    # "columns", select_sql, end_time, results_key) would be silently
+    # discarded and reloaded back to their pre-execution (typically None)
+    # values on EVERY successful query, since nothing before this point
+    # commits them. This was a real regression caught by CI integration
+    # tests across all three DB backends (sqlite/mysql/postgres) that the
+    # unit-test suite driving this fix never exercised.
+    db.session.flush()
+    db.session.refresh(query)
+    if query.status not in (QueryStatus.FAILED, QueryStatus.STOPPED):
        query.status = QueryStatus.SUCCESS
    db.session.commit()

@@ -747,7 +885,41 @@ def cancel_query(query: Query) -> bool:

    cancel_query_id = query.extra.get(QUERY_CANCEL_KEY)
    if cancel_query_id is None:
-        return False
+        # KNOWN LIMITATION (deliberately not fixed here): this read of
+        # QUERY_DISPATCHED_KEY and execute_sql_statements()'s own commit of
+        # that same flag (see the "Recorded unconditionally" comment where
+        # it's set) are two independent transactions with no lock between
+        # them. A stop request can still land in the narrow window where
+        # this read has already happened -- deciding "not dispatched yet,
+        # safe to fabricate a stop" -- but the worker's dispatch commit
+        # lands immediately after, so the statement still gets sent to the
+        # engine even though the row was just marked STOPPED. Closing this
+        # for real needs DB-level row locking (e.g. SELECT ... FOR UPDATE)
+        # or optimistic-concurrency versioning on the query row; neither is
+        # meaningfully verifiable against the sqlite backend this codebase's
+        # tests run against, so it's out of scope here rather than a
+        # false claim of safety.
+        if query.extra.get(QUERY_DISPATCHED_KEY):
+            # execute_sql_statements() already opened a connection and asked
+            # this engine spec for a cancel handle, and still got nothing --
+            # this engine genuinely has no way to cancel a query once it's
+            # running. That's a real failure, not a race window; report it
+            # honestly rather than fabricating a stop the engine can't back.
+            return False
+        # No cancel handle has been recorded and execution hasn't reached the
+        # engine yet, so "no ID" here can only mean "too early to have one" --
+        # record the same early-cancel intent Trino's own
+        # prepare_cancel_query() records for its harder case (ID only
+        # obtainable after execution starts), so the stopped check at the top
+        # of the statement-block loop honors the request instead of leaving
+        # the query stuck at RUNNING with no avenue to ever stop it.
+        #
+        # Not committed here: the caller (QueryDAO.stop_query) commits this
+        # together with status=STOPPED in one transaction, so another
+        # request can never observe the flag set but the status still
+        # RUNNING.
+        query.set_extra_json_key(QUERY_EARLY_CANCEL_KEY, True)
+        return True

    with query.database.get_sqla_engine(
        catalog=query.catalog,
@@ -296,7 +296,14 @@ def _stub_run_environment(mocker: MockerFixture) -> MagicMock:
    )
    db_mock = mocker.patch("superset.commands.database.uploaders.base.db")
    # No visible dataset over the target table.
+    db_mock.session.query.return_value.filter.return_value.one_or_none.return_value = (
+        None
+    )
    db_mock.session.query.return_value.filter_by.return_value.one_or_none.return_value = None  # noqa: E501
+    mocker.patch(
+        "superset.commands.database.uploaders.base.or_",
+        side_effect=lambda *args: mocker.MagicMock(),
+    )
    return model


@@ -368,3 +375,57 @@ def test_run_proceeds_when_no_soft_deleted_twin(
    )
    command.run()
    reader.read.assert_called_once()
+
+
+def test_run_sets_default_catalog_on_dataset_creation(
+    app_context: None, mocker: MockerFixture
+) -> None:
+    """UploadCommand sets default catalog on newly created dataset."""
+    model = _stub_run_environment(mocker)
+    model.get_default_catalog.return_value = "default_catalog"
+    mocker.patch(
+        "superset.daos.dataset.DatasetDAO.find_soft_deleted_logical_duplicate",
+        return_value=None,
+    )
+    sqla_table_mock = mocker.patch(
+        "superset.commands.database.uploaders.base.SqlaTable",
+        return_value=MagicMock(),
+    )
+    mocker.patch(
+        "superset.commands.database.uploaders.base.get_user",
+        return_value=None,
+    )
+
+    reader = MagicMock()
+    command = UploadCommand(
+        model_id=1, table_name="t", file=_file(b"x"), schema="public", reader=reader
+    )
+    command.run()
+
+    sqla_table_mock.assert_called_once()
+    assert sqla_table_mock.call_args.kwargs.get("catalog") == "default_catalog"
+
+
+def test_run_updates_catalog_on_existing_dataset_with_none_catalog(
+    app_context: None, mocker: MockerFixture
+) -> None:
+    """UploadCommand updates catalog on an existing dataset if catalog was None."""
+    model = _stub_run_environment(mocker)
+    model.get_default_catalog.return_value = "default_catalog"
+
+    existing_table = MagicMock()
+    existing_table.catalog = None
+
+    db_mock = mocker.patch("superset.commands.database.uploaders.base.db")
+    db_mock.session.query.return_value.filter.return_value.one_or_none.return_value = (
+        existing_table
+    )
+
+    reader = MagicMock()
+    command = UploadCommand(
+        model_id=1, table_name="t", file=_file(b"x"), schema="public", reader=reader
+    )
+    command.run()
+
+    assert existing_table.catalog == "default_catalog"
+    existing_table.fetch_metadata.assert_called_once()
@@ -348,6 +348,71 @@ def test_import_passes_ignore_permissions_to_all_importers(
    assert mock_import_dashboard.call_args[1].get("ignore_permissions") is True


+@patch(
+    "superset.commands.importers.v1.examples.safe_insert_dashboard_chart_relationships"
+)
+@patch("superset.commands.importers.v1.examples.import_dataset")
+@patch("superset.commands.importers.v1.examples.import_database")
+def test_import_dedupes_datasets_with_same_uuid(
+    mock_import_db,
+    mock_import_dataset,
+    mock_safe_insert,
+):
+    """_import() must import a given dataset uuid at most once per run.
+
+    Two example folders can ship a dataset config for the same
+    underlying table with an identical uuid (e.g. "world_health" and
+    "misc_charts" both shipping a config for "wb_health_population").
+    Importing it twice repeats the same column/metric sync for no
+    benefit.
+    """
+    from superset.commands.importers.v1.examples import ImportExamplesCommand
+
+    db_uuid = "a2dc77af-e654-49bb-b321-40f6b559a1ee"
+    dataset_uuid = "69e9de42-fe7f-4948-946a-f7913227aee8"
+
+    mock_db_obj = MagicMock()
+    mock_db_obj.uuid = db_uuid
+    mock_db_obj.id = 1
+    mock_import_db.return_value = mock_db_obj
+
+    mock_dataset_obj = MagicMock()
+    mock_dataset_obj.uuid = dataset_uuid
+    mock_dataset_obj.id = 10
+    mock_dataset_obj.table_name = "wb_health_population"
+    mock_import_dataset.return_value = mock_dataset_obj
+
+    configs = {
+        "databases/examples.yaml": {
+            "uuid": db_uuid,
+            "database_name": "examples",
+            "sqlalchemy_uri": "sqlite:///test.db",
+        },
+        "datasets/examples/world_health.yaml": {
+            "uuid": dataset_uuid,
+            "table_name": "wb_health_population",
+            "database_uuid": db_uuid,
+            "schema": None,
+            "sql": None,
+        },
+        "datasets/examples/wb_health_population.yaml": {
+            "uuid": dataset_uuid,
+            "table_name": "wb_health_population",
+            "database_uuid": db_uuid,
+            "schema": None,
+            "sql": None,
+        },
+    }
+
+    with patch(
+        "superset.commands.importers.v1.examples.get_example_default_schema",
+        return_value=None,
+    ):
+        ImportExamplesCommand._import(configs)
+
+    mock_import_dataset.assert_called_once()
+
+
 def test_normalize_dataset_schema_converts_main_to_null():
    """SQLite 'main' schema must be normalized to null in YAML content.

@@ -146,6 +146,11 @@ def test_query_dao_stop_query_not_found(

    db.session.add(database)
    db.session.add(query_obj)
+    # Committed (not just autoflushed) since QueryDAO.stop_query() is now
+    # wrapped in @transaction, which rolls back the session on the
+    # QueryNotFoundException raised below -- an uncommitted insert would be
+    # discarded along with it.
+    db.session.commit()

    mocker.patch("superset.sql_lab.cancel_query", return_value=False)

@@ -228,6 +233,11 @@ def test_query_dao_stop_query_failed(

    db.session.add(database)
    db.session.add(query_obj)
+    # Committed (not just autoflushed) since QueryDAO.stop_query() is now
+    # wrapped in @transaction, which rolls back the session on the
+    # SupersetCancelQueryException raised below -- an uncommitted insert
+    # would be discarded along with it.
+    db.session.commit()

    mocker.patch("superset.sql_lab.cancel_query", return_value=False)

@@ -314,6 +324,11 @@ def test_query_dao_stop_query_wrong_user(

    db.session.add(database)
    db.session.add(query_obj)
+    # Committed (not just autoflushed) since QueryDAO.stop_query() is now
+    # wrapped in @transaction, which rolls back the session on the
+    # QueryNotFoundException raised below -- an uncommitted insert would be
+    # discarded along with it.
+    db.session.commit()

    # Simulate a different user (user 2) attempting to stop user 1's query
    mocker.patch("superset.daos.query.get_user_id", return_value=2)
@@ -29,6 +29,8 @@ from superset.jinja_context import JinjaTemplateProcessor
 from superset.sql.parse import (
    _check_script_length,
    _count_weighted_table_references,
+    _find_last_token_node,
+    _get_select_trailing_child,
    BaseSQLStatement,
    count_referenced_tables,
    CTASMethod,
@@ -1358,20 +1360,12 @@ LIMIT 100
    assert "increase timeout for large scans" in formatted[hint_end:]


-@pytest.mark.xfail(
-    reason=(
-        "#38189 is not fully fixed: a `;`-terminated statement still hits "
-        "the comment-relocation branch and corrupts the hint block. Only "
-        "the no-semicolon form from the original repro was fixed."
-    ),
-    strict=True,
-)
 def test_sqlscript_format_preserves_optimizer_hint_block_with_semicolon() -> None:
    """
    Same as `test_sqlscript_format_preserves_optimizer_hint_block`, but with
-    a terminating `;` on the statement -- this still reproduces #38189: the
-    trailing `--` comment gets injected inside the `/*+ SET_VAR(...) */`
-    hint block, corrupting it for StarRocks/MySQL-style engines.
+    a terminating `;` on the statement -- verifies #38189 fix so that trailing
+    `--` comments land after the statement rather than injected into the
+    `/*+ SET_VAR(...) */` hint block for StarRocks/MySQL-style engines.
    """
    sql = """SELECT /*+ SET_VAR(query_timeout = 3000) */ col1, col2
 FROM my_table
@@ -1388,6 +1382,61 @@ LIMIT 100;
    assert "increase timeout for large scans" in formatted[hint_end:]


+def test_sqlscript_format_preserves_optimizer_hint_with_cte_and_semicolon() -> None:
+    """
+    Ensure optimizer hints with CTEs and trailing comments survive formatting intact.
+    """
+    sql = """WITH cte AS (SELECT 1 AS id)
+SELECT /*+ SET_VAR(query_timeout = 3000) */ id
+FROM cte
+WHERE id = 1;
+
+-- trailing explanation comment"""
+    statement = SQLScript(sql, "mysql").statements[0]
+    formatted = statement.format()
+
+    hint = "/*+ SET_VAR(query_timeout = 3000) */"
+    assert hint in formatted
+    assert "SET_VAR(query_timeout /*" not in formatted
+    hint_end = formatted.index(hint) + len(hint)
+    assert "trailing explanation comment" in formatted[hint_end:]
+
+
+def test_find_last_token_node_branches() -> None:
+    """
+    Directly test all branches of _find_last_token_node and _get_select_trailing_child.
+    """
+    # 1. Empty select returns None from _get_select_trailing_child
+    # and falls back to node
+    empty_select = exp.Select()
+    assert _get_select_trailing_child(empty_select) is None
+    assert _find_last_token_node(empty_select) is empty_select
+
+    # 2. Select with list clause vs single Expression clause
+    select_with_exprs = exp.Select(expressions=[exp.Literal.number(1)])
+    assert _get_select_trailing_child(select_with_exprs) == exp.Literal.number(1)
+
+    select_with_where = exp.Select(where=exp.Where(this=exp.Literal.number(2)))
+    assert _get_select_trailing_child(select_with_where) == exp.Literal.number(2)
+
+    # 3. Node with hint or comments in args is skipped during child traversal
+    col_with_comment = exp.Column(this="foo", comments=["my comment"])
+    assert _find_last_token_node(col_with_comment) is not None
+
+    table_with_hint = exp.Table(
+        this="bar", hint=exp.Hint(expressions=[exp.var("HINT")])
+    )
+    assert _find_last_token_node(table_with_hint) is not None
+
+    # 4. Non-select node with list of expressions
+    tup = exp.Tuple(expressions=[exp.Literal.number(1), exp.Literal.number(2)])
+    assert _find_last_token_node(tup) == exp.Literal.number(2)
+
+    # 5. Leaf node with no children returns itself
+    lit = exp.Literal.number(42)
+    assert _find_last_token_node(lit) is lit
+
+
@pytest.mark.parametrize(
    "sql, engine, expected",
    [
@@ -483,6 +483,11 @@ def test_get_sql_results_oauth2(mocker: MockerFixture, app) -> None:
        mocker.patch("superset.daos.key_value.KeyValueDAO.delete_expired_entries")
        mocker.patch("superset.daos.key_value.KeyValueDAO.create_entry")
        mocker.patch("superset.db_engine_specs.base.db.session.commit")
+        # handle_query_error() refreshes `query` from the DB to check for a
+        # concurrently-committed STOPPED status before overwriting it with
+        # FAILED; `query` here is a MagicMock, not a real persistent ORM
+        # instance, so the real refresh() would error introspecting it.
+        mocker.patch("superset.sql_lab.db.session.refresh", return_value=None)

        g = mocker.patch("superset.db_engine_specs.base.g")
        g.user = mocker.MagicMock()
Author	SHA1	Message	Date
Claude Code	58b16326e9	fix(chromatic): emit webpack stats so TurboSnap can trace story deps Second live-test failure on #44111: after fixing the E2BIG git-log crash, the publish step got further but then failed with "Failed to retrieve dependent story files" / "Could not retrieve dependent story files." TurboSnap (onlyChanged) needs a webpack stats.json to build the module dependency graph it uses to trace which stories a changed file affects. Normally chromaui/action builds Storybook itself and can inject stats collection; since we hand it an already-built storybookBuildDir instead (to reuse the shared Docker image rather than installing Node on the bare runner), that stats file has to already exist in the build output. Add --webpack-stats-json to the build-storybook invocation so it does. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>	2026-09-09 12:16:39 -07:00
Claude Code	04d7596974	fix(chromatic): bound fetch-depth to avoid E2BIG on a dormant project Live-tested this workflow via the draft PR stacked on it (#44111) and hit a real failure: chromaui/action's "Retrieving git information failed" / "Command failed with E2BIG: git log ...", passing hundreds of full commit SHAs as individual CLI arguments until it blew past the OS's ARG_MAX. Root cause (confirmed against chromaui/chromatic-cli#432, filed by another user hitting the identical error on a different large repo): with fetch-depth: 0, Chromatic's CLI walks git history back to the last build it knows about for this project. This project's last known build predates its ~2-year dormancy (shipped in #21095, removed in #27232) -- so on this very first run, it tries to walk back through literally every commit since then on a large, very active repo, and chokes passing that whole list to git log as arguments. The Chromatic team's own recommended workaround on that issue is exactly this: bound the fetch depth, since a baseline that old isn't useful anyway ("we should probably just bail out ... and warn the user that we are running a build with no baseline"). 500 is far more than any realistic PR needs once a recent baseline exists, which it will after this workflow's own first successful run against master. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>	2026-09-09 12:06:38 -07:00
Claude Code	9eda4c6977	ci(chromatic): add workflow_dispatch for manual runs Also lets this be triggered manually to verify the actual publish step end-to-end on a branch, since the path-filtered push/pull_request triggers won't fire on a PR that only touches this workflow file itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>	2026-09-09 11:17:15 -07:00
Claude Code	c56d46adc0	feat(ci): add Chromatic visual regression testing for Storybook Publishes superset-frontend's Storybook to Chromatic on pushes to master and on pull requests touching superset-frontend, using the existing CHROMATIC_PROJECT_TOKEN repo secret. Chromatic was implemented here once before (#21095, 2021) and removed in #27232 (Feb 2024) for being unmaintained and overlapping with Applitools, which has since also been discontinued (no live references left, only historical CHANGELOG mentions) -- so that overlap no longer applies. Design choices, to keep this from repeating the prior setup's history of "fix:"/"remove, it has errors" churn: - Fork PRs don't receive CHROMATIC_PROJECT_TOKEN (GitHub withholds repo secrets from pull_request runs triggered by a fork); the publish steps guard on CHROMATIC_PROJECT_TOKEN != '' and simply no-op for them rather than failing, following the same env-var-gate pattern frontend-bundle-size-nightly.yml already uses for an optional secret. - Non-blocking for now (exitZeroOnChanges: true): visual changes show up as a PR check + comment for review, without gating merges. Can flip to a required check once the team trusts the signal. - TurboSnap enabled (onlyChanged: true) to keep runs fast against this monorepo's large chart-plugin story set; fetch-depth: 0 on checkout so it has the git history it needs to diff against the baseline. - Reuses the existing superset-node-ci Docker target and the build-then-mount-output pattern frontend-bundle-size-nightly.yml already established, rather than installing Node directly on the runner. chromaui/action is already wildcard-allowlisted in the ASF allowlist (chromaui/action@*), so this doesn't touch that check. Verified locally: pre-commit (zizmor) passes, actionlint passes (aside from a stale-label false positive on ubuntu-26.04 and $TAG-quoting notices that are already pervasive throughout the rest of this repo's workflows), and the real check_asf_allowlist.py script accepts the new chromaui/action ref. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>	2026-09-09 11:15:06 -07:00
Ankit	e0103fa889	fix(plugin-chart-echarts): honor per-metric formats in Timeseries tooltips (#43113 )	2026-09-09 10:56:59 -07:00
Francesco.CastaldiandFrancescoCastaldi	b1185d8c05	fix(sql): preserve optimizer hints when formatting semicolon-terminated statements with trailing comments (#43565 ) Co-authored-by: FrancescoCastaldi <francesco.castaldi@mapsgroup.it>	2026-09-09 10:44:01 -07:00
Francesco.CastaldiandFrancescoCastaldi	29dd683f80	fix(database): set default catalog on dataset created from file uploads (#43586 ) Co-authored-by: FrancescoCastaldi <francesco.castaldi@mapsgroup.it>	2026-09-09 10:43:41 -07:00
Joe LiandClaude Fable 5.1	70346a51b6	perf(examples): dedupe dataset imports, right-size chunksize, add timing to load_examples (#43907 ) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>	2026-09-09 10:16:01 -07:00
Joe LiandClaude Sonnet 5	0534d569d0	fix(sqllab): mark early-cancelled query stopped, not stuck running (#43889 ) Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>	2026-09-09 09:47:58 -07:00
Joe Li	f66843ef0c	fix(plugin-chart-table): stop narrowing sticky header/footer width (#43937 )	2026-09-09 09:47:27 -07:00
Balaji Madhan	078915f4ce	docs(contributing): update legacy rst reference (#44087 )	2026-09-09 23:20:02 +07:00