feat: generate consistent QueryObject whether GenericAxis is enabled or disabled (#21519)

2026-04-20 00:24:38 +00:00 · 2022-09-21 09:41:21 +08:00
parent 6644a84f79
commit 4d12e3709e
22 changed files with 474 additions and 65 deletions
--- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts
@@ -18,12 +18,13 @@
 */
 import {
  buildQueryContext,
-  DTTM_ALIAS,
  ensureIsArray,
  normalizeOrderBy,
  PostProcessingPivot,
  QueryFormData,
  QueryObject,
+  getXAxis,
+  isXAxisSet,
 } from '@superset-ui/core';
 import {
  pivotOperator,
@@ -41,11 +42,8 @@ import {
 } from '../utils/formDataSuffix';

 export default function buildQuery(formData: QueryFormData) {
-  const { x_axis: index } = formData;
-  const is_timeseries = index === DTTM_ALIAS || !index;
  const baseFormData = {
    ...formData,
-    is_timeseries,
  };

  const formData1 = removeFormDataSuffix(baseFormData, '_b');
@@ -55,9 +53,12 @@ export default function buildQuery(formData: QueryFormData) {
    buildQueryContext(fd, baseQueryObject => {
      const queryObject = {
        ...baseQueryObject,
-        columns: [...ensureIsArray(index), ...ensureIsArray(fd.groupby)],
+        columns: [
+          ...(isXAxisSet(formData) ? ensureIsArray(getXAxis(formData)) : []),
+          ...ensureIsArray(fd.groupby),
+        ],
        series_columns: fd.groupby,
-        is_timeseries,
+        ...(isXAxisSet(formData) ? {} : { is_timeseries: true }),
      };

      const pivotOperatorInRuntime: PostProcessingPivot = isTimeComparison(
@@ -68,8 +69,6 @@ export default function buildQuery(formData: QueryFormData) {
        : pivotOperator(fd, {
            ...queryObject,
            columns: fd.groupby,
-            index,
-            is_timeseries,
          });

      const tmpQueryObject = {
@@ -83,7 +82,6 @@ export default function buildQuery(formData: QueryFormData) {
          renameOperator(fd, {
            ...queryObject,
            columns: fd.groupby,
-            is_timeseries,
          }),
          flattenOperator(fd, queryObject),
        ],
--- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
@@ -20,16 +20,16 @@
 import {
  AnnotationLayer,
  CategoricalColorNamespace,
-  DTTM_ALIAS,
  GenericDataType,
-  getColumnLabel,
  getNumberFormatter,
  isEventAnnotationLayer,
  isFormulaAnnotationLayer,
  isIntervalAnnotationLayer,
  isTimeseriesAnnotationLayer,
+  QueryFormData,
  TimeseriesChartDataResponseResult,
  TimeseriesDataRecord,
+  getXAxis,
 } from '@superset-ui/core';
 import { EChartsCoreOption, SeriesOption } from 'echarts';
 import {
@@ -152,8 +152,7 @@ export default function transformProps(

  const colorScale = CategoricalColorNamespace.getScale(colorScheme as string);

-  const xAxisCol =
-    verboseMap[xAxisOrig] || getColumnLabel(xAxisOrig || DTTM_ALIAS);
+  const xAxisCol = getXAxis(chartProps.rawFormData as QueryFormData) as string;

  const rebasedDataA = rebaseForecastDatum(data1, verboseMap);
  const rawSeriesA = extractSeries(rebasedDataA, {
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts
@@ -18,11 +18,12 @@
 */
 import {
  buildQueryContext,
-  DTTM_ALIAS,
  ensureIsArray,
  normalizeOrderBy,
  PostProcessingPivot,
  QueryFormData,
+  getXAxis,
+  isXAxisSet,
 } from '@superset-ui/core';
 import {
  rollingWindowOperator,
@@ -38,8 +39,7 @@ import {
 } from '@superset-ui/chart-controls';

 export default function buildQuery(formData: QueryFormData) {
-  const { x_axis, groupby } = formData;
-  const is_timeseries = x_axis === DTTM_ALIAS || !x_axis;
+  const { groupby } = formData;
  return buildQueryContext(formData, baseQueryObject => {
    /* the `pivotOperatorInRuntime` determines how to pivot the dataframe returned from the raw query.
       1. If it's a time compared query, there will return a pivoted dataframe that append time compared metrics. for instance:
@@ -66,18 +66,17 @@ export default function buildQuery(formData: QueryFormData) {
      baseQueryObject,
    )
      ? timeComparePivotOperator(formData, baseQueryObject)
-      : pivotOperator(formData, {
-          ...baseQueryObject,
-          index: x_axis,
-          is_timeseries,
-        });
+      : pivotOperator(formData, baseQueryObject);

    return [
      {
        ...baseQueryObject,
-        columns: [...ensureIsArray(x_axis), ...ensureIsArray(groupby)],
+        columns: [
+          ...(isXAxisSet(formData) ? ensureIsArray(getXAxis(formData)) : []),
+          ...ensureIsArray(groupby),
+        ],
        series_columns: groupby,
-        is_timeseries,
+        ...(isXAxisSet(formData) ? {} : { is_timeseries: true }),
        // todo: move `normalizeOrderBy to extractQueryFields`
        orderby: normalizeOrderBy(baseQueryObject).orderby,
        time_offsets: isTimeComparison(formData, baseQueryObject)
@@ -92,10 +91,7 @@ export default function buildQuery(formData: QueryFormData) {
          rollingWindowOperator(formData, baseQueryObject),
          timeCompareOperator(formData, baseQueryObject),
          resampleOperator(formData, baseQueryObject),
-          renameOperator(formData, {
-            ...baseQueryObject,
-            is_timeseries,
-          }),
+          renameOperator(formData, baseQueryObject),
          contributionOperator(formData, baseQueryObject),
          flattenOperator(formData, baseQueryObject),
          // todo: move prophet before flatten
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -28,9 +28,9 @@ import {
  isTimeseriesAnnotationLayer,
  TimeseriesChartDataResponseResult,
  t,
-  DTTM_ALIAS,
+  getXAxis,
 } from '@superset-ui/core';
-import { getAxis, isDerivedSeries } from '@superset-ui/chart-controls';
+import { isDerivedSeries } from '@superset-ui/chart-controls';
 import { EChartsCoreOption, SeriesOption } from 'echarts';
 import { ZRLineType } from 'echarts/types/src/util/types';
 import {
@@ -148,9 +148,7 @@ export default function transformProps(

  const colorScale = CategoricalColorNamespace.getScale(colorScheme as string);
  const rebasedData = rebaseForecastDatum(data, verboseMap);
-  // todo: if the both granularity_sqla and x_axis are `null`, should throw an error
-  const xAxisCol =
-    verboseMap[xAxisOrig] || getAxis(chartProps.rawFormData) || DTTM_ALIAS;
+  const xAxisCol = getXAxis(chartProps.rawFormData) as string;
  const isHorizontal = orientation === OrientationType.horizontal;
  const { totalStackedValues, thresholdValues } = extractDataTotalValues(
    rebasedData,
--- a/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts
@@ -267,7 +267,22 @@ test('should compile AA in query B', () => {
  });
 });

-test('should compile query objects with x-axis', () => {
+test('should convert a queryObject with x-axis although FF is disabled', () => {
+  let windowSpy: any;
+
+  beforeAll(() => {
+    // @ts-ignore
+    windowSpy = jest.spyOn(window, 'window', 'get').mockImplementation(() => ({
+      featureFlags: {
+        GENERIC_CHART_AXES: false,
+      },
+    }));
+  });
+
+  afterAll(() => {
+    windowSpy.mockRestore();
+  });
+
  const { queries } = buildQuery({
    ...formDataMixedChart,
    x_axis: 'my_index',
@@ -280,11 +295,19 @@ test('should compile query objects with x-axis', () => {
    filters: [],
    extras: {
      having: '',
-      time_grain_sqla: 'P1W',
      where: "(foo in ('a', 'b'))",
    },
    applied_time_extras: {},
-    columns: ['my_index', 'foo'],
+    columns: [
+      {
+        columnType: 'BASE_AXIS',
+        expressionType: 'SQL',
+        label: 'my_index',
+        sqlExpression: 'my_index',
+        timeGrain: 'P1W',
+      },
+      'foo',
+    ],
    metrics: ['sum(sales)'],
    annotation_layers: [],
    row_limit: 10,
@@ -296,7 +319,6 @@ test('should compile query objects with x-axis', () => {
    url_params: {},
    custom_params: {},
    custom_form_data: {},
-    is_timeseries: false,
    time_offsets: [],
    post_processing: [
      {
@@ -332,8 +354,16 @@ test('should compile query objects with x-axis', () => {
  // check the main props on the second query
  expect(queries[1]).toEqual(
    expect.objectContaining({
-      is_timeseries: false,
-      columns: ['my_index'],
+      columns: [
+        {
+          columnType: 'BASE_AXIS',
+          expressionType: 'SQL',
+          label: 'my_index',
+          sqlExpression: 'my_index',
+          timeGrain: 'P1W',
+        },
+      ],
+      granularity: 'ds',
      series_columns: [],
      metrics: ['count'],
      post_processing: [
@@ -357,3 +387,90 @@ test('should compile query objects with x-axis', () => {
    }),
  );
 });
+
+test("shouldn't convert a queryObject with axis although FF is enabled", () => {
+  let windowSpy: any;
+
+  beforeAll(() => {
+    // @ts-ignore
+    windowSpy = jest.spyOn(window, 'window', 'get').mockImplementation(() => ({
+      featureFlags: {
+        GENERIC_CHART_AXES: true,
+      },
+    }));
+  });
+
+  afterAll(() => {
+    windowSpy.mockRestore();
+  });
+
+  const { queries } = buildQuery(formDataMixedChart);
+  expect(queries[0]).toEqual(
+    expect.objectContaining({
+      granularity: 'ds',
+      columns: ['foo'],
+      series_columns: ['foo'],
+      metrics: ['sum(sales)'],
+      is_timeseries: true,
+      extras: {
+        time_grain_sqla: 'P1W',
+        having: '',
+        where: "(foo in ('a', 'b'))",
+      },
+      post_processing: [
+        {
+          operation: 'pivot',
+          options: {
+            aggregates: {
+              'sum(sales)': {
+                operator: 'mean',
+              },
+            },
+            columns: ['foo'],
+            drop_missing_columns: false,
+            index: ['__timestamp'],
+          },
+        },
+        {
+          operation: 'rename',
+          options: { columns: { 'sum(sales)': null }, inplace: true, level: 0 },
+        },
+        {
+          operation: 'flatten',
+        },
+      ],
+    }),
+  );
+  expect(queries[1]).toEqual(
+    expect.objectContaining({
+      granularity: 'ds',
+      columns: [],
+      series_columns: [],
+      metrics: ['count'],
+      is_timeseries: true,
+      extras: {
+        time_grain_sqla: 'P1W',
+        having: '',
+        where: "(name in ('c', 'd'))",
+      },
+      post_processing: [
+        {
+          operation: 'pivot',
+          options: {
+            aggregates: {
+              count: {
+                operator: 'mean',
+              },
+            },
+            columns: [],
+            drop_missing_columns: false,
+            index: ['__timestamp'],
+          },
+        },
+        {
+          operation: 'flatten',
+        },
+      ],
+    }),
+  );
+});
--- a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts
@@ -16,6 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
+import { SqlaFormData } from '@superset-ui/core';
 import buildQuery from '../../src/Timeseries/buildQuery';

 describe('Timeseries buildQuery', () => {
@@ -59,3 +60,181 @@ describe('Timeseries buildQuery', () => {
    expect(query.orderby).toEqual([['foo', true]]);
  });
 });
+
+describe('GENERIC_CHART_AXES is enabled', () => {
+  let windowSpy: any;
+
+  beforeAll(() => {
+    // @ts-ignore
+    windowSpy = jest.spyOn(window, 'window', 'get').mockImplementation(() => ({
+      featureFlags: {
+        GENERIC_CHART_AXES: true,
+      },
+    }));
+  });
+
+  afterAll(() => {
+    windowSpy.mockRestore();
+  });
+
+  const formData: SqlaFormData = {
+    datasource: '5__table',
+    viz_type: 'table',
+    granularity_sqla: 'time_column',
+    time_grain_sqla: 'P1Y',
+    time_range: '1 year ago : 2013',
+    groupby: ['col1'],
+    metrics: ['count(*)'],
+  };
+
+  it("shouldn't convert queryObject", () => {
+    const { queries } = buildQuery(formData);
+    expect(queries[0]).toEqual(
+      expect.objectContaining({
+        granularity: 'time_column',
+        time_range: '1 year ago : 2013',
+        extras: { time_grain_sqla: 'P1Y', having: '', where: '' },
+        columns: ['col1'],
+        series_columns: ['col1'],
+        metrics: ['count(*)'],
+        is_timeseries: true,
+        post_processing: [
+          {
+            operation: 'pivot',
+            options: {
+              aggregates: { 'count(*)': { operator: 'mean' } },
+              columns: ['col1'],
+              drop_missing_columns: true,
+              index: ['__timestamp'],
+            },
+          },
+          { operation: 'flatten' },
+        ],
+      }),
+    );
+  });
+
+  it('should convert queryObject', () => {
+    const { queries } = buildQuery({ ...formData, x_axis: 'time_column' });
+    expect(queries[0]).toEqual(
+      expect.objectContaining({
+        granularity: 'time_column',
+        time_range: '1 year ago : 2013',
+        extras: { having: '', where: '' },
+        columns: [
+          {
+            columnType: 'BASE_AXIS',
+            expressionType: 'SQL',
+            label: 'time_column',
+            sqlExpression: 'time_column',
+            timeGrain: 'P1Y',
+          },
+          'col1',
+        ],
+        series_columns: ['col1'],
+        metrics: ['count(*)'],
+        post_processing: [
+          {
+            operation: 'pivot',
+            options: {
+              aggregates: { 'count(*)': { operator: 'mean' } },
+              columns: ['col1'],
+              drop_missing_columns: true,
+              index: ['time_column'],
+            },
+          },
+          { operation: 'flatten' },
+        ],
+      }),
+    );
+  });
+});
+
+describe('GENERIC_CHART_AXES is disabled', () => {
+  let windowSpy: any;
+
+  beforeAll(() => {
+    // @ts-ignore
+    windowSpy = jest.spyOn(window, 'window', 'get').mockImplementation(() => ({
+      featureFlags: {
+        GENERIC_CHART_AXES: false,
+      },
+    }));
+  });
+
+  afterAll(() => {
+    windowSpy.mockRestore();
+  });
+
+  const formData: SqlaFormData = {
+    datasource: '5__table',
+    viz_type: 'table',
+    granularity_sqla: 'time_column',
+    time_grain_sqla: 'P1Y',
+    time_range: '1 year ago : 2013',
+    groupby: ['col1'],
+    metrics: ['count(*)'],
+  };
+
+  it("shouldn't convert queryObject", () => {
+    const { queries } = buildQuery(formData);
+    expect(queries[0]).toEqual(
+      expect.objectContaining({
+        granularity: 'time_column',
+        time_range: '1 year ago : 2013',
+        extras: { time_grain_sqla: 'P1Y', having: '', where: '' },
+        columns: ['col1'],
+        series_columns: ['col1'],
+        metrics: ['count(*)'],
+        is_timeseries: true,
+        post_processing: [
+          {
+            operation: 'pivot',
+            options: {
+              aggregates: { 'count(*)': { operator: 'mean' } },
+              columns: ['col1'],
+              drop_missing_columns: true,
+              index: ['__timestamp'],
+            },
+          },
+          { operation: 'flatten' },
+        ],
+      }),
+    );
+  });
+
+  it('should convert queryObject', () => {
+    const { queries } = buildQuery({ ...formData, x_axis: 'time_column' });
+    expect(queries[0]).toEqual(
+      expect.objectContaining({
+        granularity: 'time_column',
+        time_range: '1 year ago : 2013',
+        extras: { having: '', where: '' },
+        columns: [
+          {
+            columnType: 'BASE_AXIS',
+            expressionType: 'SQL',
+            label: 'time_column',
+            sqlExpression: 'time_column',
+            timeGrain: 'P1Y',
+          },
+          'col1',
+        ],
+        series_columns: ['col1'],
+        metrics: ['count(*)'],
+        post_processing: [
+          {
+            operation: 'pivot',
+            options: {
+              aggregates: { 'count(*)': { operator: 'mean' } },
+              columns: ['col1'],
+              drop_missing_columns: true,
+              index: ['time_column'],
+            },
+          },
+          { operation: 'flatten' },
+        ],
+      }),
+    );
+  });
+});