Skip to content

Fix aggregation NPE when child input finishes early#18171

Open
Caideyipi wants to merge 1 commit into
apache:masterfrom
Caideyipi:fix/aggregation-missing-child
Open

Fix aggregation NPE when child input finishes early#18171
Caideyipi wants to merge 1 commit into
apache:masterfrom
Caideyipi:fix/aggregation-missing-child

Conversation

@Caideyipi

Copy link
Copy Markdown
Collaborator

Description

  • Skip missing child TsBlocks when merging partial aggregation results.
  • Avoid advancing null child input blocks after a child operator has finished.
  • Add a regression test for group-by intermediate aggregation when one child has fewer windows than another.

Tests

  • mvn -nsu -pl iotdb-core/datanode -am -Dtest=AggregationOperatorTest#testGroupByIntermediateResultWithFinishedChild -DfailIfNoTests=false "-Dsurefire.failIfNoSpecifiedTests=false" test

@PDGGK

PDGGK commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Thanks for fixing this — the null-guards read well. Two small questions to make sure I follow the intended behavior:

  1. In the isInputFinal branch, when the input block is null the method returns without calling accumulator.setFinal(...). Is the intent that the accumulator keeps its prior state (no final result set for this call) and a later non-null block sets it? Just confirming there's no path where the final result ends up unset because the producing child finished with a null block.
  2. In the intermediate branch, hasMissingInput breaks on the first null and skips the whole group. I believe that's safe because a given InputLocation[] group's columns all come from the same child's TsBlock (same tsBlockIndex), so if one is null they all are — is that assumption always true, or could a group ever span multiple children where one finished early?

Neither is blocking — just making sure I understand the model. Thanks!

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