From 4155dc8ecaf0f9704615df3987a648cd7f3d6831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Bedi?= Date: Tue, 12 Jul 2022 14:11:54 +0200 Subject: [PATCH] SQL: Fix couple of issues in SQLDatasource (#52080) * Fix: sql plugins feature (cherry picked from commit 035862bade2304567f59172db673e4edc3ac06e7) * SQLDS: Use builtin annotation editor Plus strict rule fixes (cherry picked from commit fee2eb3716b501b769b45e137e4171652a43e970) --- .betterer.results | 34 ------ package.json | 2 + public/app/features/plugins/sql/constants.ts | 30 +++-- .../plugins/sql/datasource/SqlDatasource.ts | 112 ++++++++---------- public/app/features/plugins/sql/defaults.ts | 15 +-- public/app/features/plugins/sql/types.ts | 13 +- .../plugins/sql/utils/useSqlChange.ts | 2 +- public/app/features/variables/utils.ts | 6 +- yarn.lock | 33 +++++- 9 files changed, 125 insertions(+), 122 deletions(-) diff --git a/.betterer.results b/.betterer.results index a623348052f..d1ac5fb17b0 100644 --- a/.betterer.results +++ b/.betterer.results @@ -5559,15 +5559,6 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use any type assertions.", "1"], [0, 0, 0, "Unexpected any. Specify a different type.", "2"] ], - "public/app/features/plugins/sql/datasource/SqlDatasource.ts:5381": [ - [0, 0, 0, "Unexpected any. Specify a different type.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"], - [0, 0, 0, "Unexpected any. Specify a different type.", "2"], - [0, 0, 0, "Unexpected any. Specify a different type.", "3"], - [0, 0, 0, "Do not use any type assertions.", "4"], - [0, 0, 0, "Unexpected any. Specify a different type.", "5"], - [0, 0, 0, "Unexpected any. Specify a different type.", "6"] - ], "public/app/features/plugins/tests/datasource_srv.test.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], @@ -7807,36 +7798,11 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"] ], - "public/app/plugins/datasource/mssql/query_ctrl.ts:5381": [ - [0, 0, 0, "Unexpected any. Specify a different type.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"], - [0, 0, 0, "Unexpected any. Specify a different type.", "2"], - [0, 0, 0, "Unexpected any. Specify a different type.", "3"] - ], "public/app/plugins/datasource/mssql/response_parser.ts:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], [0, 0, 0, "Do not use any type assertions.", "2"] ], - "public/app/plugins/datasource/mssql/specs/datasource.test.ts:5381": [ - [0, 0, 0, "Unexpected any. Specify a different type.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"], - [0, 0, 0, "Unexpected any. Specify a different type.", "2"], - [0, 0, 0, "Unexpected any. Specify a different type.", "3"], - [0, 0, 0, "Unexpected any. Specify a different type.", "4"], - [0, 0, 0, "Unexpected any. Specify a different type.", "5"], - [0, 0, 0, "Unexpected any. Specify a different type.", "6"], - [0, 0, 0, "Unexpected any. Specify a different type.", "7"], - [0, 0, 0, "Unexpected any. Specify a different type.", "8"] - ], - "public/app/plugins/datasource/mssql/types.ts:5381": [ - [0, 0, 0, "Unexpected any. Specify a different type.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"], - [0, 0, 0, "Unexpected any. Specify a different type.", "2"], - [0, 0, 0, "Unexpected any. Specify a different type.", "3"], - [0, 0, 0, "Unexpected any. Specify a different type.", "4"], - [0, 0, 0, "Unexpected any. Specify a different type.", "5"] - ], "public/app/plugins/datasource/mysql/datasource.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], diff --git a/package.json b/package.json index c381e44aa5f..7dd8d03cd7e 100644 --- a/package.json +++ b/package.json @@ -87,6 +87,7 @@ "@babel/preset-env": "7.18.2", "@babel/preset-react": "7.18.6", "@babel/preset-typescript": "7.18.6", + "@babel/runtime": "7.18.6", "@betterer/betterer": "5.3.5", "@betterer/cli": "5.3.5", "@betterer/eslint": "5.3.5", @@ -386,6 +387,7 @@ "semver": "7.3.7", "slate": "0.47.8", "slate-plain-serializer": "0.7.10", + "sql-formatter-plus": "^1.3.6", "symbol-observable": "4.0.0", "test": "link:./public/test", "tether-drop": "https://github.com/torkelo/drop", diff --git a/public/app/features/plugins/sql/constants.ts b/public/app/features/plugins/sql/constants.ts index 38eaff650ee..7518715682e 100644 --- a/public/app/features/plugins/sql/constants.ts +++ b/public/app/features/plugins/sql/constants.ts @@ -9,7 +9,7 @@ export const AGGREGATE_FNS = [ expression ) [OVER (...)] - + Returns the average of non-NULL input values, or NaN if the input contains a NaN.`, }, { @@ -17,13 +17,13 @@ export const AGGREGATE_FNS = [ name: 'COUNT', description: `COUNT(*) [OVER (...)] Returns the number of rows in the input. - + COUNT( [DISTINCT] expression ) [OVER (...)] - + Returns the number of rows with expression evaluated to any value other than NULL. `, }, @@ -34,7 +34,7 @@ export const AGGREGATE_FNS = [ expression ) [OVER (...)] - + Returns the maximum value of non-NULL expressions. Returns NULL if there are zero input rows or expression evaluates to NULL for all rows. Returns NaN if the input contains a NaN. `, }, @@ -45,7 +45,7 @@ export const AGGREGATE_FNS = [ expression ) [OVER (...)] - + Returns the minimum value of non-NULL expressions. Returns NULL if there are zero input rows or expression evaluates to NULL for all rows. Returns NaN if the input contains a NaN. `, }, @@ -57,9 +57,9 @@ export const AGGREGATE_FNS = [ expression ) [OVER (...)] - + Returns the sum of non-null values. - + If the expression is a floating point value, the sum is non-deterministic, which means you might receive a different result each time you use this function. `, }, @@ -110,3 +110,19 @@ export const OPERATORS = [ { type: OperatorType.Logical, id: 'AND', operator: 'AND' }, { type: OperatorType.Logical, id: 'OR', operator: 'OR' }, ]; + +export const MACRO_NAMES = [ + '$__time', + '$__timeEpoch', + '$__timeFilter', + '$__timeFrom', + '$__timeTo', + '$__timeGroup', + '$__timeGroupAlias', + '$__unixEpochFilter', + '$__unixEpochNanoFilter', + '$__unixEpochNanoFrom', + '$__unixEpochNanoTo', + '$__unixEpochGroup', + '$__unixEpochGroupAlias', +]; diff --git a/public/app/features/plugins/sql/datasource/SqlDatasource.ts b/public/app/features/plugins/sql/datasource/SqlDatasource.ts index 73e18ba4b18..8039583dad2 100644 --- a/public/app/features/plugins/sql/datasource/SqlDatasource.ts +++ b/public/app/features/plugins/sql/datasource/SqlDatasource.ts @@ -2,15 +2,14 @@ import { lastValueFrom, of } from 'rxjs'; import { catchError, map } from 'rxjs/operators'; import { - AnnotationEvent, DataFrame, DataFrameView, - DataQueryRequest, - DataQueryResponse, + DataQuery, DataSourceInstanceSettings, DataSourceRef, MetricFindValue, ScopedVars, + TimeRange, } from '@grafana/data'; import { BackendDataSourceResponse, @@ -20,10 +19,12 @@ import { getTemplateSrv, TemplateSrv, } from '@grafana/runtime'; -import { toTestingStatus } from '@grafana/runtime/src/utils/queryResponse'; +import { toDataQueryResponse, toTestingStatus } from '@grafana/runtime/src/utils/queryResponse'; +import { getTimeSrv } from 'app/features/dashboard/services/TimeSrv'; import { VariableWithMultiSupport } from '../../../variables/types'; -import { getSearchFilterScopedVar } from '../../../variables/utils'; +import { getSearchFilterScopedVar, SearchFilterOptions } from '../../../variables/utils'; +import { MACRO_NAMES } from '../constants'; import { DB, SQLQuery, @@ -39,6 +40,7 @@ export abstract class SqlDatasource extends DataSourceWithBackend, @@ -121,78 +123,56 @@ export abstract class SqlDatasource extends DataSourceWithBackend { - if (!options.annotation.rawQuery) { - return Promise.reject({ - message: 'Query missing in annotation definition', - }); - } + async metricFindQuery(query: string, optionalOptions?: MetricFindQueryOptions): Promise { + const rawSql = this.templateSrv.replace( + query, + getSearchFilterScopedVar({ query, wildcardChar: '%', options: optionalOptions }), + this.interpolateVariable + ); - const query = { - refId: options.annotation.name, + const interpolatedQuery: SQLQuery = { + refId: 'tempvar', datasource: this.getRef(), - rawSql: this.templateSrv.replace(options.annotation.rawQuery, options.scopedVars, this.interpolateVariable), - format: 'table', + rawSql, + format: QueryFormat.Table, }; + const response = await this.runMetaQuery(interpolatedQuery, optionalOptions); + return this.getResponseParser().transformMetricFindResponse(response); + } + + async runSql(query: string, options?: MetricFindQueryOptions) { + const frame = await this.runMetaQuery({ rawSql: query, format: QueryFormat.Table }, options); + return new DataFrameView(frame); + } + + private runMetaQuery(request: Partial, options?: MetricFindQueryOptions): Promise { + const range = getTimeSrv().timeRange(); + const refId = request.refId || 'meta'; + const queries: DataQuery[] = [{ ...request, datasource: request.datasource || this.getRef(), refId }]; + return lastValueFrom( getBackendSrv() .fetch({ url: '/api/ds/query', method: 'POST', data: { - from: options.range.from.valueOf().toString(), - to: options.range.to.valueOf().toString(), - queries: [query], + from: options?.range?.from.valueOf().toString() || range.from.valueOf().toString(), + to: options?.range?.to.valueOf().toString() || range.to.valueOf().toString(), + queries, }, - requestId: options.annotation.name, + requestId: refId, }) .pipe( - map( - async (res: FetchResponse) => - await this.getResponseParser().transformAnnotationResponse(options, res.data) - ) + map((res: FetchResponse) => { + const rsp = toDataQueryResponse(res, queries); + return rsp.data[0]; + }) ) ); } - async metricFindQuery(query: string, optionalOptions: any): Promise { - const rawSql = this.templateSrv.replace( - query, - getSearchFilterScopedVar({ query, wildcardChar: '%', options: optionalOptions }), - this.interpolateVariable - ); - - const interpolatedQuery = { - datasourceId: this.id, - datasource: this.getRef(), - rawSql, - format: QueryFormat.Table, - }; - - const response = await this.runQuery(interpolatedQuery, optionalOptions); - return this.getResponseParser().transformMetricFindResponse(response); - } - - async runSql(query: string) { - const frame = await this.runQuery({ rawSql: query, format: QueryFormat.Table }, {}); - return new DataFrameView(frame); - } - - private runQuery(request: Partial, options?: any): Promise { - return new Promise((resolve) => { - const req = { - targets: [{ ...request, refId: String(Math.random()) }], - range: options?.range, - } as DataQueryRequest; - this.query(req).subscribe((res: DataQueryResponse) => { - resolve(res.data[0] || { fields: [] }); - }); - }); - } - - testDatasource(): Promise { + testDatasource(): Promise<{ status: string; message: string }> { return lastValueFrom( getBackendSrv() .fetch({ @@ -223,7 +203,15 @@ export abstract class SqlDatasource extends DataSourceWithBackend { + queryWithoutMacros = queryWithoutMacros?.replace(value, '') || ''; + }); + return this.templateSrv.containsTemplate(queryWithoutMacros); } } + +interface MetricFindQueryOptions extends SearchFilterOptions { + range?: TimeRange; +} diff --git a/public/app/features/plugins/sql/defaults.ts b/public/app/features/plugins/sql/defaults.ts index 6093df7adc0..e67a4128acb 100644 --- a/public/app/features/plugins/sql/defaults.ts +++ b/public/app/features/plugins/sql/defaults.ts @@ -3,20 +3,21 @@ import { EditorMode } from '@grafana/experimental'; import { QueryFormat, SQLQuery } from './types'; import { createFunctionField, setGroupByField } from './utils/sql.utils'; -export function applyQueryDefaults(q: SQLQuery): SQLQuery { - let editorMode = q.editorMode || EditorMode.Builder; +export function applyQueryDefaults(q?: SQLQuery): SQLQuery { + let editorMode = q?.editorMode || EditorMode.Builder; // Switching to code editor if the query was created before visual query builder was introduced. - if (q.editorMode === undefined && q.rawSql !== undefined) { + if (q?.editorMode === undefined && q?.rawSql !== undefined) { editorMode = EditorMode.Code; } - const result = { + const result: SQLQuery = { ...q, - format: q.format !== undefined ? q.format : QueryFormat.Table, - rawSql: q.rawSql || '', + refId: q?.refId || 'A', + format: q?.format !== undefined ? q.format : QueryFormat.Table, + rawSql: q?.rawSql || '', editorMode, - sql: q.sql || { + sql: q?.sql || { columns: [createFunctionField()], groupBy: [setGroupByField()], limit: 50, diff --git a/public/app/features/plugins/sql/types.ts b/public/app/features/plugins/sql/types.ts index e6869254b9a..b5509dbba2e 100644 --- a/public/app/features/plugins/sql/types.ts +++ b/public/app/features/plugins/sql/types.ts @@ -1,7 +1,6 @@ import { JsonTree } from 'react-awesome-query-builder'; import { - AnnotationEvent, DataFrame, DataQuery, DataSourceJsonData, @@ -11,7 +10,6 @@ import { toOption as toOptionFromData, } from '@grafana/data'; import { CompletionItemKind, EditorMode, LanguageCompletionProvider } from '@grafana/experimental'; -import { BackendDataSourceResponse } from '@grafana/runtime'; import { QueryWithDefaults } from './defaults'; import { @@ -23,7 +21,7 @@ import { export interface SqlQueryForInterpolation { dataset?: string; alias?: string; - format?: ResultFormat; + format?: QueryFormat; rawSql?: string; refId: string; hide?: boolean; @@ -34,8 +32,6 @@ export interface SQLOptions extends DataSourceJsonData { database: string; } -export type ResultFormat = 'time_series' | 'table'; - export enum QueryFormat { Timeseries = 'time_series', Table = 'table', @@ -43,7 +39,7 @@ export enum QueryFormat { export interface SQLQuery extends DataQuery { alias?: string; - format?: ResultFormat | QueryFormat | string | undefined; + format?: QueryFormat; rawSql?: string; dataset?: string; table?: string; @@ -119,7 +115,7 @@ export interface DB { tables: (dataset?: string) => Promise; fields: (query: SQLQuery, order?: boolean) => Promise; validateQuery: (query: SQLQuery, range?: TimeRange) => Promise; - dsID: () => string; + dsID: () => number; dispose?: (dsID?: string) => void; lookup: (path?: string) => Promise>; getSqlCompletionProvider: () => LanguageCompletionProvider; @@ -135,7 +131,7 @@ export interface QueryEditorProps { export interface ValidationResults { query: SQLQuery; - rawSql: string; + rawSql?: string; error: string; isError: boolean; isValid: boolean; @@ -150,7 +146,6 @@ export interface SqlQueryModel { } export interface ResponseParser { - transformAnnotationResponse: (options: object, data: BackendDataSourceResponse) => Promise; transformMetricFindResponse: (frame: DataFrame) => MetricFindValue[]; } diff --git a/public/app/features/plugins/sql/utils/useSqlChange.ts b/public/app/features/plugins/sql/utils/useSqlChange.ts index d326261ef1d..b7e0cbdfd51 100644 --- a/public/app/features/plugins/sql/utils/useSqlChange.ts +++ b/public/app/features/plugins/sql/utils/useSqlChange.ts @@ -14,7 +14,7 @@ export function useSqlChange({ query, onQueryChange, db }: UseSqlChange) { const onSqlChange = useCallback( (sql: SQLExpression) => { const toRawSql = db.toRawSql || defaultToRawSql; - const rawSql = toRawSql({ sql, dataset: query.dataset, table: query.table, refId: db.dsID() }); + const rawSql = toRawSql({ sql, dataset: query.dataset, table: query.table, refId: query.refId }); const newQuery: SQLQuery = { ...query, sql, rawSql }; onQueryChange(newQuery); }, diff --git a/public/app/features/variables/utils.ts b/public/app/features/variables/utils.ts index 06984535888..fc5e9dd861d 100644 --- a/public/app/features/variables/utils.ts +++ b/public/app/features/variables/utils.ts @@ -33,10 +33,14 @@ export const SEARCH_FILTER_VARIABLE = '__searchFilter'; export const containsSearchFilter = (query: string | unknown): boolean => query && typeof query === 'string' ? query.indexOf(SEARCH_FILTER_VARIABLE) !== -1 : false; +export interface SearchFilterOptions { + searchFilter?: string; +} + export const getSearchFilterScopedVar = (args: { query: string; wildcardChar: string; - options: { searchFilter?: string }; + options?: SearchFilterOptions; }): ScopedVars => { const { query, wildcardChar } = args; if (!containsSearchFilter(query)) { diff --git a/yarn.lock b/yarn.lock index 71913426369..fbed4c0aff6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3177,6 +3177,16 @@ __metadata: languageName: node linkType: hard +"@babel/polyfill@npm:^7.6.0": + version: 7.12.1 + resolution: "@babel/polyfill@npm:7.12.1" + dependencies: + core-js: ^2.6.5 + regenerator-runtime: ^0.13.4 + checksum: 3f59a9d85a41b390b044a1be13e11ae6d8efbfcf4e07217964585c7cef337b828eecfc5e164083227189146d2b6efc1affae8f59c831438eb40b848ab6fe5f39 + languageName: node + linkType: hard + "@babel/preset-env@npm:7.18.2": version: 7.18.2 resolution: "@babel/preset-env@npm:7.18.2" @@ -3447,6 +3457,15 @@ __metadata: languageName: node linkType: hard +"@babel/runtime@npm:7.18.6": + version: 7.18.6 + resolution: "@babel/runtime@npm:7.18.6" + dependencies: + regenerator-runtime: ^0.13.4 + checksum: 8b707b64ae0524db617d0c49933b258b96376a38307dc0be8fb42db5697608bcc1eba459acce541e376cff5ed5c5287d24db5780bd776b7c75ba2c2e26ff8a2c + languageName: node + linkType: hard + "@babel/runtime@npm:^7.0.0, @babel/runtime@npm:^7.1.2, @babel/runtime@npm:^7.10.1, @babel/runtime@npm:^7.11.1, @babel/runtime@npm:^7.11.2, @babel/runtime@npm:^7.12.0, @babel/runtime@npm:^7.12.1, @babel/runtime@npm:^7.12.13, @babel/runtime@npm:^7.12.5, @babel/runtime@npm:^7.13.10, @babel/runtime@npm:^7.15.4, @babel/runtime@npm:^7.5.5, @babel/runtime@npm:^7.6.2, @babel/runtime@npm:^7.7.2, @babel/runtime@npm:^7.8.4, @babel/runtime@npm:^7.8.7, @babel/runtime@npm:^7.9.2": version: 7.15.4 resolution: "@babel/runtime@npm:7.15.4" @@ -16295,7 +16314,7 @@ __metadata: languageName: node linkType: hard -"core-js@npm:^2.0.0, core-js@npm:^2.4.0": +"core-js@npm:^2.0.0, core-js@npm:^2.4.0, core-js@npm:^2.6.5": version: 2.6.12 resolution: "core-js@npm:2.6.12" checksum: 44fa9934a85f8c78d61e0c8b7b22436330471ffe59ec5076fe7f324d6e8cf7f824b14b1c81ca73608b13bdb0fef035bd820989bf059767ad6fa13123bb8bd016 @@ -20937,6 +20956,7 @@ __metadata: "@babel/preset-env": 7.18.2 "@babel/preset-react": 7.18.6 "@babel/preset-typescript": 7.18.6 + "@babel/runtime": 7.18.6 "@betterer/betterer": 5.3.5 "@betterer/cli": 5.3.5 "@betterer/eslint": 5.3.5 @@ -21217,6 +21237,7 @@ __metadata: sinon: 14.0.0 slate: 0.47.8 slate-plain-serializer: 0.7.10 + sql-formatter-plus: ^1.3.6 style-loader: 3.3.1 stylelint: 14.9.1 stylelint-config-prettier: 9.0.3 @@ -33883,6 +33904,16 @@ __metadata: languageName: node linkType: hard +"sql-formatter-plus@npm:^1.3.6": + version: 1.3.6 + resolution: "sql-formatter-plus@npm:1.3.6" + dependencies: + "@babel/polyfill": ^7.6.0 + lodash: ^4.17.15 + checksum: 5c215f85936b3465d6778af868f0a8a2d3dd4b714ab95025c3b17b9748fe420ae2e128c523d50be0a43667bf8329a974f3248828971299146786e28d9714e752 + languageName: node + linkType: hard + "sqlstring@npm:^2.3.2": version: 2.3.3 resolution: "sqlstring@npm:2.3.3"