Skip to content

Commit

Permalink
Fix removing comments (#523)
Browse files Browse the repository at this point in the history
* Fix removing comments

* Fix comments removing algorithm

* Add doesQueryContainOrderBy unit tests
  • Loading branch information
artemipanchuk authored Jan 24, 2024
1 parent 8bb2525 commit 7a06799
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 9 deletions.
6 changes: 2 additions & 4 deletions src/server/modes/charts/plugins/ql/js.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ import preparePreviewTable from './preparers/preview-table';
import prepareTable from './preparers/table';
import {LINEAR_VISUALIZATIONS, PIE_VISUALIZATIONS} from './utils/constants';
import {
doesQueryContainOrderBy,
getColumnsAndRows,
log,
prepareQuery,
visualizationCanHaveContinuousAxis,
} from './utils/misc-helpers';
import {
Expand Down Expand Up @@ -224,9 +224,7 @@ export default ({shared, ChartEditor}: {shared: QlConfig; ChartEditor: IChartEdi

const available = [...(fields as unknown as Field[])];

const preparedQuery = prepareQuery(shared.queryValue);

const disableDefaultSorting = /order by/gi.test(preparedQuery);
const disableDefaultSorting = doesQueryContainOrderBy(shared.queryValue);

const prepareSingleResultArgs = {
resultData,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type {QLConfigQuery} from '../../../../../../../shared';
import {convertConnectionType} from '../connection';
import {buildSource, iterateThroughVisibleQueries} from '../misc-helpers';
import {buildSource, doesQueryContainOrderBy, iterateThroughVisibleQueries} from '../misc-helpers';

const MOCK_ID = 'MOCK_ID';

Expand Down Expand Up @@ -115,3 +115,31 @@ describe('iterateThroughVisibleQueries', () => {
);
});
});

describe('doesQueryContainOrderBy', () => {
it('should return true when order by is used normally', () => {
const queryContainsOrderBy = doesQueryContainOrderBy('select a from b order by a desc');

expect(queryContainsOrderBy).toEqual(true);
});

it('should return false when order by is not used', () => {
const queryContainsOrderBy = doesQueryContainOrderBy('select a from b');

expect(queryContainsOrderBy).toEqual(false);
});

it('should return false when order by is commented out', () => {
const queryContainsOrderBy = doesQueryContainOrderBy('select a from b -- order by a desc');

expect(queryContainsOrderBy).toEqual(false);
});

it('should return true when order by is used normally, but query contains -- in quotes', () => {
const queryContainsOrderBy = doesQueryContainOrderBy(
`select a from b where a like '%--%' order by a`,
);

expect(queryContainsOrderBy).toEqual(true);
});
});
28 changes: 24 additions & 4 deletions src/server/modes/charts/plugins/ql/utils/misc-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -739,10 +739,30 @@ export function iterateThroughVisibleQueries(
}

export const prepareQuery = (query: string) => {
return query
.replace(/--[^\n]*\n/g, '')
.trim()
.replace(/;+$/g, '');
return query.trim().replace(/;+$/g, '');
};

const removeComments = (query: string) => {
const result = query.replace(
/("(""|[^"])*")|('(''|[^'])*')|(--[^\n\r]*)|(\/\*[\w\W]*?(?=\*\/)\*\/)/gm,
(match) => {
if (
(match[0] === '"' && match[match.length - 1] === '"') ||
(match[0] === "'" && match[match.length - 1] === "'")
)
return match;

return '';
},
);

return result;
};

export const doesQueryContainOrderBy = (query: string) => {
const queryWithoutComments = removeComments(query);

return /order by/gim.test(queryWithoutComments);
};

export const visualizationCanHaveContinuousAxis = (visualization: ServerVisualization) => {
Expand Down

0 comments on commit 7a06799

Please sign in to comment.