fix unused code, bump coverage - add todo to revisit corner cases later for now mark nocover as unreachable from parser and this is the only entrypoint we want for majority of the tests

Author: collerek

sql_metadata/column_extractor.py

@@ -336,11 +336,10 @@ def _is_star_inside_function(star: exp.Star) -> bool:
     # -------------------------------------------------------------------
 
     def _walk(
-        self, node: Optional[exp.Expression], clause: str = "", depth: int = 0
+        self, node: exp.Expression, clause: str = "", depth: int = 0
     ) -> None:
         """Depth-first walk of the AST in ``arg_types`` key order."""
-        if node is None:
-            return
+        assert node is not None
 
         if self._dispatch_leaf(node, clause, depth):
             return
@@ -368,9 +367,11 @@ def _dispatch_leaf(self, node: exp.Expression, clause: str, depth: int) -> bool:
         Returns ``True`` if handled (stop recursion), ``False`` to continue.
         """
         if isinstance(node, (exp.Values, exp.Star, exp.ColumnDef, exp.Identifier)):
-            if isinstance(node, exp.Star):
+            # TODO: revisit if Stars appear outside Select.expressions
+            if isinstance(node, exp.Star):  # pragma: no cover
                 self._handle_star(node, clause)
-            elif isinstance(node, exp.ColumnDef):
+            # TODO: revisit if CREATE TABLE walk stops returning early
+            elif isinstance(node, exp.ColumnDef):  # pragma: no cover
                 self._collector.add_column(node.name, clause)
             elif isinstance(node, exp.Identifier):
                 self._handle_identifier(node, clause)
@@ -416,7 +417,8 @@ def _recurse_child(self, child: Any, clause: str, depth: int) -> None:
     # Node handlers
     # -------------------------------------------------------------------
 
-    def _handle_star(self, node: exp.Star, clause: str) -> None:
+    # TODO: revisit if Stars reach _dispatch_leaf
+    def _handle_star(self, node: exp.Star, clause: str) -> None:  # pragma: no cover
         """Handle a standalone Star node (not inside a Column or function)."""
         not_in_col = not isinstance(node.parent, exp.Column)
         if not_in_col and not self._is_star_inside_function(node):
@@ -428,7 +430,8 @@ def _handle_identifier(self, node: exp.Identifier, clause: str) -> None:
             node.parent,
             (exp.Column, exp.Table, exp.TableAlias, exp.CTE),
         ):
-            if clause == "join":
+            # TODO: revisit if JOIN produces bare Identifiers
+            if clause == "join":  # pragma: no cover
                 self._collector.add_column(node.name, clause)
 
     def _handle_insert_schema(self, node: exp.Insert) -> None:
@@ -456,7 +459,8 @@ def _handle_column(self, col: exp.Column, clause: str) -> None:
             if table:
                 table = self._resolve_table_alias(table)
                 c.add_column(f"{table}.*", clause)
-            else:
+            # TODO: revisit if Column(Star) without table
+            else:  # pragma: no cover
                 c.add_column("*", clause)
             return
 
@@ -475,10 +479,9 @@ def _handle_column(self, col: exp.Column, clause: str) -> None:
 
         c.add_column(full, clause)
 
-    def _handle_select_exprs(self, exprs: Any, clause: str, depth: int) -> None:
+    def _handle_select_exprs(self, exprs: list, clause: str, depth: int) -> None:
         """Handle the expressions list of a SELECT clause."""
-        if not isinstance(exprs, list):
-            return
+        assert isinstance(exprs, list)
 
         for expr in exprs:
             if isinstance(expr, exp.Alias):
@@ -540,7 +543,8 @@ def _handle_cte(self, cte: exp.CTE, depth: int) -> None:
         """Handle a CTE (Common Table Expression) AST node."""
         c = self._collector
         alias = cte.alias
-        if not alias:
+        # TODO: revisit if sqlglot ever produces CTE nodes without aliases
+        if not alias:  # pragma: no cover
             return
 
         c.cte_names.append(alias)
@@ -605,7 +609,8 @@ def _collect_column_from_node(
                 if table:
                     table = self._resolve_table_alias(table)
                     return f"{table}.*"
-                return "*"
+                # TODO: revisit if Column(Star) without table
+                return "*"  # pragma: no cover
             return self._column_full_name(child)
         if isinstance(child, exp.Star):
             if id(child) not in seen_stars and not isinstance(child.parent, exp.Column):
@@ -615,9 +620,8 @@ def _collect_column_from_node(
 
     def _flat_columns(self, node: exp.Expression) -> list:
         """Extract all column names from an expression subtree via DFS."""
+        assert node is not None
         cols = []
-        if node is None:
-            return cols
         seen_stars: set[int] = set()
         for child in _dfs(node):
             name = self._collect_column_from_node(child, seen_stars)

sql_metadata/comments.py

@@ -90,7 +90,8 @@ def extract_comments(sql: str) -> List[str]:
         return []
     try:
         tokens = list(_choose_tokenizer(sql).tokenize(sql))
-    except Exception:
+    # TODO: revisit if sqlglot tokenizer starts raising on specific inputs
+    except Exception:  # pragma: no cover
         return []
     comments: list[str] = []
     prev_end = -1

sql_metadata/dialect_parser.py

@@ -139,7 +139,8 @@ def _try_dialects(
                     raise ValueError("This query is wrong")
                 continue
 
-        if last_result is not None:
+        # TODO: revisit if sqlglot starts returning None from parse for last dialect
+        if last_result is not None:  # pragma: no cover
             return last_result, winning_dialect
         raise ValueError("This query is wrong")
 
@@ -161,9 +162,9 @@ def _parse_with_dialect(clean_sql: str, dialect: Any) -> Optional[exp.Expression
         if not results or results[0] is None:
             return None
         result = results[0]
-        if result is None:
-            return None
-        if isinstance(result, exp.Subquery) and not result.alias:
+        assert result is not None  # guaranteed by check above
+        # TODO: revisit if sqlglot returns top-level Subquery
+        if isinstance(result, exp.Subquery) and not result.alias:  # pragma: no cover
             inner = result.this
             if isinstance(inner, exp.Expression):
                 return inner

sql_metadata/nested_resolver.py

@@ -52,7 +52,8 @@ def coalesce_sql(self, expression: exp.Expression) -> str:
         args = [expression.this] + expression.expressions
         if len(args) == 2:
             return f"IFNULL({self.sql(args[0])}, {self.sql(args[1])})"
-        return super().coalesce_sql(expression)  # type: ignore[misc, no-any-return]
+        args_sql = ", ".join(self.sql(a) for a in args)
+        return f"COALESCE({args_sql})"
 
     def dateadd_sql(self, expression: exp.Expression) -> str:
         return (
@@ -140,7 +141,7 @@ def __init__(
     def _cte_nodes(self) -> list:
         """Return all ``exp.CTE`` nodes from the AST (cached)."""
         if self._cached_cte_nodes is None:
-            if self._ast is None:
+            if self._ast is None:  # pragma: no cover — callers check first
                 self._cached_cte_nodes = []
             else:
                 self._cached_cte_nodes = list(self._ast.find_all(exp.CTE))
@@ -154,7 +155,7 @@ def extract_cte_names(
 
         Called by :attr:`Parser.with_names`.
         """
