siddharthteotia commented on a change in pull request #4877: Results in
ResultTable if responseFormat=sql
URL: https://github.com/apache/incubator-pinot/pull/4877#discussion_r353925043
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java
##########
@@ -89,35 +107,39 @@ public void reduceAndSetResults(String tableName,
DataSchema dataSchema, Map<Ser
int resultSize = 0;
Collection<DataTable> dataTables = dataTableMap.values();
- // Aggregation group-by query.
- // read results as records if GROUP_BY_MODE is explicitly set to SQL
-
if (_groupByModeSql) {
- // if RESPONSE_FORMAT is SQL, return results in {@link ResultTable}
if (_responseFormatSql) {
- setSQLGroupByOrderByResults(brokerResponseNative, dataSchema,
_brokerRequest.getAggregationsInfo(),
- _brokerRequest.getGroupBy(), _brokerRequest.getOrderBy(),
dataTables);
+ setSQLGroupByInResultTable(brokerResponseNative, dataSchema,
dataTables);
resultSize = brokerResponseNative.getResultTable().getRows().size();
} else {
- setPQLGroupByOrderByResults(brokerResponseNative, dataSchema,
_brokerRequest.getAggregationsInfo(),
- _brokerRequest.getGroupBy(), _brokerRequest.getOrderBy(),
dataTables);
+ setSQLGroupByInAggregationResults(brokerResponseNative, dataSchema,
dataTables);
if (!brokerResponseNative.getAggregationResults().isEmpty()) {
resultSize =
brokerResponseNative.getAggregationResults().get(0).getGroupByResult().size();
}
}
} else {
- boolean[] aggregationFunctionSelectStatus =
-
AggregationFunctionUtils.getAggregationFunctionsSelectStatus(_brokerRequest.getAggregationsInfo());
- setGroupByHavingResults(brokerResponseNative,
aggregationFunctionSelectStatus, _brokerRequest.getGroupBy(),
- dataTables, _brokerRequest.getHavingFilterQuery(),
_brokerRequest.getHavingFilterSubQueryMap());
- // We emit the group by size when the result isn't empty. All the sizes
among group-by results should be the same.
- // Thus, we can just emit the one from the 1st result.
- if (!brokerResponseNative.getAggregationResults().isEmpty()) {
- resultSize =
brokerResponseNative.getAggregationResults().get(0).getGroupByResult().size();
+ if (_responseFormatSql && _numAggregationFunctions > 1) {
+ brokerResponseNative.addToExceptions(new
QueryProcessingException(QueryException.MERGE_RESPONSE_ERROR_CODE,
Review comment:
Would be good to add a test for the same in QueryValidationTest -- it
already has some tests for invalid queries that call into
BaseBrokerRequestHandler.validateRequest()
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
