From 167d6a26edfbdbd4b8b5e66bfade7bbacf068909 Mon Sep 17 00:00:00 2001 From: justin-park Date: Tue, 2 May 2023 11:28:16 -0700 Subject: [PATCH 1/4] fix(sqllab): clean comments within quotes --- .../src/SqlLab/actions/sqlLab.js | 49 ++++++++++++++++++- .../src/SqlLab/actions/sqlLab.test.js | 13 ++++- 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 6b7802a4cff99..3af9edd2c99a2 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -357,12 +357,59 @@ 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; + + str.split('').forEach((currentChar, i) => { + if ( + (currentQuote && currentChar === currentQuote) || + (!currentQuote && quotes.includes(currentChar)) + ) { + if (currentQuote) { + chunks.push(str.substring(chunkStart, i + 1)); + chunkStart = i + 1; + currentQuote = ''; + } else { + chunks.push(str.substring(chunkStart, i)); + chunkStart = i; + currentQuote = currentChar; + } + } + }); + + 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 -> -- - return sql.replace(/(--.*?$|\/\*[\s\S]*?\*\/)\n?/gm, '\n').trim(); + const chunks = splitByQuotedBlock(sql); + return chunks + .map((chunk, index) => + quotes.includes(chunk[0]) ? `${quotedBlockHash}:${index}:` : chunk, + ) + .join('') + .replace(/(--.*?$|\/\*[\s\S]*?\*\/)\n?/gm, '\n') + .replace( + quotedBlockMatch, + quotedBlock => chunks[quotedBlock.match(/:\d+/)[0].substring(1)], + ) + .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 1e6dcf78a4285..46a9536fc9278 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -312,7 +312,16 @@ describe('async actions', () => { const makeRequest = () => { const request = actions.runQuery({ ...query, - sql: '/*\nSELECT * FROM\n */\nSELECT 213--, {{ds}}\n/*\n{{new_param1}}\n{{new_param2}}*/\n\nFROM table', + sql: `/* + SELECT * FROM +*/ +SELECT 213--, {{ds}} "quote out" +/* +{{new_param1}} +{{new_param2}}*/ + +FROM table +WHERE value = '--"NULL"--' --{{test_param}}`, }); return request(dispatch, () => initialState); }; @@ -326,7 +335,7 @@ describe('async actions', () => { expect(fetchMock.calls(runQueryEndpoint)).toHaveLength(1); expect( JSON.parse(fetchMock.calls(runQueryEndpoint)[0][1].body).sql, - ).toEqual('SELECT 213\n\n\nFROM table'); + ).toEqual(`SELECT 213\n\n\nFROM table\nWHERE value = '--"NULL"--'`); }); }); }); From 7ef533cebf753bfc081114c1ec0f6d25d2b90c7f Mon Sep 17 00:00:00 2001 From: justin-park Date: Tue, 2 May 2023 19:50:05 -0700 Subject: [PATCH 2/4] add comments for code detail --- .../src/SqlLab/actions/sqlLab.js | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 3af9edd2c99a2..0632872798f35 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -399,17 +399,22 @@ export function cleanSqlComments(sql) { // group 1 -> /* */ // group 2 -> -- const chunks = splitByQuotedBlock(sql); - return chunks - .map((chunk, index) => - quotes.includes(chunk[0]) ? `${quotedBlockHash}:${index}:` : chunk, - ) - .join('') - .replace(/(--.*?$|\/\*[\s\S]*?\*\/)\n?/gm, '\n') - .replace( - quotedBlockMatch, - quotedBlock => chunks[quotedBlock.match(/:\d+/)[0].substring(1)], - ) - .trim(); + 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)], + ) + ); } export function runQuery(query) { From da2810d334aaa4fc33ae5e4d12a8049aaf687a83 Mon Sep 17 00:00:00 2001 From: justin-park Date: Wed, 3 May 2023 07:07:10 -0700 Subject: [PATCH 3/4] fix the matching quote condition --- superset-frontend/src/SqlLab/actions/sqlLab.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 0632872798f35..f8bb9dd188174 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -368,8 +368,7 @@ function splitByQuotedBlock(str) { str.split('').forEach((currentChar, i) => { if ( - (currentQuote && currentChar === currentQuote) || - (!currentQuote && quotes.includes(currentChar)) + currentQuote ? currentChar === currentQuote : quotes.includes(currentChar) ) { if (currentQuote) { chunks.push(str.substring(chunkStart, i + 1)); From 0d1cc1550703605355654d39e3f055170d179de5 Mon Sep 17 00:00:00 2001 From: justin-park Date: Wed, 3 May 2023 09:27:48 -0700 Subject: [PATCH 4/4] optimize forEach operation by while-loop --- superset-frontend/src/SqlLab/actions/sqlLab.js | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index f8bb9dd188174..d617fa23f5432 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -366,21 +366,28 @@ function splitByQuotedBlock(str) { let currentQuote = ''; let chunkStart = 0; - str.split('').forEach((currentChar, i) => { + let i = 0; + while (i < str.length) { + const currentChar = str[i]; if ( currentQuote ? currentChar === currentQuote : quotes.includes(currentChar) ) { + let chunk; if (currentQuote) { - chunks.push(str.substring(chunkStart, i + 1)); + chunk = str.substring(chunkStart, i + 1); chunkStart = i + 1; currentQuote = ''; } else { - chunks.push(str.substring(chunkStart, i)); + 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);