-        if self._ast is None:
+        if self._ast is None:  # pragma: no cover — Parser ensures AST exists
             return []
         cte_name_map = cte_name_map or {}
         reverse_map = _make_reverse_cte_map(cte_name_map)
@@ -171,7 +172,7 @@ def extract_subquery_names(ast: Optional[exp.Expression]) -> List[str]:
 
         Called by :attr:`Parser.subqueries_names`.
         """
-        if ast is None:
+        if ast is None:  # pragma: no cover — Parser ensures AST exists
             return []
         names = UniqueList()
         NestedResolver._collect_subquery_names_postorder(ast, names)
@@ -310,7 +311,8 @@ def _resolve_and_filter(
             result = self._resolve_sub_queries(col)
             if isinstance(result, list):
                 resolved.extend(result)
-            else:
+            # TODO: revisit if _resolve_sub_queries returns non-list
+            else:  # pragma: no cover
                 resolved.append(result)
 
         final = UniqueList()
@@ -370,7 +372,8 @@ def _lookup_alias_in_nested(
 
         for nested_name in names:
             nested_def = definitions.get(nested_name)
-            if not nested_def:
+            # TODO: revisit if extract_*_bodies can produce missing entries
+            if not nested_def:  # pragma: no cover
                 continue
             nested_parser = parser_cache.setdefault(nested_name, Parser(nested_def))
             if col_name in nested_parser.columns_aliases_names:
@@ -425,7 +428,8 @@ def _resolve_nested_query(
             return subquery_alias
         sub_query, column_name = parts[0], parts[-1]
         sub_query_definition = nested_queries.get(sub_query)
-        if not sub_query_definition:
+        # TODO: revisit if names/definitions can diverge between extraction steps
+        if not sub_query_definition:  # pragma: no cover
             return subquery_alias
         subparser = already_parsed.setdefault(sub_query, Parser(sub_query_definition))
         return NestedResolver._resolve_column_in_subparser(

sql_metadata/parser.py

@@ -148,7 +148,8 @@ def tokens(self) -> List[str]:
             sg_tokens = list(
                 _choose_tokenizer(self._raw_query).tokenize(self._raw_query)
             )
-        except Exception:
+        # TODO: revisit if sqlglot tokenizer starts raising on specific inputs
+        except Exception:  # pragma: no cover
             sg_tokens = []
         self._tokens = [t.text.strip("`").strip('"') for t in sg_tokens]
         return self._tokens
@@ -175,7 +176,7 @@ def columns(self) -> list:
             self._columns_aliases = {}
             return self._columns
 
-        if ast is None:
+        if ast is None:  # pragma: no cover — tables_aliases raises for None ast
             self._columns = UniqueList()
             self._columns_dict = {}
             self._columns_aliases_names = UniqueList()
@@ -192,9 +193,7 @@ def columns(self) -> list:
         self._columns_aliases_dict = result.alias_dict
         self._columns_aliases = result.alias_map if result.alias_map else {}
 
-        # Cache CTE/subquery names from the same extraction
-        if self._with_names is None:
-            self._with_names = result.cte_names
+        # Cache subquery names from the same extraction
         if self._subqueries_names is None:
             self._subqueries_names = result.subquery_names
 
@@ -343,7 +342,7 @@ def _extract_int_from_node(node: Any) -> Optional[int]:
             return None
         try:
             return int(node.expression.this)
-        except (ValueError, AttributeError):
+        except (ValueError, AttributeError, TypeError):
             return None
 
     @property
@@ -387,7 +386,8 @@ def values_dict(self) -> Optional[Dict]:
             return self._values_dict
         try:
             columns = self.columns
-        except ValueError:
+        # TODO: revisit if .columns starts propagating ValueError to callers
+        except ValueError:  # pragma: no cover
             columns = []
         if not columns:
             columns = [f"column_{ind + 1}" for ind in range(len(values))]
@@ -438,7 +438,8 @@ def _extract_values(self) -> List:
             if isinstance(tup, exp.Tuple):
                 for val in tup.expressions:
                     values.append(self._convert_value(val))
-            else:
+            # TODO: revisit if sqlglot stops wrapping VALUES items in Tuple
+            else:  # pragma: no cover
                 values.append(self._convert_value(tup))
         return values
 

sql_metadata/query_type_extractor.py

@@ -6,7 +6,7 @@
 """
 
 import logging
