Skip to content

Preserve NULLs in array_agg pushdown - #340

Open
fallintoplace wants to merge 1 commit into
ClickHouse:mainfrom
fallintoplace:fix/array-agg-null-preservation
Open

Preserve NULLs in array_agg pushdown#340
fallintoplace wants to merge 1 commit into
ClickHouse:mainfrom
fallintoplace:fix/array-agg-null-preservation

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

Summary

ClickHouse groupArray() skips NULL values, while PostgreSQL array_agg() preserves them. Wrap nullable inputs in a tuple before groupArray and unwrap them with arrayMap so NULL elements survive the pushdown.

Non-nullable array_agg() plans keep their existing SQL. Ordered array_agg() stays local because the wrapper does not preserve ordering.

Testing

  • Built and installed with -Werror
  • make installcheck REGRESS=aggregates

@theory theory left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find. Please add a changelog item and write a descriptive commit 72/50 message in a single commit.

Comment thread src/include/fdw.h Outdated
* length(arr)-n) */
CF_ARRAY_SORT_DESC, /* array_sort(arr,desc) →
* arrayReverseSort/arraySort */
CF_ARRAY_AGG,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs explanatory comment

Comment thread src/deparse.c
Comment on lines +1219 to +1221
if ((agg->aggfnoid == F_STRING_AGG_TEXT_TEXT ||
agg->aggfnoid == F_ARRAY_AGG_ANYNONARRAY) &&
agg->aggorder != NIL) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need test rejecting use of ORDER with F_ARRAY_AGG_ANYNONARRAY

Comment thread test/sql/aggregates.sql
EXPLAIN (VERBOSE, COSTS OFF) SELECT array_agg(v) FROM agg_bin.null_agg;
SELECT array_agg(v) FROM agg_bin.null_agg;
EXPLAIN (VERBOSE, COSTS OFF) SELECT array_agg(v) FROM agg_http.null_agg;
SELECT array_agg(v) FROM agg_http.null_agg;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is useful but we also need tests to cover all of the variants in deparseArrayAggref(): filter, distinct, and nullable.

@theory theory added pushdown Improvements to query pushdown aggregates Improve aggregate pushdown bug Something isn't working labels Aug 6, 2026
@fallintoplace
fallintoplace force-pushed the fix/array-agg-null-preservation branch from 3a33223 to f4c4699 Compare August 6, 2026 20:44
Wrap nullable inputs in tuples before passing them to ClickHouse
groupArray so NULL elements are preserved. Keep ordered array_agg local
because groupArray does not preserve PostgreSQL ordering.

Add regression coverage for nullable, FILTER, DISTINCT, and ORDER BY
array_agg variants.
@fallintoplace
fallintoplace force-pushed the fix/array-agg-null-preservation branch from f4c4699 to 197602a Compare August 6, 2026 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aggregates Improve aggregate pushdown bug Something isn't working pushdown Improvements to query pushdown

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants