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 updates the escape_name function in parse_utils.py to unconditionally quote identifiers with backticks, while adding strict type checking and validation against ASCII control characters and invalid qualified identifiers. It also adds new reserved keywords (GRAPH_TABLE and QUALIFY) to maintain backwards compatibility. The review feedback suggests a performance-conscious improvement to use re.search instead of a generator expression for detecting control characters, which should be benchmarked before merging.
Previously, the DB-API identifier escaping helper attempted selective quoting based on a hardcoded list of reserved SQL keywords and simple alphanumeric rules. This required continuous maintenance as new Spanner keywords were introduced, left unlisted keywords unquoted, and could misparse multipart names that contained dots inside already-quoted parts. This change: - Unconditionally quotes identifiers using GoogleSQL backticks and properly escapes embedded backticks and backslashes, eliminating the need to maintain keyword lists for quoting. - Properly handles multipart qualified identifiers (such as schema.table) without corrupting dots that appear inside already-quoted segments. - Rejects ASCII control characters (such as newlines and null bytes) to prevent SQL injection. - Validates that identifiers are strings and non-empty. - Retains and updates the reserved keywords list for backwards compatibility with downstream libraries that import it.
9431cc5 to
4caac46
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the Spanner DB-API driver's identifier escaping logic in parse_utils.py. The escape_name function has been refactored to unconditionally quote identifiers with backticks (unless already quoted) and escape embedded delimiters, rather than only quoting reserved keywords or invalid identifiers. Additionally, the function now includes type annotations and strict validation, raising a TypeError for non-string inputs and a ValueError for identifiers containing ASCII control characters or invalid qualified formats. The SPANNER_RESERVED_KEYWORDS set has been updated with GRAPH_TABLE and QUALIFY for backwards compatibility with downstream packages. Corresponding unit tests have been updated and expanded to cover these changes, including new tests for invalid inputs and keyword compatibility. There are no review comments, and I have no feedback to provide.
Previously, the DB-API identifier escaping helper attempted selective quoting based on a hardcoded list of reserved SQL keywords and simple alphanumeric rules. This required continuous maintenance as new Spanner keywords were introduced, left unlisted keywords unquoted, and could misparse multipart names that contained dots inside already-quoted parts.
This change: