From 2e61bfbe5e56bc93b9438232a673b05ec09fd063 Mon Sep 17 00:00:00 2001 From: Kacper Kula Date: Wed, 15 Jan 2025 17:48:33 +0000 Subject: [PATCH] fix: fixed SQL parsing by changing underlying parsing library --- CHANGELOG.md | 9 +- package.json | 2 +- pnpm-lock.yaml | 31 +--- src/codeblockHandler/CodeblockProcessor.ts | 8 +- .../inline/InlineProcessor.ts | 8 +- src/sql/sqlTransformer.ts | 37 +++++ src/sql/transformer.test.ts | 38 ++++- src/sql/transformer.ts | 143 ------------------ 8 files changed, 93 insertions(+), 183 deletions(-) create mode 100644 src/sql/sqlTransformer.ts delete mode 100644 src/sql/transformer.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2113b03..ae00f02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,11 @@ # UNRELEASED -fix: fix issue with local links not being clickable -fix: query is now less case sensitive. You can use keywords like `TABLE`, `HTML`, `MARKDOWN` in any casing you want +- fix: fix issue with local links not being clickable +- fix: query is now less case sensitive. You can use keywords like `TABLE`, `HTML`, `MARKDOWN` in any casing you want +- fix: improved SQL parser - now more complex syntax like recursive CTE, window functions, etc. should work properly. Migrated from `node-sql-parser` to `sql-parser-cst` # 0.14.1 -fix: fixed the issue where rows with extra data in them (rows with more columns that a header) were not synchronised correctly -fix: fixed the issue where queries with lowercase SELECT would not work in certain cases. +- fix: fixed the issue where rows with extra data in them (rows with more columns that a header) were not synchronised correctly +- fix: fixed the issue where queries with lowercase SELECT would not work in certain cases. # 0.14.0 Added support for inline code blocks. Create a codeblock (backtick `) and use prefix S> to indicate it's SQLSeal query. diff --git a/package.json b/package.json index 58b3af0..28f0eb5 100644 --- a/package.json +++ b/package.json @@ -51,8 +51,8 @@ "json5": "^2.2.3", "lodash": "^4.17.21", "markdown-table-ts": "^1.0.3", - "node-sql-parser": "^5.3.4", "papaparse": "^5.4.1", + "sql-parser-cst": "^0.32.0", "util": "^0.12.5", "uuid": "^11.0.4" } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 33ce674..dee4eac 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -38,12 +38,12 @@ importers: markdown-table-ts: specifier: ^1.0.3 version: 1.0.3 - node-sql-parser: - specifier: ^5.3.4 - version: 5.3.4 papaparse: specifier: ^5.4.1 version: 5.4.1 + sql-parser-cst: + specifier: ^0.32.0 + version: 0.32.0 util: specifier: ^0.12.5 version: 0.12.5 @@ -1058,9 +1058,6 @@ packages: '@types/papaparse@5.3.15': resolution: {integrity: sha512-JHe6vF6x/8Z85nCX4yFdDslN11d+1pr12E526X8WAfhadOeaOTx5AuIkvDKIBopfvlzpzkdMx4YyvSKCM9oqtw==} - '@types/pegjs@0.10.6': - resolution: {integrity: sha512-eLYXDbZWXh2uxf+w8sXS8d6KSoXTswfps6fvCUuVAGN8eRpfe7h9eSRydxiSJvo9Bf+GzifsDOr9TMQlmJdmkw==} - '@types/responselike@1.0.3': resolution: {integrity: sha512-H/+L+UkTV33uf49PH5pCAUBVPNj2nDBXTN+qS1dOwyyg24l3CcicicCA7ca+HMvJBZcFgl5r8e+RR6elsb4Lyw==} @@ -1358,10 +1355,6 @@ packages: base64-js@1.5.1: resolution: {integrity: sha512-AKpaYlHn8t4SVbOHCy+b5+KKgvR4vrsD8vbvrbiQJps7fKDTkjkDry6ji0rUJjC0kzbNePLwzxq8iypo41qeWA==} - big-integer@1.6.52: - resolution: {integrity: sha512-QxD8cf2eVqJOOz63z6JIN9BzvVs/dlySa5HGSBH5xtR8dPteIRQnBxxKqkNTiT6jbDTF6jAfrd4oMcND9RGbQg==} - engines: {node: '>=0.6'} - birpc@0.2.19: resolution: {integrity: sha512-5WeXXAvTmitV1RqJFppT5QtUiz2p1mRSYU000Jkft5ZUCLJIk4uQriYNO50HknxKwM6jd8utNc66K1qGIwwWBQ==} @@ -2465,10 +2458,6 @@ packages: node-releases@2.0.18: resolution: {integrity: sha512-d9VeXT4SJ7ZeOqGX6R5EM022wpL+eWPooLI+5UpWn2jCT1aosUQEhQP214x33Wkwx3JQMvIm+tIoVOdodFS40g==} - node-sql-parser@5.3.4: - resolution: {integrity: sha512-uOgTAADMc4pMjKGkSPELkQUoYAlQBz+JqC9zw/MFs7/wr0fkYoxkexB3sNjslnPttlJlxiaseHFQ36xsXhmnUQ==} - engines: {node: '>=8'} - nopt@6.0.0: resolution: {integrity: sha512-ZwLpbTgdhuZUnZzjd7nb1ZV+4DoiC6/sfiVKok72ym/4Tlf+DFdlHYmT2JPmcNNWV6Pi3SDf1kT+A4r9RTuT9g==} engines: {node: ^12.13.0 || ^14.15.0 || >=16.0.0} @@ -2810,6 +2799,9 @@ packages: sprintf-js@1.1.3: resolution: {integrity: sha512-Oo+0REFV59/rz3gfJNKQiBlwfHaSESl1pcGyABQsnnIfWOFt6JNj5gCog2U6MLZ//IGYD+nA8nI+mTShREReaA==} + sql-parser-cst@0.32.0: + resolution: {integrity: sha512-J/8IeKGBGftFbssmWlE7NnIOnkH0EW8FwJoctcgZB9cY0aeCb1QFp/rl9uYPX87SY38PwCKiPUJO8+fssqg5vQ==} + ssri@9.0.1: resolution: {integrity: sha512-o57Wcn66jMQvfHG1FlYbWeZWW/dHZhJXjpIcTfXldXEk5nz5lStPo3mK0OJQfGR3RbZUlbISexbljkJzuEj/8Q==} engines: {node: ^12.13.0 || ^14.15.0 || >=16.0.0} @@ -4076,8 +4068,6 @@ snapshots: dependencies: '@types/node': 22.9.3 - '@types/pegjs@0.10.6': {} - '@types/responselike@1.0.3': dependencies: '@types/node': 22.9.3 @@ -4445,8 +4435,6 @@ snapshots: base64-js@1.5.1: {} - big-integer@1.6.52: {} - birpc@0.2.19: {} bl@4.1.0: @@ -5824,11 +5812,6 @@ snapshots: node-releases@2.0.18: {} - node-sql-parser@5.3.4: - dependencies: - '@types/pegjs': 0.10.6 - big-integer: 1.6.52 - nopt@6.0.0: dependencies: abbrev: 1.1.1 @@ -6158,6 +6141,8 @@ snapshots: sprintf-js@1.1.3: {} + sql-parser-cst@0.32.0: {} + ssri@9.0.1: dependencies: minipass: 3.3.6 diff --git a/src/codeblockHandler/CodeblockProcessor.ts b/src/codeblockHandler/CodeblockProcessor.ts index 3ecfcdc..ab2fb56 100644 --- a/src/codeblockHandler/CodeblockProcessor.ts +++ b/src/codeblockHandler/CodeblockProcessor.ts @@ -2,9 +2,9 @@ import { OmnibusRegistrator } from "@hypersphere/omnibus"; import { App, MarkdownPostProcessorContext, MarkdownRenderChild } from "obsidian"; import { SqlSealDatabase } from "src/database/database"; import { Sync } from "src/datamodel/sync"; -import { transformQuery } from "src/sql/transformer"; import { parseLanguage, Table } from "src/grammar/newParser"; import { RendererRegistry, RenderReturn } from "src/renderer/rendererRegistry"; +import { transformQuery } from "src/sql/sqlTransformer"; import { displayError, displayNotice } from "src/utils/ui"; export class CodeblockProcessor extends MarkdownRenderChild { @@ -55,7 +55,9 @@ export class CodeblockProcessor extends MarkdownRenderChild { try { const registeredTablesForContext = await this.sync.getTablesMappingForContext(this.ctx.sourcePath) - const tranformedQuery = transformQuery(this.query, registeredTablesForContext) + + const res = transformQuery(this.query, registeredTablesForContext) + const transformedQuery = res.sql this.registrator.offAll() Object.values(registeredTablesForContext).forEach(v => { @@ -74,7 +76,7 @@ export class CodeblockProcessor extends MarkdownRenderChild { return } const fileCache = this.app.metadataCache.getFileCache(file) - const { data, columns } = await this.db.select(tranformedQuery, fileCache?.frontmatter ?? {}) + const { data, columns } = await this.db.select(transformedQuery, fileCache?.frontmatter ?? {}) this.renderer.render({ data, columns }) } catch (e) { this.renderer.error(e.toString()) diff --git a/src/codeblockHandler/inline/InlineProcessor.ts b/src/codeblockHandler/inline/InlineProcessor.ts index 8060d30..a47ddf3 100644 --- a/src/codeblockHandler/inline/InlineProcessor.ts +++ b/src/codeblockHandler/inline/InlineProcessor.ts @@ -1,9 +1,9 @@ import { OmnibusRegistrator } from "@hypersphere/omnibus"; -import { App, MarkdownPostProcessorContext, MarkdownRenderChild } from "obsidian"; +import { App, MarkdownRenderChild } from "obsidian"; import { SqlSealDatabase } from "src/database/database"; import { Sync } from "src/datamodel/sync"; -import { RendererRegistry, RenderReturn } from "src/renderer/rendererRegistry"; -import { transformQuery } from "src/sql/transformer"; +import { RenderReturn } from "src/renderer/rendererRegistry"; +import { transformQuery } from "src/sql/sqlTransformer"; import { displayError } from "src/utils/ui"; export class InlineProcessor extends MarkdownRenderChild { @@ -57,7 +57,7 @@ export class InlineProcessor extends MarkdownRenderChild { const fileCache = this.app.metadataCache.getFileCache(file); const { data, columns } = await this.db.select( - transformedQuery, + transformedQuery.sql, fileCache?.frontmatter ?? {} ); diff --git a/src/sql/sqlTransformer.ts b/src/sql/sqlTransformer.ts new file mode 100644 index 0000000..a112a91 --- /dev/null +++ b/src/sql/sqlTransformer.ts @@ -0,0 +1,37 @@ +import { uniq } from 'lodash'; +import { parse, show, Node, cstVisitor } from 'sql-parser-cst'; + +/** + * Function transforms SQL query and updates tables into actual names in te database. + * @param query SQL query string + * @param tableNames mappings of user-defined values (keys) and actual table names in the database (values) + * @returns returns object containing new query (sql) and all tables that has been mapped (mappedTables) + */ +export const transformQuery = (query: string, tableNames: Record) => { + const cst = parse(query, { + dialect: 'sqlite', + includeSpaces: true, + includeComments: true, + includeNewlines: true + }) + + const watchTables: string[] = [] + + const tableMapper = cstVisitor({ + identifier: (identifier) => { + if (tableNames[identifier.name]) { + const newName = tableNames[identifier.name] + identifier.name = newName + identifier.text = newName + watchTables.push(newName) + } + } + }) + + tableMapper(cst) + + return { + sql: show(cst), + mappedTables: uniq(watchTables) + } +} \ No newline at end of file diff --git a/src/sql/transformer.test.ts b/src/sql/transformer.test.ts index b8fa5e4..7aff626 100644 --- a/src/sql/transformer.test.ts +++ b/src/sql/transformer.test.ts @@ -1,16 +1,18 @@ import { describe, it, expect } from '@jest/globals' -import { transformQuery } from './transformer' +import { transformQuery } from './sqlTransformer' describe('SQL Transformer', () => { it('should properly transform basic SQL query', () => { const transformed = transformQuery('SELECT * FROM x', { x: 'file_123' }) - expect(transformed).toEqual('SELECT * FROM "file_123"') + expect(transformed.sql).toEqual('SELECT * FROM file_123') + expect(transformed.mappedTables).toEqual(['file_123']) }) it('should properly transform SQL with CTE', () => { - const transformed = transformQuery('WITH virtual AS (SELECT * FROM x) SELECT * FROM virtual, x', { x: 'y' }) - expect(transformed).toEqual("WITH \"virtual\" AS (SELECT * FROM \"y\") SELECT * FROM \"virtual\", \"y\"") + const transformed = transformQuery('WITH v AS (SELECT * FROM x) SELECT * FROM v, x', { x: 'y' }) + expect(transformed.sql).toEqual("WITH v AS (SELECT * FROM y) SELECT * FROM v, y") + expect(transformed.mappedTables).toEqual(['y']) }) it('should properly persist table functions like json_each', () => { @@ -18,6 +20,32 @@ describe('SQL Transformer', () => { FROM y, json_each(y.list) as x WHERE list IS NOT NULL`, { y: 'file_123' }) - expect(transformed).toEqual("SELECT \"file_123\".\"name\", \"x\".\"value\" FROM \"file_123\", json_each(\"file_123\".\"list\") AS \"x\" WHERE \"list\" IS NOT NULL") + expect(transformed).toEqual({ + sql: `SELECT file_123.name, x.value +FROM file_123, json_each(file_123.list) as x +WHERE list IS NOT NULL`, + mappedTables: ['file_123'] + }) + }) + + it('should properly transform window function', () => { + const input = `WITH RECURSIVE +numbers AS ( +SELECT 1 AS num +UNION ALL +SELECT num + 1 +FROM numbers +WHERE num < 10 +) +SELECT +num, +SUM(num) OVER (ORDER BY num) AS running_total +FROM +numbers` + + expect(transformQuery(input, { files: 'files' })).toEqual({ + sql: input, + mappedTables: [] + }) }) }) \ No newline at end of file diff --git a/src/sql/transformer.ts b/src/sql/transformer.ts deleted file mode 100644 index a064df2..0000000 --- a/src/sql/transformer.ts +++ /dev/null @@ -1,143 +0,0 @@ -import { Parser } from 'node-sql-parser'; - -export function transformQuery(query: string, tablesAliases: Record): string { - const parser = new Parser(); - - // Parse the query into an AST - const ast = parser.astify(query, { - database: 'Sqlite' - }); - - // Handle both single queries and multiple queries (like UNION) - const queries = Array.isArray(ast) ? ast : [ast]; - - // Process each query in the array - queries.forEach(queryAst => { - transformQueryPart(queryAst, tablesAliases); - }); - - // Convert back to SQL - return parser.sqlify(ast, { database: 'Sqlite' }); -} - -function transformQueryPart(ast: any, tablesAliases: Record): void { - if (!ast || typeof ast !== 'object') return; - - // Handle FROM clause - if (ast.from) { - ast.from = ast.from.map((fromItem: any) => { - // Handle regular table references - if (fromItem.table && tablesAliases[fromItem.table]) { - fromItem.table = tablesAliases[fromItem.table]; - } - - // Handle function expressions in FROM clause (like json_each) - if (fromItem.expr) { - transformExpression(fromItem.expr, tablesAliases); - } - - // Handle nested queries in FROM - if (fromItem.stmt) { - transformQueryPart(fromItem.stmt, tablesAliases); - } - - return fromItem; - }); - } - - // Handle WITH clause (CTEs) - if (ast.with && Array.isArray(ast.with)) { - ast.with.forEach((withItem: any) => { - if (withItem.stmt) { - transformQueryPart(withItem.stmt.ast, tablesAliases); - } - }); - } - - // Handle legacy WITH clause structure - if (ast.with && ast.with.ctes && Array.isArray(ast.with.ctes)) { - ast.with.ctes.forEach((withItem: any) => { - if (withItem.stmt) { - transformQueryPart(withItem.stmt, tablesAliases); - } - }); - } - - // Handle JOINs - if (ast.join) { - ast.join = ast.join.map((joinItem: any) => { - if (joinItem.table && joinItem.table.table && tablesAliases[joinItem.table.table]) { - joinItem.table.table = tablesAliases[joinItem.table.table]; - } - if (joinItem.on) { - transformExpression(joinItem.on, tablesAliases); - } - return joinItem; - }); - } - - // Handle WHERE clause - if (ast.where) { - transformExpression(ast.where, tablesAliases); - } - - // Handle UNION queries - if (ast.union) { - transformQueryPart(ast.union[0], tablesAliases); - transformQueryPart(ast.union[1], tablesAliases); - } - - // Handle SELECT columns - if (ast.columns) { - ast.columns.forEach((column: any) => { - transformExpression(column.expr, tablesAliases); - }); - } -} - -function transformExpression(expr: any, tablesAliases: Record): void { - if (!expr || typeof expr !== 'object') return; - - // Handle column references - if (expr.type === 'column_ref' && expr.table && tablesAliases[expr.table]) { - expr.table = tablesAliases[expr.table]; - } - - // Handle function calls - if (expr.type === 'function') { - if (expr.args && Array.isArray(expr.args)) { - expr.args.forEach((arg: any) => { - if (arg.type === 'column_ref' && arg.table && tablesAliases[arg.table]) { - arg.table = tablesAliases[arg.table]; - } - transformExpression(arg, tablesAliases); - }); - } - } - - // Handle binary expressions - if (expr.type === 'binary_expr') { - transformExpression(expr.left, tablesAliases); - transformExpression(expr.right, tablesAliases); - } - - // Handle CASE expressions - if (expr.type === 'case') { - if (expr.args && Array.isArray(expr.args)) { - expr.args.forEach((arg: any) => transformExpression(arg, tablesAliases)); - } - } - - // Recursively process other potential expressions - Object.keys(expr).forEach(key => { - if (Array.isArray(expr[key])) { - expr[key].forEach((item: any) => { - if (item && typeof item === 'object') { - transformExpression(item, tablesAliases); - } - }); - } else if (expr[key] && typeof expr[key] === 'object') { - transformExpression(expr[key], tablesAliases); - } - }); -}