Update SQL parser to handle more modern syntax (#28211)
For #26366 # Checklist for submitter - [X] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files) for more information. # Details This PR fixes an issue where the SQL parser in the UI doesn't recognize window functions like `OVER()` and marks the SQL as having syntax errors. The fix here is to update to a more modern parsing library. This involved updating some AST-parsing code we have for determining which tables are used in a query, for the purposes of feeding autocomplete and determining query compatibility. # Testing I tested this with the query mentioned in #26366 in Chrome, Firefox and Safari on MacOS. I also added new unit tests for our SQL helper functions. # Notes During testing I discovered that we were bundling two versions of the ACE editor into our frontend package. By upgrading one version by a couple of patches to make the two dependencies equal, we chop out ~300k from our bundle.
This commit is contained in:
@@ -1,12 +1,11 @@
|
||||
import sqliteParser from "sqlite-parser";
|
||||
import { Parser } from "node-sql-parser";
|
||||
import { includes, some } from "lodash";
|
||||
|
||||
const BLACKLISTED_ACTIONS = [];
|
||||
const invalidQueryErrorMessage = "Blacklisted query action";
|
||||
const invalidQueryResponse = (message) => {
|
||||
return { valid: false, error: message };
|
||||
};
|
||||
const validQueryResponse = { valid: true, error: null };
|
||||
const parser = new Parser();
|
||||
|
||||
export const validateQuery = (queryText) => {
|
||||
if (!queryText) {
|
||||
@@ -14,19 +13,12 @@ export const validateQuery = (queryText) => {
|
||||
}
|
||||
|
||||
try {
|
||||
const ast = sqliteParser(queryText);
|
||||
const { statement } = ast;
|
||||
const invalidQuery = some(statement, (obj) => {
|
||||
return includes(BLACKLISTED_ACTIONS, obj.variant.toLowerCase());
|
||||
});
|
||||
|
||||
if (invalidQuery) {
|
||||
return invalidQueryResponse(invalidQueryErrorMessage);
|
||||
}
|
||||
|
||||
parser.astify(queryText, { database: "sqlite" });
|
||||
return validQueryResponse;
|
||||
} catch (error) {
|
||||
return invalidQueryResponse(error.message);
|
||||
return invalidQueryResponse(
|
||||
"There is a syntax error in your query; please resolve in order to save."
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -22,7 +22,9 @@ describe("validateQuery", () => {
|
||||
const { error, valid } = validateQuery(query);
|
||||
|
||||
expect(valid).toEqual(false);
|
||||
expect(error).toMatch(/Syntax error found near .+/);
|
||||
expect(error).toMatch(
|
||||
"There is a syntax error in your query; please resolve in order to save."
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user