Skip to content

fix(sqlite): keep the COALESCE wrapping when CONCAT is generated as || [CLAUDE] - #8283

Open
ebarkhordar wants to merge 1 commit into
tobymao:mainfrom
ebarkhordar:fix/sqlite-concat-coalesce-dpipe
Open

fix(sqlite): keep the COALESCE wrapping when CONCAT is generated as || [CLAUDE]#8283
ebarkhordar wants to merge 1 commit into
tobymao:mainfrom
ebarkhordar:fix/sqlite-concat-coalesce-dpipe

Conversation

@ebarkhordar

Copy link
Copy Markdown

concat_to_dpipe_sql reduces straight over expression.expressions, so it is the one
generation path for exp.Concat that never calls Generator.convert_concat_args. That helper
is what carries the source dialect's NULL semantics across: when the parsed CONCAT has
coalesce=True and the target's own CONCAT does not coalesce, each operand is wrapped in
COALESCE(x, '').

SQLite maps exp.Concat to that helper, so the wrapping is dropped and a CONCAT that skipped
NULLs upstream becomes a || chain that returns NULL.

import duckdb, sqlite3, sqlglot

duck, lite = duckdb.connect(), sqlite3.connect(":memory:")
duck.execute("CREATE TABLE t (x VARCHAR, y VARCHAR)")
duck.execute("INSERT INTO t VALUES ('a', NULL)")
lite.execute("CREATE TABLE t (x TEXT, y TEXT)")
lite.execute("INSERT INTO t VALUES ('a', NULL)")

src = "SELECT CONCAT(x, y) FROM t"
out = sqlglot.transpile(src, read="duckdb", write="sqlite")[0]
print(out)                            # SELECT x || y FROM t
print(duck.execute(src).fetchall())   # [('a',)]
print(lite.execute(out).fetchall())   # [(None,)]

Run on 45158c3 with duckdb 1.5.5, sqlite 3.46.1 and CPython 3.12.14, against pure Python
sqlglot with no mypyc build. Postgres and T-SQL sources behave the same way, as does
SELECT CONCAT('a', NULL, 'b'), which is ab in DuckDB and NULL once transpiled.

CONCAT_WS is already right on SQLite, because exp.ConcatWs is not routed to a dpipe helper
there: it comes out as CONCAT_WS('-', 'a', COALESCE(NULL, ''), 'b') and both engines return
a-b. That is the same wrapping this change gives CONCAT.

Redshift is the only other dialect mapping exp.Concat to concat_to_dpipe_sql, and it sets
CONCAT_COALESCE = True, so convert_concat_args hands the operands back untouched and its
output does not move. Nor does concat_sql's own fallback into the helper, which is only
reached when the dialect coalesces and the expression does not.

For the blast radius I generated every registered dialect's output for the 7265
(source dialect, target dialect, statement) rows in this repo's fixtures and dialect tests that
mention CONCAT or ||, before and after the change. 11 rows move and all 11 target SQLite.
The doubled COALESCE(COALESCE(a, ''), '') and the wrapping of numeric args visible in a few of
them are what convert_concat_args already produces for MySQL, Oracle, BigQuery, Snowflake,
ClickHouse and Teradata today, so this does not introduce them, it stops SQLite being the
exception.

The new SQLite cases pin both directions: coalescing sources (duckdb, postgres, tsql) get the
wrapping, and a NULL propagating source (mysql, snowflake) still gets a bare ||. The first
fails on main with 'SELECT a || b FROM t' != "SELECT COALESCE(a, '') || COALESCE(b, '') FROM t".

SKIP_INTEGRATION=1 python -m unittest is 1349 tests, OK (skipped=45), and ruff check,
ruff format --check and mypy sqlglot tests are clean.

concat_to_dpipe_sql reduced over expression.expressions directly, so it was
the one exp.Concat generation path that never called convert_concat_args.
A CONCAT parsed with coalesce=True therefore lost its COALESCE wrapping on
SQLite and became a || chain that returns NULL.

Redshift also maps exp.Concat to this helper but sets CONCAT_COALESCE, so
convert_concat_args returns its operands untouched and its output is
unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant