From 0703b12871920e110b6be688905ca80967233da6 Mon Sep 17 00:00:00 2001 From: spicysharkco <321699413+spicysharkco@users.noreply.github.com> Date: Mon, 26 Oct 2026 06:24:12 +0000 Subject: [PATCH] Prefer a dialect that parses without errors The parser asks sqlglot to recover from syntax errors instead of raising, so a dialect can return a tree even when it did not understand the query. That tree was accepted as soon as it did not look degraded, and a later dialect that parses the same query without errors was never tried. For MySQL syntax such as GROUP_CONCAT(... SEPARATOR ...), STRAIGHT_JOIN, USE INDEX and hex literals, the default dialect recovered by dropping part of the statement, so tables or columns went missing or the properties raised. Record whether sqlglot reported errors for each dialect. A result with errors is kept as a fallback while the remaining dialects are tried. It is returned when none of them parses without errors. Only the first statement's tree is used, but sqlglot collects errors from all statements, so an error in a later statement would move the first one to another dialect. When a query has several statements and errors were recorded, the first statement is parsed again on its own and only its errors count. The tree still comes from the whole query, because a procedure body continues past its first semicolon. --- ARCHITECTURE.md | 5 ++- sql_metadata/dialect_parser.py | 73 +++++++++++++++++++++++++--------- test/test_create_table.py | 6 +++ test/test_getting_columns.py | 42 +++++++++++++++++++ test/test_getting_tables.py | 9 +++++ 5 files changed, 114 insertions(+), 21 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 7313c36a..a4a46468 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -202,10 +202,11 @@ flowchart TD `_try_dialects` iterates through the dialect list. For each dialect: -1. Parse with `sqlglot.parse()` (warnings suppressed) +1. Parse with the dialect's sqlglot parser in `ErrorLevel.WARN` mode (warnings suppressed) and note whether the first statement reported syntax errors 2. Check for degradation via `_is_degraded` — phantom tables (`IGNORE`, `""`), keyword-as-column names (`UNIQUE`, `DISTINCT`) 3. If degraded and not the last dialect, try the next one -4. If all fail, raise `InvalidQueryDefinition` (a `ValueError` subclass from [`exceptions.py`](sql_metadata/exceptions.py)) +4. If the parse reported syntax errors, keep the first such result as a fallback and try the next one +5. If no dialect parsed without errors, return the fallback; with no fallback, raise `InvalidQueryDefinition` (a `ValueError` subclass from [`exceptions.py`](sql_metadata/exceptions.py)) --- diff --git a/sql_metadata/dialect_parser.py b/sql_metadata/dialect_parser.py index 84343b79..c578e012 100644 --- a/sql_metadata/dialect_parser.py +++ b/sql_metadata/dialect_parser.py @@ -16,6 +16,7 @@ class so that callers only need to call :meth:`DialectParser.parse`. from sqlglot.errors import ParseError, TokenError from sqlglot.parsers.redshift import RedshiftParser from sqlglot.tokens import Tokenizer as BaseTokenizer +from sqlglot.tokens import TokenType from sql_metadata.comments import _has_hash_variables from sql_metadata.exceptions import InvalidQueryDefinition @@ -187,45 +188,67 @@ def _try_dialects( """Try each candidate dialect in order and return the first good result. Iterates over *dialects*, calling :meth:`_parse_with_dialect` for - each. A result is accepted immediately if it is the last dialect - in the list (best-effort) or if :meth:`_is_degraded` reports no - quality issues. Degraded results from non-last dialects are - skipped so the next candidate gets a chance. + each. A result is accepted immediately if sqlglot reported no + syntax errors and it is either the last dialect in the list + (best-effort) or :meth:`_is_degraded` reports no quality issues. + Degraded results from non-last dialects are skipped so the next + candidate gets a chance. + + A result that sqlglot parsed with syntax errors is not accepted + straight away, because sqlglot recovers from an error by + dropping or misreading part of the statement while a later + dialect may parse the same SQL without any error. The first such + result is kept as a fallback and returned when no later dialect + parses without errors. :param clean_sql: Preprocessed SQL string. :param dialects: Priority-ordered list from :meth:`_detect_dialects`. :returns: 2-tuple of ``(ast_root_node, winning_dialect)``. :raises InvalidQueryDefinition: If the last dialect raises a - parse error, or if no dialect produces a usable AST. + parse error and no earlier dialect gave a usable result, or + if no dialect produces a usable AST. """ + fallback: tuple[exp.Expression, DialectType] | None = None for dialect in dialects: try: - result = self._parse_with_dialect(clean_sql, dialect) - if result is None: + parsed = self._parse_with_dialect(clean_sql, dialect) + if parsed is None: continue + result, has_errors = parsed is_last = dialect == dialects[-1] if not is_last and self._is_degraded(result, clean_sql): continue - return result, dialect + if not has_errors: + return result, dialect + # Keep only the first errored result as the fallback. + fallback = fallback or (result, dialect) except (ParseError, TokenError): - if dialect is not None and dialect == dialects[-1]: + if fallback is None and dialect is not None and dialect == dialects[-1]: raise InvalidQueryDefinition( "Query could not be parsed — SQL syntax error" ) continue + if fallback is not None: + return fallback raise InvalidQueryDefinition( "Query could not be parsed — no dialect could handle this SQL" ) @staticmethod - def _parse_with_dialect(clean_sql: str, dialect: Any) -> exp.Expression | None: + def _parse_with_dialect( + clean_sql: str, dialect: Any + ) -> tuple[exp.Expression, bool] | None: """Parse *clean_sql* with a single sqlglot dialect. Uses ``ErrorLevel.WARN`` so that sqlglot returns a best-effort AST instead of raising on the first syntax problem — the caller decides whether the result is good enough via - :meth:`_is_degraded`. + :meth:`_is_degraded` and the returned error flag. The flag is + read from the parser's error list, which is how a clean parse is + told apart from one that sqlglot recovered from. Only the first + statement is used, so with several statements the flag is read for + the first one parsed on its own. The sqlglot logger is temporarily raised to ``CRITICAL`` during the parse call because ``WARN`` mode emits noisy warnings for @@ -237,24 +260,36 @@ def _parse_with_dialect(clean_sql: str, dialect: Any) -> exp.Expression | None: :param clean_sql: Preprocessed SQL string. :param dialect: A sqlglot dialect identifier, class, or ``None`` for the default dialect. - :returns: The root AST node, or ``None`` if sqlglot could not - produce any result. + :returns: 2-tuple of ``(root_node, has_errors)`` where + ``has_errors`` is ``True`` when sqlglot reported at least one + syntax error, or ``None`` if sqlglot could not produce any + result. """ + sqlglot_dialect = Dialect.get_or_raise(dialect) + parser = sqlglot_dialect.parser(error_level=sqlglot.ErrorLevel.WARN) logger = logging.getLogger("sqlglot") old_level = logger.level logger.setLevel(logging.CRITICAL) try: - results = sqlglot.parse( - clean_sql, - dialect=dialect, - error_level=sqlglot.ErrorLevel.WARN, - ) + tokens = sqlglot_dialect.tokenize(clean_sql) + results = parser.parse(tokens, clean_sql) + has_errors = bool(parser.errors) + if has_errors and len(results) > 1: + # Only the first statement is used, but ``parser.errors`` + # covers every statement: parse the first one on its own. + end = next( + i + for i, token in enumerate(tokens) + if token.token_type == TokenType.SEMICOLON + ) + parser.parse(tokens[:end], clean_sql) + has_errors = bool(parser.errors) finally: logger.setLevel(old_level) if not results or results[0] is None: return None - return results[0] # type: ignore[return-value] + return results[0], has_errors # type: ignore[return-value] # -- quality checks ----------------------------------------------------- diff --git a/test/test_create_table.py b/test/test_create_table.py index 2e5d1fec..6fe18a2e 100644 --- a/test/test_create_table.py +++ b/test/test_create_table.py @@ -242,3 +242,9 @@ def test_create_table_as_select_with_cte_same_name(): query = "CREATE TABLE foo AS WITH foo AS (SELECT 1 as id) SELECT * FROM foo" parser = Parser(query) assert parser.tables == ["foo"] + + +def test_create_procedure_keeps_every_statement_of_its_body(): + """The body runs past the first semicolon, so it is one statement.""" + query = "CREATE PROCEDURE p() BEGIN SELECT * FROM a; SELECT * FROM b; END" + assert Parser(query).tables == ["p", "a", "b"] diff --git a/test/test_getting_columns.py b/test/test_getting_columns.py index 00594b98..db57474f 100644 --- a/test/test_getting_columns.py +++ b/test/test_getting_columns.py @@ -752,6 +752,48 @@ def test_separator_not_column(): assert "separator" not in columns_lower +def test_use_index_hint_keeps_where_columns(): + """USE INDEX is MySQL-only: the default dialect drops the WHERE clause.""" + query = ( + "SELECT wp_posts.ID FROM wp_posts USE INDEX (type_status_date) " + "WHERE wp_posts.post_status = 'publish' AND wp_posts.post_type = 'post'" + ) + assert Parser(query).columns == [ + "wp_posts.ID", + "wp_posts.post_status", + "wp_posts.post_type", + ] + + +def test_hex_string_literal_is_not_a_column(): + """X'..' is a MySQL hex literal: it is not a column and WHERE continues.""" + query = "SELECT * FROM users WHERE token = X'0A1B2C' AND deleted = 0" + assert Parser(query).columns == ["*", "token", "deleted"] + + +def test_straight_join_modifier_does_not_crash(): + """STRAIGHT_JOIN after SELECT is MySQL-only; it used to raise AttributeError.""" + query = "SELECT STRAIGHT_JOIN t1.a FROM t1 JOIN t2 ON t1.x = t2.x WHERE t1.y = 1" + assert Parser(query).columns == ["t1.a", "t1.x", "t2.x", "t1.y"] + + +def test_string_ending_with_backslash_after_default_dialect_errors(): + """The mysql dialect cannot tokenize a string ending with a backslash. + + The default dialect parses it, with an error, and its result is kept. + """ + query = "SELECT name FROM authors WHERE path = 'C:\\' AND name LIKE" + parser = Parser(query) + assert parser.tables == ["authors"] + assert parser.columns == ["name", "path"] + + +def test_error_in_later_statement_keeps_first_statement_dialect(): + """Only the first statement counts: a MySQL-only second one changes nothing.""" + query = "SELECT \"a\" FROM t1; SELECT GROUP_CONCAT(x SEPARATOR ',') FROM t2" + assert Parser(query).columns == ["a"] + + def test_mssql_top_columns(): # solved: https://github.com/macbre/sql-metadata/issues/318 query = "SELECT TOP 10 id, name FROM foo" diff --git a/test/test_getting_tables.py b/test/test_getting_tables.py index 8b67ccfe..fbe173b4 100644 --- a/test/test_getting_tables.py +++ b/test/test_getting_tables.py @@ -988,6 +988,15 @@ def test_degraded_parse_falls_through_to_last_dialect(): assert "t" in p.tables +def test_group_concat_separator_keeps_tables(): + """SEPARATOR is MySQL-only: the default dialect drops the FROM clause.""" + query = ( + "SELECT user_id, GROUP_CONCAT(tag SEPARATOR ',') AS tags " + "FROM user_tags GROUP BY user_id" + ) + assert Parser(query).tables == ["user_tags"] + + def test_parenthesized_select_unwrapping(): """Parenthesized top-level SELECT is correctly unwrapped.""" p = Parser("(SELECT a, b FROM t)")