Repository navigation
feat(plugins): let drivers declare their SQL syntax - #1200
Merged
Merged
Conversation
Plugins can now declare `sql_dialect: "bigquery"`. The statement splitter follows GoogleSQL lexing and scripting rules: - backslash escapes in '...', "..." and `...`, no doubled-quote escape - triple-quoted '''...''' / """...""" strings and r/b/rb/br prefixes - `#`, `--` and `/* */` comments - procedural blocks stay one statement: BEGIN ... END (but not BEGIN TRANSACTION or BEGIN;), EXCEPTION WHEN ERROR THEN, IF ... END IF, LOOP, WHILE, REPEAT, FOR ... DO, CASE ... END CASE, labels and CREATE PROCEDURE bodies. Only statement-position keywords open a block, since BEGIN/LOOP/WHILE/REPEAT are not reserved and IF/REPEAT are also functions. The Rust SqlDialect enum gains Bigquery. A manifest that declares a dialect this host doesn't know now loads as generic with a warning instead of failing the whole plugin, so future dialects don't break older hosts. Connection metadata overrides stay strict. Other dialects and existing plugins are unchanged.
…igquery For plugins that declare the bigquery dialect: - filter bar, foreign-key filters, copy as INSERT / IN list and clipboard import backslash-escape string literals, since GoogleSQL has no doubled-quote escape - "contains" / "starts with" / "ends with" filters escape LIKE wildcards with backslashes and omit ESCAPE, which GoogleSQL LIKE doesn't support - autocomplete adds GoogleSQL keywords and functions (QUALIFY, PIVOT, UNNEST, SAFE_CAST, REGEXP_*, JSON_*, ST_*, APPROX_*, date functions...) - Format SQL uses the formatter's bigquery language Other dialects keep their current output.
The AI/MCP read-only classifier tokenized SQL one way only, so a payload could hide a second statement in text that the target engine reads differently: `#` comments (`SELECT 1 # '<newline>; DROP TABLE t` on MySQL), backslash escapes in "..." and `...`, MySQL `--` without a space and executable /*! */ comments, PostgreSQL nested comments, CR-terminated line comments, `$` inside identifiers and non-ASCII dollar-quote tags, SQL Server [brackets] and triple-quoted strings. Statements are now lexed once per tokenization profile plausible for the connection's sql_dialect capability and are a read only when every pass sees exactly one read statement. Generic, Oracle, undeclared and future dialects use every profile, so they fail closed. MCP run_query (initial and approver-edited queries) passes the driver's dialect, and the DROP DATABASE sidebar sync uses the same multi-pass separator check.
The database dump picked identifier quoting, schema qualification and literal syntax from the driver id: every plugin got `"table"` with no schema, and only the `postgres`/`postgresql` ids scoped the table listing. A plugin that quotes with backticks and exposes datasets as schemas produced statements its engine could not read back. Dump formatting now lives in `DumpDialect`, built from the driver's `identifier_quote`, `schemas` and `sql_dialect` capabilities. Schema-aware plugins get a scoped listing, a scoped `get_table_ddl` call and `schema`.`table` references; drivers without schemas never receive a stray schema. Golden tests compare the builtin MySQL, PostgreSQL and SQLite output with the previous implementation byte for byte.
…tifier quote
Cell references (`{{cell_N}}`) are inlined as a CTE whose column aliases
were always double-quoted, and backslash escaping was chosen by comparing
the driver id with "mysql"/"mariadb". Engines that read "..." as a string
literal (BigQuery, MySQL plugins) rejected the aliases.
`variableOptionsForDriver` now derives both settings from the active
capabilities (`identifier_quote`, `sql_dialect`), falling back to the
builtin rules for a bare driver id.
`stripLeadingComments` only knew `--` and `/* */`, so a MySQL statement starting with a `# comment` line got an empty leading keyword: it was not treated as a SELECT, result set or explainable statement. The helper and `leadingKeyword` take a `hashComments` option, and the splitter passes its dialect's `hashComments` flag, so PostgreSQL (where `#` is an operator) is unchanged. The dialect-agnostic safety checks (`isDataModifyingQuery`, `classifyDangerousQuery`) also look past a leading `#` line, since no dialect can start a statement with `#`.
Column statistics and the filter value picker run `COUNT(DISTINCT col)` and `GROUP BY col`, which engines with nested types reject for arrays, structs and repeated records. Plugin drivers report those as `ARRAY<...>`, `STRUCT<...>`, `MAP<...>`, `RECORD` or `... REPEATED`. `isNestedColumnType` matches those spellings without a driver check; such columns get row and NULL counts only and no value picker. A bare `ARRAY` (PostgreSQL's report for `int[]`, which supports equality) is unchanged.
Instead of a hard-coded `bigquery` SqlDialect, a driver plugin now describes how its engine lexes and quotes SQL in the new optional `capabilities.sql_syntax` object. It extends one of the existing presets (`extends`, default `sql_dialect`) and overrides individual rules: quotes, triple-quoted strings, string prefixes, `#` comments, nested comments, `--` spacing, script blocks, dollar quoting, how generated string literals are escaped, LIKE ESCAPE support, the formatter language and extra autocomplete words. It is data only. `sql_syntax` is separate from `sql_dialect`, so older hosts ignore it and keep using `sql_dialect` as the fallback. Every field is validated and bounded; an invalid field is logged and ignored and never fails the manifest. The sanitized descriptor is what the frontend receives. `SqlSyntax::resolve` turns capabilities into the effective rules. A driver without `sql_syntax` resolves to exactly its `sql_dialect` preset. Clipboard import and dump literals now follow the resolved `string_escape`, so built-in drivers keep their output and a GoogleSQL plugin gets backslash-escaped literals. The `bigquery` SqlDialect variant is gone; a manifest that still names it loads as `generic` through the lenient dialect parser.
The AI/MCP read-only classifier lexes a query once per tokenization profile of the driver's `sql_dialect` and treats it as a read only when every pass sees one read statement. A driver that declares `sql_syntax` now adds one more pass, derived from that syntax (backslash escapes per quote, `#` comments, `--` spacing, nesting, triple quotes, dollar quotes, bracket identifiers). Rules the descriptor can't set come from the preset it extends. The pass is added, never substituted, so a descriptor can only make the check stricter: a wrong or hostile descriptor can't hide a `;` that the dialect's own passes see, and generic, oracle or undeclared dialects still run every profile. The trade-off is fail-closed false positives when the fallback dialect reads a literal differently, e.g. `'''it's; ok'''` with a `mysql` fallback. MCP run_query and approver-edited queries classify with the driver's capabilities. The former BigQuery lexer profile stays in the all-profiles set as a dialect-neutral triple-quote/backslash pass.
`strip_leading_sql_comments` and `returns_result_set` only knew `--` and `/* */`, so on MySQL a statement starting with a `# note` line went down the affected-rows path instead of fetching its rows, and a commented `START TRANSACTION` missed the text protocol. New `_for` variants take whether the resolved syntax has `#` comments. The MySQL driver passes its resolved syntax's `hash_comments` (true); the existing functions keep ignoring `#` for every other caller.
`resolveSqlSyntax(capabilities)` (src/utils/sqlSyntax.ts) is the single place the frontend turns a driver into its SQL rules: splitter options, literal escape style, LIKE ESCAPE support, formatter language and extra autocomplete words. It extends the `sql_dialect` preset with the declared `sql_syntax`, validating and bounding every field as the backend does. Without `sql_syntax` it returns the exact preset object, cached, so it is as stable as the dialect string it replaces. The splitter entry points, non-code scanning, query parameters, folding, invisible-character cleanup, Format SQL and the Editor accept the resolved syntax wherever they took a dialect name. Filters, FK filters, Copy as INSERT / IN list and notebook cell CTEs take their literal style from it (`stringEscapeStyle` / `quoteSqlString` in sqlLiteral.ts), and literal LIKE filters drop `ESCAPE` when `like_escape` is false. GoogleSQL support moves to plugin data. The BIGQUERY splitter preset and keyword list are removed; the splitter keeps triple-quoted strings and script blocks as generic features and gains `stringPrefixes` and a `kind` per quote rule. The former BigQuery tests now run through the GoogleSQL descriptor fixture (tests/fixtures/sql-syntax), and new tests check that every preset resolves and splits exactly as before.
Adds a SQL Syntax section to the plugin guide with every field, its default and limits, what a descriptor can't change, how it affects the AI/MCP read-only check, and the full GoogleSQL descriptor a BigQuery driver ships with `sql_dialect: "mysql"` as its fallback. `sql_syntax` joins the manifest schema and the Tabularium extension schema, and `bigquery` leaves the `sql_dialect` enum. Connection metadata can't override `sql_syntax`. Tests on both sides check the schemas against the host limits and formatter allowlist, and the GoogleSQL fixture against both schemas.
Dump booleans: only PostgreSQL (the postgres dialect, or the postgres / postgresql driver ids) writes TRUE/FALSE. Plugins with no sql_dialect or a generic one go back to the 1/0 the dump always wrote, since an engine nobody verified may not accept the keywords. A postgres-flavoured driver without the schemas capability is still schema-qualified, as before. The golden tests now compare against the old id-based implementation for the builtin drivers, the postgresql plugin (with and without a dialect) and dialect-less plugins: literals are byte-identical. The one intended difference is table references for a plugin that declares a non-default identifier_quote or the schemas capability: they are now quoted with that quote and schema-qualified instead of always being written as "table". Notebook variables: when the capabilities name no concrete dialect (none or generic), the old driver-id rule applies again, so a mariadb plugin without sql_dialect keeps doubling backslashes in cell literals.
Option::is_none_or is stable since Rust 1.82, above the crate's rust-version 1.77.2. Use map_or(true, ...) in the dash-comment check.
…syntax DumpDialect keeps the resolved string_escape and takes the restored boolean rule (TRUE/FALSE only for PostgreSQL, 1/0 otherwise). Notebook literals use the resolved syntax via stringEscapeStyle, which now takes the active driver id: when the capabilities declare neither sql_syntax nor a concrete sql_dialect (none or generic), the mysql / mariadb id rule still applies.
std::sync::LazyLock is stable since Rust 1.80, above the crate's rust-version 1.77.2. Use once_cell::sync::Lazy like the rest of the crate.
ConnectionCapabilityOverrides.sql_dialect was parsed strictly, so a plugin returning a dialect this host doesn't know from get_connection_metadata failed discovery for the whole connection. Parse it leniently like the manifest, but drop the unknown override (with a warning) instead of mapping it to generic, so the manifest's dialect stays in force. A non-string value is still rejected.
A descriptor declaring string_escape "standard" on a mysql base (extends mysql, or sql_dialect mysql without extends) made generated literals stop doubling backslashes, although the engine still reads `\` as an escape character: a value ending in `\` could swallow its closing quote. Both resolvers (Rust SqlSyntax::resolve and TS resolveSqlSyntax) now keep the mysql style and log a warning; "backslash" is still honoured since it escapes backslashes too. The backslash literal style also writes NUL as `\0` in Rust and TS, as the mysql style already did.
Explain what a plugin without a dialect gets: the strictest AI/MCP read-only classification (with the kinds of reads it may reject) and standard literals with 1/0 booleans in dumps. Document that string_escape "standard" is ignored on a mysql base and that the backslash style escapes NUL.
GoogleSQL has no implicit INT64 to BOOL coercion, so the 1/0 a dump writes for a plugin on the mysql fallback makes the generated INSERTs fail. A plugin can now declare `sql_syntax.boolean_literals: "keywords" | "numeric"`. The declared style is used by dumps, Copy as INSERT / IN list, foreign-key and cell-value filters, filters on BOOL/BOOLEAN columns, notebook cell CTEs and the BOOLEAN columns a clipboard import creates. Without the field every generator keeps its historical form (dumps: TRUE/FALSE on postgres, 1/0 elsewhere), so built-in drivers and existing plugins are unchanged. An invalid value is logged and ignored like other descriptor fields. The GoogleSQL fixture, both manifest schemas and PLUGIN_GUIDE.md declare and document the field.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| // `execute()` so we can return the actual `rows_affected`. In text mode | ||
| // they must use `raw_sql` (COM_QUERY) since the bastion rejects prepares. | ||
| if !crate::drivers::common::returns_result_set(query) { | ||
| if !crate::drivers::common::returns_result_set_for(query, *HASH_COMMENTS) { |
Contributor
There was a problem hiding this comment.
WARNING: #-comment handling is only half applied on the MySQL path
returns_result_set_for(query, *HASH_COMMENTS) now treats # note\nSELECT 1 as a result-set statement, but is_select_query(query) a few lines below (line 1461) still uses strip_leading_sql_comments, which keeps a leading #. Such a query takes the fetch path with is_select == false, so no LIMIT/OFFSET pagination is pushed down and pagination stays None (only the client-side manual_limit cap applies). is_explainable_query (drivers/common/query.rs) has the same blind spot. The doc on is_select_query says it must match the comment stripping used for result-set detection, so route it through strip_leading_sql_comments_for(query, *HASH_COMMENTS) for MySQL, and add a test for is_select_query with a leading #.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Collaborator
Author
There was a problem hiding this comment.
Fixed in e6ab202. Added is_select_query_for and is_explainable_query_for (the plain versions delegate with hash_comments = false). The MySQL driver now uses is_select_query_for(query, *HASH_COMMENTS), so # note\nSELECT 1 gets LIMIT/OFFSET pagination. explain_query_plan now runs the explainable check after resolving the driver, using the hash_comments of its resolved SqlSyntax. Tests cover both helpers with and without hash comments.
| case "backslash": | ||
| return `'${escapeBackslashString(value)}'`; | ||
| case "mysql": | ||
| return `'${value.replace(/\\/g, "\\\\").replace(/'/g, "''")}'`; |
Contributor
There was a problem hiding this comment.
WARNING: TS mysql literal style does not escape NUL, unlike the Rust side
The PR states backslash literals escape NUL as \0, and Rust quote_string_literal for LiteralStyle::Mysql does .replace('\0', "\\0") (asserted in sql_syntax_tests.rs and dump_utils/tests.rs). This mysql branch only doubles backslashes and quotes, so Copy as INSERT / filters / notebook literals for a MySQL value containing NUL differ from the dump output. Add .replace(/\0/g, "\\0") here and a test for the mysql style (the existing NUL test in tests/utils/sqlLiteral.test.ts only covers backslash).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Collaborator
Author
There was a problem hiding this comment.
Fixed in cad2aa6: the mysql style now writes NUL as \0, same as the Rust side. I also added a shared fixture, tests/fixtures/sql-syntax/string-literals.json, that both the Rust quote_string_literal test and the TS quoteSqlStringWithStyle test read, covering all three styles. That keeps the two sides identical from now on.
| continue; | ||
| } | ||
| if (options.blockComments && source.startsWith('/*', p)) { | ||
| const close = source.indexOf('*/', p + 2); |
Contributor
There was a problem hiding this comment.
SUGGESTION: skipTrivia does not follow the tokenizer's comment rules
The look-ahead ends a block comment at the first */ and treats every -- as a line comment, while the tokenizer honours nested_block_comments and line_comment_requires_space. For a plugin declaring script_blocks together with either option, END /* /* x */ */ IF or END --IF is read differently by the look-ahead (peekWord / beginsTransaction) than by the main scanner, which can mis-detect block closers. Reuse the tokenizer's comment scanning (e.g. scanBlockCommentLength) or pass the options through.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Collaborator
Author
There was a problem hiding this comment.
Fixed in ddd29f1. The comment scanning moved out of scanToken into an exported scanComment in tokenizer.ts, and skipTrivia now uses it. The look-ahead therefore follows hash_comments, nested_block_comments and the -- space rule exactly like the main scanner (an operator -- or an executable comment counts as code). Tests cover END IF closers past nested comments, --x with and without the space rule, # comments and BEGIN TRANSACTION.
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge All three previous findings are resolved in the latest commits:
Files Reviewed (incremental, PR-specific commits e6ab202, cad2aa6, ddd29f1)
The remaining incremental diff comes from merging Previous Review Summary (commit b8adba5)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit b8adba5)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Other notes (not inline, low severity):
Fix these issues in Kilo Cloud Files Reviewed (70 files)
Reviewed by claude-sonnet-5.5 · Input: 256 · Output: 2.1K · Cached: 263.8K Review guidance: REVIEW.md from base branch |
# Conflicts: # src-tauri/src/ai_activity.rs # src-tauri/src/ai_activity_lexer_tests.rs # src-tauri/src/dump_utils.rs # src-tauri/src/dump_utils/tests.rs # src-tauri/src/mcp/mod.rs # src/utils/notebookVariables.ts # tests/utils/notebookVariables.test.ts
Contributor
Preview buildThe preview build of commit |
# Conflicts: # plugins/PLUGIN_GUIDE.md
This was referenced Oct 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sql_dialectonly offers fixed presets, so an engine none of them describes (GoogleSQL: triple-quoted and raw strings,#comments, proceduralBEGIN ... ENDblocks, backslash escapes, noLIKE ... ESCAPE, no implicit integer-to-boolean coercion) had its scripts split wrongly and got invalid generated SQL. Instead of hard-coding one more dialect in the host, plugins can now declare their syntax as data and extend a preset.Changes
sql_syntax:extends(a preset, defaults tosql_dialect) plus overrides forquotes,triple_quoted_strings,string_prefixes,hash_comments,nested_block_comments,line_comment_requires_space,script_blocks,dollar_quoting,string_escape,like_escape,boolean_literals,formatter(allowlist) and autocompletekeywords/functions. Data only, every field bounded; an invalid field is ignored with a warning and never fails plugin loading.drivers/sql_syntax.rs,src/utils/sqlSyntax.ts), used by the statement splitter, folding, parameters, invisible-character cleanup, Format SQL, autocomplete, filter bar, foreign-key filters, Copy as INSERT, clipboard import, dumps and notebook literals.BEGIN TRANSACTIONstays a plain statement).mysqlbase, and backslash literals escape NUL as\0.sql_dialectvalues load asgenericwith a warning (manifest) or are ignored (connection metadata), so future dialects never break older hosts.#comment now take the result-set path.PLUGIN_GUIDE.mddocuments the capability with a complete GoogleSQL example and asks plugins to declaresql_dialectorsql_syntax. Both schemas declare the field; the Tabulariumdriverkind already accepts it.Hosts that predate
sql_syntaxignore it and usesql_dialect, so a plugin keeps the closest preset there as a fallback.Compatibility
Without
sql_syntaxevery path resolves to the same preset as before. Golden tests cover split, batch and scan output for every preset, dumps, clipboard import and generated literals for built-in drivers. NoSqlDialectvalue is removed compared withmain.Tests
sql_syntax_tests.rs,sql_dialect_tests.rs,sqlSyntax.test.ts,sqlLiteral.test.ts, splitter tests run through the GoogleSQL fixture (tests/fixtures/sql-syntax/googlesql.json), schema contract tests.cargo test --all-targets,pnpm build,vitest,pnpm lintpass. Merged together with feat(plugins): support plugin-owned connection fields and secrets #629 and feat(plugins): send connection context to DDL builders and add drop_table_sql #1199 the full suite also passes (1921 Rust, 6583 vitest).