[To dev/1.3] Pipe: Added max / min function to aggregate-processor (#15459) #15469
Conversation
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
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds new max and min aggregate operator functionality to the pipe aggregate-processor, updating both the aggregated result and intermediate result operator sets, and extends test coverage accordingly.
- New imports and operator registrations for MaxValueOperator and MinValueOperator have been added.
- New operator classes (MaxOperator, MinOperator, MaxValueOperator, MinValueOperator) are introduced with parallel behavior to existing operators.
- The integration tests are updated to reflect the increased number of operators.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| StandardStatisticsOperatorProcessor.java | Registers new max/min operators in both aggregated result and intermediate result operator sets. |
| MinOperator.java, MaxOperator.java | Implements new numeric operator classes for min and max values. |
| AbsoluteMaxOperator.java | Updates parameters with final modifiers for improved consistency. |
| MinValueOperator.java, MaxValueOperator.java | Introduces new aggregated result operator classes for min and max. |
| IoTDBPipeAggregateIT.java | Updates operator list and expected result count in integration tests. |
Comments suppressed due to low confidence (1)
integration-test/src/test/java/org/apache/iotdb/pipe/it/single/IoTDBPipeAggregateIT.java:119
- [nitpick] The expected result count has changed from 20 to 24 due to the inclusion of max and min operators; consider adding a comment in the test to explain the update.
Collections.singleton("24,"));
| () -> new IntegralPoweredSumOperator(3), | ||
| () -> new IntegralPoweredSumOperator(4)))); | ||
| () -> new IntegralPoweredSumOperator(4), | ||
| MaxOperator::new, |
There was a problem hiding this comment.
[nitpick] Ensure that the ordering of supplier operator additions reflects the intended operator evaluation sequence for clarity and maintainability.
Comment on lines
+29
to
+35
| public boolean initAndGetIsSupport(int initialInput, long initialTimestamp) { | ||
| intValue = initialInput; | ||
| return super.initAndGetIsSupport(initialInput, initialTimestamp); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean initAndGetIsSupport(long initialInput, long initialTimestamp) { |
There was a problem hiding this comment.
[nitpick] For consistency with other similar operator implementations, consider declaring the method parameters as final.
Suggested change
| public boolean initAndGetIsSupport(int initialInput, long initialTimestamp) { | |
| intValue = initialInput; | |
| return super.initAndGetIsSupport(initialInput, initialTimestamp); | |
| } | |
| @Override | |
| public boolean initAndGetIsSupport(long initialInput, long initialTimestamp) { | |
| public boolean initAndGetIsSupport(final int initialInput, final long initialTimestamp) { | |
| intValue = initialInput; | |
| return super.initAndGetIsSupport(initialInput, initialTimestamp); | |
| } | |
| @Override | |
| public boolean initAndGetIsSupport(final long initialInput, final long initialTimestamp) { |
SteveYurongSu
approved these changes
May 8, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
As the title said.
#15459
This PR has:
for an unfamiliar reader.
for code coverage.
Key changed/added classes (or packages if there are too many classes) in this PR