From fdd69d374138a7cde881c3a9c1679f3732ffdefc Mon Sep 17 00:00:00 2001 From: justin-park Date: Wed, 10 May 2023 10:20:01 -0700 Subject: [PATCH 1/2] Revert "fix(sqllab): clean comments within quotes (#23908)" This reverts commit 841726d4325bfdad13eec81cbca537f9dcd93284. --- .../src/SqlLab/actions/sqlLab.js | 60 +------------------ .../src/SqlLab/actions/sqlLab.test.js | 13 +--- 2 files changed, 3 insertions(+), 70 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index d617fa23f5432..6b7802a4cff99 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -357,70 +357,12 @@ export function fetchQueryResults(query, displayLimit) { }; } -const quotes = '\'"`'.split(''); -const quotedBlockHash = shortid.generate(); -const quotedBlockMatch = new RegExp(`${quotedBlockHash}:\\d+:`, 'g'); - -function splitByQuotedBlock(str) { - const chunks = []; - let currentQuote = ''; - let chunkStart = 0; - - let i = 0; - while (i < str.length) { - const currentChar = str[i]; - if ( - currentQuote ? currentChar === currentQuote : quotes.includes(currentChar) - ) { - let chunk; - if (currentQuote) { - chunk = str.substring(chunkStart, i + 1); - chunkStart = i + 1; - currentQuote = ''; - } else { - chunk = str.substring(chunkStart, i); - chunkStart = i; - currentQuote = currentChar; - } - if (chunk) { - chunks.push(chunk); - } - } - i += 1; - } - - if (chunkStart < str.length) { - const lastChunk = str.substring(chunkStart); - if (lastChunk) { - chunks.push(lastChunk); - } - } - - return chunks; -} - export function cleanSqlComments(sql) { if (!sql) return ''; // it sanitizes the following comment block groups // group 1 -> /* */ // group 2 -> -- - const chunks = splitByQuotedBlock(sql); - return ( - chunks - // replace quoted blocks in a hash format - .map((chunk, index) => - quotes.includes(chunk[0]) ? `${quotedBlockHash}:${index}:` : chunk, - ) - .join('') - // Clean out the commented-out blocks - .replace(/(--.*?$|\/\*[\s\S]*?\*\/)\n?/gm, '\n') - .trim() - // restore quoted block to the original value - .replace( - quotedBlockMatch, - quotedBlock => chunks[quotedBlock.match(/:\d+/)[0].substring(1)], - ) - ); + return sql.replace(/(--.*?$|\/\*[\s\S]*?\*\/)\n?/gm, '\n').trim(); } export function runQuery(query) { diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index 46a9536fc9278..1e6dcf78a4285 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -312,16 +312,7 @@ describe('async actions', () => { const makeRequest = () => { const request = actions.runQuery({ ...query, - sql: `/* - SELECT * FROM -*/ -SELECT 213--, {{ds}} "quote out" -/* -{{new_param1}} -{{new_param2}}*/ - -FROM table -WHERE value = '--"NULL"--' --{{test_param}}`, + sql: '/*\nSELECT * FROM\n */\nSELECT 213--, {{ds}}\n/*\n{{new_param1}}\n{{new_param2}}*/\n\nFROM table', }); return request(dispatch, () => initialState); }; @@ -335,7 +326,7 @@ WHERE value = '--"NULL"--' --{{test_param}}`, expect(fetchMock.calls(runQueryEndpoint)).toHaveLength(1); expect( JSON.parse(fetchMock.calls(runQueryEndpoint)[0][1].body).sql, - ).toEqual(`SELECT 213\n\n\nFROM table\nWHERE value = '--"NULL"--'`); + ).toEqual('SELECT 213\n\n\nFROM table'); }); }); }); From 67ac8e8756100e69e7deeeb8383d1ea26002e2cd Mon Sep 17 00:00:00 2001 From: justin-park Date: Wed, 10 May 2023 10:20:29 -0700 Subject: [PATCH 2/2] Revert "fix(sqllab): throw errors of commented out query (#23378)" This reverts commit d1947f735485986364f0930c66f68bc6c3292383. --- .../src/SqlLab/actions/sqlLab.js | 10 +------- .../src/SqlLab/actions/sqlLab.test.js | 23 ------------------- .../components/RunQueryActionButton/index.tsx | 5 ++-- 3 files changed, 4 insertions(+), 34 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 6b7802a4cff99..3b9e68b9c182a 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -357,14 +357,6 @@ 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)); @@ -374,7 +366,7 @@ export function runQuery(query) { json: true, runAsync: query.runAsync, schema: query.schema, - sql: cleanSqlComments(query.sql), + sql: 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 index 1e6dcf78a4285..01886d6f77d08 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -308,29 +308,6 @@ 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 index f9181188f9817..57a22b0aec4e7 100644 --- a/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx +++ b/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx @@ -26,7 +26,6 @@ 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; @@ -106,7 +105,9 @@ const RunQueryActionButton = ({ : Button; const sqlContent = selectedText || sql || ''; - const isDisabled = cleanSqlComments(sqlContent).length === 0; + const isDisabled = + !sqlContent || + !sqlContent.replace(/(\/\*[^*]*\*\/)|(\/\/[^*]*)|(--[^.].*)/gm, '').trim(); const stopButtonTooltipText = useMemo( () =>