You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/03/04 04:37:02 UTC
[GitHub] [pinot] weixiangsun commented on a change in pull request #8029: Add Time-Series Gapfilling functionality.
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org