fix(sqllab): throw errors of commented out query (#23378)

apache · Mar 23, 2023 · d1947f7 · d1947f7
1 parent 8f846b7
commit d1947f7
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 4 deletions.
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js
@@ -353,6 +353,14 @@ export function fetchQueryResults(query, displayLimit) {
   };
 }
 
+export function cleanSqlComments(sql) {
+  if (!sql) return '';
+  // it sanitizes the following comment block groups
+  // group 1 -> /* */
+  // group 2 -> --
+  return sql.replace(/(--.*?$|\/\*[\s\S]*?\*\/)\n?/gm, '\n').trim();
+}
+
 export function runQuery(query) {
   return function (dispatch) {
     dispatch(startQuery(query));
@@ -362,7 +370,7 @@ export function runQuery(query) {
       json: true,
       runAsync: query.runAsync,
       schema: query.schema,
-      sql: query.sql,
+      sql: cleanSqlComments(query.sql),
       sql_editor_id: query.sqlEditorId,
       tab: query.tab,
       tmp_table_name: query.tempTable,

diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js
@@ -308,6 +308,29 @@ describe('async actions', () => {
     });
   });
 
+  describe('runQuery with comments', () => {
+    const makeRequest = () => {
+      const request = actions.runQuery({
+        ...query,
+        sql: '/*\nSELECT * FROM\n */\nSELECT 213--, {{ds}}\n/*\n{{new_param1}}\n{{new_param2}}*/\n\nFROM table',
+      });
+      return request(dispatch, () => initialState);
+    };
+
+    it('makes the fetch request without comments', async () => {
+      const runQueryEndpoint = 'glob:*/api/v1/sqllab/execute/';
+      fetchMock.post(runQueryEndpoint, '{}', {
+        overwriteRoutes: true,
+      });
+      await makeRequest().then(() => {
+        expect(fetchMock.calls(runQueryEndpoint)).toHaveLength(1);
+        expect(
+          JSON.parse(fetchMock.calls(runQueryEndpoint)[0][1].body).sql,
+        ).toEqual('SELECT 213\n\n\nFROM table');
+      });
+    });
+  });
+
   describe('reRunQuery', () => {
     let stub;
     beforeEach(() => {

diff --git a/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx b/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx
@@ -26,6 +26,7 @@ import { DropdownButton } from 'src/components/DropdownButton';
 import { detectOS } from 'src/utils/common';
 import { QueryButtonProps } from 'src/SqlLab/types';
 import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor';
+import { cleanSqlComments } from 'src/SqlLab/actions/sqlLab';
 
 export interface RunQueryActionButtonProps {
   queryEditorId: string;
@@ -105,9 +106,7 @@ const RunQueryActionButton = ({
     : Button;
 
   const sqlContent = selectedText || sql || '';
-  const isDisabled =
-    !sqlContent ||
-    !sqlContent.replace(/(\/\*[^*]*\*\/)|(\/\/[^*]*)|(--[^.].*)/gm, '').trim();
+  const isDisabled = cleanSqlComments(sqlContent).length === 0;
 
   const stopButtonTooltipText = useMemo(
     () =>