feat: support case-insensitive query parameter matching#823
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces case-insensitive matching for named parameters in Spanner statements, along with corresponding unit tests for both GoogleSQL and PostgreSQL dialects. The reviewer recommended checking for an exact match first before performing case-insensitive matching. This ensures determinism (since map iteration in Go is non-deterministic and could lead to incorrect matches if multiple casing variations exist) and improves performance by avoiding unnecessary loops and case-insensitive comparisons in the common case.
|
Great feature to support case-insensitive query parameter matching. However, I have one critical correctness recommendation: To fix this and avoid non-deterministic behavior, we should check for an exact match first before falling back to the case-insensitive search loop. Checking for an exact match first will also optimize the hot path for queries where the casing matches exactly, avoiding the (N \times M)$ comparison loop entirely. |
Normalize bound parameter names case-insensitively against the parameter names parsed from the SQL statement. This allows using any case in database/sql named arguments (e.g. sql.Named('id', ...)) to match case-insensitive query parameters in the statement (e.g. @id).
Fixes #594