Push down string_agg to pgduck_server#440
Merged
Merged
Conversation
Ship string_agg(text, text) to pgduck_server/DuckDB when the delimiter is a non-NULL constant. That is the only condition under which the two engines diverge: DuckDB returns NULL for a NULL separator (PostgreSQL concatenates) and rejects a non-constant separator, whereas every other behavior (NULL/empty values, ORDER BY, DISTINCT, FILTER) is identical. The bytea variant is excluded automatically (DuckDB has no string_agg(BLOB, VARCHAR)) as is a non-constant delimiter. Aggregates reach the shippability callback as an Aggref, so IsStringAggShippable unwraps the delimiter from Aggref->args rather than using GetConstArg (which assumes a FuncExpr). Adds test_string_agg_pushdown.py covering pushed cases (ORDER BY / DISTINCT / FILTER / GROUP BY, unicode) and non-pushed cases (NULL delimiter, per-row delimiter, bytea variant). Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Onder KALACI <oender.kalaci@snowflake.com> Co-authored-by: Cursor <cursoragent@cursor.com>
d1eeee4 to
b78c5b1
Compare
The negative cases assert the pushed SQL does not contain "string_agg", but the schema and S3 path were named "string_agg_pushdown", so the substring matched the table/path name rather than the function. Rename to "sagg_pushdown". Signed-off-by: Onder KALACI <oender.kalaci@snowflake.com> Co-authored-by: Cursor <cursoragent@cursor.com>
sfc-gh-mslot
approved these changes
Jul 10, 2026
| return false; | ||
|
|
||
| /* a NULL delimiter yields NULL in DuckDB but concatenation in PostgreSQL */ | ||
| return !((Const *) delimNode)->constisnull; |
Collaborator
There was a problem hiding this comment.
we should perhaps open an issue to add a new implementation
Collaborator
Author
There was a problem hiding this comment.
Opened #441 to track this. The NULL-constant-delimiter case is a straightforward transform (deparse a NULL delimiter as '', since that matches PostgreSQL's "no separator" semantics); the per-row/non-constant delimiter case is harder and captured there too, with examples.
Cover a single-codepoint emoji and a multi-codepoint emoji (with skin-tone modifier) alongside the existing Japanese and accented-Latin values, since these are 4-byte UTF-8 sequences worth exercising through the pushdown path. Signed-off-by: Onder KALACI <oender.kalaci@snowflake.com> Co-authored-by: Cursor <cursoragent@cursor.com>
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.
Summary
Adds
string_agg(text, text)to the list of aggregates shipped to pgduck_server / DuckDB, gated on the delimiter being a non-NULL constant.This is a customer feature request (Zendesk 11742, previously 10799). Investigation showed the delimiter is the only place
string_aggdiverges between PostgreSQL and DuckDB:NULL, PostgreSQL treats it as "no separator" and concatenates.Separator argument to StringAgg must be a constant), PostgreSQL evaluates it per row.With a non-NULL constant delimiter, every other behavior is byte-for-byte identical between the engines: NULL/empty values (dropped/kept the same way),
ORDER BY(incl. NULLS FIRST/LAST, multi-key),DISTINCT,FILTER,GROUP BY, unicode, and large inputs. This was verified with an extensive engine-vs-engine torture test (44+ scenarios).Implementation notes
IsStringAggShippablereceives anAggref(not aFuncExpr), so it unwraps the delimiter fromAggref->argsinstead of usingGetConstArg(). The delimiter is always the 2nd arg because sort-only junkTargetEntrys are appended after the real args.byteavariant is excluded automatically (DuckDB has nostring_agg(BLOB, VARCHAR)), as is any non-constant delimiter.float8::text→1.0vs1) are governed by their own cast-pushdown behavior and are out of scope here.Test plan
pg_lake_table/tests/pytests/test_string_agg_pushdown.py:ORDER BYfor determinism): basic,ORDER BY DESC, separate sort key with NULLS LAST/FIRST,DISTINCT,FILTER, empty/unicode delimiter,GROUP BY.byteavariant.Locally verified end-to-end via
EXPLAIN (VERBOSE)on an Iceberg table: the constant-delimiter case becomes aCustom Scan (Query Pushdown)withstring_agg(...)in the remote SQL, while the NULL/column-delimiter cases stay local. Relying on CI for the full pytest run.Made with Cursor