fix: null parameter handling (#64)

Enhance handling of null parameters in SQL expressions to prevent binding errors and ensure parameter count validation.

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.9"
+    rev: "v0.12.10"
     hooks:
       - id: ruff
         args: ["--fix"]

sqlspec/adapters/adbc/driver.py

@@ -1,15 +1,15 @@
 """ADBC driver implementation for Arrow Database Connectivity.
 
-Provides ADBC driver integration with multi-dialect database connections,
-Arrow-native data handling with type coercion, parameter style conversion
-for different database backends, and transaction management.
+Provides database connectivity through ADBC with support for multiple
+database dialects, parameter style conversion, and transaction management.
 """
 
 import contextlib
 import datetime
 import decimal
 from typing import TYPE_CHECKING, Any, Optional, cast
 
+from adbc_driver_manager.dbapi import DatabaseError, IntegrityError, OperationalError, ProgrammingError
 from sqlglot import exp
 
 from sqlspec.core.cache import get_cache_config
@@ -53,22 +53,88 @@
 }
 
 
-def _adbc_ast_transformer(expression: Any, parameters: Any) -> tuple[Any, Any]:
-    """AST transformer for NULL parameter handling.
+def _count_placeholders(expression: Any) -> int:
+    """Count the number of unique parameter placeholders in a SQLGlot expression.
 
-    For PostgreSQL, replaces NULL parameter placeholders with NULL literals
-    in the AST to prevent Arrow from inferring 'na' types which cause binding errors.
+    For PostgreSQL ($1, $2) style: counts highest numbered parameter (e.g., $1, $1, $2 = 2)
+    For QMARK (?) style: counts total occurrences (each ? is a separate parameter)
+    For named (:name) style: counts unique parameter names
 
     Args:
         expression: SQLGlot AST expression
-        parameters: Parameter values that may contain None
 
     Returns:
-        Tuple of (modified_expression, cleaned_parameters)
+        Number of unique parameter placeholders expected
     """
-    if not parameters:
-        return expression, parameters
+    numeric_params = set()  # For $1, $2 style
+    qmark_count = 0  # For ? style
+    named_params = set()  # For :name style
+
+    def count_node(node: Any) -> Any:
+        nonlocal qmark_count
+        if isinstance(node, exp.Parameter):
+            # PostgreSQL style: $1, $2, etc.
+            param_str = str(node)
+            if param_str.startswith("$") and param_str[1:].isdigit():
+                numeric_params.add(int(param_str[1:]))
+            elif ":" in param_str:
+                # Named parameter: :name
+                named_params.add(param_str)
+            else:
+                # Other parameter formats
+                named_params.add(param_str)
+        elif isinstance(node, exp.Placeholder):
+            # QMARK style: ?
+            qmark_count += 1
+        return node
+
+    expression.transform(count_node)
+
+    # Return the appropriate count based on parameter style detected
+    if numeric_params:
+        # PostgreSQL style: return highest numbered parameter
+        return max(numeric_params)
+    if named_params:
+        # Named parameters: return count of unique names
+        return len(named_params)
+    # QMARK style: return total count
+    return qmark_count
+
+
+def _is_execute_many_parameters(parameters: Any) -> bool:
+    """Check if parameters are in execute_many format (list/tuple of lists/tuples)."""
+    return isinstance(parameters, (list, tuple)) and len(parameters) > 0 and isinstance(parameters[0], (list, tuple))
+
 
+def _validate_parameter_counts(expression: Any, parameters: Any, dialect: str) -> None:
+    """Validate parameter count against placeholder count in SQL."""
+    placeholder_count = _count_placeholders(expression)
+    is_execute_many = _is_execute_many_parameters(parameters)
+
+    if is_execute_many:
+        # For execute_many, validate each inner parameter set
+        for i, param_set in enumerate(parameters):
+            param_count = len(param_set) if isinstance(param_set, (list, tuple)) else 0
+            if param_count != placeholder_count:
+                msg = f"Parameter count mismatch in set {i}: {param_count} parameters provided but {placeholder_count} placeholders in SQL (dialect: {dialect})"
+                raise SQLSpecError(msg)
+    else:
+        # For single execution, validate the parameter set directly
+        param_count = (
+            len(parameters)
+            if isinstance(parameters, (list, tuple))
+            else len(parameters)
+            if isinstance(parameters, dict)
+            else 0
+        )
+
+        if param_count != placeholder_count:
+            msg = f"Parameter count mismatch: {param_count} parameters provided but {placeholder_count} placeholders in SQL (dialect: {dialect})"
+            raise SQLSpecError(msg)
+
+
+def _find_null_positions(parameters: Any) -> set[int]:
+    """Find positions of None values in parameters for single execution."""
     null_positions = set()
     if isinstance(parameters, (list, tuple)):
         for i, param in enumerate(parameters):
@@ -83,7 +149,37 @@ def _adbc_ast_transformer(expression: Any, parameters: Any) -> tuple[Any, Any]:
                         null_positions.add(param_num - 1)
                 except ValueError:
                     pass
+    return null_positions
+
+
+def _adbc_ast_transformer(expression: Any, parameters: Any, dialect: str = "postgres") -> tuple[Any, Any]:
+    """Transform AST to handle NULL parameters.
 
