Skip to content

Fix crash on malformed SQL like "{ =" - #673

Open
eeshsaxena wants to merge 1 commit into
macbre:masterfrom
eeshsaxena:fix/malformed-sql-partial-ast-crash
Open

Fix crash on malformed SQL like "{ ="#673
eeshsaxena wants to merge 1 commit into
macbre:masterfrom
eeshsaxena:fix/malformed-sql-partial-ast-crash

Conversation

@eeshsaxena

Copy link
Copy Markdown

The bug

from sql_metadata import Parser
Parser("{ =").columns
# AttributeError: 'NoneType' object has no attribute 'name'

Every public accessor crashes on the same three-character input: .tables, .columns, .query_type, .tables_aliases, .columns_dict. SELECT { = and x { = behave the same.

Why

DialectParser._parse_with_dialect runs sqlglot with ErrorLevel.WARN so it returns a best-effort partial AST instead of raising on the first problem. For { = sqlglot builds a bracket/map node whose key is None and then raises AttributeError while evaluating e.this.name on it. That is neither ParseError nor TokenError, so _try_dialects (which only catches those two) never sees it and the exception propagates all the way out to the accessors, even though the method is documented to raise InvalidQueryDefinition when no dialect produces a usable AST.

The fix

Catch any sqlglot-side failure in _parse_with_dialect and treat it as 'this dialect produced no result' (returns None), so a malformed query is reported through the normal InvalidQueryDefinition path rather than crashing. ParseError/TokenError are re-raised so the existing syntax-error handling is untouched.

After the fix { = no longer crashes: query_type/tables raise InvalidQueryDefinition and the best-effort accessors come back empty.

Tests

Added test/test_malformed_input.py covering the three inputs across the accessors. Full suite passes and dialect_parser.py stays at 100% coverage.

DialectParser runs sqlglot in WARN mode so it returns a best-effort AST instead
of raising. For a handful of short inputs like "{ =" sqlglot instead raises an
AttributeError while assembling that partial tree (a node whose key is None).
That was neither ParseError nor TokenError, so it escaped _try_dialects and
every public accessor (tables, columns, query_type, ...) crashed with a raw
AttributeError.

Catch any such sqlglot-side failure in _parse_with_dialect and treat it as 'this
dialect produced no result', so the query is reported through the normal
InvalidQueryDefinition path. ParseError/TokenError are still re-raised so the
existing syntax-error handling is unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant