fix(editor): lex SQL the way each engine does, so Safe Mode sees every statement - #3005
Merged
Merged
Conversation
…grammar # Conflicts: # CHANGELOG.md # TablePro/Core/Compare/CompareSyncExecutor.swift # TablePro/Core/Services/Execution/AutocommitOnlyStatement.swift
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
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.
Safe Mode, the MCP and AI gates and the iOS write check all decide what a statement is by reading the SQL, and the app read it with several lexers that disagreed with each other and with the engines. Where the app's reading was wrong, text it counted as one safe statement ran as several on the server.
Measured live, each one a hidden
DROP TABLE usersbehind a leadingSELECTthe classifier tiered safe:SELECT 'C:\' AS p; DROP TABLE usersdropped the table on PostgreSQL 17 (the driver forcesstandard_conforming_stringson and sends throughPQexec), on DuckDB, on SQL Server through FreeTDS, and on Dameng. The app treated\'as an escape for every engine but Oracle. MySQL, MariaDB, ClickHouse, Snowflake and GoogleSQL do escape with it, so the rule is per engine, not a constant.SELECT 1 /* /* */ ' */; DROP TABLE users; --'dropped the table on PostgreSQL 17, DuckDB 1.5.4 and Azure SQL Edge. PostgreSQL, DuckDB and SQL Server nest; SQLite, MySQL, Oracle and DM8 do not.SELECT [it's] FROM t; DROP TABLE users; SELECT 'x'was one safe statement and dropped the table on SQL Server, where[...]quotes an identifier and]]escapes.SELECT $$it's$$; DROP TABLE users; SELECT 'x'did the same on DuckDB.$$bodies were read only on PostgreSQL, so Snowflake, Cassandra, Databend and DuckDB were all exposed, and PostgreSQL itself missed non-ASCII tags such as$ü$.SQLWriteClassifier.isWriteQuerycalledSELECT $$'$$; DELETE FROM ta read on PostgreSQL, and the server deleted every row. The same held forE'\'', MySQL'a\'', T-SQL[a'b]and Oracleq'[it's]'.What changed
TableProSQLGrammar, owns the lexer, the statement scanner, the boundary trackers and one grammar value type. Both apps read it, so the Mac and iOS now lex a statement the same way. The first commit is the move with no behaviour change; the second is the behaviour.SQLLexicalGrammarstates the facts a lexer needs (which quotes escape with a backslash, whether block comments nest, bracket and backtick identifiers, dollar-quote style,#and//comments,q'[...]',E'', triple quotes,/*! */,--needing whitespace, a carriage return ending a line comment,$/#in identifiers).SQLLexicalProfilecarries one curated profile per engine TablePro ships, each with the server and version it was measured on in its doc comment, and marked "not measured" with the reference it came from where no server was available (ClickHouse, Snowflake, Databend, CQL, SurrealQL, BigQuery).SQLDialectDescriptor.lexicalFeatures, then what the connected driver reports about its own session (PluginDatabaseDriver.sessionLexicalState: MySQL'sNO_BACKSLASH_ESCAPES, Dameng'sBACKSLASH_ESCAPE). The session only decides how a script is split for execution.SQLLexicalReadings.distinct(for:)drops any fact the text cannot exercise first, so a query with no backslash is still lexed once.QueryClassifier(through one code projection, replacingstrippingStringLiterals/skipQuoted),CatalogChangeClassifier, and iOSSQLWriteClassifier.SqlDialectstays what it was, the SQL generation dialect for export and statement building; it no longer decides how text is read.SQLStatementSplittinggets the dollar-quote state its doc comment always claimed, through a grammar-aware overload. DuckDB's transaction tracking and the MySQL session footprint pass their own grammar, so a Databend connection served by the MySQL plugin is lexed as Databend.CREATE PROCEDURE p IS BEGIN EXECUTE IMMEDIATE 'DROP TABLE users'; END;stays a write, as fix(plugin-oracle): run PL/SQL blocks and units whole, with the terminator Oracle needs #2988 settled, while an anonymous block that runs it is still destructive.ABI
Additive, checked with
scripts/check-pluginkit-abi.shagainst the merge base: no symbol removed.SQLDialectDescriptorkeeps its existing initializer exactly as it was, marked@_disfavoredOverload, beside a new one that takeslexicalFeatures.PluginDatabaseDrivergainssessionLexicalStatewith a default. Kit 33 is pending and unreleased, so no bump.Tests
ExternalStatementGateLexicalTests,MCPStatementGateTests,QueryClassifierLexicalTests, andSQLWriteClassifierTestsin the package for iOS.SQLLexicalFeatureMappingTestsholds every grammar fact to its PluginKit feature bit, so the two sets cannot drift.TableProCorepackage tests,AllPlugins, the iOS build and the shared-isolation check.Measured against live PostgreSQL 17.11, MySQL 8.4.11, MariaDB 11.8.9, SQLite 3.54, DuckDB 1.5.2 and 1.5.4, Oracle 23.26, Dameng DM8 (compatibility modes 0, 2, 4 and 7), Azure SQL Edge 15.0 and the Spanner emulator in both dialects.
Not in this PR
The statement boundary work that sits on top of this: per-engine routine-body trackers (a column named
beginstill merges statements on PostgreSQL, MySQL, SQLite and SQL Server), the T-SQL batch classifier, the import parser's boundaries, Dameng's PL/SQL units, Snowflake Scripting blocks, and enforcing one statement at the engine (PostgreSQL's extended protocol refuses a merged text; measured).