+    Replaces NULL parameter placeholders with NULL literals in the AST
+    to prevent Arrow from inferring 'na' types which cause binding errors.
+    Validates parameter count before transformation.
+
+    Args:
+        expression: SQLGlot AST expression parsed with proper dialect
+        parameters: Parameter values that may contain None
+        dialect: SQLGlot dialect used for parsing (default: "postgres")
+
+    Returns:
+        Tuple of (modified_expression, cleaned_parameters)
+    """
+    if not parameters:
+        return expression, parameters
+
+    # Validate parameter count before transformation
+    _validate_parameter_counts(expression, parameters, dialect)
+
+    # For execute_many operations, skip AST transformation as different parameter
+    # sets may have None values in different positions, making transformation complex
+    if _is_execute_many_parameters(parameters):
+        return expression, parameters
+
+    # Find positions of None values for single execution
+    null_positions = _find_null_positions(parameters)
     if not null_positions:
         return expression, parameters
 
@@ -183,14 +279,28 @@ def get_adbc_statement_config(detected_dialect: str) -> StatementConfig:
 
 
 def _convert_array_for_postgres_adbc(value: Any) -> Any:
-    """Convert array values for PostgreSQL compatibility."""
+    """Convert array values for PostgreSQL compatibility.
+
+    Args:
+        value: Value to convert
+
+    Returns:
+        Converted value (tuples become lists)
+    """
     if isinstance(value, tuple):
         return list(value)
     return value
 
 
 def get_type_coercion_map(dialect: str) -> "dict[type, Any]":
-    """Get type coercion map for Arrow type handling."""
+    """Get type coercion map for Arrow type handling.
+
+    Args:
+        dialect: Database dialect name
+
+    Returns:
+        Mapping of Python types to conversion functions
+    """
     type_map = {
         datetime.datetime: lambda x: x,
         datetime.date: lambda x: x,
@@ -245,8 +355,6 @@ def __exit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None:
             return
 
         try:
-            from adbc_driver_manager.dbapi import DatabaseError, IntegrityError, OperationalError, ProgrammingError
-
             if issubclass(exc_type, IntegrityError):
                 e = exc_val
                 msg = f"Integrity constraint violation: {e}"
@@ -282,9 +390,8 @@ def __exit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None:
 class AdbcDriver(SyncDriverAdapterBase):
     """ADBC driver for Arrow Database Connectivity.
 
