Skip to content

Commit 4f36f29

Browse files
committed
raise more meaningful error on invalid queries, raise on cte without name instead of silently skipping, extract mypy and ruff into separate workflows
1 parent 404754e commit 4f36f29

12 files changed

Lines changed: 113 additions & 30 deletions

.github/workflows/lint.yml

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
name: Lint
2+
3+
on:
4+
push:
5+
branches: [ master ]
6+
pull_request:
7+
8+
jobs:
9+
lint:
10+
runs-on: ubuntu-latest
11+
12+
steps:
13+
- uses: actions/checkout@v6
14+
- name: Set up Python
15+
uses: actions/setup-python@v6
16+
with:
17+
python-version: "3.12"
18+
19+
- name: Install Poetry
20+
uses: snok/install-poetry@v1.4.1
21+
with:
22+
version: latest
23+
virtualenvs-create: true
24+
virtualenvs-in-project: true
25+
26+
- name: Install dependencies with poetry
27+
run: poetry install --no-root
28+
29+
- name: Lint with ruff
30+
run: make lint

.github/workflows/python-ci.yml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,5 @@ jobs:
7474
pip install coveralls
7575
poetry run coveralls --service=github
7676
77-
- name: Lint with ruff
78-
run: make lint
79-
80-
- name: Type check with mypy
81-
run: make type_check
82-
8377
- name: Build a distribution package
8478
run: poetry build -vvv

.github/workflows/type-check.yml

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
name: Type Check
2+
3+
on:
4+
push:
5+
branches: [ master ]
6+
pull_request:
7+
8+
jobs:
9+
type-check:
10+
runs-on: ubuntu-latest
11+
12+
steps:
13+
- uses: actions/checkout@v6
14+
- name: Set up Python
15+
uses: actions/setup-python@v6
16+
with:
17+
python-version: "3.12"
18+
19+
- name: Install Poetry
20+
uses: snok/install-poetry@v1.4.1
21+
with:
22+
version: latest
23+
virtualenvs-create: true
24+
virtualenvs-in-project: true
25+
26+
- name: Install dependencies with poetry
27+
run: poetry install --no-root
28+
29+
- name: Type check with mypy
30+
run: make type_check

