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

feat: update ChartFormData and QueryObject to support filters. #164

Merged
merged 14 commits into from
May 23, 2019
4 changes: 2 additions & 2 deletions packages/superset-ui-chart/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export { default as DatasourceKey } from './query/DatasourceKey';
export { default as ChartDataProvider } from './components/ChartDataProvider';

export * from './types/Annotation';
export * from './types/Datasource';
export * from './types/ChartFormData';
export * from './types/Query';
export * from './types/Datasource';
export * from './types/Metric';
export * from './types/Query';
68 changes: 0 additions & 68 deletions packages/superset-ui-chart/src/query/Metrics.ts

This file was deleted.

66 changes: 36 additions & 30 deletions packages/superset-ui-chart/src/query/buildQueryObject.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import Metrics from './Metrics';
/* eslint-disable camelcase */
import { QueryObject } from '../types/Query';
import { ChartFormData, DruidFormData, SqlaFormData } from '../types/ChartFormData';
import { ChartFormData, isSqlaFormData } from '../types/ChartFormData';
import convertMetric from './convertMetric';
import processFilters from './processFilters';
import processMetrics from './processMetrics';
import processExtras from './processExtras';

const DTTM_ALIAS = '__timestamp';
export const DTTM_ALIAS = '__timestamp';

function getGranularity(formData: ChartFormData): string {
return 'granularity_sqla' in formData ? formData.granularity_sqla : formData.granularity;
function processGranularity(formData: ChartFormData): string {
return isSqlaFormData(formData) ? formData.granularity_sqla : formData.granularity;
}

// Build the common segments of all query objects (e.g. the granularity field derived from
Expand All @@ -14,38 +18,40 @@ 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 {
time_range,
since,
until,
columns = [],
groupby = [],
order_desc,
row_limit,
limit,
timeseries_limit_metric,
} = formData;

const { columns = [], groupby = [] } = formData;
const groupbySet = new Set([...columns, ...groupby]);
const limit = formData.limit ? Number(formData.limit) : 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),
const queryObject: QueryObject = {
extras: processExtras(formData),
granularity: processGranularity(formData),
groupby: Array.from(groupbySet),
is_prequery: false,
is_timeseries: isTimeseries,
metrics: new Metrics(formData).getMetrics(),
order_desc: orderDesc,
is_timeseries: groupbySet.has(DTTM_ALIAS),
metrics: processMetrics(formData),
order_desc: order_desc === undefined ? true : order_desc,
Copy link
Contributor

@williaster williaster May 23, 2019

Choose a reason for hiding this comment

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

typeof order_desc === 'undefined'?

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)
row_limit: Number(row_limit),
since,
time_range,
timeseries_limit: limit ? Number(limit) : 0,
timeseries_limit_metric: timeseries_limit_metric
? convertMetric(timeseries_limit_metric)
: null,
until: formData.until,
until,
...processFilters(formData),
};

return queryObject;
}
30 changes: 30 additions & 0 deletions packages/superset-ui-chart/src/query/convertFilter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { SimpleAdhocFilter, isBinaryAdhocFilter, isUnaryAdhocFilter } from '../types/Filter';
import { QueryObjectFilterClause } from '../types/Query';

export default function convertFilter(filter: SimpleAdhocFilter): QueryObjectFilterClause {
const { subject } = filter;
if (isUnaryAdhocFilter(filter)) {
const { operator } = filter;
Copy link
Contributor

Choose a reason for hiding this comment

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

We can put it outside the current block to reduce the duplication.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Has to be inside the block to infer the correct Operator type


return {
col: subject,
op: operator,
};
} else if (isBinaryAdhocFilter(filter)) {
const { operator } = filter;

return {
col: subject,
op: operator,
val: filter.comparator,
};
}

const { operator } = filter;

return {
col: subject,
op: operator,
val: filter.comparator,
};
}
38 changes: 38 additions & 0 deletions packages/superset-ui-chart/src/query/convertMetric.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { ChartFormDataMetric } from '../types/ChartFormData';
import { QueryObjectMetric } from '../types/Query';
import { AdhocMetric } from '../types/Metric';

export const LABEL_MAX_LENGTH = 43;

function getDefaultLabel(metric: AdhocMetric) {
let label: string;
if (metric.expressionType === 'SIMPLE') {
label = `${metric.aggregate}(${metric.column.columnName})`;
} else {
label = metric.sqlExpression;
}

return label.length <= LABEL_MAX_LENGTH
? label
: `${label.substring(0, LABEL_MAX_LENGTH - 3)}...`;
}

