JackieTien97 commented on a change in pull request #1650:
URL: https://github.com/apache/incubator-iotdb/pull/1650#discussion_r484157676
##########
File path:
server/src/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByEngineDataSet.java
##########
@@ -31,18 +32,13 @@
public abstract class GroupByEngineDataSet extends QueryDataSet {
protected long queryId;
- protected long interval;
- protected long slidingStep;
- // total query [startTime, endTime)
- protected long startTime;
- protected long endTime;
+ protected GroupByTimePlan plan;
Review comment:
Why here use GroupByTimePlan to replace the former ones? GroupByTimePlan
contains some useless info that will waste some memory space.
##########
File path:
server/src/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByWithoutValueFilterDataSet.java
##########
@@ -140,9 +142,37 @@ protected RowRecord nextWithoutConstraint() throws
IOException {
return record;
}
- protected GroupByExecutor getGroupByExecutor(PartialPath path, Set<String>
allSensors, TSDataType dataType,
- QueryContext context, Filter
timeFilter, TsFileFilter fileFilter)
- throws StorageEngineException, QueryProcessException {
- return new LocalGroupByExecutor(path, allSensors, dataType, context,
timeFilter, fileFilter);
+ @Override
+ public Pair<Long, Object> peekNextNotNullValue(Path path, int i) throws
IOException {
+ Pair<Long, Object> result = null;
+ long nextStartTime = curStartTime;
+ long nextEndTime;
+ do {
+ if (ascending) {
+ nextStartTime += plan.getSlidingStep();
+ if (nextStartTime < plan.getEndTime()) {
+ nextEndTime = Math.min(nextStartTime + plan.getInterval(),
plan.getEndTime());
+ } else {
+ return null;
+ }
+ } else {
Review comment:
You don't need to consider the `ascending` case, the
`peekNextNotNullValue` method will only be used in `desc` case.
##########
File path:
server/src/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByWithValueFilterDataSet.java
##########
@@ -160,6 +171,24 @@ protected RowRecord nextWithoutConstraint() throws
IOException {
return constructRowRecord(aggregateResultList);
}
+ @Override
+ public Pair<Long, Object> peekNextNotNullValue(Path path, int i) throws
IOException {
+ long[] timestampArray = new long[1];
+ AggregateResult aggrResultByName =
AggregateResultFactory.getAggrResultByName(
+ groupByTimePlan.getDeduplicatedAggregations().get(i),
+ groupByTimePlan.getDeduplicatedDataTypes().get(i));
+
+ int timeArrayLength = 0;
+ if (hasCachedTimestamp) {
+ timestampArray[timeArrayLength++] = timestamp;
+
+ aggrResultByName
+ .updateResultUsingTimestamps(timestampArray, timeArrayLength,
allDataReaderList.get(i));
+ return new Pair<>(timestamp, aggrResultByName.getResult());
+ }
+ return null;
Review comment:
`timestamp` is the a cached timestamp for all sensors in this query, it
may not be the current one which is needed to fill value.
----------------------------------------------------------------
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
