Use forked node-sql-parser, fix CTE issues in parsed SQL#38744
Use forked node-sql-parser, fix CTE issues in parsed SQL#38744
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #38744 +/- ##
==========================================
- Coverage 66.07% 66.06% -0.02%
==========================================
Files 2414 2414
Lines 192683 192722 +39
Branches 8424 8550 +126
==========================================
+ Hits 127321 127327 +6
- Misses 53799 53832 +33
Partials 11563 11563
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Added tests for the specific queries mentioned in issues. These will act as regression tests as new patches are made to the node-sql-parser package.
| @@ -1,5 +1,5 @@ | |||
| // @ts-ignore | |||
There was a problem hiding this comment.
nit: we could also add an alias for this in tsconfig.json (the same way you did on the webpack.config.js) and remove this ts-ignore directive:
"paths": {
"node-sql-parser": ["../node_modules/@sgress454/node-sql-parser"]
}
There was a problem hiding this comment.
great idea! done.
Related issue: Resolves #34635
Details
This PR switches us to a fork of node-sql-parser that I'm maintaining to fast-track fixes to the SQLite implementation. The first published version of the fork is 5.4.0-fork.1 (forked from v5.4.0 of the upstream), and includes fixes for #34635 and #30109 that haven't made it to the upstream yet.
Fixes in 5.4.0-fork.1:
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing