cleanup in parser and docstring refactor, cleanup of thr remaining todoes

Author: collerek

sql_metadata/ast_parser.py

@@ -55,6 +55,8 @@ def dialect(self) -> DialectType:
         Set as a side-effect of :attr:`ast` access.  May be ``None``
         (default dialect), a string like ``"mysql"``, or a custom
         :class:`Dialect` subclass such as :class:`HashVarDialect`.
+
+        :rtype: DialectType
         """
         _ = self.ast
         return self._dialect
@@ -67,6 +69,8 @@ def is_replace(self) -> bool:
         (sqlglot otherwise produces an opaque ``Command`` node).  This
         flag allows :attr:`Parser.query_type` to restore the correct
         :class:`QueryType.REPLACE` value.
+
+        :rtype: bool
         """
         _ = self.ast
         return self._is_replace
@@ -77,6 +81,8 @@ def cte_name_map(self) -> dict[str, str]:
 
         Keys are underscore-separated placeholders (``db__DOT__name``),
         values are the original dotted names (``db.name``).
+
+        :rtype: dict[str, str]
         """
         _ = self.ast
         return self._cte_name_map

sql_metadata/column_extractor.py

@@ -28,13 +28,15 @@
 class ExtractionResult:
     """Immutable container for column extraction results.
 
-    Replaces the earlier 7-tuple return value with named fields.
+    Returned by :meth:`ColumnExtractor.extract` and consumed by
+    :class:`Parser` to populate its column/alias/CTE properties.
+    Each field corresponds to a public ``Parser`` property.
     """
 
     columns: UniqueList
     columns_dict: dict[str, UniqueList]
     alias_names: UniqueList
-    alias_dict: dict[str, UniqueList] | None
+    alias_dict: dict[str, UniqueList]
     alias_map: dict[str, str | list[str]]
     cte_names: UniqueList
     subquery_names: UniqueList
@@ -119,7 +121,17 @@ def _dfs(node: exp.Expression) -> Any:
 
 
 def _is_date_part_unit(node: exp.Column) -> bool:
-    """Return True if *node* is the first arg of a date-part function."""
+    """Return ``True`` if *node* is the date-part unit argument of a function.
+
+    Functions like ``DATEADD``, ``DATEDIFF``, and ``DATE_TRUNC`` accept a
+    date-part keyword (``DAY``, ``MONTH``, …) as their first argument.
+    sqlglot parses these keywords as ``exp.Column`` nodes, but they are not
+    real columns and must be skipped during extraction.
+
+    :param node: A column AST node to inspect.
+    :type node: exp.Column
+    :rtype: bool
+    """
     parent = node.parent
     if (
         isinstance(parent, exp.Anonymous)
@@ -167,13 +179,28 @@ def __init__(self, table_aliases: dict[str, str]):
         self.output_columns: list[str] = []
 
     def add_column(self, name: str, clause: str) -> None:
-        """Record a column name, filing it into the appropriate section."""
+        """Record a column name, filing it into the appropriate clause section.
+
+        :param name: The column name to record.
+        :type name: str
+        :param clause: The SQL clause section (e.g. ``"select"``, ``"where"``).
+        :type clause: str
+        """
         self.columns.append(name)
         if clause:
             self.columns_dict.setdefault(clause, UniqueList()).append(name)
 
     def add_alias(self, name: str, target: Any, clause: str) -> None:
-        """Record a column alias and its target expression."""
+        """Record a column alias and its target expression.
+
+        :param name: The alias name.
+        :type name: str
+        :param target: The source column name or expression the alias refers
+            to, or ``None`` if not determinable.
+        :type target: Any
+        :param clause: The SQL clause section where the alias was defined.
+        :type clause: str
+        """
         self.alias_names.append(name)
         if clause:
             self.alias_dict.setdefault(clause, UniqueList()).append(name)
@@ -267,7 +294,7 @@ def extract(self) -> ExtractionResult:
         for name in c.cte_names:
             final_cte.append(self._reverse_cte_map.get(name, name))
 
-        alias_dict = c.alias_dict if c.alias_dict else None
+        alias_dict = c.alias_dict
         return ExtractionResult(
             columns=c.columns,
             columns_dict=c.columns_dict,

sql_metadata/comments.py

@@ -21,6 +21,7 @@
 import re
 from typing import Any
 
+from sqlglot.errors import TokenError
 from sqlglot.tokens import Tokenizer
 
 
@@ -90,8 +91,7 @@ def extract_comments(sql: str) -> list[str]:
         return []
     try:
         tokens = list(_choose_tokenizer(sql).tokenize(sql))
-    # TODO: revisit if sqlglot tokenizer starts raising on specific inputs
-    except Exception:  # pragma: no cover
+    except TokenError:
         return []
     comments: list[str] = []
     prev_end = -1
@@ -119,7 +119,19 @@ def _scan_gap(sql: str, start: int, end: int, out: list[str]) -> None:
 
 
 def _reconstruct_from_tokens(sql: str, tokens: list[Any]) -> str:
-    """Rebuild SQL from token spans, collapsing gaps to single spaces."""
+    """Rebuild SQL from token spans, collapsing gaps to single spaces.
+
+    Concatenates the text of each token using its ``start`` / ``end``
+    positions.  Any gap between consecutive tokens (where comments or
+    extra whitespace lived) is replaced by a single space.
+
+    :param sql: The original SQL string.
+    :type sql: str
+    :param tokens: Sqlglot token objects with ``start`` and ``end`` attrs.
+    :type tokens: list[Any]
+    :returns: Reconstructed SQL with comments removed.
+    :rtype: str
+    """
     if not tokens:
         return ""
     parts = [sql[tokens[0].start : tokens[0].end + 1]]
@@ -158,7 +170,7 @@ def strip_comments_for_parsing(sql: str) -> str:
         tokenizer = MySQL.Tokenizer()
     try:
         tokens = list(tokenizer.tokenize(sql))
-    except Exception:
+    except TokenError:
         return sql.strip()
     return _reconstruct_from_tokens(sql, tokens)
 
@@ -183,6 +195,6 @@ def strip_comments(sql: str) -> str:
         return sql or ""
     try:
         tokens = list(_choose_tokenizer(sql).tokenize(sql))
-    except Exception:
+    except TokenError:
         return sql.strip()
     return _reconstruct_from_tokens(sql, tokens)

sql_metadata/dialect_parser.py

@@ -43,17 +43,32 @@ class HashVarDialect(Dialect):
     """
 
     class Tokenizer(BaseTokenizer):