export default function convertMetric(metric: ChartFormDataMetric): QueryObjectMetric {
let formattedMetric;
if (typeof metric === 'string') {
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 || getDefaultLabel(metric);
formattedMetric = {
...metric,
label,
};
}

return formattedMetric;
}
17 changes: 17 additions & 0 deletions packages/superset-ui-chart/src/query/processExtras.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/* eslint-disable camelcase */
import { ChartFormData, isDruidFormData } from '../types/ChartFormData';
import { QueryObjectExtras } from '../types/Query';

export default function processExtras(formData: ChartFormData): QueryObjectExtras {
const { where = '' } = formData;

if (isDruidFormData(formData)) {
const { druid_time_origin, having_druid } = formData;

return { druid_time_origin, having_druid, where };
}

const { time_grain_sqla, having } = formData;

return { having, time_grain_sqla, where };
}
51 changes: 51 additions & 0 deletions packages/superset-ui-chart/src/query/processFilters.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { ChartFormData } from '../types/ChartFormData';
import { QueryObjectFilterClause } from '../types/Query';
import { isSimpleAdhocFilter } from '../types/Filter';
import convertFilter from './convertFilter';

/** Logic formerly in viz.py's process_query_filters */
export default function processFilters(formData: ChartFormData) {
// TODO: Implement
// utils.convert_legacy_filters_into_adhoc(self.form_data)

// TODO: Implement
Copy link
Contributor

Choose a reason for hiding this comment

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

Will these two in the scope of this PR?

Copy link
Contributor Author

@kristw kristw May 23, 2019

Choose a reason for hiding this comment

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

These will be beyond the scope. Right now this PR will handle adhoc_filters (which is the standard filters) correctly plus provide typings.

Copy link
Contributor

Choose a reason for hiding this comment

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

do you think we should console.warn if we see legacy filters? (fine if not)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The legacy filters are using fields filters and having_filters which do not exist on type ChartFormData, so typescript would complain.

I am wondering if we want to support this utils.convert_legacy_filters_into_adhoc(self.form_data) going forward. Not sure how long since we have moved from the legacy filters.

// merge_extra_filters(self.form_data)

// utils.split_adhoc_filters_into_base_filters(self.form_data)
Copy link
Contributor

Choose a reason for hiding this comment

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

is this just indicating what the logic below mapped to? (i.e., it's not part of the TODOs above) should we remove it or keep for context?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove or change to simple sentence. Seem not necessary any more

// eslint-disable-next-line camelcase
const { adhoc_filters } = formData;
if (Array.isArray(adhoc_filters)) {
const simpleWhere: QueryObjectFilterClause[] = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

(not blocking) the type helps but I find these a little confusing name-wise, not sure what to expect simple to mean, maybe whereFilter <> whereFreeform and havingFilter <> havingFreeform?

const simpleHaving: QueryObjectFilterClause[] = [];
const freeformWhere: string[] = [];
const freeformHaving: string[] = [];

adhoc_filters.forEach(filter => {
const { clause } = filter;
if (isSimpleAdhocFilter(filter)) {
const filterClause = convertFilter(filter);
if (clause === 'WHERE') {
simpleWhere.push(filterClause);
} else {
simpleHaving.push(filterClause);
}
} else {
const { sqlExpression } = filter;
if (clause === 'WHERE') {
freeformWhere.push(sqlExpression);
} else {
freeformHaving.push(sqlExpression);
}
}
});

return {
filters: simpleWhere,
having: freeformHaving.map(exp => `(${exp})`).join(' AND '),
having_filters: simpleHaving,
where: freeformWhere.map(exp => `(${exp})`).join(' AND '),
};
}

return {};
}
25 changes: 25 additions & 0 deletions packages/superset-ui-chart/src/query/processMetrics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { ChartFormData } from '../types/ChartFormData';
import { QueryObjectMetric } from '../types/Query';
import { MetricKey } from '../types/Metric';
import convertMetric from './convertMetric';

export default function processMetrics(formData: ChartFormData) {
// Use Array to maintain insertion order
// for metrics that are order sensitive
const metrics: QueryObjectMetric[] = [];

Object.keys(MetricKey).forEach(key => {
const metric = formData[MetricKey[key as keyof typeof MetricKey]];
if (metric) {
if (Array.isArray(metric)) {
metric.forEach(m => {
metrics.push(convertMetric(m));
});
} else {
metrics.push(convertMetric(metric));
}
}
});

return metrics;
}
Loading