Skip to content

fix(aggregate): preserve listagg result ownership - #281

Merged
lxy-9602 merged 4 commits into
apache:mainfrom
ChaomingZhangCN:codex/fix-listagg-result-lifetime
Sep 4, 2026
Merged

fix(aggregate): preserve listagg result ownership#281
lxy-9602 merged 4 commits into
apache:mainfrom
ChaomingZhangCN:codex/fix-listagg-result-lifetime

Conversation

@ChaomingZhangCN

@ChaomingZhangCN ChaomingZhangCN commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Purpose

Linked issue: N/A

FieldListaggAgg returned a std::string_view backed by a mutable member string shared by all calls to the same aggregator instance.

The merge path reuses one field aggregator across primary keys, while SortMergeReaderBatchProducer may retain results for multiple keys before converting the batch to Arrow. As a result, aggregating a later key could overwrite or invalidate the string data referenced by an earlier key, causing cross-key data corruption.

This change:

  • builds each concatenated result independently;
  • stores the result in a pool-backed BinaryString;
  • removes the shared mutable result buffer;
  • preserves the lifetime of previously returned aggregation results.

Tests

Added:

  • FieldListaggAggTest.TestResultOwnershipAcrossAggregations for the aggregator result-lifetime contract;
  • WriteAndReadInteTest.TestPKListAggPreservesResultsAcrossKeys for the real PK merge path in write_and_read_inte_test.cpp.

The integration test creates a PK aggregation table, writes two keys over two commits, and verifies that each key retains its independently accumulated listagg value.

Before the production fix, the integration test fails deterministically for all three enabled formats (Parquet, ORC, and Avro):

Expected: alpha,beta
Actual:   one,two\0\0\0

After the fix:

  • paimon-write-and-read-inte-test builds successfully;
  • TestPKListAggPreservesResultsAcrossKeys: 3/3 format variants passed;
  • neighboring TestPKSimple: 3/3 format variants passed;
  • paimon-core-test builds and links successfully on macOS arm64 after replacing the non-portable BIGINT test literals;
  • clang-format, cpplint, codespell, and git diff --check passed.

The focused FieldListaggAggTest.* execution is still blocked in this local build by an existing macOS static/shared plugin registration issue (Could not find a FileFormatFactory implementation class for format 'parquet'). This does not affect the integration-test target above.

API and Format

No public API, storage format, or protocol changes.

Documentation

No documentation changes are required.

Generative AI tooling

Generated-by: OpenAI Codex (GPT-5)

@lxy-9602 lxy-9602 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@lxy-9602
lxy-9602 merged commit ead9207 into apache:main Sep 4, 2026
16 checks passed
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.

2 participants