fix(sqllab): clean comments within quotes (apache#23908)

superset-frontend/src/SqlLab/actions/sqlLab.js

@@ -357,12 +357,70 @@ 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 -> --
-  return sql.replace(/(--.*?$|\/\*[\s\S]*?\*\/)\n?/gm, '\n').trim();
+  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)],
+      )
+  );
 }
 
 export function runQuery(query) {

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"--'`);
       });
     });
   });