-from typing import Optional
+from typing import NoReturn, Optional
 
 from sqlglot import exp
 
@@ -56,7 +56,6 @@ def extract(self) -> QueryType:
         """
         if self._ast is None:
             self._raise_for_none_ast()
-            assert self._ast is not None  # unreachable; for mypy
 
         root = self._unwrap_parens(self._ast)
         node_type = type(root)
@@ -80,7 +79,8 @@ def extract(self) -> QueryType:
     @staticmethod
     def _unwrap_parens(ast: exp.Expression) -> exp.Expression:
         """Remove Paren and Subquery wrappers to reach the real statement."""
-        if isinstance(ast, (exp.Paren, exp.Subquery)):
+        # TODO: revisit if sqlglot stops stripping outer parens before this is called
+        if isinstance(ast, (exp.Paren, exp.Subquery)):  # pragma: no cover
             return QueryTypeExtractor._unwrap_parens(ast.this)
         return ast
 
@@ -94,7 +94,7 @@ def _resolve_command_type(root: exp.Expression) -> Optional[QueryType]:
             return QueryType.CREATE
         return None
 
-    def _raise_for_none_ast(self) -> None:
+    def _raise_for_none_ast(self) -> "NoReturn":
         """Raise an appropriate error when the AST is None."""
         from sql_metadata.comments import strip_comments
 

sql_metadata/table_extractor.py

@@ -27,7 +27,8 @@ def _assemble_dotted_name(catalog: str, db: object, name: str) -> str:
         parts.append(catalog)
     if db is not None:
         db_str = str(db)
-        if db_str == "" and catalog:
+        # TODO: revisit if catalog..table bypasses shortcut
+        if db_str == "" and catalog:  # pragma: no cover
             parts.append("")
         elif db_str:
             parts.append(db_str)
@@ -49,8 +50,6 @@ def _collect_node_parts(node: object, parts: list[str]) -> None:
         for sub in [node.this, node.expression]:
             if isinstance(sub, exp.Identifier):
                 parts.append(_ident_str(sub))
-    elif node == "":
-        parts.append("")
 
 
 def _bracketed_full_name(table: exp.Table) -> str:
@@ -63,11 +62,6 @@ def _bracketed_full_name(table: exp.Table) -> str:
     return ".".join(parts) if parts else ""
 
 
-def _is_word_char(c: str) -> bool:
-    """Check whether *c* is an alphanumeric character or underscore."""
-    return c.isalnum() or c == "_"
-
-
 def _ends_with_table_keyword(before: str) -> bool:
     """Check whether *before* ends with a table-introducing keyword."""
     return any(before.endswith(kw) for kw in _TABLE_CONTEXT_KEYWORDS)
@@ -139,7 +133,7 @@ def extract(self) -> List[str]:
         Sorts results by first occurrence in raw SQL (left-to-right order).
         For ``CREATE TABLE`` statements the target table is always first.
         """