-        """Tokenizer subclass that includes ``#`` in variable tokens."""
+        """Tokenizer subclass that includes ``#`` in variable tokens.
+
+        Removes ``#`` from ``SINGLE_TOKENS`` and adds it to
+        ``VAR_SINGLE_TOKENS`` so that ``#temp`` is lexed as a single
+        ``VAR`` token instead of ``#`` + ``temp``.
+        """
 
         SINGLE_TOKENS = {**BaseTokenizer.SINGLE_TOKENS}
         SINGLE_TOKENS.pop("#", None)
         VAR_SINGLE_TOKENS = {*BaseTokenizer.VAR_SINGLE_TOKENS, "#"}
 
 
 class _RedshiftAppendParser(RedshiftParser):
-    """Redshift parser extended with ``ALTER TABLE ... APPEND FROM``."""
+    """Redshift parser extended with ``ALTER TABLE … APPEND FROM``.
+
+    Adds an ``APPEND`` entry to ``ALTER_PARSERS`` so that the Redshift-
+    specific ``ALTER TABLE t APPEND FROM src`` syntax produces a proper
+    ``exp.Alter`` node instead of degrading to ``exp.Command``.
+    """
 
     def _parse_alter_table_append(self) -> "exp.Expr | None":
+        """Parse the ``FROM <table>`` portion of an ``APPEND FROM`` clause.
+
+        :returns: The parsed table expression, or ``None``.
+        :rtype: exp.Expr | None
+        """
         self._match_text_seq("FROM")
         return self._parse_table()
 

