gortiz commented on code in PR #15245:
URL: https://github.com/apache/pinot/pull/15245#discussion_r2005253188
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/plan/MultiStageQueryStats.java:
##########
@@ -95,32 +90,25 @@ private MultiStageQueryStats(int stageId) {
_closedStats = new ArrayList<>();
}
- private static MultiStageQueryStats create(int stageId,
MultiStageOperator.Type type, @Nullable StatMap<?> opStats) {
- MultiStageQueryStats multiStageQueryStats = new
MultiStageQueryStats(stageId);
- multiStageQueryStats.getCurrentStats().addLastOperator(type, opStats);
- return multiStageQueryStats;
+ private MultiStageQueryStats(MultiStageQueryStats other) {
+ _currentStageId = other._currentStageId;
+ _currentStats = StageStats.Open.copy(other._currentStats);
+ _closedStats = new ArrayList<>(other._closedStats.size());
+ for (StageStats.Closed closed : other._closedStats) {
+ if (closed == null) {
Review Comment:
But then `StageStats.Closed.copy` would be nullable, which means it should
be annotated with `@Nullable`. Therefore, all usages of that method should
either assume null will not be returned or have to check for null values.
By the way, the first goes against something that would be cool to add in
the future: [NullAway](https://github.com/apache/pinot/pull/13741).
In general, when some types are usually not nullable, it is better to add
explicit null checks outside utility methods than pollute all calls to that
method with nullability
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