-        if self._ast is None:
+        if self._ast is None:  # pragma: no cover — Parser always provides an AST
             return []
 
         if isinstance(self._ast, exp.Command):
@@ -166,7 +160,7 @@ def extract_aliases(self, tables: List[str]) -> Dict[str, str]:
         :param tables: List of known table names.
         :returns: Mapping of ``{alias: table_name}``.
         """
-        if self._ast is None:
+        if self._ast is None:  # pragma: no cover — Parser always provides an AST
             return {}
 
         aliases = {}
@@ -213,7 +207,8 @@ def _first_position(self, name: str) -> int:
 
         last_part = last_segment(name_upper)
         pos = self._find_word_in_table_context(last_part)
-        if pos >= 0:
+        # TODO: revisit if qualified table names stop being found by full name above
+        if pos >= 0:  # pragma: no cover
             return pos
 
         pos = self._find_word(name_upper)
@@ -255,17 +250,20 @@ def _extract_create_target(self) -> Optional[str]:
         """Extract the target table name from a CREATE TABLE statement."""
         assert self._ast is not None
         target = self._ast.this
-        if not target:
+        # TODO: revisit if sqlglot produces CREATE without .this target
+        if not target:  # pragma: no cover
             return None
         target_table = (
             target.find(exp.Table) if not isinstance(target, exp.Table) else target
         )
-        if not target_table:
+        # TODO: revisit if sqlglot produces CREATE target without a Table node
+        if not target_table:  # pragma: no cover
             return None
         name = self._table_full_name(target_table)
         if name and name not in self._cte_names:
             return name
-        return None
+        # TODO: revisit if CTE-named CREATE targets become possible
+        return None  # pragma: no cover
 
     def _collect_lateral_aliases(self) -> List[str]:
         """Collect alias names from LATERAL VIEW clauses in the AST."""

test/test_alter.py

@@ -11,3 +11,9 @@ def test_alter_table_indices_index():
     parser = Parser("ALTER TABLE foo_table ADD INDEX `idx_foo` (`bar`);")
     assert parser.query_type == QueryType.ALTER
     assert parser.tables == ["foo_table"]
+
+
+def test_alter_table_add_column():
+    """ALTER TABLE ADD COLUMN is parsed correctly."""
+    p = Parser("ALTER TABLE t ADD COLUMN new_col INT")
+    assert p.query_type == "ALTER TABLE"