siddharthteotia commented on a change in pull request #7047:
URL: https://github.com/apache/incubator-pinot/pull/7047#discussion_r649629237
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/BrokerReduceService.java
##########
@@ -232,6 +232,7 @@ public BrokerResponseNative reduceOnDataTable(BrokerRequest
brokerRequest,
}
// Set execution statistics.
+ ResultTable resultTable = brokerResponseNative.getResultTable();
Review comment:
Since ResultTable is only available in SQL mode, I suggest doing the
following
@JsonProperty("resultTable")
public void setResultTable(ResultTable resultTable) {
_resultTable = resultTable;
_numRowsResultSet = resultTable.getRows().size()
}
This will ensure we don't run into NPE. Also, this will populate non zero
numRowsResultSet only for SQL mode but that is fine as PQL will be removed
anyway
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/BrokerReduceService.java
##########
@@ -232,6 +232,7 @@ public BrokerResponseNative reduceOnDataTable(BrokerRequest
brokerRequest,
}
// Set execution statistics.
+ ResultTable resultTable = brokerResponseNative.getResultTable();
Review comment:
Since ResultTable is only available in SQL mode, I suggest doing the
following
```
@JsonProperty("resultTable")
public void setResultTable(ResultTable resultTable) {
_resultTable = resultTable;
_numRowsResultSet = resultTable.getRows().size()
}
```
This will ensure we don't run into NPE. Also, this will populate non zero
numRowsResultSet only for SQL mode but that is fine as PQL will be removed
anyway
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/BrokerReduceService.java
##########
@@ -232,6 +232,7 @@ public BrokerResponseNative reduceOnDataTable(BrokerRequest
brokerRequest,
}
// Set execution statistics.
+ ResultTable resultTable = brokerResponseNative.getResultTable();
Review comment:
Since ResultTable is only available in SQL mode, I suggest doing the
following in BrokerResponseNative
```
@JsonProperty("resultTable")
public void setResultTable(ResultTable resultTable) {
_resultTable = resultTable;
_numRowsResultSet = resultTable.getRows().size()
}
```
This will ensure we don't run into NPE. Also, this will populate non zero
numRowsResultSet only for SQL mode but that is fine as PQL will be removed
anyway
##########
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/api/RequestStatistics.java
##########
@@ -220,6 +222,10 @@ public long getOfflineThreadCpuTimeNs() {
return _offlineThreadCpuTimeNs;
}
+ public int getNumRowsResultSet() { return _numRowsResultSet; }
+
+ public void setNumRowsResultSet(int numRowsResultSet) { _numRowsResultSet =
numRowsResultSet; }
Review comment:
Can we remove this method as no one seems to be calling this ?
##########
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/api/RequestStatistics.java
##########
@@ -220,6 +222,10 @@ public long getOfflineThreadCpuTimeNs() {
return _offlineThreadCpuTimeNs;
}
+ public int getNumRowsResultSet() { return _numRowsResultSet; }
+
+ public void setNumRowsResultSet(int numRowsResultSet) { _numRowsResultSet =
numRowsResultSet; }
Review comment:
We still need the getter as we will use it internally
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
