fix(core): don't add metrics to query object when in raw records mode (…

…#995)

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/constants.ts

@@ -17,6 +17,7 @@
  * under the License.
  */
 import { t } from '@superset-ui/core';
+import { ColumnMeta } from './types';
 
 // eslint-disable-next-line import/prefer-default-export
 export const TIME_FILTER_LABELS = {
@@ -26,3 +27,9 @@ export const TIME_FILTER_LABELS = {
   druid_time_origin: t('Origin'),
   granularity: t('Time Granularity'),
 };
+
+export const TIME_COLUMN_OPTION: ColumnMeta = {
+  verbose_name: t('Time'),
+  column_name: '__timestamp',
+  description: t('A reference to the [Time] configuration, taking granularity into account'),
+};

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx

@@ -19,12 +19,7 @@
  */
 import { t, validateNonEmpty } from '@superset-ui/core';
 import { ExtraControlProps, SharedControlConfig } from '../types';
-
-const timeColumnOption = {
-  verbose_name: t('Time'),
-  column_name: '__timestamp',
-  description: t('A reference to the [Time] configuration, taking granularity into account'),
-};
+import { TIME_COLUMN_OPTION } from '../constants';
 
 export const dndGroupByControl: SharedControlConfig<'DndColumnSelect'> = {
   type: 'DndColumnSelect',
@@ -36,7 +31,7 @@ export const dndGroupByControl: SharedControlConfig<'DndColumnSelect'> = {
     if (state.datasource) {
       const options = state.datasource.columns.filter(c => c.groupby);
       if (includeTime) {
-        options.unshift(timeColumnOption);
+        options.unshift(TIME_COLUMN_OPTION);
       }
       newState.options = Object.fromEntries(options.map(option => [option.column_name, option]));
     }

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/shared-controls/index.tsx

@@ -47,7 +47,7 @@ import {
 } from '@superset-ui/core';
 
 import { mainMetric, formatSelectOptions } from '../utils';
-import { TIME_FILTER_LABELS } from '../constants';
+import { TIME_FILTER_LABELS, TIME_COLUMN_OPTION } from '../constants';
 import {
   Metric,
   SharedControlConfig,
@@ -103,12 +103,6 @@ export const D3_TIME_FORMAT_OPTIONS = [
 
 export const D3_TIME_FORMAT_DOCS = t('D3 time format syntax: https://github.com/d3/d3-time-format');
 
-const timeColumnOption = {
-  verbose_name: t('Time'),
-  column_name: '__timestamp',
-  description: t('A reference to the [Time] configuration, taking granularity into account'),
-};
-
 type Control = {
   savedMetrics?: Metric[] | null;
   default?: unknown;
@@ -137,7 +131,7 @@ const groupByControl: SharedControlConfig<'SelectControl', ColumnMeta> = {
     if (state.datasource) {
       const options = state.datasource.columns.filter(c => c.groupby);
       if (includeTime) {
-        options.unshift(timeColumnOption);
+        options.unshift(TIME_COLUMN_OPTION);
       }
       newState.options = options;
     }

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/types.ts

@@ -18,7 +18,7 @@
  * under the License.
  */
 import React, { ReactNode, ReactText, ReactElement } from 'react';
-import { QueryFormData, DatasourceType, Metric, JsonValue } from '@superset-ui/core';
+import { QueryFormData, DatasourceType, Metric, JsonValue, Column } from '@superset-ui/core';
 import sharedControls from './shared-controls';
 import sharedControlComponents from './shared-controls/components';
 
@@ -38,16 +38,10 @@ export type SharedControlComponents = typeof sharedControlComponents;
 /** ----------------------------------------------
  * Input data/props while rendering
  * ---------------------------------------------*/
-export interface ColumnMeta extends AnyDict {
-  column_name: string;
-  groupby?: string;
-  verbose_name?: string;
-  description?: string;
-  expression?: string;
-  is_dttm?: boolean;
+export type ColumnMeta = Omit<Column, 'id' | 'type'> & {
+  id?: number;
   type?: string;
-  filterable?: boolean;
-}
+} & AnyDict;
 
 export interface DatasourceMeta {
   id: number;

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/query/extractQueryFields.ts

@@ -20,7 +20,15 @@ import { t } from '../translation';
 import { removeDuplicates } from '../utils';
 import { DTTM_ALIAS } from './buildQueryObject';
 import getMetricLabel from './getMetricLabel';
-import { QueryFields, QueryFieldAliases, FormDataResidual, QueryMode } from './types/QueryFormData';
+import {
+  QueryFields,
+  QueryFormColumn,
+  QueryFormMetric,
+  QueryFormOrderBy,
+  QueryFieldAliases,
+  FormDataResidual,
+  QueryMode,
+} from './types/QueryFormData';
 
 /**
  * Extra SQL query related fields from chart form data.
@@ -47,13 +55,12 @@ export default function extractQueryFields(
     order_by_cols: 'orderby',
     ...aliases,
   };
-  const finalQueryFields: QueryFields = {
-    columns: [],
-    metrics: [],
-    orderby: [],
-  };
   const { query_mode: queryMode, include_time: includeTime, ...restFormData } = formData;
 
+  let columns: QueryFormColumn[] = [];
+  let metrics: QueryFormMetric[] = [];
+  let orderby: QueryFormOrderBy[] = [];
+
   Object.entries(restFormData).forEach(([key, value]) => {
     // ignore `null` or `undefined` value
     if (value == null) {
@@ -62,14 +69,6 @@ export default function extractQueryFields(
 
     let normalizedKey: string = queryFieldAliases[key] || key;
 
-    // ignore groupby and metrics when in raw records mode
-    if (
-      queryMode === QueryMode.raw &&
-      (normalizedKey === 'groupby' || normalizedKey === 'metrics')
-    ) {
-      return;
-    }
-
     // ignore columns when (specifically) in aggregate mode.
     // For charts that support both aggregate and raw records mode,
     // we store both `groupby` and `columns` in `formData`, so users can
@@ -78,42 +77,50 @@ export default function extractQueryFields(
       return;
     }
 
+    // for the same reason, ignore groupby and metrics in raw records mode
+    if (
+      queryMode === QueryMode.raw &&
+      (normalizedKey === 'groupby' || normalizedKey === 'metrics')
+    ) {
+      return;
+    }
+
     // groupby has been deprecated in QueryObject: https://github.com/apache/superset/pull/9366
-    // We translate all `groupby` to `columns`.
     if (normalizedKey === 'groupby') {
       normalizedKey = 'columns';
     }
 
     if (normalizedKey === 'metrics') {
-      finalQueryFields[normalizedKey] = finalQueryFields[normalizedKey].concat(value);
+      metrics = metrics.concat(value);
     } else if (normalizedKey === 'columns') {
       // currently the columns field only accept pre-defined columns (string shortcut)
-      finalQueryFields[normalizedKey] = finalQueryFields[normalizedKey]
-        .concat(value)
-        .filter(x => typeof x === 'string' && x);
+      columns = columns.concat(value);
     } else if (normalizedKey === 'orderby') {
-      finalQueryFields[normalizedKey] = finalQueryFields[normalizedKey].concat(value).map(item => {
-        // value can be in the format of `['["col1", true]', '["col2", false]'],
-        // where the option strings come directly from `order_by_choices`.
-        if (typeof item === 'string') {
-          try {
-            return JSON.parse(item);
-          } catch (error) {
-            throw new Error(t('Found invalid orderby options'));
-          }
-        }
-        return item;
-      });
+      orderby = orderby.concat(value);
     }
   });
 
-  if (includeTime && !finalQueryFields.columns.includes(DTTM_ALIAS)) {
-    finalQueryFields.columns.unshift(DTTM_ALIAS);
+  if (includeTime && !columns.includes(DTTM_ALIAS)) {
+    columns.unshift(DTTM_ALIAS);
   }
 
-  // remove duplicate columns and metrics
-  finalQueryFields.columns = removeDuplicates(finalQueryFields.columns);
-  finalQueryFields.metrics = removeDuplicates(finalQueryFields.metrics, getMetricLabel);
-
-  return finalQueryFields;
+  return {
+    columns: removeDuplicates(columns.filter(x => typeof x === 'string' && x)),
+    metrics: queryMode === QueryMode.raw ? undefined : removeDuplicates(metrics, getMetricLabel),
+    orderby:
+      orderby.length > 0
+        ? orderby.map(item => {
+            // value can be in the format of `['["col1", true]', '["col2", false]'],
+            // where the option strings come directly from `order_by_choices`.
+            if (typeof item === 'string') {
+              try {
+                return JSON.parse(item);
+              } catch (error) {
+                throw new Error(t('Found invalid orderby options'));
+              }
+            }
+            return item;
+          })
+        : undefined,
+  };
 }

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/query/getMetricLabel.ts

@@ -8,7 +8,7 @@ export default function getMetricLabel(metric: QueryFormMetric) {
     return metric.label;
   }
   if (metric.expressionType === 'SIMPLE') {
-    return `${metric.aggregate}(${metric.column.columnName})`;
+    return `${metric.aggregate}(${metric.column.columnName || metric.column.column_name})`;
   }
   return metric.sqlExpression;
 }

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/query/types/Column.ts

@@ -41,7 +41,15 @@ export enum ColumnType {
 export interface Column {
   id: number;
   type: ColumnType;
-  columnName: string;
+  column_name: string;
+  groupby?: boolean;
+  is_dttm?: boolean;
+  filterable?: boolean;
+  verbose_name?: string | null;
+  description?: string | null;
+  expression?: string | null;
+  database_expression?: string | null;
+  python_date_format?: string | null;
 }
 
 export default {};

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/query/types/Metric.ts

@@ -29,7 +29,10 @@ export interface AdhocMetricBase {
 
 export interface AdhocMetricSimple extends AdhocMetricBase {
   expressionType: 'SIMPLE';
-  column: Column;
+  column: Omit<Column, 'column_name'> & {
+    column_name?: string;
+    columnName?: string;
+  };
   aggregate: Aggregate;
 }
 

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/query/types/QueryFormData.ts

@@ -61,8 +61,8 @@ export enum QueryMode {
  */
 export interface QueryFields {
   columns: QueryFormColumn[];
-  metrics: QueryFormMetric[];
-  orderby: QueryFormOrderBy[];
+  metrics?: QueryFormMetric[];
+  orderby?: QueryFormOrderBy[];
 }
 
 /**

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/fixtures.ts

@@ -0,0 +1,38 @@
+/**
+ * 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 { AdhocMetric, ColumnType } from '../src';
+
+export const NUM_METRIC: AdhocMetric = {
+  expressionType: 'SIMPLE',
+  label: 'Sum(num)',
+  column: {
+    id: 336,
+    type: ColumnType.BIGINT,
+    column_name: 'num',
+    verbose_name: null,
+    description: null,
+    expression: '',
+    filterable: false,
+    groupby: false,
+    is_dttm: false,
+    database_expression: null,
+    python_date_format: null,
+  },
+  aggregate: 'SUM',
+};