sql_metadata/__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@
1515
MSSQL, MySQL, Hive/Spark, and TSQL bracket notation.
1616
"""
1717

18+
from sql_metadata.exceptions import InvalidQueryDefinition
1819
from sql_metadata.keywords_lists import QueryType
1920
from sql_metadata.parser import Parser
2021

21-
__all__ = ["Parser", "QueryType"]
22+
__all__ = ["InvalidQueryDefinition", "Parser", "QueryType"]

sql_metadata/column_extractor.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
from sqlglot import exp
1818

19+
from sql_metadata.exceptions import InvalidQueryDefinition
1920
from sql_metadata.utils import UniqueList, _make_reverse_cte_map, last_segment
2021

2122
# ---------------------------------------------------------------------------
@@ -509,9 +510,10 @@ def _handle_cte(self, cte: exp.CTE, depth: int) -> None:
509510
"""Handle a CTE (Common Table Expression) AST node."""
510511
c = self._collector
511512
alias = cte.alias
512-
# TODO: revisit if sqlglot ever produces CTE nodes without aliases
513-
if not alias: # pragma: no cover
514-
return
513+
if not alias:
514+
raise InvalidQueryDefinition(
515+
"All CTEs require an alias, not a valid SQL"
516+
)
515517

516518
c.cte_names.append(alias)
517519

sql_metadata/dialect_parser.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ class so that callers only need to call :meth:`DialectParser.parse`.
1515
from sqlglot.tokens import Tokenizer as BaseTokenizer
1616

1717
from sql_metadata.comments import _has_hash_variables
18+
from sql_metadata.exceptions import InvalidQueryDefinition
1819

1920
#: Table names that indicate a degraded parse result.
2021
_BAD_TABLE_NAMES = frozenset({"IGNORE", ""})
@@ -136,13 +137,17 @@ def _try_dialects(
136137
return result, dialect
137138
except (ParseError, TokenError):
138139
if dialect is not None and dialect == dialects[-1]:
139-
raise ValueError("This query is wrong")
140+
raise InvalidQueryDefinition(
141+
"Query could not be parsed — SQL syntax error"
142+
)
140143
continue
141144

142145
# TODO: revisit if sqlglot starts returning None from parse for last dialect
143146
if last_result is not None: # pragma: no cover
144147
return last_result, winning_dialect
145-
raise ValueError("This query is wrong")
148+
raise InvalidQueryDefinition(
149+
"Query could not be parsed — no dialect could handle this SQL"
150+
)
146151

147152
@staticmethod
148153
def _parse_with_dialect(clean_sql: str, dialect: Any) -> Optional[exp.Expression]:

sql_metadata/exceptions.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
"""Custom exceptions for the sql-metadata package."""
2+
3+
4+
class InvalidQueryDefinition(ValueError):
5+
"""Raised when the SQL query is structurally invalid or unsupported."""

sql_metadata/query_type_extractor.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
from sqlglot import exp
1212

13+
from sql_metadata.exceptions import InvalidQueryDefinition
1314
from sql_metadata.keywords_lists import QueryType
1415

1516
logger = logging.getLogger(__name__)
@@ -61,7 +62,9 @@ def extract(self) -> QueryType:
6162
node_type = type(root)
6263

6364
if node_type is exp.With:
64-
raise ValueError("This query is wrong")
65+
raise InvalidQueryDefinition(
66+
"WITH clause without a main statement is not valid SQL"
67+
)
6568

6669
simple = _SIMPLE_TYPE_MAP.get(node_type)
6770
if simple is not None:
@@ -74,7 +77,7 @@ def extract(self) -> QueryType:
7477

7578
shorten_query = " ".join(self._raw_query.split(" ")[:3])
7679
logger.error("Not supported query type: %s", shorten_query)
77-
raise ValueError("Not supported query type!")
80+
raise InvalidQueryDefinition("Not supported query type!")
7881

7982
@staticmethod
8083
def _unwrap_parens(ast: exp.Expression) -> exp.Expression:
@@ -100,5 +103,7 @@ def _raise_for_none_ast(self) -> "NoReturn":
100103

101104
stripped = strip_comments(self._raw_query) if self._raw_query else ""
102105
if stripped.strip():
103-
raise ValueError("This query is wrong")
104-
raise ValueError("Empty queries are not supported!")
106+
raise InvalidQueryDefinition(
107+
"Could not parse the query — the SQL syntax appears to be invalid"
108+
)
109+
raise InvalidQueryDefinition("Empty queries are not supported!")

sql_metadata/sql_cleaner.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from typing import NamedTuple, Optional
1212

1313
from sql_metadata.comments import strip_comments_for_parsing as _strip_comments
14+
from sql_metadata.exceptions import InvalidQueryDefinition
1415
from sql_metadata.utils import DOT_PLACEHOLDER
1516

1617

@@ -174,4 +175,6 @@ def _detect_malformed_with(clean_sql: str) -> None:
174175
if re.search(
175176
r"\)\s+AS\s+" + main_kw + r"\b", clean_sql, re.IGNORECASE
176177
) or re.search(r"\)\s+AS\s+\w+\s+" + main_kw + r"\b", clean_sql, re.IGNORECASE):
177-
raise ValueError("This query is wrong")
178+
raise InvalidQueryDefinition(
179+
"Malformed WITH clause — extra AS keyword after CTE body"
180+
)

test/test_create_table.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import pytest
22

3-
from sql_metadata import Parser
3+
from sql_metadata import InvalidQueryDefinition, Parser
44
from sql_metadata.keywords_lists import QueryType
55

66

77
def test_is_create_table_query():
8-
with pytest.raises(ValueError):
8+
with pytest.raises(InvalidQueryDefinition):
99
assert Parser("BEGIN").query_type
1010

1111
assert Parser("SELECT * FROM `foo` ()").query_type == QueryType.SELECT

0 commit comments

Comments
 (0)