liran-funaro commented on a change in pull request #12185:
URL: https://github.com/apache/druid/pull/12185#discussion_r798354833
##########
File path:
processing/src/main/java/org/apache/druid/segment/incremental/OnheapIncrementalIndex.java
##########
@@ -353,31 +383,51 @@ public String getOutOfRowsReason()
@Override
public float getMetricFloatValue(int rowOffset, int aggOffset)
{
- return concurrentGet(rowOffset)[aggOffset].getFloat();
+ if (isPreserveExistingMetrics()) {
+ return (float) getCombinedMetric(getMetricAggs(),
concurrentGet(rowOffset), aggOffset);
+ } else {
+ return concurrentGet(rowOffset)[aggOffset].getFloat();
+ }
}
@Override
public long getMetricLongValue(int rowOffset, int aggOffset)
{
- return concurrentGet(rowOffset)[aggOffset].getLong();
+ if (isPreserveExistingMetrics()) {
+ return (long) getCombinedMetric(getMetricAggs(),
concurrentGet(rowOffset), aggOffset);
+ } else {
+ return concurrentGet(rowOffset)[aggOffset].getLong();
+ }
}
@Override
public Object getMetricObjectValue(int rowOffset, int aggOffset)
{
- return concurrentGet(rowOffset)[aggOffset].get();
+ if (isPreserveExistingMetrics()) {
+ return getCombinedMetric(getMetricAggs(), concurrentGet(rowOffset),
aggOffset);
+ } else {
+ return concurrentGet(rowOffset)[aggOffset].get();
+ }
}
@Override
protected double getMetricDoubleValue(int rowOffset, int aggOffset)
{
- return concurrentGet(rowOffset)[aggOffset].getDouble();
+ if (isPreserveExistingMetrics()) {
+ return (double) getCombinedMetric(getMetricAggs(),
concurrentGet(rowOffset), aggOffset);
+ } else {
+ return concurrentGet(rowOffset)[aggOffset].getDouble();
+ }
Review comment:
This `if/else` logic repeats itself. It makes sense to put it in a
function or move the entire logic to `getCombinedMetric()`.
##########
File path:
processing/src/main/java/org/apache/druid/segment/incremental/OnheapIncrementalIndex.java
##########
@@ -248,9 +268,14 @@ private void factorizeAggs(
)
{
rowContainer.set(row);
- for (int i = 0; i < metrics.length; i++) {
+ int aggLength = isPreserveExistingMetrics() ? aggs.length / 2 :
aggs.length;
Review comment:
You can use here `metrics.length` instead.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
