googleapis / googleapis/google-cloud-python
visit_like_op_binary raises NotImplementedError for ESCAPE instead of omitting the clause
- Ngôn ngữ chính
- Python
- Star
- 5.4k
- Fork
- 1.8k
- Merge trung bình
- 3 ngày 4 giờ
- Pull request đã merge (30 ngày)
- 122
Mô tả
## Summary
`visit_like_op_binary` in `sqlalchemy_spanner.py` raises `NotImplementedError("ESCAPE keyword is not supported by Spanner")` when `autoescape=True` is passed to SQLAlchemy's `.endswith()`, `.startswith()`, or `.contains()` methods. This prevents portable use of these operators for literal pattern matching.
Spanner **does** support backslash escaping in LIKE patterns natively — it just doesn't support the explicit `ESCAPE` SQL clause. From the [Spanner LIKE operator docs](https://cloud.google.com/spanner/docs/reference/standard-sql/operators#like_operator):
> You can escape `\`, `_`, or `%` using two backslashes.
However, `autoescape=True` uses `/` (forward slash) as its default escape character, not `\`. This means simply omitting the `ESCAPE` clause is not sufficient — the pattern would contain `/`-escaped values (e.g., `foo/%bar`) but Spanner would interpret `\` as the escape character, producing wrong results.
## Current behavior
```python
Model.column.endswith("_operators", autoescape=True)
# raises: NotImplementedError: ESCAPE keyword is not supported by Spanner
```
The problematic code ([sqlalchemy_spanner.py L306-315](https://github.com/googleapis/python-spanner-sqlalchemy/blob/main/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py)):
```python
def visit_like_op_binary(self, binary, operator, **kw):
"""Build a LIKE clause."""
if binary.modifiers.get("escape", None):
raise NotImplementedError("ESCAPE keyword is not supported by Spanner")
return "%s LIKE %s" % (
binary.left._compiler_dispatch(self, **kw),
binary.right._compiler_dispatch(self, **kw),
)
```
## Root cause
Spanner hardcodes `\` as the only LIKE escape character with no `ESCAPE` clause to change it. SQLAlchemy's `autoescape=True` defaults to `/` as the escape character. These two assumptions are incompatible:
| Scenario | Escape char | Pattern value | ESCAPE clause | Works on Spanner? |
|---|---|---|---|---|
| `autoescape=True` (default) | `/` | `foo/%bar` | `ESCAPE '/'` | No — Spanner has no ESCAPE clause, and `\` is the only escape char |
| `autoescape=True, escape="\\"` | `\` | `foo\%bar` | `ESCAPE '\'` | Would work if ESCAPE clause is omitted (matches Spanner's default) |
| `escape="#"` | `#` | user-provided | `ESCAPE '#'` | No — Spanner cannot use `#` as escape |
## Proposed fix
**Override the default escape character to `\` for the Spanner dialect**, so that `autoescape=True` produces `\`-escaped patterns that work with Spanner's built-in escape behavior. Then omit the `ESCAPE` clause from the generated SQL.
```python
def visit_like_op_binary(self, binary, operator, **kw):
"""Build a LIKE clause."""
escape = binary.modifiers.get("escape", None)
if escape and escape != "\\":
raise NotImplementedError(
f"ESCAPE '{escape}' is not supported by Spanner — "
f"only backslash is supported. Use escape='\\' explicitly."
)
# Omit ESCAPE clause — Spanner uses \ as default escape
return "%s LIKE %s" % (
binary.left._compiler_dispatch(self, **kw),
binary.right._compiler_dispatch(self, **kw),
)
```
However, this alone only fixes the case where users pass `escape="\\"` explicitly. For `autoescape=True` (which defaults to `/`), the dialect would additionally need to either:
1. **Override the autoescape default** at the dialect level so that `autoescape=True` uses `\` instead of `/` for Spanner. This would mean the pattern value is `\`-escaped from the start, and omitting the ESCAPE clause is correct.
2. **Re-translate the pattern at compile time** — detect the non-backslash escape char, substitute it with `\` in the bind parameter value, and omit the ESCAPE clause. This is more complex but doesn't require changes to how `autoescape` works.
Option 1 (overriding the default escape char) is the cleaner approach. The relevant logic is in SQLAlchemy's `_literal_as_text` / `_apply_autoescape` path — the Spanner dialect could override whatever determines the default escape character to return `\` instead of `/`.
## Workarounds (available now)
### 1. Manual LIKE with `\_` escaping (simplest, portable across Spanner and Postgres)
```python
column.like("%" + suffix.replace("_", r"\_"))
```
### 2. Explicit `escape="\\"` with `autoescape=True` (requires dialect fix above)
```python
column.endswith("_operators", autoescape=True, escape="\\")
```
This would work once the dialect accepts `escape="\\"` without raising, but does **not** work today.
### 3. Local `@compiles` override
```python
from sqlalchemy.ext.compiler import compiles
from sqlalchemy.sql import operators
@compiles(operators.like_op, "spanner+spanner")
def _spanner_like_op(binary, compiler, **kw):
escape = binary.modifiers.get("escape", None)
if escape and escape != "\\":
raise NotImplementedError(
f"ESCAPE '{escape}' is not supported by Spanner (only backslash is supported)"
)
return "%s LIKE %s" % (
compiler.process(binary.left, **kw),
compiler.process(binary.right, **kw),
)
```
Note: this only helps if you also pass `escape="\\"` explicitly to your `endswith`/`startswith`/`contains` calls.
## Impact
Without this fix, users who need to match literal `_` or `%` characters in LIKE patterns on Spanner must use raw `.like()` with manual `\` escaping, which is less portable and less discoverable than the standard `autoescape=True` API.
## Environment
- `sqlalchemy`: 2.0.x
- `sqlalchemy-spanner`: latest (tested against Spanner emulator)
Hướng dẫn đóng góp
Đánh giá
Issue này chưa được đánh giá.