feat: refactor builder code to reduce duplication (#74)

Refactor the builder code to minimize duplication by introducing utility functions that check for specific attributes in SQL objects. This enhances code readability and maintainability.

Author: cofin

.pre-commit-config.yaml

@@ -17,7 +17,7 @@ repos:
       - id: mixed-line-ending
       - id: trailing-whitespace
   - repo: https://github.com/charliermarsh/ruff-pre-commit
-    rev: "v0.12.11"
+    rev: "v0.12.12"
     hooks:
       - id: ruff
         args: ["--fix"]

sqlspec/builder/_base.py

@@ -19,7 +19,7 @@
 from sqlspec.core.statement import SQL, StatementConfig
 from sqlspec.exceptions import SQLBuilderError
 from sqlspec.utils.logging import get_logger
-from sqlspec.utils.type_guards import has_sql_method, has_with_method
+from sqlspec.utils.type_guards import has_expression_and_parameters, has_sql_method, has_with_method
 
 if TYPE_CHECKING:
     from sqlspec.core.result import SQLResult
@@ -455,7 +455,7 @@ def _to_statement(self, config: "Optional[StatementConfig]" = None) -> "SQL":
 
         if isinstance(safe_query.parameters, dict):
             kwargs = safe_query.parameters
-            parameters: Optional[tuple] = None
+            parameters: Optional[tuple[Any, ...]] = None
         else:
             kwargs = None
             parameters = (
@@ -479,7 +479,7 @@ def _to_statement(self, config: "Optional[StatementConfig]" = None) -> "SQL":
             config.dialect is not None
             and config.dialect != safe_query.dialect
             and self._expression is not None
-            and hasattr(self._expression, "sql")
+            and has_sql_method(self._expression)
         ):
             try:
                 sql_string = self._expression.sql(dialect=config.dialect, pretty=True)
@@ -498,26 +498,34 @@ def __str__(self) -> str:
         Returns:
             str: The SQL string for this query.
         """
-        try:
-            return self.build().sql
-        except Exception:
-            return super().__str__()
+        return self.build().sql
 
     @property
     def dialect_name(self) -> "Optional[str]":
         """Returns the name of the dialect, if set."""
         if isinstance(self.dialect, str):
             return self.dialect
-        if self.dialect is not None:
-            if isinstance(self.dialect, type) and issubclass(self.dialect, Dialect):
-                return self.dialect.__name__.lower()
-            if isinstance(self.dialect, Dialect):
-                return type(self.dialect).__name__.lower()
-            try:
-                return self.dialect.__name__.lower()
-            except AttributeError:
-                pass
-        return None
+        if self.dialect is None:
+            return None
+        if isinstance(self.dialect, type) and issubclass(self.dialect, Dialect):
+            return self.dialect.__name__.lower()
+        if isinstance(self.dialect, Dialect):
+            return type(self.dialect).__name__.lower()
+        return getattr(self.dialect, "__name__", str(self.dialect)).lower()
+
+    def _merge_sql_object_parameters(self, sql_obj: Any) -> None:
+        """Merge parameters from a SQL object into the builder.
+
+        Args:
+            sql_obj: Object with parameters attribute containing parameter mappings
+        """
+        if not has_expression_and_parameters(sql_obj):
+            return
+
+        sql_parameters = getattr(sql_obj, "parameters", {})
+        for param_name, param_value in sql_parameters.items():
+            unique_name = self._generate_unique_parameter_name(param_name)
+            self.add_parameter(param_value, name=unique_name)
 
     @property
     def parameters(self) -> dict[str, Any]:

sqlspec/builder/_ddl.py

@@ -12,6 +12,7 @@
 
 from sqlspec.builder._base import QueryBuilder, SafeQuery
 from sqlspec.core.result import SQLResult
+from sqlspec.utils.type_guards import has_sqlglot_expression, has_with_method
 
 if TYPE_CHECKING:
     from sqlspec.builder._column import ColumnExpression
@@ -436,8 +437,8 @@ def check_constraint(self, condition: Union[str, "ColumnExpression"], name: "Opt
             self._raise_sql_builder_error("Check constraint must have a condition")
 
         condition_str: str
-        if hasattr(condition, "sqlglot_expression"):
-            sqlglot_expr = getattr(condition, "sqlglot_expression", None)
+        if has_sqlglot_expression(condition):
+            sqlglot_expr = condition.sqlglot_expression
             condition_str = sqlglot_expr.sql(dialect=self.dialect) if sqlglot_expr else str(condition)
         else:
             condition_str = str(condition)
@@ -970,15 +971,15 @@ def _create_base_expression(self) -> exp.Expression:
 
         if isinstance(self._select_query, SQL):
             select_expr = self._select_query.expression
-            select_parameters = getattr(self._select_query, "parameters", None)
+            select_parameters = self._select_query.parameters
         elif isinstance(self._select_query, Select):
-            select_expr = getattr(self._select_query, "_expression", None)
-            select_parameters = getattr(self._select_query, "_parameters", None)
+            select_expr = self._select_query._expression
+            select_parameters = self._select_query._parameters
 
-            with_ctes = getattr(self._select_query, "_with_ctes", {})
+            with_ctes = self._select_query._with_ctes
             if with_ctes and select_expr and isinstance(select_expr, exp.Select):
                 for alias, cte in with_ctes.items():
-                    if hasattr(select_expr, "with_"):
+                    if has_with_method(select_expr):
                         select_expr = select_expr.with_(cte.this, as_=alias, copy=False)
         elif isinstance(self._select_query, str):
             select_expr = exp.maybe_parse(self._select_query)
@@ -1097,10 +1098,10 @@ def _create_base_expression(self) -> exp.Expression:
 
         if isinstance(self._select_query, SQL):
             select_expr = self._select_query.expression
-            select_parameters = getattr(self._select_query, "parameters", None)
+            select_parameters = self._select_query.parameters
         elif isinstance(self._select_query, Select):
-            select_expr = getattr(self._select_query, "_expression", None)
-            select_parameters = getattr(self._select_query, "_parameters", None)
+            select_expr = self._select_query._expression
+            select_parameters = self._select_query._parameters
         elif isinstance(self._select_query, str):
             select_expr = exp.maybe_parse(self._select_query)
             select_parameters = None
@@ -1195,10 +1196,10 @@ def _create_base_expression(self) -> exp.Expression:
 
         if isinstance(self._select_query, SQL):
             select_expr = self._select_query.expression
-            select_parameters = getattr(self._select_query, "parameters", None)
+            select_parameters = self._select_query.parameters
         elif isinstance(self._select_query, Select):
-            select_expr = getattr(self._select_query, "_expression", None)
-            select_parameters = getattr(self._select_query, "_parameters", None)
+            select_expr = self._select_query._expression
+            select_parameters = self._select_query._parameters
         elif isinstance(self._select_query, str):
             select_expr = exp.maybe_parse(self._select_query)
             select_parameters = None

sqlspec/builder/_insert.py

@@ -13,6 +13,7 @@
 from sqlspec.builder.mixins import InsertFromSelectMixin, InsertIntoClauseMixin, InsertValuesMixin, ReturningClauseMixin
 from sqlspec.core.result import SQLResult
 from sqlspec.exceptions import SQLBuilderError
+from sqlspec.utils.type_guards import has_expression_and_sql
 
 if TYPE_CHECKING:
     from collections.abc import Mapping, Sequence
@@ -124,12 +125,9 @@ def values(self, *values: Any, **kwargs: Any) -> "Self":
             return self.values_from_dict(kwargs)
 
         if len(values) == 1:
-            try:
-                values_0 = values[0]
-                if hasattr(values_0, "items"):
-                    return self.values_from_dict(values_0)
-            except (AttributeError, TypeError):
-                pass
+            values_0 = values[0]
+            if hasattr(values_0, "items") and hasattr(values_0, "keys"):
+                return self.values_from_dict(values_0)
 
         insert_expr = self._get_insert_expression()
 
@@ -141,24 +139,18 @@ def values(self, *values: Any, **kwargs: Any) -> "Self":
         for i, value in enumerate(values):
             if isinstance(value, exp.Expression):
                 value_placeholders.append(value)
-            elif hasattr(value, "expression") and hasattr(value, "sql"):
+            elif has_expression_and_sql(value):
                 # Handle SQL objects (from sql.raw with parameters)
                 expression = getattr(value, "expression", None)
                 if expression is not None and isinstance(expression, exp.Expression):
                     # Merge parameters from SQL object into builder
-                    if hasattr(value, "parameters"):
-                        sql_parameters = getattr(value, "parameters", {})
-                        for param_name, param_value in sql_parameters.items():
-                            self.add_parameter(param_value, name=param_name)
+                    self._merge_sql_object_parameters(value)
                     value_placeholders.append(expression)
                 else:
                     # If expression is None, fall back to parsing the raw SQL
                     sql_text = getattr(value, "sql", "")
                     # Merge parameters even when parsing raw SQL
-                    if hasattr(value, "parameters"):
-                        sql_parameters = getattr(value, "parameters", {})
-                        for param_name, param_value in sql_parameters.items():
-                            self.add_parameter(param_value, name=param_name)
+                    self._merge_sql_object_parameters(value)
                     # Check if sql_text is callable (like Expression.sql method)
                     if callable(sql_text):
                         sql_text = str(value)
@@ -376,7 +368,7 @@ def do_update(self, **kwargs: Any) -> "Insert":
         # Create SET expressions for the UPDATE
         set_expressions = []
         for col, val in kwargs.items():
-            if hasattr(val, "expression") and hasattr(val, "sql"):
+            if has_expression_and_sql(val):
                 # Handle SQL objects (from sql.raw with parameters)
                 expression = getattr(val, "expression", None)
                 if expression is not None and isinstance(expression, exp.Expression):

sqlspec/builder/_parsing_utils.py

@@ -10,7 +10,12 @@
 from sqlglot import exp, maybe_parse, parse_one
 
 from sqlspec.core.parameters import ParameterStyle
-from sqlspec.utils.type_guards import has_expression_attr, has_parameter_builder
+from sqlspec.utils.type_guards import (
+    has_expression_and_parameters,
+    has_expression_and_sql,
+    has_expression_attr,
+    has_parameter_builder,
+)
 
 
 def parse_column_expression(
@@ -38,32 +43,29 @@ def parse_column_expression(
         return column_input
 
     # Handle SQL objects (from sql.raw with parameters)
-    if hasattr(column_input, "expression") and hasattr(column_input, "sql"):
+    if has_expression_and_sql(column_input):
         # This is likely a SQL object
         expression = getattr(column_input, "expression", None)
         if expression is not None and isinstance(expression, exp.Expression):
             # Merge parameters from SQL object into builder if available
-            if builder and hasattr(column_input, "parameters") and hasattr(builder, "add_parameter"):
+            if builder and has_expression_and_parameters(column_input) and hasattr(builder, "add_parameter"):
                 sql_parameters = getattr(column_input, "parameters", {})
                 for param_name, param_value in sql_parameters.items():
                     builder.add_parameter(param_value, name=param_name)
             return cast("exp.Expression", expression)
         # If expression is None, fall back to parsing the raw SQL
         sql_text = getattr(column_input, "sql", "")
         # Merge parameters even when parsing raw SQL
-        if builder and hasattr(column_input, "parameters") and hasattr(builder, "add_parameter"):
+        if builder and has_expression_and_parameters(column_input) and hasattr(builder, "add_parameter"):
             sql_parameters = getattr(column_input, "parameters", {})
             for param_name, param_value in sql_parameters.items():
                 builder.add_parameter(param_value, name=param_name)
         return exp.maybe_parse(sql_text) or exp.column(str(sql_text))
 
     if has_expression_attr(column_input):
-        try:
-            attr_value = column_input._expression
-            if isinstance(attr_value, exp.Expression):
-                return attr_value
-        except AttributeError:
-            pass
+        attr_value = getattr(column_input, "_expression", None)
+        if isinstance(attr_value, exp.Expression):
+            return attr_value
 
     return exp.maybe_parse(column_input) or exp.column(str(column_input))
 
@@ -178,14 +180,10 @@ def parse_condition_expression(
                 )
         condition_input = converted_condition
 
-    try:
-        return exp.condition(condition_input)
-    except Exception:
-        try:
-            parsed = exp.maybe_parse(condition_input)  # type: ignore[var-annotated]
-            return parsed or exp.condition(condition_input)
-        except Exception:
-            return exp.condition(condition_input)
+    parsed: Optional[exp.Expression] = exp.maybe_parse(condition_input)
+    if parsed:
+        return parsed
+    return exp.condition(condition_input)
 
 
 __all__ = ("parse_column_expression", "parse_condition_expression", "parse_order_expression", "parse_table_expression")

sqlspec/builder/_update.py

@@ -176,7 +176,7 @@ def build(self) -> "SafeQuery":
             msg = "No UPDATE expression to build or expression is of the wrong type."
             raise SQLBuilderError(msg)
 
-        if getattr(self._expression, "this", None) is None:
+        if self._expression.this is None:
             msg = "No table specified for UPDATE statement."
             raise SQLBuilderError(msg)