-    Provides database connectivity through ADBC with multi-database dialect
-    support, Arrow-native data handling with type coercion, parameter style
-    conversion for different backends, and transaction management.
+    Provides database connectivity through ADBC with support for multiple
+    database dialects, parameter style conversion, and transaction management.
     """
 
     __slots__ = ("_detected_dialect", "dialect")
@@ -309,15 +416,26 @@ def __init__(
 
     @staticmethod
     def _ensure_pyarrow_installed() -> None:
-        """Ensure PyArrow is installed."""
+        """Ensure PyArrow is installed.
+
+        Raises:
+            MissingDependencyError: If PyArrow is not installed
+        """
         from sqlspec.typing import PYARROW_INSTALLED
 
         if not PYARROW_INSTALLED:
             raise MissingDependencyError(package="pyarrow", install_package="arrow")
 
     @staticmethod
     def _get_dialect(connection: "AdbcConnection") -> str:
-        """Detect database dialect from connection information."""
+        """Detect database dialect from connection information.
+
+        Args:
+            connection: ADBC connection
+
+        Returns:
+            Detected dialect name (defaults to 'postgres')
+        """
         try:
             driver_info = connection.adbc_get_info()
             vendor_name = driver_info.get("vendor_name", "").lower()
@@ -334,31 +452,53 @@ def _get_dialect(connection: "AdbcConnection") -> str:
         return "postgres"
 
     def _handle_postgres_rollback(self, cursor: "Cursor") -> None:
-        """Execute rollback for PostgreSQL after transaction failure."""
+        """Execute rollback for PostgreSQL after transaction failure.
+
+        Args:
+            cursor: Database cursor
+        """
         if self.dialect == "postgres":
             with contextlib.suppress(Exception):
                 cursor.execute("ROLLBACK")
                 logger.debug("PostgreSQL rollback executed after transaction failure")
 
     def _handle_postgres_empty_parameters(self, parameters: Any) -> Any:
-        """Process empty parameters for PostgreSQL compatibility."""
+        """Process empty parameters for PostgreSQL compatibility.
+
+        Args:
+            parameters: Parameter values
+
+        Returns:
+            Processed parameters
+        """
         if self.dialect == "postgres" and isinstance(parameters, dict) and not parameters:
             return None
         return parameters
 
     def with_cursor(self, connection: "AdbcConnection") -> "AdbcCursor":
-        """Create context manager for cursor."""
+        """Create context manager for cursor.
+
+        Args:
+            connection: Database connection
+
+        Returns:
+            Cursor context manager
+        """
         return AdbcCursor(connection)
 
     def handle_database_exceptions(self) -> "AbstractContextManager[None]":
-        """Handle database-specific exceptions and wrap them appropriately."""
+        """Handle database-specific exceptions and wrap them appropriately.
+
+        Returns:
+            Exception handler context manager
+        """
         return AdbcExceptionHandler()
 
     def _try_special_handling(self, cursor: "Cursor", statement: SQL) -> "Optional[SQLResult]":
         """Handle special operations.
 
         Args:
-            cursor: Cursor object
+            cursor: Database cursor
             statement: SQL statement to analyze
 
         Returns:
@@ -368,7 +508,15 @@ def _try_special_handling(self, cursor: "Cursor", statement: SQL) -> "Optional[S
         return None
 
     def _execute_many(self, cursor: "Cursor", statement: SQL) -> "ExecutionResult":
-        """Execute SQL with multiple parameter sets."""
+        """Execute SQL with multiple parameter sets.
+
+        Args:
+            cursor: Database cursor
+            statement: SQL statement to execute
+
+        Returns:
+            Execution result with row counts
+        """
         sql, prepared_parameters = self._get_compiled_sql(statement, self.statement_config)
 
         try:
@@ -398,7 +546,15 @@ def _execute_many(self, cursor: "Cursor", statement: SQL) -> "ExecutionResult":
         return self.create_execution_result(cursor, rowcount_override=row_count, is_many_result=True)
 
     def _execute_statement(self, cursor: "Cursor", statement: SQL) -> "ExecutionResult":
-        """Execute single SQL statement."""
+        """Execute single SQL statement.
+
+        Args:
+            cursor: Database cursor
+            statement: SQL statement to execute
+
+        Returns:
+            Execution result with data or row count
+        """
         sql, prepared_parameters = self._get_compiled_sql(statement, self.statement_config)
 
         try:
@@ -430,7 +586,15 @@ def _execute_statement(self, cursor: "Cursor", statement: SQL) -> "ExecutionResu
         return self.create_execution_result(cursor, rowcount_override=row_count)
 
     def _execute_script(self, cursor: "Cursor", statement: "SQL") -> "ExecutionResult":
-        """Execute SQL script."""
+        """Execute SQL script containing multiple statements.
+
+        Args:
+            cursor: Database cursor
+            statement: SQL script to execute
+
+        Returns:
+            Execution result with statement counts
+        """
         if statement.is_script:
             sql = statement._raw_sql
             prepared_parameters: list[Any] = []