weixiangsun commented on a change in pull request #8029: URL: https://github.com/apache/pinot/pull/8029#discussion_r817134430
########## File path: pinot-core/src/main/java/org/apache/pinot/core/query/request/context/utils/BrokerRequestToQueryContextConverter.java ########## @@ -53,12 +55,82 @@ private BrokerRequestToQueryContextConverter() { * Converts the given {@link BrokerRequest} into a {@link QueryContext}. */ public static QueryContext convert(BrokerRequest brokerRequest) { - return brokerRequest.getPinotQuery() != null ? convertSQL(brokerRequest) : convertPQL(brokerRequest); + QueryContext queryContext; + if (brokerRequest.getPinotQuery() != null) { + queryContext = convertSQL(brokerRequest.getPinotQuery(), brokerRequest); + } else { + queryContext = convertPQL(brokerRequest); + } + queryContext.setGapfillType(GapfillUtils.getGapfillType(queryContext)); Review comment: Should move it to sql case Fixed ########## File path: pinot-core/src/main/java/org/apache/pinot/core/query/reduce/PostAggregationHandler.java ########## @@ -50,6 +54,9 @@ public PostAggregationHandler(QueryContext queryContext, DataSchema dataSchema) _filteredAggregationsIndexMap = queryContext.getFilteredAggregationsIndexMap(); assert _filteredAggregationsIndexMap != null; List<ExpressionContext> groupByExpressions = queryContext.getGroupByExpressions(); + if (groupByExpressions == null) { Review comment: Maybe should be removed? Fixed ########## File path: pinot-core/src/test/java/org/apache/pinot/queries/BaseQueriesTest.java ########## @@ -234,8 +234,8 @@ private BrokerResponseNative getBrokerResponse(QueryContext queryContext, PlanMa Utils.rethrowException(e); } - BrokerResponseNative brokerResponse = - brokerReduceService.reduceOnDataTable(queryContext.getBrokerRequest(), dataTableMap, + BrokerResponseNative brokerResponse = brokerReduceService Review comment: Revert the change Fixed ########## File path: pinot-common/src/main/java/org/apache/pinot/common/request/DataSource.java ########## @@ -97,12 +102,14 @@ public short getThriftFieldId() { } // isset id assignments - private static final _Fields optionals[] = {_Fields.TABLE_NAME}; + private static final _Fields optionals[] = {_Fields.TABLE_NAME,_Fields.SUBQUERY}; public static final java.util.Map<_Fields, org.apache.thrift.meta_data.FieldMetaData> metaDataMap; static { java.util.Map<_Fields, org.apache.thrift.meta_data.FieldMetaData> tmpMap = new java.util.EnumMap<_Fields, org.apache.thrift.meta_data.FieldMetaData>(_Fields.class); tmpMap.put(_Fields.TABLE_NAME, new org.apache.thrift.meta_data.FieldMetaData("tableName", org.apache.thrift.TFieldRequirementType.OPTIONAL, new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.STRING))); + tmpMap.put(_Fields.SUBQUERY, new org.apache.thrift.meta_data.FieldMetaData("subquery", org.apache.thrift.TFieldRequirementType.OPTIONAL, + new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.STRUCT , "PinotQuery"))); Review comment: Remove the spaces? Fixed -- 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: commits-unsubscr...@pinot.apache.org 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