fix(sqlite): keep the COALESCE wrapping when CONCAT is generated as || [CLAUDE] - #8283
Open
ebarkhordar wants to merge 1 commit into
Open
fix(sqlite): keep the COALESCE wrapping when CONCAT is generated as || [CLAUDE]#8283ebarkhordar wants to merge 1 commit into
ebarkhordar wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
concat_to_dpipe_sqlreduces straight overexpression.expressions, so it is the onegeneration path for
exp.Concatthat never callsGenerator.convert_concat_args. That helperis what carries the source dialect's NULL semantics across: when the parsed
CONCAThascoalesce=Trueand the target's ownCONCATdoes not coalesce, each operand is wrapped inCOALESCE(x, '').SQLite maps
exp.Concatto that helper, so the wrapping is dropped and aCONCATthat skippedNULLs upstream becomes a
||chain that returns NULL.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 isabin DuckDB and NULL once transpiled.CONCAT_WSis already right on SQLite, becauseexp.ConcatWsis not routed to a dpipe helperthere: it comes out as
CONCAT_WS('-', 'a', COALESCE(NULL, ''), 'b')and both engines returna-b. That is the same wrapping this change givesCONCAT.Redshift is the only other dialect mapping
exp.Concattoconcat_to_dpipe_sql, and it setsCONCAT_COALESCE = True, soconvert_concat_argshands the operands back untouched and itsoutput does not move. Nor does
concat_sql's own fallback into the helper, which is onlyreached 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
CONCATor||, 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 ofthem are what
convert_concat_argsalready 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 firstfails on main with
'SELECT a || b FROM t' != "SELECT COALESCE(a, '') || COALESCE(b, '') FROM t".SKIP_INTEGRATION=1 python -m unittestis 1349 tests, OK (skipped=45), andruff check,ruff format --checkandmypy sqlglot testsare clean.