sql_metadata/nested_resolver.py

@@ -46,25 +46,53 @@ class _PreservingGenerator(Generator):
     }
 
     def coalesce_sql(self, expression: exp.Expression) -> str:
+        """Render ``COALESCE`` back to ``IFNULL`` for two-argument calls.
+
+        :param expression: The ``exp.Coalesce`` AST node.
+        :type expression: exp.Expression
+        :returns: SQL string using ``IFNULL`` (2 args) or ``COALESCE``.
+        :rtype: str
+        """
         args = [expression.this] + expression.expressions
         if len(args) == 2:
             return f"IFNULL({self.sql(args[0])}, {self.sql(args[1])})"
         args_sql = ", ".join(self.sql(a) for a in args)
         return f"COALESCE({args_sql})"
 
     def dateadd_sql(self, expression: exp.Expression) -> str:
+        """Render ``exp.DateAdd`` back to ``DATE_ADD(…)`` syntax.
+
+        :param expression: The ``exp.DateAdd`` AST node.
+        :type expression: exp.Expression
+        :rtype: str
+        """
         return (
             f"DATE_ADD({self.sql(expression, 'this')}, "
             f"{self.sql(expression, 'expression')})"
         )
 
     def datesub_sql(self, expression: exp.Expression) -> str:
+        """Render ``exp.DateSub`` back to ``DATE_SUB(…)`` syntax.
+
+        :param expression: The ``exp.DateSub`` AST node.
+        :type expression: exp.Expression
+        :rtype: str
+        """
         return (
             f"DATE_SUB({self.sql(expression, 'this')}, "
             f"{self.sql(expression, 'expression')})"
         )
 
     def tsordsadd_sql(self, expression: exp.Expression) -> str:
+        """Render ``exp.TsOrDsAdd`` as ``DATE_ADD`` or ``DATE_SUB``.
+
+        When the interval multiplier is ``-1`` the expression is rendered
+        as ``DATE_SUB`` instead, preserving the original SQL intent.
+
+        :param expression: The ``exp.TsOrDsAdd`` AST node.
+        :type expression: exp.Expression
+        :rtype: str
+        """
         this = self.sql(expression, "this")
         expr_node = expression.expression
         if isinstance(expr_node, exp.Mul):
@@ -79,6 +107,16 @@ def tsordsadd_sql(self, expression: exp.Expression) -> str:
         return f"DATE_ADD({this}, {self.sql(expression, 'expression')})"
 
     def not_sql(self, expression: exp.Expression) -> str:
+        """Render ``NOT`` expressions preserving ``IS NOT NULL`` and ``NOT IN``.
+
+        sqlglot normalises ``IS NOT NULL`` to ``NOT (x IS NULL)`` and
+        ``NOT IN`` to ``NOT (x IN (...))``; this override renders them
+        back to their original idiomatic forms.
+
+        :param expression: The ``exp.Not`` AST node.
+        :type expression: exp.Expression
+        :rtype: str
+        """
         child = expression.this
         if isinstance(child, exp.Is) and isinstance(child.expression, exp.Null):
             return f"{self.sql(child, 'this')} IS NOT NULL"
@@ -96,14 +134,26 @@ def not_sql(self, expression: exp.Expression) -> str:
 
 
 def _is_qualified_reference(result: list[str]) -> bool:
-    """Check if result is a single dotted reference like ``['cte.col']``."""
+    """Check if *result* is a single dotted reference like ``['cte.col']``.
+
+    :param result: Resolved column list to inspect.
+    :type result: list[str]
+    :rtype: bool
+    """
     return len(result) == 1 and "." in result[0]
 
 
 def _is_not_already_resolved_qualified_reference(
     result: list[str], column: str
 ) -> bool:
-    """Check if result is a qualified reference that changed from the input."""
+    """Check if *result* is a qualified reference that differs from *column*.
+
+    :param result: Resolved column list to inspect.
+    :type result: list[str]
+    :param column: The original column name before resolution.
+    :type column: str
+    :rtype: bool
+    """
     return _is_qualified_reference(result) and result != [column]