From 7430ed552881ef5695840eade3483e2cff645610 Mon Sep 17 00:00:00 2001 From: Spencer Torres Date: Tue, 27 Feb 2024 15:16:56 -0500 Subject: [PATCH] Fix converting from SQL Editor back to Query Builder --- src/components/queryBuilder/utils.test.ts | 367 +++++++++++++++++++++- src/components/queryBuilder/utils.ts | 92 +++--- src/data/sqlGenerator.ts | 8 +- src/views/CHQueryEditor.tsx | 12 +- 4 files changed, 427 insertions(+), 52 deletions(-) diff --git a/src/components/queryBuilder/utils.test.ts b/src/components/queryBuilder/utils.test.ts index 1afe2462..dff30839 100644 --- a/src/components/queryBuilder/utils.test.ts +++ b/src/components/queryBuilder/utils.test.ts @@ -1,4 +1,6 @@ -import { isDateTimeType, isDateType, isNumberType } from './utils'; +import { generateSql } from 'data/sqlGenerator'; +import { getQueryOptionsFromSql, isDateTimeType, isDateType, isNumberType } from './utils'; +import { AggregateType, BuilderMode, ColumnHint, DateFilterWithoutValue, FilterOperator, MultiFilter, OrderByDirection, QueryBuilderOptions, QueryType } from 'types/queryBuilder'; describe('isDateType', () => { it('returns true for Date type', () => { @@ -114,3 +116,366 @@ describe('isNumberType', () => { expect(isNumberType('Nullable')).toBe(false); }); }); + +describe('getQueryOptionsFromSql', () => { + testCondition('handles a table without a database', 'SELECT name FROM "foo"', { + queryType: QueryType.Table, + mode: BuilderMode.List, + database: '', + table: 'foo', + columns: [{ name: 'name', alias: undefined }], + aggregates: [], + }); + + testCondition('handles a database with a special character', 'SELECT name FROM "foo-bar"."buzz"', { + queryType: QueryType.Table, + mode: BuilderMode.List, + database: 'foo-bar', + table: 'buzz', + columns: [{ name: 'name', alias: undefined }], + aggregates: [], + }); + + testCondition('handles a database and a table', 'SELECT name FROM "db"."foo"', { + queryType: QueryType.Table, + mode: BuilderMode.List, + database: 'db', + table: 'foo', + columns: [{ name: 'name', alias: undefined }], + aggregates: [], + }); + + testCondition('handles a database and a table with a dot', 'SELECT name FROM "db"."foo.bar"', { + queryType: QueryType.Table, + mode: BuilderMode.List, + database: 'db', + table: 'foo.bar', + columns: [{ name: 'name', alias: undefined }], + aggregates: [], + }); + + testCondition('handles 2 columns', 'SELECT field1, field2 FROM "db"."foo"', { + queryType: QueryType.Table, + mode: BuilderMode.List, + database: 'db', + table: 'foo', + columns: [{ name: 'field1', alias: undefined }, { name: 'field2', alias: undefined }], + aggregates: [], + }); + + testCondition('handles a limit', 'SELECT field1, field2 FROM "db"."foo" LIMIT 20', { + queryType: QueryType.Table, + mode: BuilderMode.List, + database: 'db', + table: 'foo', + columns: [{ name: 'field1', alias: undefined }, { name: 'field2', alias: undefined }], + aggregates: [], + limit: 20, + }); + + testCondition( + 'handles empty orderBy array', + 'SELECT field1, field2 FROM "db"."foo" LIMIT 20', + { + queryType: QueryType.Table, + mode: BuilderMode.List, + database: 'db', + table: 'foo', + columns: [{ name: 'field1', alias: undefined }, { name: 'field2', alias: undefined }], + aggregates: [], + orderBy: [], + limit: 20, + }, + false + ); + + testCondition('handles order by', 'SELECT field1, field2 FROM "db"."foo" ORDER BY field1 ASC LIMIT 20', { + queryType: QueryType.Table, + mode: BuilderMode.List, + database: 'db', + table: 'foo', + columns: [{ name: 'field1', alias: undefined }, { name: 'field2', alias: undefined }], + aggregates: [], + orderBy: [{ name: 'field1', dir: OrderByDirection.ASC }], + limit: 20, + }); + + testCondition( + 'handles no select', + 'SELECT FROM "db"', + { + queryType: QueryType.Table, + mode: BuilderMode.Aggregate, + database: 'db', + table: '', + columns: [], + aggregates: [], + }, + false + ); + + testCondition( + 'does not escape * field', + 'SELECT * FROM "db"', + { + queryType: QueryType.Table, + mode: BuilderMode.Aggregate, + database: 'db', + table: '', + columns: [{ name: '*' }], + aggregates: [], + limit: undefined + }, + false + ); + + testCondition('handles aggregation function', 'SELECT sum(field1) FROM "db"."foo"', { + queryType: QueryType.Table, + mode: BuilderMode.Aggregate, + database: 'db', + table: 'foo', + columns: [], + aggregates: [{ column: 'field1', aggregateType: AggregateType.Sum, alias: undefined }], + limit: undefined + }); + + testCondition('handles aggregation with alias', 'SELECT sum(field1) as total_records FROM "db"."foo"', { + queryType: QueryType.Table, + mode: BuilderMode.Aggregate, + database: 'db', + table: 'foo', + columns: [], + aggregates: [{ column: 'field1', aggregateType: AggregateType.Sum, alias: 'total_records' }], + limit: undefined + }); + + testCondition( + 'handles 2 aggregations', + 'SELECT sum(field1) as total_records, count(field2) as total_records2 FROM "db"."foo"', + { + queryType: QueryType.Table, + mode: BuilderMode.Aggregate, + table: 'foo', + database: 'db', + columns: [], + aggregates: [ + { column: 'field1', aggregateType: AggregateType.Sum, alias: 'total_records' }, + { column: 'field2', aggregateType: AggregateType.Count, alias: 'total_records2' }, + ], + limit: undefined + } + ); + + testCondition( + 'handles aggregation with groupBy', + 'SELECT sum(field1) as total_records, count(field2) as total_records2 FROM "db"."foo" GROUP BY field3', + { + database: 'db', + table: 'foo', + queryType: QueryType.Table, + mode: BuilderMode.Aggregate, + columns: [], + aggregates: [ + { column: 'field1', aggregateType: AggregateType.Sum, alias: 'total_records' }, + { column: 'field2', aggregateType: AggregateType.Count, alias: 'total_records2' }, + ], + groupBy: ['field3'], + }, + false + ); + + testCondition( + 'handles aggregation with groupBy with columns having group by value', + 'SELECT field3, sum(field1) as total_records, count(field2) as total_records2 FROM "db"."foo" GROUP BY field3', + { + queryType: QueryType.Table, + mode: BuilderMode.Aggregate, + table: 'foo', + database: 'db', + columns: [{ name: 'field3' }], + aggregates: [ + { column: 'field1', aggregateType: AggregateType.Sum, alias: 'total_records' }, + { column: 'field2', aggregateType: AggregateType.Count, alias: 'total_records2' }, + ], + groupBy: ['field3'], + limit: undefined + } + ); + + testCondition( + 'handles aggregation with group by and order by', + 'SELECT StageName, Type, count(Id) as count_of, sum(Amount) FROM "db"."foo" GROUP BY StageName, Type ORDER BY count_of DESC, StageName ASC', + { + mode: BuilderMode.Aggregate, + database: 'db', + table: 'foo', + queryType: QueryType.Table, + columns: [ + { name: 'StageName' }, + { name: 'Type' }, + ], + aggregates: [ + { column: 'Id', aggregateType: AggregateType.Count, alias: 'count_of' }, + { column: 'Amount', aggregateType: AggregateType.Sum, alias: undefined }, + ], + groupBy: ['StageName', 'Type'], + orderBy: [ + { name: 'count_of', dir: OrderByDirection.DESC }, + { name: 'StageName', dir: OrderByDirection.ASC }, + ], + }, + false + ); + + testCondition( + 'handles aggregation with a IN filter', + `SELECT count(id) FROM "db"."foo" WHERE ( stagename IN ('Deal Won', 'Deal Lost') )`, + { + queryType: QueryType.Table, + mode: BuilderMode.Aggregate, + database: 'db', + table: 'foo', + columns: [], + aggregates: [{ column: 'id', aggregateType: AggregateType.Count, alias: undefined }], + filters: [ + { + key: 'stagename', + operator: FilterOperator.In, + value: ['Deal Won', 'Deal Lost'], + type: 'string' + } as MultiFilter, + ] + } + ); + + testCondition( + 'handles aggregation with a NOT IN filter', + `SELECT count(id) FROM "db"."foo" WHERE ( stagename NOT IN ('Deal Won', 'Deal Lost') )`, + { + queryType: QueryType.Table, + mode: BuilderMode.Aggregate, + database: 'db', + table: 'foo', + columns: [], + aggregates: [{ column: 'id', aggregateType: AggregateType.Count, alias: undefined }], + filters: [ + { + key: 'stagename', + operator: FilterOperator.NotIn, + value: ['Deal Won', 'Deal Lost'], + type: 'string', + } as MultiFilter, + ], + limit: undefined + } + ); + + testCondition( + 'handles aggregation with datetime filter', + `SELECT count(id) FROM "db"."foo" WHERE ( createddate >= $__fromTime AND createddate <= $__toTime )`, + { + queryType: QueryType.Table, + mode: BuilderMode.Aggregate, + database: 'db', + table: 'foo', + columns: [], + aggregates: [{ column: 'id', aggregateType: AggregateType.Count, alias: undefined }], + filters: [ + { + key: 'createddate', + operator: FilterOperator.WithInGrafanaTimeRange, + type: 'datetime', + } as DateFilterWithoutValue, + ], + limit: undefined + } + ); + + testCondition( + 'handles aggregation with date filter', + `SELECT count(id) FROM "db"."foo" WHERE ( NOT ( closedate >= $__fromTime AND closedate <= $__toTime ) )`, + { + queryType: QueryType.Table, + mode: BuilderMode.Aggregate, + database: 'db', + table: 'foo', + columns: [], + aggregates: [{ column: 'id', aggregateType: AggregateType.Count, alias: undefined }], + filters: [ + { + key: 'closedate', + operator: FilterOperator.OutsideGrafanaTimeRange, + type: 'datetime', + } as DateFilterWithoutValue, + ], + limit: undefined + } + ); + + testCondition( + 'handles timeseries function with "timeFieldType: DateType"', + 'SELECT $__timeInterval(time) as time FROM "db"."foo" GROUP BY time', + { + queryType: QueryType.TimeSeries, + mode: BuilderMode.Trend, + database: 'db', + table: 'foo', + columns: [{ name: 'time', type: 'datetime', hint: ColumnHint.Time }], + aggregates: [], + filters: [], + }, + false + ); + + testCondition( + 'handles timeseries function with "timeFieldType: DateType" with a filter', + 'SELECT $__timeInterval(time) as time FROM "db"."foo" WHERE ( base IS NOT NULL ) GROUP BY time', + { + queryType: QueryType.TimeSeries, + mode: BuilderMode.Trend, + database: 'db', + table: 'foo', + columns: [{ name: 'time', type: 'datetime', hint: ColumnHint.Time }], + aggregates: [], + filters: [ + { + condition: 'AND', + filterType: 'custom', + key: 'base', + operator: FilterOperator.IsNotNull, + type: 'LowCardinality(String)' + }, + ], + }, + false + ); + + it('Handles brackets and Grafana macros/variables', () => { + const sql = ` + SELECT + *, + \$__timeInterval(timestamp), + '{"a": 1, "b": { "c": 2, "d": [1, 2, 3] }}'::json as bracketTest + FROM default.table + WHERE $__timeFilter(timestamp) + AND col != \${variable} + AND col != \${variable.key} + AND col != \${variable.key:singlequote} + AND col != '\${variable}' + AND col != '\${__variable}' + AND col != ('\${__variable.key}') + `; + + const builderOptions = getQueryOptionsFromSql(sql); + expect(builderOptions).not.toBeUndefined(); + }); +}); + +function testCondition(name: string, sql: string, builder: QueryBuilderOptions, testQueryOptionsFromSql = true) { + it(name, () => { + expect(generateSql(builder)).toBe(sql); + if (testQueryOptionsFromSql) { + expect(getQueryOptionsFromSql(sql)).toEqual(builder); + } + }); +} diff --git a/src/components/queryBuilder/utils.ts b/src/components/queryBuilder/utils.ts index da0d42d4..a1cf5677 100644 --- a/src/components/queryBuilder/utils.ts +++ b/src/components/queryBuilder/utils.ts @@ -32,6 +32,7 @@ import { QueryType, } from 'types/queryBuilder'; import { sqlToStatement } from 'data/ast'; +import { getColumnByHint } from 'data/sqlGenerator'; export const isBooleanType = (type: string): boolean => { @@ -94,39 +95,41 @@ export function getQueryOptionsFromSql(sql: string): QueryBuilderOptions | strin } const fromTable = ast.from[0] as FromTable; - const columnsAndAggregates = getAggregatesFromAst(ast.columns ? ast.columns : null); + const columnsAndAggregates = getAggregatesFromAst(ast.columns || null); - let builder = { + const builderOptions = { + database: fromTable.name.schema || '', + table: fromTable.name.name || '', queryType: QueryType.Table, mode: BuilderMode.List, - database: fromTable.name.schema, - table: fromTable.name.name, + columns: [], + aggregates: [], } as QueryBuilderOptions; - if (columnsAndAggregates.columns) { - builder.columns = columnsAndAggregates.columns.map(f => ({ name: f })); + if (columnsAndAggregates.columns.length > 0) { + builderOptions.columns = columnsAndAggregates.columns; } if (columnsAndAggregates.aggregates.length > 0) { - builder.mode = BuilderMode.Aggregate; - builder.aggregates = columnsAndAggregates.aggregates; + builderOptions.mode = BuilderMode.Aggregate; + builderOptions.aggregates = columnsAndAggregates.aggregates; } - if (columnsAndAggregates.timeField) { - builder.queryType = QueryType.TimeSeries; - builder.mode = BuilderMode.Trend; - const columns: CHSelectedColumn[] = builder.columns || []; - columns.push({ name: columnsAndAggregates.timeField, type: 'datetime', hint: ColumnHint.Time }); - builder.columns = columns; + const timeColumn = getColumnByHint(builderOptions, ColumnHint.Time); + if (timeColumn) { + builderOptions.queryType = QueryType.TimeSeries; + if (builderOptions.aggregates?.length || 0) { + builderOptions.mode = BuilderMode.Trend; + } } if (ast.where) { - builder.filters = getFiltersFromAst(ast.where, columnsAndAggregates.timeField); + builderOptions.filters = getFiltersFromAst(ast.where, timeColumn?.name || ''); } const orderBy = ast.orderBy ?.map((ob) => { - if (ob.by.type !== 'ref' || ob.by.name === 'time') { + if (ob.by.type !== 'ref') { return {} as OrderBy; } return { name: ob.by.name, dir: ob.order } as OrderBy; @@ -134,23 +137,23 @@ export function getQueryOptionsFromSql(sql: string): QueryBuilderOptions | strin .filter((x) => x.name); if (orderBy && orderBy.length > 0) { - builder.orderBy = orderBy!; + builderOptions.orderBy = orderBy!; } - builder.limit = ast.limit?.limit?.type === 'integer' ? ast.limit?.limit.value : undefined; + builderOptions.limit = ast.limit?.limit?.type === 'integer' ? ast.limit?.limit.value : undefined; const groupBy = ast.groupBy ?.map((gb) => { - if (gb.type !== 'ref' || gb.name === 'time') { + if (gb.type !== 'ref') { return ''; } return gb.name; }) .filter((x) => x !== ''); if (groupBy && groupBy.length > 0) { - builder.groupBy = groupBy; + builderOptions.groupBy = groupBy; } - return builder; + return builderOptions; } function getFiltersFromAst(expr: Expr, timeField: string): Filter[] { @@ -297,7 +300,10 @@ function getBinaryFilter(e: ExprBinary, filters: Filter[], i: number, notFlag: b } else if (Object.values(FilterOperator).find((x) => e.op === x)) { if (i === 0) { filters.unshift({} as Filter); + } else if (!filters[i]) { + filters.push({ condition: 'AND' } as Filter); } + filters[i].operator = e.op as FilterOperator; if (notFlag && filters[i].operator === FilterOperator.Like) { filters[i].operator = FilterOperator.NotLike; @@ -307,9 +313,9 @@ function getBinaryFilter(e: ExprBinary, filters: Filter[], i: number, notFlag: b return notFlag; } -function selectCallFunc(s: SelectedColumn): AggregateColumn | string { +function selectCallFunc(s: SelectedColumn): [AggregateColumn | string, string | undefined] { if (s.expr.type !== 'call') { - return {} as AggregateColumn; + return [{} as AggregateColumn, undefined]; } let fields = s.expr.args.map((x) => { if (x.type !== 'ref') { @@ -318,55 +324,51 @@ function selectCallFunc(s: SelectedColumn): AggregateColumn | string { return x.name; }); if (fields.length > 1) { - return ''; + return ['', undefined]; } if ( Object.values(AggregateType).includes( s.expr.function.name.toLowerCase() as AggregateType ) ) { - return { + return [{ aggregateType: s.expr.function.name as AggregateType, column: fields[0], alias: s.alias?.name, - } as AggregateColumn; + } as AggregateColumn, s.alias?.name]; } - return fields[0]; + return [fields[0], s.alias?.name]; } -function getAggregatesFromAst(selectClauses: SelectedColumn[] | null): { - timeField: string; - aggregates: AggregateColumn[]; - columns: string[]; -} { +function getAggregatesFromAst(selectClauses: SelectedColumn[] | null): { columns: CHSelectedColumn[]; aggregates: AggregateColumn[]; } { if (!selectClauses) { - return { timeField: '', aggregates: [], columns: [] }; + return { columns: [], aggregates: [] }; } + + const columns: CHSelectedColumn[] = []; const aggregates: AggregateColumn[] = []; - const columns: string[] = []; - let timeField = ''; for (let s of selectClauses) { switch (s.expr.type) { case 'ref': - columns.push(s.expr.name); + columns.push({ name: s.expr.name, alias: s.alias?.name }); break; case 'call': - const f = selectCallFunc(s); - if (!f) { - return { timeField: '', aggregates: [], columns: [] }; + const [columnOrAggregate, alias] = selectCallFunc(s); + if (!columnOrAggregate) { + break; } - if (isString(f)) { - timeField = f; + + if (isString(columnOrAggregate)) { + columns.push({ name: columnOrAggregate, type: 'datetime', alias, hint: ColumnHint.Time }); } else { - aggregates.push(f); + aggregates.push(columnOrAggregate); } break; - default: - return { timeField: '', aggregates: [], columns: [] }; } } - return { timeField, aggregates, columns }; + + return { columns, aggregates }; } export const operMap = new Map([ diff --git a/src/data/sqlGenerator.ts b/src/data/sqlGenerator.ts index fa20097f..668515fd 100644 --- a/src/data/sqlGenerator.ts +++ b/src/data/sqlGenerator.ts @@ -515,12 +515,12 @@ const getColumnIdentifier = (col: SelectedColumn): string => { } const getTableIdentifier = (database: string, table: string): string => { - const sep = (database === '' || table === '') ? '' : '.'; + const sep = (!database || !table) ? '' : '.'; return `${escapeIdentifier(database)}${sep}${escapeIdentifier(table)}`; } const escapeIdentifier = (id: string): string => { - return id === '' ? '' : `"${id}"`; + return id ? `"${id}"` : ''; } const escapeValue = (value: string): string => { @@ -696,7 +696,7 @@ const getFilters = (options: QueryBuilderOptions): string => { filterParts.push(escapeValue((filter as StringFilter).value || '')); } } else if (isMultiFilter(type, filter.operator)) { - filterParts.push(`(${(filter as MultiFilter).value?.map(v => escapeValue(v)).join(', ')})`); + filterParts.push(`(${(filter as MultiFilter).value?.map(v => escapeValue(v.trim())).join(', ')})`); } else { filterParts.push(escapeValue((filter as StringFilter).value || '')); } @@ -724,7 +724,7 @@ const numberTypes = ['int', 'float', 'decimal']; const isNumberType = (type: string): boolean => numberTypes.some(t => type?.toLowerCase().includes(t)); const isDateType = (type: string): boolean => type?.toLowerCase().startsWith('date') || type?.toLowerCase().startsWith('nullable(date'); // const isDateTimeType = (type: string): boolean => type?.toLowerCase().startsWith('datetime') || type?.toLowerCase().startsWith('nullable(datetime'); -const isStringType = (type: string): boolean => type === 'String' && !(isBooleanType(type) || isNumberType(type) || isDateType(type)); +const isStringType = (type: string): boolean => type.toLowerCase() === 'string' && !(isBooleanType(type) || isNumberType(type) || isDateType(type)); const isNullFilter = (operator: FilterOperator): boolean => operator === FilterOperator.IsNull || operator === FilterOperator.IsNotNull; const isBooleanFilter = (type: string): boolean => isBooleanType(type); const isNumberFilter = (type: string): boolean => isNumberType(type); diff --git a/src/views/CHQueryEditor.tsx b/src/views/CHQueryEditor.tsx index ff612afe..35b1deb5 100644 --- a/src/views/CHQueryEditor.tsx +++ b/src/views/CHQueryEditor.tsx @@ -50,6 +50,15 @@ const CHEditorByType = (props: CHQueryEditorProps) => { lastKey.current = queryKey; } + /** + * Sync builder options when switching from SQL Editor to Query Builder + */ + const lastEditorType = useRef(); + if (query.editorType !== lastEditorType.current && query.editorType === EditorType.Builder) { + builderOptionsDispatch(setAllOptions((query as CHBuilderQuery).builderOptions || {})); + lastEditorType.current = query.editorType; + } + // Prevent trying to run empty query on load const shouldSkipChanges = useRef(true); if (isBuilderOptionsRunnable(builderOptions)) { @@ -61,12 +70,11 @@ const CHEditorByType = (props: CHQueryEditorProps) => { return; } - const sql = generateSql(builderOptions); onChange({ ...query, pluginVersion, editorType: EditorType.Builder, - rawSql: sql, + rawSql: generateSql(builderOptions), builderOptions, format: mapQueryBuilderOptionsToGrafanaFormat(builderOptions) });