feat: correctly merge parameter names & adds or builder support (#73)

Implement unique naming for parameters in CTEs to prevent collisions. Introduce support for an `or` builder to improve query flexibility.

Author: cofin

sqlspec/builder/_base.py

@@ -208,6 +208,43 @@ def _generate_unique_parameter_name(self, base_name: str) -> str:
 
         return f"{base_name}_{uuid.uuid4().hex[:8]}"
 
+    def _merge_cte_parameters(self, cte_name: str, parameters: dict[str, Any]) -> dict[str, str]:
+        """Merge CTE parameters with unique naming to prevent collisions.
+
+        Args:
+            cte_name: The name of the CTE for parameter prefixing
+            parameters: The CTE's parameter dictionary
+
+        Returns:
+            Mapping of old parameter names to new unique names
+        """
+        param_mapping = {}
+        for old_name, value in parameters.items():
+            new_name = self._generate_unique_parameter_name(f"{cte_name}_{old_name}")
+            param_mapping[old_name] = new_name
+            self.add_parameter(value, name=new_name)
+        return param_mapping
+
+    def _update_placeholders_in_expression(
+        self, expression: exp.Expression, param_mapping: dict[str, str]
+    ) -> exp.Expression:
+        """Update parameter placeholders in expression to use new names.
+
+        Args:
+            expression: The SQLGlot expression to update
+            param_mapping: Mapping of old parameter names to new names
+
+        Returns:
+            Updated expression with new placeholder names
+        """
+
+        def placeholder_replacer(node: exp.Expression) -> exp.Expression:
+            if isinstance(node, exp.Placeholder) and str(node.this) in param_mapping:
+                return exp.Placeholder(this=param_mapping[str(node.this)])
+            return node
+
+        return expression.transform(placeholder_replacer, copy=False)
+
     def _generate_builder_cache_key(self, config: "Optional[StatementConfig]" = None) -> str:
         """Generate cache key based on builder state and configuration.
 
@@ -276,9 +313,12 @@ def with_cte(self: Self, alias: str, query: "Union[QueryBuilder, exp.Select, str
                 msg = f"CTE query builder expression must be a Select, got {type(query._expression).__name__}."
                 self._raise_sql_builder_error(msg)
             cte_select_expression = query._expression
-            for p_name, p_value in query.parameters.items():
-                unique_name = self._generate_unique_parameter_name(p_name)
-                self.add_parameter(p_value, unique_name)
+            param_mapping = self._merge_cte_parameters(alias, query.parameters)
+            updated_expression = self._update_placeholders_in_expression(cte_select_expression, param_mapping)
+            if not isinstance(updated_expression, exp.Select):
+                msg = f"Updated CTE expression must be a Select, got {type(updated_expression).__name__}."
+                self._raise_sql_builder_error(msg)
+            cte_select_expression = updated_expression
 
         elif isinstance(query, str):
             try:
@@ -297,7 +337,6 @@ def with_cte(self: Self, alias: str, query: "Union[QueryBuilder, exp.Select, str
         else:
             msg = f"Invalid query type for CTE: {type(query).__name__}"
             self._raise_sql_builder_error(msg)
-            return self
 
         self._with_ctes[alias] = exp.CTE(this=cte_select_expression, alias=exp.to_table(alias))
         return self

sqlspec/builder/mixins/_cte_and_set_ops.py

@@ -31,6 +31,18 @@ def add_parameter(self, value: Any, name: Optional[str] = None) -> tuple[Any, st
         msg = "Method must be provided by QueryBuilder subclass"
         raise NotImplementedError(msg)
 
+    def _generate_unique_parameter_name(self, base_name: str) -> str:
+        """Generate unique parameter name - provided by QueryBuilder."""
+        msg = "Method must be provided by QueryBuilder subclass"
+        raise NotImplementedError(msg)
+
+    def _update_placeholders_in_expression(
+        self, expression: exp.Expression, param_mapping: dict[str, str]
+    ) -> exp.Expression:
+        """Update parameter placeholders - provided by QueryBuilder."""
+        msg = "Method must be provided by QueryBuilder subclass"
+        raise NotImplementedError(msg)
+
     def with_(
         self, name: str, query: Union[Any, str], recursive: bool = False, columns: Optional[list[str]] = None
     ) -> Self:
@@ -69,8 +81,15 @@ def with_(
             parameters = built_query.parameters
             if parameters:
                 if isinstance(parameters, dict):
+                    param_mapping = {}
                     for param_name, param_value in parameters.items():
-                        self.add_parameter(param_value, name=param_name)
+                        unique_name = self._generate_unique_parameter_name(f"{name}_{param_name}")
+                        param_mapping[param_name] = unique_name
+                        self.add_parameter(param_value, name=unique_name)
+
+                    # Update placeholders in the parsed expression
+                    if cte_expr and param_mapping:
+                        cte_expr = self._update_placeholders_in_expression(cte_expr, param_mapping)
                 elif isinstance(parameters, (list, tuple)):
                     for param_value in parameters:
                         self.add_parameter(param_value)