Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

feat: 🎸 Improved QueryObject to handle more fields #116

Merged
merged 3 commits into from
Mar 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions packages/superset-ui-chart/src/query/Metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,31 @@ export default class Metrics {
return this.metrics.map(m => m.label);
}

private addMetric(metric: FormDataMetric) {
static formatMetric(metric: FormDataMetric): Metric {
let formattedMetric;
if (typeof metric === 'string') {
this.metrics.push({
formattedMetric = {
label: metric,
});
};
} else {
// Note we further sanitize the metric label for BigQuery datasources
// TODO: move this logic to the client once client has more info on the
// the datasource
const label = metric.label || this.getDefaultLabel(metric);
this.metrics.push({
formattedMetric = {
...metric,
label,
});
};
}

return formattedMetric;
}

private addMetric(metric: FormDataMetric) {
this.metrics.push(Metrics.formatMetric(metric));
}

private getDefaultLabel(metric: AdhocMetric) {
static getDefaultLabel(metric: AdhocMetric) {
let label: string;
if (metric.expressionType === ExpressionType.SIMPLE) {
label = `${metric.aggregate}(${metric.column.columnName})`;
Expand Down
34 changes: 33 additions & 1 deletion packages/superset-ui-chart/src/query/buildQueryObject.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import Metrics from './Metrics';
import { QueryObject } from '../types/Query';
import { ChartFormData } from '../types/ChartFormData';
import { ChartFormData, DruidFormData, SqlaFormData } from '../types/ChartFormData';

const DTTM_ALIAS = '__timestamp';

function getGranularity(formData: ChartFormData): string {
return 'granularity_sqla' in formData ? formData.granularity_sqla : formData.granularity;
Expand All @@ -12,8 +14,38 @@ function getGranularity(formData: ChartFormData): string {
// Note the type of the formData argument passed in here is the type of the formData for a
// specific viz, which is a subtype of the generic formData shared among all viz types.
export default function buildQueryObject<T extends ChartFormData>(formData: T): QueryObject {
const extras = {
druid_time_origin: (formData as DruidFormData).druid_time_origin || '',
having: (formData as SqlaFormData).having || '',
having_druid: (formData as DruidFormData).having_druid || '',
time_grain_sqla: (formData as SqlaFormData).time_grain_sqla || '',
where: formData.where || '',
};

const { columns = [], groupby = [] } = formData;
const groupbySet = new Set([...columns, ...groupby]);
const limit = formData.limit ? Number(formData.limit) : 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does limit=0 have any weird query side effects?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the original logic from viz.py:

limit = int(form_data.get('limit') or 0)

const rowLimit = Number(formData.row_limit);
const orderDesc = formData.order_desc === undefined ? true : formData.order_desc;
const isTimeseries = groupbySet.has(DTTM_ALIAS);

return {
extras,
granularity: getGranularity(formData),
groupby: Array.from(groupbySet),
is_prequery: false,
is_timeseries: isTimeseries,
metrics: new Metrics(formData).getMetrics(),
order_desc: orderDesc,
orderby: [],
prequeries: [],
row_limit: rowLimit,
since: formData.since,
time_range: formData.time_range,
timeseries_limit: limit,
timeseries_limit_metric: formData.timeseries_limit_metric
? Metrics.formatMetric(formData.timeseries_limit_metric)
: null,
until: formData.until,
};
}
19 changes: 17 additions & 2 deletions packages/superset-ui-chart/src/types/ChartFormData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,30 @@ type BaseFormData = {
datasource: string;
viz_type: string;
annotation_layers?: AnnotationLayerMetadata[];
where?: string;
groupby?: string[];
columns?: string[];
all_columns?: string[];
limit?: number;
row_limit?: number;
order_desc?: boolean;
timeseries_limit_metric?: FormDataMetric;
time_range?: string;
since?: string;
until?: string;
} & Metrics;

// FormData is either sqla-based or druid-based
type SqlaFormData = {
export type SqlaFormData = {
granularity_sqla: string;
time_grain_sqla?: string;
having?: string;
} & BaseFormData;

type DruidFormData = {
export type DruidFormData = {
granularity: string;
having_druid?: string;
druid_time_origin?: string;
} & BaseFormData;

export type ChartFormData = SqlaFormData | DruidFormData;
15 changes: 15 additions & 0 deletions packages/superset-ui-chart/src/types/Query.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint camelcase: 0 */
import ChartProps from '../models/ChartProps';
import { DatasourceType } from './Datasource';
import { ChartFormData } from './ChartFormData';
Expand All @@ -7,6 +8,20 @@ export interface QueryObject {
granularity: string;
groupby?: string[];
metrics?: Metric[];
extras?: {
[key: string]: string;
};
timeseries_limit?: number;
timeseries_limit_metric?: Metric | null;
time_range?: string;
since?: string;
until?: string;
row_limit?: number;
order_desc?: boolean;
is_timeseries?: boolean;
prequeries?: string[];
is_prequery?: boolean;
orderby?: Array<[Metric, boolean]>;
}

export interface QueryContext {
Expand Down
33 changes: 33 additions & 0 deletions packages/superset-ui-chart/test/query/buildQueryObject.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,37 @@ describe('queryObjectBuilder', () => {
});
expect(query.metrics).toEqual([{ label: 'sum__num' }]);
});

it('should build limit', () => {
const limit = 2;
query = buildQueryObject({
datasource: '5__table',
granularity_sqla: 'ds',
viz_type: 'table',
limit,
});
expect(query.timeseries_limit).toEqual(limit);
});

it('should build order_desc', () => {
const orderDesc = false;
query = buildQueryObject({
datasource: '5__table',
granularity_sqla: 'ds',
viz_type: 'table',
order_desc: orderDesc,
});
expect(query.order_desc).toEqual(orderDesc);
});

it('should build timeseries_limit_metric', () => {
const metric = 'country';
query = buildQueryObject({
datasource: '5__table',
granularity_sqla: 'ds',
viz_type: 'table',
timeseries_limit_metric: metric,
});
expect(query.timeseries_limit_metric).toEqual({ label: metric });
});
});