From eb5ef3943db2e5323964376d386bcbcac7eb6849 Mon Sep 17 00:00:00 2001 From: Vishal Mishra Date: Wed, 29 Jul 2026 21:47:12 +0000 Subject: [PATCH] Apply readability refactors in sql_metadata and format test fixtures Simplify regex match indexing, ternary patterns, and min() default usage in the core package; ruff-format test SQL fixtures with trailing whitespace. --- sql_metadata/column_extractor.py | 2 +- sql_metadata/generalizator.py | 5 +++-- sql_metadata/parser.py | 10 +++++----- sql_metadata/sql_cleaner.py | 14 ++++++++------ sql_metadata/table_extractor.py | 2 +- test/test_create_table.py | 9 ++++++++- test/test_getting_columns.py | 21 +++++++++++---------- test/test_getting_tables.py | 3 +-- test/test_limit_and_offset.py | 12 +++--------- test/test_multiple_subqueries.py | 9 ++------- test/test_redshift.py | 3 +-- test/test_values.py | 4 +--- test/test_with_statements.py | 19 ++++++------------- 13 files changed, 51 insertions(+), 62 deletions(-) diff --git a/sql_metadata/column_extractor.py b/sql_metadata/column_extractor.py index 1efdb8d8..dff16f92 100644 --- a/sql_metadata/column_extractor.py +++ b/sql_metadata/column_extractor.py @@ -584,7 +584,7 @@ def _handle_alias(self, alias_node: exp.Alias, clause: str, depth: int) -> None: target = ( target_cols[0] if len(target_cols) == 1 - else (target_cols if target_cols else None) + else target_cols or None ) c.add_alias(alias_name, target, clause) return diff --git a/sql_metadata/generalizator.py b/sql_metadata/generalizator.py index f0639517..049779fc 100644 --- a/sql_metadata/generalizator.py +++ b/sql_metadata/generalizator.py @@ -56,12 +56,13 @@ def _normalize_likes(sql: str) -> str: # or all_groups LIKE X or all_groups LIKE X found = re.finditer(r"(or|and) [^\s]+ LIKE X", sql, flags=re.IGNORECASE) - like_matches = [m.group(0) for m in found] + like_matches = [m[0] for m in found] if like_matches: for match in set(like_matches): + replacement = " " + match + " ..." sql = re.sub( - r"(\s?" + re.escape(match) + ")+", " " + match + " ...", sql + r"(\s?" + re.escape(match) + ")+", replacement, sql ) return sql diff --git a/sql_metadata/parser.py b/sql_metadata/parser.py index c92538fc..3fcebd0f 100644 --- a/sql_metadata/parser.py +++ b/sql_metadata/parser.py @@ -581,8 +581,8 @@ def _extract_limit_regex(self) -> tuple[int, int] | None: sql = strip_comments(self._raw_query) match = re.search(r"LIMIT\s+(\d+)\s*,\s*(\d+)", sql, re.IGNORECASE) if match: - offset_val = int(match.group(1)) - limit_val = int(match.group(2)) + offset_val = int(match[1]) + limit_val = int(match[2]) self._limit_and_offset = limit_val, offset_val return self._limit_and_offset @@ -592,8 +592,8 @@ def _extract_limit_regex(self) -> tuple[int, int] | None: re.IGNORECASE, ) if match: - limit_val = int(match.group(1)) - offset_val = int(match.group(2)) if match.group(2) else 0 + limit_val = int(match[1]) + offset_val = int(match[2]) if match[2] else 0 self._limit_and_offset = limit_val, offset_val return self._limit_and_offset return None @@ -615,7 +615,7 @@ def _extract_columns_regex(self) -> list[str]: if not match: return [] cols = [] - for col in match.group(1).split(","): + for col in match[1].split(","): col = col.strip().strip("`").strip('"').strip("'") if col: cols.append(col) diff --git a/sql_metadata/sql_cleaner.py b/sql_metadata/sql_cleaner.py index 44993c64..d9fbdc5b 100644 --- a/sql_metadata/sql_cleaner.py +++ b/sql_metadata/sql_cleaner.py @@ -91,9 +91,9 @@ def _normalize_cte_names(sql: str) -> tuple[str, dict[str, str]]: ) def replacer(match: re.Match[str]) -> str: - prefix = match.group(1) - qualified_name = match.group(2) - suffix = match.group(3) + prefix = match[1] + qualified_name = match[2] + suffix = match[3] placeholder = qualified_name.replace(".", DOT_PLACEHOLDER) name_map[placeholder] = qualified_name return f"{prefix}{placeholder}{suffix}" @@ -247,9 +247,11 @@ def _detect_malformed_with(clean_sql: str) -> None: if not re.match(r"\s*WITH\b", clean_sql, re.IGNORECASE): return main_kw = r"(?:SELECT|INSERT|UPDATE|DELETE)" - if re.search( - r"\)\s+AS\s+" + main_kw + r"\b", clean_sql, re.IGNORECASE - ) or re.search(r"\)\s+AS\s+\w+\s+" + main_kw + r"\b", clean_sql, re.IGNORECASE): + pattern_a = r"\)\s+AS\s+" + main_kw + r"\b" + pattern_b = r"\)\s+AS\s+\w+\s+" + main_kw + r"\b" + if re.search(pattern_a, clean_sql, re.IGNORECASE) or re.search( + pattern_b, clean_sql, re.IGNORECASE + ): raise InvalidQueryDefinition( "Malformed WITH clause — extra AS keyword after CTE body" ) diff --git a/sql_metadata/table_extractor.py b/sql_metadata/table_extractor.py index 1c37492d..51902ca9 100644 --- a/sql_metadata/table_extractor.py +++ b/sql_metadata/table_extractor.py @@ -304,4 +304,4 @@ def _table_start_position(table: exp.Table) -> int: if isinstance((n := table.args.get(key)), exp.Identifier) and "start" in n.meta ] - return min(positions) if positions else 0 + return min(positions, default=0) diff --git a/test/test_create_table.py b/test/test_create_table.py index 2e5d1fec..86164a08 100644 --- a/test/test_create_table.py +++ b/test/test_create_table.py @@ -233,7 +233,14 @@ def test_create_table_with_comments_and_keyword_columns(): """) assert p.tables == ["accounts"] assert p.columns == [ - "id", "username", "status", "online_at", "hash", "uid", "test", "usage" + "id", + "username", + "status", + "online_at", + "hash", + "uid", + "test", + "usage", ] diff --git a/test/test_getting_columns.py b/test/test_getting_columns.py index fb0d59ae..4aa2f90a 100644 --- a/test/test_getting_columns.py +++ b/test/test_getting_columns.py @@ -129,7 +129,11 @@ def test_output_columns(): dj.field_3 as field_3 FROM dj""") assert parser.output_columns == [ - "dj.field_1", "field_1_count", "dj.field_2", "field_2_count", "field_3" + "dj.field_1", + "field_1_count", + "dj.field_2", + "field_2_count", + "field_3", ] # Simple alias @@ -156,7 +160,10 @@ def test_output_columns(): JOIN customers c ON o.customer_id = c.customer_id JOIN order_items oi ON o.order_id = oi.order_id""") assert parser.output_columns == [ - "month", "customers.customer_id", "revenue", "revenue_rank" + "month", + "customers.customer_id", + "revenue", + "revenue_rank", ] assert "revenue_rank" in parser.columns_aliases @@ -759,18 +766,12 @@ def test_columns_via_regex_on_completely_invalid_sql(): def test_cte_with_more_column_aliases_than_body(): """CTE defines more column names than the body SELECT produces.""" - p = Parser( - "WITH cte(a, b, c) AS (SELECT x FROM t) " - "SELECT a FROM cte" - ) + p = Parser("WITH cte(a, b, c) AS (SELECT x FROM t) SELECT a FROM cte") assert "a" in p.columns_aliases_names def test_cte_with_table_star_in_body(): """CTE body uses table.* — exercises _flat_columns with table-qualified star.""" - p = Parser( - "WITH cte(a) AS (SELECT t.* FROM t) " - "SELECT a FROM cte" - ) + p = Parser("WITH cte(a) AS (SELECT t.* FROM t) SELECT a FROM cte") assert p.columns == ["t.*"] assert p.columns_aliases_names == ["a"] diff --git a/test/test_getting_tables.py b/test/test_getting_tables.py index 70f34e89..c6525fb1 100644 --- a/test/test_getting_tables.py +++ b/test/test_getting_tables.py @@ -888,8 +888,7 @@ def test_presto_unnest_not_table(): def test_bigquery_unnest_not_table(): # Solved: https://github.com/macbre/sql-metadata/issues/352 p = Parser( - "SELECT A, B, metrics.C, metrics.D " - "FROM table1, UNNEST(metrics) as metrics" + "SELECT A, B, metrics.C, metrics.D FROM table1, UNNEST(metrics) as metrics" ) assert p.tables == ["table1"] assert "metrics" in p.columns diff --git a/test/test_limit_and_offset.py b/test/test_limit_and_offset.py index b1ae7cba..e606533c 100644 --- a/test/test_limit_and_offset.py +++ b/test/test_limit_and_offset.py @@ -75,27 +75,21 @@ def test_limit_regex_mysql_comma_via_subquery(): LIMIT ALL makes sqlglot produce a non-integer limit node, triggering the regex fallback which then matches the inner subquery's LIMIT 10, 20. """ - p = Parser( - "SELECT * FROM (SELECT id FROM t LIMIT 10, 20) AS sub LIMIT ALL" - ) + p = Parser("SELECT * FROM (SELECT id FROM t LIMIT 10, 20) AS sub LIMIT ALL") assert p.limit_and_offset == (20, 10) def test_limit_regex_standard_via_subquery(): """Regex fallback finds standard LIMIT in subquery.""" p = Parser( - "SELECT * FROM (SELECT id FROM t LIMIT 30) AS sub" - " FETCH FIRST 5 ROWS ONLY" + "SELECT * FROM (SELECT id FROM t LIMIT 30) AS sub FETCH FIRST 5 ROWS ONLY" ) assert p.limit_and_offset == (30, 0) def test_limit_regex_with_offset_via_subquery(): """Regex fallback finds LIMIT with OFFSET when outer is unparseable.""" - p = Parser( - "SELECT * FROM (SELECT id FROM t LIMIT 50 OFFSET 100)" - " AS sub LIMIT ALL" - ) + p = Parser("SELECT * FROM (SELECT id FROM t LIMIT 50 OFFSET 100) AS sub LIMIT ALL") assert p.limit_and_offset == (50, 100) diff --git a/test/test_multiple_subqueries.py b/test/test_multiple_subqueries.py index a03442b1..d9a28750 100644 --- a/test/test_multiple_subqueries.py +++ b/test/test_multiple_subqueries.py @@ -578,9 +578,7 @@ def test_unaliased_subquery(): def test_multiple_unaliased_subqueries(): p = Parser( - "SELECT * FROM t " - "WHERE a IN (SELECT id FROM t2) " - "AND b IN (SELECT id FROM t3)" + "SELECT * FROM t WHERE a IN (SELECT id FROM t2) AND b IN (SELECT id FROM t3)" ) assert p.subqueries_names == ["subquery_1", "subquery_2"] assert "subquery_1" in p.subqueries @@ -588,10 +586,7 @@ def test_multiple_unaliased_subqueries(): def test_mixed_aliased_and_unaliased_subqueries(): - p = Parser( - "SELECT * FROM (SELECT id FROM t2) sub " - "WHERE a IN (SELECT id FROM t3)" - ) + p = Parser("SELECT * FROM (SELECT id FROM t2) sub WHERE a IN (SELECT id FROM t3)") assert "sub" in p.subqueries_names assert "subquery_1" in p.subqueries_names assert "sub" in p.subqueries diff --git a/test/test_redshift.py b/test/test_redshift.py index 83f2049b..12a5e39f 100644 --- a/test/test_redshift.py +++ b/test/test_redshift.py @@ -1,5 +1,4 @@ -from sql_metadata import Parser -from sql_metadata import QueryType +from sql_metadata import Parser, QueryType def test_redshift(): diff --git a/test/test_values.py b/test/test_values.py index 07668a53..a4e294a3 100644 --- a/test/test_values.py +++ b/test/test_values.py @@ -120,9 +120,7 @@ def test_negative_integer_values(): def test_negative_float_values(): """INSERT with a negative float value.""" - p = Parser( - "INSERT INTO measurements (sensor, reading) VALUES ('temp', -3.14)" - ) + p = Parser("INSERT INTO measurements (sensor, reading) VALUES ('temp', -3.14)") assert p.values == ["temp", -3.14] assert p.values_dict == {"sensor": "temp", "reading": -3.14} diff --git a/test/test_with_statements.py b/test/test_with_statements.py index 67f0c0d0..de063398 100644 --- a/test/test_with_statements.py +++ b/test/test_with_statements.py @@ -628,10 +628,7 @@ def test_group_by_not_table_alias_in_cte(): def test_coalesce_three_args_in_cte(): """COALESCE with 3+ args should render as COALESCE, not IFNULL.""" - p = Parser( - "WITH cte AS (SELECT COALESCE(a, b, c) FROM t) " - "SELECT * FROM cte" - ) + p = Parser("WITH cte AS (SELECT COALESCE(a, b, c) FROM t) SELECT * FROM cte") body = p.with_queries["cte"] assert "COALESCE" in body.upper() @@ -658,20 +655,14 @@ def test_date_sub_in_cte(): def test_not_expression_in_cte(): """NOT applied to a boolean expression (not IS NULL or IN) in CTE body.""" - p = Parser( - "WITH cte AS (SELECT * FROM t WHERE NOT (active > 0)) " - "SELECT * FROM cte" - ) + p = Parser("WITH cte AS (SELECT * FROM t WHERE NOT (active > 0)) SELECT * FROM cte") body = p.with_queries["cte"] assert "NOT" in body.upper() def test_nested_resolver_unresolvable_reference(): """A dotted column reference not matching any CTE/subquery stays as-is.""" - p = Parser( - "WITH cte AS (SELECT id FROM t) " - "SELECT nonexistent.col FROM cte" - ) + p = Parser("WITH cte AS (SELECT id FROM t) SELECT nonexistent.col FROM cte") assert "nonexistent.col" in p.columns @@ -686,7 +677,9 @@ def test_cte_with_subquery_and_star_alias(): ) q inner join x on q.s2_fk = x.af_pk""") assert p.tables == [ - "test_db.test_table3", "testdb.test_table", "testdb.test_table2" + "test_db.test_table3", + "testdb.test_table", + "testdb.test_table2", ] assert p.with_names == ["x"] assert "testdb.test_table.*" in p.columns