dbt-labs / dbt-labs/dbt-adapters
default__concat produces invalid SQL for single-field input when adapters override with CONCAT() function
- Dominant language
- Python
- Stars
- 233
- Forks
- 362
- Avg merge
- 3d 22h
- Merged PRs (30d)
- 9
Description
## Summary
The current `default__concat` macro joins fields with ` || `, which works correctly for a single-field list (it simply returns the field). However, many adapters override `default__concat` to emit a SQL `CONCAT(...)` function call instead of the `||` operator. On engines like SQL Server / Microsoft Fabric T-SQL, `CONCAT()` requires at least two arguments — `CONCAT(x)` raises a syntax error.
Because the default macro propagates a single-field list to overrides, every override that uses the function form must add its own defensive branch (or break on single-field input).
## Repro
When `dbt_utils.generate_surrogate_key([single_field])` is called, the package generates `dbt.hash(dbt.concat([field]))`. On Fabric this becomes `HASHBYTES(... CONCAT(field) ...)`, which fails to compile.
## Affected adapters / known workarounds
- dbt-spark: overrides `default__concat` with `concat({{ fields|join(', ') }})`.
- dbt-fabric (community fork): currently ships a duplicated override of `dbt_utils.generate_surrogate_key` to work around this exact problem — see https://github.com/sdebruyn/dbt-fabric/blob/main/src/dbt/include/fabric/macros/dbt_package_support/dbt_utils/sql/generate_surrogate_key.sql
Any adapter that uses the `CONCAT(...)` function form needs the same fix.
## Proposal
Short-circuit `default__concat` on `length == 1` and return the field unchanged. Behavior is identical for multi-field lists; the change is purely defensive.
```jinja
{% macro default__concat(fields) -%}
{%- if fields | length == 1 -%}
{{ fields[0] }}
{%- else -%}
{{ fields | join(' || ') }}
{%- endif -%}
{%- endmacro %}
```
A dispatched-down override would have to repeat the same guard in every adapter, so the fix belongs in the default.
I'll open a PR with the change plus a `BaseConcatSingleField` test in `dbt-tests-adapter` so every adapter inherits coverage.
Contributor guide
Assessment
This issue has not been assessed yet.