Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Spanner DB-API to prevent dummy WHERE clauses from being automatically added to partitioned DML statements. The logic for adding dummy WHERE clauses to UPDATE and DELETE statements is moved from the classification phase in parse_utils.py to specific cursor execution paths, and comprehensive tests are added to verify the new behavior. The review feedback suggests using the NamedTuple._replace() method in _ensure_where_clause to update the ParsedStatement and Statement objects more cleanly and safely.
Partitioned DML executed through the Cloud Spanner DB-API driver previously failed whenever query parameters were used. The driver passed raw positional placeholders and parameter lists directly to the database rather than converting them into the named parameters and data types expected by Cloud Spanner. Additionally, statement parsing historically mutated SQL statements in place to automatically append a dummy "WHERE 1=1" clause to update and delete statements. Injecting this dummy clause into Partitioned DML is dangerous because Partitioned DML is non-atomic, irreversible, and cannot be rolled back. Bypassing Cloud Spanner's requirement for an explicit WHERE clause risks unintended full-table modifications. This change addresses these issues by: - Properly translating and passing query parameters and their data types during Partitioned DML execution. - Making statement parsing pure and side-effect free so SQL statements are no longer mutated during classification. - Ensuring Partitioned DML statements never receive an automatic dummy WHERE clause, keeping Cloud Spanner's safety checks intact. - Guarding Partitioned DML mode so that statements executed inside an explicit transaction always run as atomic transactional DML. - Retaining automatic dummy WHERE clause injection exclusively for transactional and batch DML to preserve full backwards compatibility for existing applications.
47b8ba8 to
218d44b
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors how dummy WHERE clauses are added to unqualified UPDATE and DELETE statements in the Spanner DB API. Instead of automatically appending 'WHERE 1=1' during statement classification in parse_utils.py, the logic is moved to the cursor execution level via a new _ensure_where_clause helper. This ensures that partitioned DML statements do not receive a dummy WHERE clause, while transactional DML, batch DML, and executemany statements still do. Comprehensive unit and mock server tests have been added to verify these behaviors, including idempotency during transaction retries. I have no further feedback to provide.
Partitioned DML executed through the Cloud Spanner DB-API driver previously failed whenever query parameters were used. The driver passed raw positional placeholders and parameter lists directly to the database rather than converting them into the named parameters and data types expected by Cloud Spanner.
Additionally, statement parsing historically mutated SQL statements in place to automatically append a dummy "WHERE 1=1" clause to update and delete statements. Injecting this dummy clause into Partitioned DML is dangerous because Partitioned DML is non-atomic, irreversible, and cannot be rolled back. Bypassing Cloud Spanner's requirement for an explicit WHERE clause risks unintended full-table modifications.
This change addresses these issues by: