googleapis / googleapis/google-cloud-python

visit_like_op_binary raises NotImplementedError for ESCAPE instead of omitting the clause

Abierto
#15,673 1 comentario 0 reacciones 1 asignado Reclamado por @olavloite Ver en GitHub
api: spanner
Lenguaje dominante
Python
Estrellas
5.4k
Forks
1.8k
Merge medio
3 d 4 h
PR fusionados (30 d)
122

Descripción

## 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)

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.