raghavyadav01 commented on code in PR #14413:
URL: https://github.com/apache/pinot/pull/14413#discussion_r1835153031
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/timeseries/TimeSeriesAggregationOperator.java:
##########
@@ -152,23 +151,26 @@ public ExecutionStatistics getExecutionStatistics() {
return new ExecutionStatistics(0, 0, 0, 0);
}
- private int[] getTimeValueIndex(long[] actualTimeValues, TimeUnit timeUnit,
int numDocs) {
- if (timeUnit == TimeUnit.MILLISECONDS) {
+ @VisibleForTesting
+ protected int[] getTimeValueIndex(long[] actualTimeValues, int numDocs) {
+ if (_storedTimeUnit == TimeUnit.MILLISECONDS) {
return getTimeValueIndexMillis(actualTimeValues, numDocs);
}
int[] timeIndexes = new int[numDocs];
+ final long reference = _timeBuckets.getTimeRangeStartExclusive();
+ final long divisor = _timeBuckets.getBucketSize().getSeconds();
for (int index = 0; index < numDocs; index++) {
- timeIndexes[index] = (int) ((actualTimeValues[index] -
_timeBuckets.getStartTime())
- / _timeBuckets.getBucketSize().getSeconds());
+ timeIndexes[index] = (int) ((actualTimeValues[index] - reference - 1) /
divisor);
}
return timeIndexes;
}
private int[] getTimeValueIndexMillis(long[] actualTimeValues, int numDocs) {
int[] timeIndexes = new int[numDocs];
+ final long reference = _timeBuckets.getTimeRangeStartExclusive() * 1000L;
+ final long divisor = _timeBuckets.getBucketSize().toMillis();
for (int index = 0; index < numDocs; index++) {
- timeIndexes[index] = (int) ((actualTimeValues[index] -
_timeBuckets.getStartTime() * 1000L)
- / _timeBuckets.getBucketSize().toMillis());
+ timeIndexes[index] = (int) ((actualTimeValues[index] - reference - 1) /
divisor);
Review Comment:
May be we can add a comment here that this is (]
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/timeseries/TimeSeriesAggregationOperator.java:
##########
@@ -240,14 +242,12 @@ public void processStringExpression(BlockValSet
blockValSet, Map<Long, BaseTimeS
}
}
- public static long[] applyTimeshift(long timeshift, long[] timeValues, int
numDocs) {
- if (timeshift == 0) {
- return timeValues;
+ private void applyTimeOffset(long[] timeValues, int numDocs) {
Review Comment:
Method only has name change correct ?
##########
pinot-timeseries/pinot-timeseries-spi/src/main/java/org/apache/pinot/tsdb/spi/TimeBuckets.java:
##########
@@ -67,13 +67,11 @@ public int resolveIndex(long timeValue) {
if (_timeBuckets.length == 0) {
return -1;
}
- if (timeValue < _timeBuckets[0]) {
+ if (timeValue <= getTimeRangeStartExclusive() || timeValue >
getTimeRangeEndInclusive()) {
return -1;
}
- if (timeValue >= _timeBuckets[_timeBuckets.length - 1] +
_bucketSize.getSeconds()) {
- return -1;
- }
- return (int) ((timeValue - _timeBuckets[0]) / _bucketSize.getSeconds());
+ long offsetFromRangeStart = timeValue - getTimeRangeStartExclusive();
+ return (int) ((offsetFromRangeStart - 1) / _bucketSize.getSeconds());
Review Comment:
nit: We should add some commentary for -1 math, just for new readers to
understand.
--
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
