Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Star Tree Request/Response structure #227

Closed
wants to merge 3 commits into from
Closed

Star Tree Request/Response structure #227

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
da0056b
Star stree request/response changes
sandeshkr419 Aug 13, 2024
dcddc5c
Merge branch 'startree-file-formats-codec-merge' into poc1
sandeshkr419 Aug 13, 2024
54b9148
fix sum aggregator code
sandeshkr419 Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions server/src/main/java/org/opensearch/search/aggregations/metrics/SumAggregator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,14 @@
package org.opensearch.search.aggregations.metrics;

import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.index.SortedNumericDocValues;
import org.apache.lucene.search.ScoreMode;
import org.opensearch.common.lease.Releasables;
import org.opensearch.common.util.BigArrays;
import org.opensearch.common.util.DoubleArray;
import org.opensearch.index.codec.composite.CompositeIndexFieldInfo;
import org.opensearch.index.codec.composite.datacube.startree.StarTreeValues;
import org.opensearch.index.compositeindex.datacube.startree.utils.StarTreeHelper;
import org.opensearch.index.fielddata.SortedNumericDoubleValues;
import org.opensearch.search.DocValueFormat;
import org.opensearch.search.aggregations.Aggregator;
Expand Down Expand Up @@ -137,8 +139,9 @@ private LeafBucketCollector getStarTreeLeafCollector(LeafReaderContext ctx, Leaf

StarTreeValues starTreeValues = getStarTreeValues(ctx, starTree);

//
String fieldName = ((ValuesSource.Numeric.FieldData) valuesSource).getIndexFieldName();
String metricName = StarTreeHelper.fullFieldNameForStarTreeMetricsDocValues(starTree.getField(), fieldName, "sum");


return new LeafBucketCollectorBase(sub, starTreeValues) {
@Override
Expand All @@ -147,7 +150,23 @@ public void collect(int doc, long bucket) throws IOException {
sums = bigArrays.grow(sums, bucket + 1);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we extract out the default implementation getDefaultLeafCollector and reuse the same logic.

I really like the approach of reusing the existing aggregators.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'll try and do that, I am inclined to on refactoring & re-using same implementations wherever possible.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final SortedNumericDoubleValues values = valuesSource.doubleValues(ctx);

we can check if we are able to get sortedNumericDoubleValues , otherwise we need to convert to double for each doc

compensations = bigArrays.grow(compensations, bucket + 1);
compensations.set(bucket, kahanSummation.delta());
sums.set(bucket, kahanSummation.value());

SortedNumericDocValues dv = (SortedNumericDocValues) starTreeValues.getMetricDocValuesIteratorMap().get(metricName);

if (dv.advanceExact(doc)) {
final int valuesCount = dv.docValueCount();
double sum = sums.get(bucket);
double compensation = compensations.get(bucket);
kahanSummation.reset(sum, compensation);

for (int i = 0; i < valuesCount; i++) {
double value = Double.longBitsToDouble(dv.nextValue());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we do this ?

 public static Double sortableLongtoDouble(Long value) {
        return NumericUtils.sortableLongToDouble(value);
    }

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also lets see if we can get double sorted numeric dv ahead

@Override
        public SortedNumericDoubleValues getDoubleValues() {
            try {
                SortedNumericDocValues raw = DocValues.getSortedNumeric(reader, field);
                return FieldData.sortableLongBitsToDoubles(raw);
            } catch (IOException e) {
                throw new IllegalStateException("Cannot load doc values", e);
            }
        }

kahanSummation.add(value);
}

compensations.set(bucket, kahanSummation.delta());
sums.set(bucket, kahanSummation.value());
}
}
};
}
Expand Down
Loading