MySQL: Quote identifiers that include special characters (#61135)

* SQL: toRawSQL required and escape table

* Fix autocomplete for MySQL

* Change the way we escape for builder

* Rework escape ident to be smart instead

* Fix A11y for alias

* Add first e2e test

* Add test for code editor

* Add doc

* Review comments

* Move functions to sqlUtil

docs/sources/datasources/mysql/_index.md

@@ -118,6 +118,8 @@ The response from MySQL can be formatted as either a table or as a time series.
 
 ### Dataset and Table selection
 
+> **Note:** If your table or database name contains a reserved word or a [not permitted character](https://dev.mysql.com/doc/refman/8.0/en/identifiers.html) the editor will put quotes around them. For example a table name like `table-name` will be quoted with backticks `` `table-name` ``.
+
 In the dataset dropdown, choose the MySQL database to query. The dropdown is be populated with the databases that the user has access to.
 When the dataset is selected, the table dropdown is populated with the tables that are available.
 

e2e/sql-suite/datasets-response.json

@@ -0,0 +1,21 @@
+{
+  "results": {
+    "datasets": {
+      "status": 200,
+      "frames": [
+        {
+          "schema": {
+            "refId": "datasets",
+            "meta": {
+              "executedQueryString": "SELECT DISTINCT TABLE_SCHEMA from information_schema.TABLES where TABLE_TYPE != 'SYSTEM VIEW' ORDER BY TABLE_SCHEMA"
+            },
+            "fields": [
+              { "name": "TABLE_SCHEMA", "type": "string", "typeInfo": { "frame": "string", "nullable": true } }
+            ]
+          },
+          "data": { "values": [["DataMaker", "mysql", "performance_schema", "sys"]] }
+        }
+      ]
+    }
+  }
+}

e2e/sql-suite/fields-response.json

@@ -0,0 +1,27 @@
+{
+  "results": {
+    "fields": {
+      "status": 200,
+      "frames": [
+        {
+          "schema": {
+            "refId": "fields",
+            "meta": {
+              "executedQueryString": "SELECT column_name, data_type FROM information_schema.columns WHERE table_schema = 'DataMaker' AND table_name = 'RandomIntsWithTimes' ORDER BY column_name"
+            },
+            "fields": [
+              { "name": "COLUMN_NAME", "type": "string", "typeInfo": { "frame": "string", "nullable": true } },
+              { "name": "DATA_TYPE", "type": "string", "typeInfo": { "frame": "string", "nullable": true } }
+            ]
+          },
+          "data": {
+            "values": [
+              ["createdAt", "id", "time", "updatedAt", "bigint"],
+              ["datetime", "int", "datetime", "datetime", "int"]
+            ]
+          }
+        }
+      ]
+    }
+  }
+}

e2e/sql-suite/mysql.spec.ts

@@ -0,0 +1,59 @@
+import { e2e } from '@grafana/e2e';
+
+import datasetResponse from './datasets-response.json';
+import fieldsResponse from './fields-response.json';
+import tablesResponse from './tables-response.json';
+
+const tableNameWithSpecialCharacter = tablesResponse.results.tables.frames[0].data.values[0][1];
+const normalTableName = tablesResponse.results.tables.frames[0].data.values[0][0];
+
+describe('MySQL datasource', () => {
+  it('code editor autocomplete should handle table name escaping/quoting', () => {
+    e2e.flows.login('admin', 'admin');
+
+    e2e().intercept('POST', '**/api/ds/query', (req) => {
+      if (req.body.queries[0].refId === 'datasets') {
+        req.alias = 'datasets';
+        req.reply({
+          body: datasetResponse,
+        });
+      } else if (req.body.queries[0].refId === 'tables') {
+        req.alias = 'tables';
+        req.reply({
+          body: tablesResponse,
+        });
+      } else if (req.body.queries[0].refId === 'fields') {
+        req.alias = 'fields';
+        req.reply({
+          body: fieldsResponse,
+        });
+      }
+    });
+
+    e2e.pages.Explore.visit();
+
+    e2e.components.DataSourcePicker.container().should('be.visible').type('gdev-mysql{enter}');
+
+    e2e().get("label[for^='option-code']").should('be.visible').click();
+    e2e().get('textarea').type('S{downArrow}{enter}');
+    e2e().wait('@tables');
+    e2e().get('.suggest-widget').contains(tableNameWithSpecialCharacter).should('be.visible');
+    e2e().get('textarea').type('{enter}');
+    e2e().get('textarea').should('have.value', `SELECT  FROM grafana.\`${tableNameWithSpecialCharacter}\``);
+
+    const deleteTimes = new Array(tableNameWithSpecialCharacter.length + 2).fill(
+      '{backspace}',
+      0,
+      tableNameWithSpecialCharacter.length + 2
+    );
+    e2e().get('textarea').type(deleteTimes.join(''));
+
+    e2e().get('textarea').type('{command}i');
+    e2e().get('.suggest-widget').contains(tableNameWithSpecialCharacter).should('be.visible');
+    e2e().get('textarea').type('S{downArrow}{enter}');
+    e2e().get('textarea').should('have.value', `SELECT  FROM grafana.${normalTableName}`);
+
+    e2e().get('textarea').type('.');
+    e2e().get('.suggest-widget').contains('No suggestions.').should('be.visible');
+  });
+});

e2e/sql-suite/tables-response.json

@@ -0,0 +1,19 @@
+{
+  "results": {
+    "tables": {
+      "status": 200,
+      "frames": [
+        {
+          "schema": {
+            "refId": "tables",
+            "meta": {
+              "executedQueryString": "SELECT table_name FROM information_schema.tables WHERE table_schema = 'DataMaker' ORDER BY table_name"
+            },
+            "fields": [{ "name": "TABLE_NAME", "type": "string", "typeInfo": { "frame": "string", "nullable": true } }]
+          },
+          "data": { "values": [["normalTable", "table-name"]] }
+        }
+      ]
+    }
+  }
+}

public/app/features/plugins/sql/components/QueryHeader.tsx

@@ -3,15 +3,13 @@ import { useCopyToClipboard } from 'react-use';
 
 import { SelectableValue } from '@grafana/data';
 import { EditorField, EditorHeader, EditorMode, EditorRow, FlexItem, InlineSelect, Space } from '@grafana/experimental';
-import { Button, InlineField, InlineSwitch, RadioButtonGroup, Select, Tooltip } from '@grafana/ui';
+import { Button, InlineSwitch, RadioButtonGroup, Tooltip } from '@grafana/ui';
 
 import { QueryWithDefaults } from '../defaults';
 import { SQLQuery, QueryFormat, QueryRowFilter, QUERY_FORMAT_OPTIONS, DB } from '../types';
-import { defaultToRawSql } from '../utils/sql.utils';
 
 import { ConfirmModal } from './ConfirmModal';
 import { DatasetSelector } from './DatasetSelector';
-import { ErrorBoundary } from './ErrorBoundary';
 import { TableSelector } from './TableSelector';
 
 export interface QueryHeaderProps {
@@ -43,7 +41,7 @@ export function QueryHeader({
   const { editorMode } = query;
   const [_, copyToClipboard] = useCopyToClipboard();
   const [showConfirm, setShowConfirm] = useState(false);
-  const toRawSql = db.toRawSql || defaultToRawSql;
+  const toRawSql = db.toRawSql;
 
   const onEditorModeChange = useCallback(
     (newEditorMode: EditorMode) => {
@@ -94,28 +92,14 @@ export function QueryHeader({
   return (
     <>
       <EditorHeader>
-        {/* Backward compatibility check. Inline select uses SelectContainer that was added in 8.3 */}
-        <ErrorBoundary
-          fallBackComponent={
-            <InlineField label="Format" labelWidth={15}>
-              <Select
-                placeholder="Select format"
-                value={query.format}
-                onChange={onFormatChange}
-                options={QUERY_FORMAT_OPTIONS}
-              />
-            </InlineField>
-          }
-        >
-          <InlineSelect
-            label="Format"
-            value={query.format}
-            placeholder="Select format"
-            menuShouldPortal
-            onChange={onFormatChange}
-            options={QUERY_FORMAT_OPTIONS}
-          />
-        </ErrorBoundary>
+        <InlineSelect
+          label="Format"
+          value={query.format}
+          placeholder="Select format"
+          menuShouldPortal
+          onChange={onFormatChange}
+          options={QUERY_FORMAT_OPTIONS}
+        />
 
         {editorMode === EditorMode.Builder && (
           <>

public/app/features/plugins/sql/components/visual-query-builder/SelectRow.tsx

@@ -137,6 +137,7 @@ export function SelectRow({ sql, format, columns, onSqlChange, functions }: Sele
             <EditorField label="Alias" optional width={15}>
               <Select
                 value={item.alias ? toOption(item.alias) : null}
+                inputId={`select-alias-${index}-${uniqueId()}`}
                 options={timeSeriesAliasOpts}
                 onChange={onAliasChange(item, index)}
                 isClearable

public/app/features/plugins/sql/types.ts

@@ -132,7 +132,7 @@ export interface DB {
   dispose?: (dsID?: string) => void;
   lookup?: (path?: string) => Promise<Array<{ name: string; completion: string }>>;
   getEditorLanguageDefinition: () => LanguageDefinition;
-  toRawSql?: (query: SQLQuery) => string;
+  toRawSql: (query: SQLQuery) => string;
   functions?: () => string[];
 }
 

public/app/features/plugins/sql/utils/sql.utils.ts

@@ -1,53 +1,13 @@
-import { isEmpty } from 'lodash';
-
 import {
   QueryEditorExpressionType,
   QueryEditorFunctionExpression,
   QueryEditorGroupByExpression,
   QueryEditorPropertyExpression,
   QueryEditorPropertyType,
 } from '../expressions';
-import { SQLQuery, SQLExpression } from '../types';
-
-export function defaultToRawSql({ sql, dataset, table }: SQLQuery): string {
-  let rawQuery = '';
-
-  // Return early with empty string if there is no sql column
-  if (!sql || !haveColumns(sql.columns)) {
-    return rawQuery;
-  }
-
-  rawQuery += createSelectClause(sql.columns);
-
-  if (dataset && table) {
-    rawQuery += `FROM ${dataset}.${table} `;
-  }
-
-  if (sql.whereString) {
-    rawQuery += `WHERE ${sql.whereString} `;
-  }
-
-  if (sql.groupBy?.[0]?.property.name) {
-    const groupBy = sql.groupBy.map((g) => g.property.name).filter((g) => !isEmpty(g));
-    rawQuery += `GROUP BY ${groupBy.join(', ')} `;
-  }
-
-  if (sql.orderBy?.property.name) {
-    rawQuery += `ORDER BY ${sql.orderBy.property.name} `;
-  }
-
-  if (sql.orderBy?.property.name && sql.orderByDirection) {
-    rawQuery += `${sql.orderByDirection} `;
-  }
-
-  // Altough LIMIT 0 doesn't make sense, it is still possible to have LIMIT 0
-  if (sql.limit !== undefined && sql.limit >= 0) {
-    rawQuery += `LIMIT ${sql.limit} `;
-  }
-  return rawQuery;
-}
+import { SQLExpression } from '../types';
 
-function createSelectClause(sqlColumns: NonNullable<SQLExpression['columns']>): string {
+export function createSelectClause(sqlColumns: NonNullable<SQLExpression['columns']>): string {
   const columns = sqlColumns.map((c) => {
     let rawColumn = '';
     if (c.name && c.alias) {

public/app/features/plugins/sql/utils/useSqlChange.ts

@@ -2,8 +2,6 @@ import { useCallback } from 'react';
 
 import { DB, SQLExpression, SQLQuery } from '../types';
 
-import { defaultToRawSql } from './sql.utils';
-
 interface UseSqlChange {
   db: DB;
   query: SQLQuery;
@@ -13,7 +11,7 @@ interface UseSqlChange {
 export function useSqlChange({ query, onQueryChange, db }: UseSqlChange) {
   const onSqlChange = useCallback(
     (sql: SQLExpression) => {
-      const toRawSql = db.toRawSql || defaultToRawSql;
+      const toRawSql = db.toRawSql;
       const rawSql = toRawSql({ sql, dataset: query.dataset, table: query.table, refId: query.refId });
       const newQuery: SQLQuery = { ...query, sql, rawSql };
       onQueryChange(newQuery);

public/app/plugins/datasource/mysql/MySqlDatasource.ts

@@ -1,14 +1,13 @@
-import { DataSourceInstanceSettings, ScopedVars, TimeRange } from '@grafana/data';
+import { DataSourceInstanceSettings, TimeRange } from '@grafana/data';
 import { CompletionItemKind, LanguageDefinition, TableIdentifier } from '@grafana/experimental';
-import { TemplateSrv } from '@grafana/runtime';
 import { SqlDatasource } from 'app/features/plugins/sql/datasource/SqlDatasource';
 import { DB, SQLQuery } from 'app/features/plugins/sql/types';
 import { formatSQL } from 'app/features/plugins/sql/utils/formatSQL';
 
-import MySQLQueryModel from './MySqlQueryModel';
 import { mapFieldsToTypes } from './fields';
 import { buildColumnQuery, buildTableQuery, showDatabases } from './mySqlMetaQuery';
 import { getSqlCompletionProvider } from './sqlCompletionProvider';
+import { quoteIdentifierIfNecessary, quoteLiteral, toRawSql } from './sqlUtil';
 import { MySQLOptions } from './types';
 
 export class MySqlDatasource extends SqlDatasource {
@@ -18,20 +17,20 @@ export class MySqlDatasource extends SqlDatasource {
     super(instanceSettings);
   }
 
-  getQueryModel(target?: Partial<SQLQuery>, templateSrv?: TemplateSrv, scopedVars?: ScopedVars): MySQLQueryModel {
-    return new MySQLQueryModel(target!, templateSrv, scopedVars);
+  getQueryModel() {
+    return { quoteLiteral };
   }
 
-  getSqlLanguageDefinition(db: DB): LanguageDefinition {
+  getSqlLanguageDefinition(): LanguageDefinition {
     if (this.sqlLanguageDefinition !== undefined) {
       return this.sqlLanguageDefinition;
     }
 
     const args = {
-      getMeta: { current: (identifier?: TableIdentifier) => this.fetchMeta(identifier) },
+      getMeta: (identifier?: TableIdentifier) => this.fetchMeta(identifier),
     };
     this.sqlLanguageDefinition = {
-      id: 'sql',
+      id: 'mysql',
       completionProvider: getSqlCompletionProvider(args),
       formatter: formatSQL,
     };
@@ -40,21 +39,27 @@ export class MySqlDatasource extends SqlDatasource {
 
   async fetchDatasets(): Promise<string[]> {
     const datasets = await this.runSql<string[]>(showDatabases(), { refId: 'datasets' });
-    return datasets.map((t) => t[0]);
+    return datasets.map((t) => quoteIdentifierIfNecessary(t[0]));
   }
 
   async fetchTables(dataset?: string): Promise<string[]> {
     const tables = await this.runSql<string[]>(buildTableQuery(dataset), { refId: 'tables' });
-    return tables.map((t) => t[0]);
+    return tables.map((t) => quoteIdentifierIfNecessary(t[0]));
   }
 
   async fetchFields(query: Partial<SQLQuery>) {
     if (!query.dataset || !query.table) {
       return [];
     }
-    const queryString = buildColumnQuery(this.getQueryModel(query), query.table!);
+    const queryString = buildColumnQuery(query.table, query.dataset);
     const frame = await this.runSql<string[]>(queryString, { refId: 'fields' });
-    const fields = frame.map((f) => ({ name: f[0], text: f[0], value: f[0], type: f[1], label: f[0] }));
+    const fields = frame.map((f) => ({
+      name: f[0],
+      text: f[0],
+      value: quoteIdentifierIfNecessary(f[0]),
+      type: f[1],
+      label: f[0],
+    }));
     return mapFieldsToTypes(fields);
   }
 
@@ -67,12 +72,12 @@ export class MySqlDatasource extends SqlDatasource {
       const datasets = await this.fetchDatasets();
       return datasets.map((d) => ({ name: d, completion: `${d}.`, kind: CompletionItemKind.Module }));
     } else {
-      if (!identifier?.table && !defaultDB) {
+      if (!identifier?.table && (!defaultDB || identifier?.schema)) {
         const tables = await this.fetchTables(identifier?.schema);
         return tables.map((t) => ({ name: t, completion: t, kind: CompletionItemKind.Class }));
       } else if (identifier?.table && identifier.schema) {
         const fields = await this.fetchFields({ dataset: identifier.schema, table: identifier.table });
-        return fields.map((t) => ({ name: t.value, completion: t.value, kind: CompletionItemKind.Field }));
+        return fields.map((t) => ({ name: t.name, completion: t.value, kind: CompletionItemKind.Field }));
       } else {
         return [];
       }
@@ -90,8 +95,9 @@ export class MySqlDatasource extends SqlDatasource {
       validateQuery: (query: SQLQuery, range?: TimeRange) =>
         Promise.resolve({ query, error: '', isError: false, isValid: true }),
       dsID: () => this.id,
+      toRawSql,
       functions: () => ['VARIANCE', 'STDDEV'],
-      getEditorLanguageDefinition: () => this.getSqlLanguageDefinition(this.db),
+      getEditorLanguageDefinition: () => this.getSqlLanguageDefinition(),
     };
   }
 }