fix(sqllab): rollback clean comments out (#24009)

apache · May 11, 2023 · 7a55625 · 7a55625
1 parent c963416
commit 7a55625
Show file tree

Hide file tree

Showing 3 changed files with 4 additions and 101 deletions.
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js
@@ -357,72 +357,6 @@ 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)],
- )
- );
-}
-
 export function runQuery(query) {
  return function (dispatch) {
  dispatch(startQuery(query));
@@ -432,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
@@ -308,38 +308,6 @@ describe('async actions', () => {
  });
  });
 
- describe('runQuery with comments', () => {
- 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}}`,
- });
- 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\nWHERE value = '--"NULL"--'`);
- });
- });
- });
-
  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,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(
  () =>