Copilot commented on code in PR #15977:
URL: https://github.com/apache/pinot/pull/15977#discussion_r2123040716
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/NonEquiJoinOperator.java:
##########
@@ -106,6 +113,7 @@ protected List<Object[]> buildJoinedRows(MseBlock.Data
leftBlock) {
@Override
protected List<Object[]> buildNonMatchRightRows() {
+ assert _matchedRightRows != null : "Matched right rows should not be null
when building non-matched right rows";
Review Comment:
Replace the assertion with an explicit null check and throw an appropriate
exception for production environments where assertions might be disabled.
```suggestion
if (_matchedRightRows == null) {
throw new IllegalStateException("Matched right rows should not be null
when building non-matched right rows");
}
```
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/AggregateOperator.java:
##########
@@ -205,13 +207,17 @@ protected MseBlock getNextBlock() {
if (finalBlock.isError()) {
return finalBlock;
}
- return produceAggregatedBlock();
+ MseBlock mseBlock = produceAggregatedBlock();
+ _aggregationExecutor = null;
+ _groupByExecutor = null;
+ return mseBlock;
}
private MseBlock produceAggregatedBlock() {
if (_aggregationExecutor != null) {
return new RowHeapDataBlock(_aggregationExecutor.getResult(),
_resultSchema, _aggFunctions);
} else {
+ assert _groupByExecutor != null;
Review Comment:
Consider adding an explicit null-check with a clear error message instead of
relying solely on assertions, to improve error handling in production.
```suggestion
if (_groupByExecutor == null) {
throw new RuntimeException("GroupByExecutor is null in
AggregateOperator. This indicates a misconfiguration or unexpected state.");
}
```
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/HashJoinOperator.java:
##########
@@ -93,18 +96,27 @@ public String toExplainString() {
@Override
protected void addRowsToRightTable(List<Object[]> rows) {
+ assert _rightTable != null : "Right table should not be null when adding
rows";
Review Comment:
Replace the assertion with a robust null-check and explicit exception
handling to ensure reliability in production builds.
```suggestion
if (_rightTable == null) {
throw new IllegalStateException("Right table should not be null when
adding rows");
}
```
--
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]
