Skip to content

[ES|QL] ToAggregateMetricDouble function #124595

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

Merged
merged 20 commits into from
Mar 18, 2025
Merged

[ES|QL] ToAggregateMetricDouble function #124595

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
2f64c49
ToAggregateMetricDouble function
limotova Mar 11, 2025
7a49ca5
Merge branch 'main' into to-aggregate-metric-double-function
limotova Mar 13, 2025
efab117
add aggmetricliteral to serialization tests named writable registery
limotova Mar 13, 2025
58104fb
flip order of args of factory builders
limotova Mar 13, 2025
61c3fef
agg metric docs
limotova Mar 15, 2025
a65a1d3
multi-values and Cast.cast
limotova Mar 15, 2025
227de7d
except undo cast because it doesn't cooperate with multivalue
limotova Mar 15, 2025
2ccfd46
introduce AggregateMetricDoubleVector?
limotova Mar 15, 2025
5c4d965
fix tests
limotova Mar 15, 2025
0594990
fix last test
limotova Mar 15, 2025
d2531de
Merge branch 'main' into to-aggregate-metric-double-function
limotova Mar 18, 2025
ce814cf
move AggMetricVectorBuilder to To Function and use in U/Long/Int fact…
limotova Mar 18, 2025
804e980
add different case for single-valued
limotova Mar 18, 2025
ec2862b
remove redundant closed check
limotova Mar 18, 2025
4749325
Merge branch 'main' into to-aggregate-metric-double-function
limotova Mar 18, 2025
b5afb7f
Merge branch 'main' into to-aggregate-metric-double-function
limotova Mar 18, 2025
9b47fc3
Update docs/changelog/124595.yaml
limotova Mar 18, 2025
a2613f3
fix changelog and make logic in double block same as others
limotova Mar 18, 2025
9e4f3b4
fix loop start point
limotova Mar 18, 2025
e469df8
Merge branch 'main' into to-aggregate-metric-double-function
limotova Mar 18, 2025
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
1 change: 1 addition & 0 deletions server/src/main/java/org/elasticsearch/TransportVersions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ static TransportVersion def(int id) {
public static final TransportVersion ESQL_THREAD_NAME_IN_DRIVER_PROFILE = def(9_027_0_00);
public static final TransportVersion INFERENCE_CONTEXT = def(9_028_0_00);
public static final TransportVersion ML_INFERENCE_DEEPSEEK = def(9_029_00_0);
public static final TransportVersion ESQL_AGGREGATE_METRIC_DOUBLE_LITERAL = def(9_030_0_00);

/*
* STOP! READ THIS FIRST! No, really,
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,6 @@ public static boolean isRepresentable(DataType t) {
&& t != SOURCE
&& t != HALF_FLOAT
&& t != PARTIAL_AGG
&& t != AGGREGATE_METRIC_DOUBLE
&& t.isCounter() == false;
}

Expand Down
39 changes: 38 additions & 1 deletion ...mpute/src/main/java/org/elasticsearch/compute/data/AggregateMetricDoubleBlockBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,17 @@

package org.elasticsearch.compute.data;

import org.elasticsearch.TransportVersion;
import org.elasticsearch.TransportVersions;
import org.elasticsearch.common.io.stream.GenericNamedWriteable;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.core.Releasables;
import org.elasticsearch.index.mapper.BlockLoader;

import java.io.IOException;

public class AggregateMetricDoubleBlockBuilder extends AbstractBlockBuilder implements BlockLoader.AggregateMetricDoubleBuilder {

private DoubleBlockBuilder minBuilder;
Expand Down Expand Up @@ -161,11 +169,40 @@ public String getLabel() {
}
}

public record AggregateMetricDoubleLiteral(Double min, Double max, Double sum, Integer count) {
public record AggregateMetricDoubleLiteral(Double min, Double max, Double sum, Integer count) implements GenericNamedWriteable {
public AggregateMetricDoubleLiteral {
min = min.isNaN() ? null : min;
max = max.isNaN() ? null : max;
sum = sum.isNaN() ? null : sum;
}

public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(
GenericNamedWriteable.class,
"AggregateMetricDoubleLiteral",
AggregateMetricDoubleLiteral::new
);

@Override
public String getWriteableName() {
return "AggregateMetricDoubleLiteral";
}

public AggregateMetricDoubleLiteral(StreamInput input) throws IOException {
this(input.readOptionalDouble(), input.readOptionalDouble(), input.readOptionalDouble(), input.readOptionalInt());
}

@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeOptionalDouble(min);
out.writeOptionalDouble(max);
out.writeOptionalDouble(sum);
out.writeOptionalInt(count);
}

@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersions.ESQL_AGGREGATE_METRIC_DOUBLE_LITERAL;
}

}
}
14 changes: 13 additions & 1 deletion x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BlockUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,19 @@ private static Object valueAtOffset(Block block, int offset) {
DocVector v = ((DocBlock) block).asVector();
yield new Doc(v.shards().getInt(offset), v.segments().getInt(offset), v.docs().getInt(offset));
}
case COMPOSITE -> throw new IllegalArgumentException("can't read values from composite blocks");
case COMPOSITE -> {
CompositeBlock compositeBlock = (CompositeBlock) block;
Copy link
Member

@martijnvg martijnvg Mar 17, 2025

Choose a reason for hiding this comment

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

Can in a follow up the usage of CompositeBlock for aggregate metric double field be replaced with AggregateMetricDoubleBlockBuilder?

Copy link
Contributor Author

@limotova limotova Mar 18, 2025

Choose a reason for hiding this comment

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

I'm not sure I understand? There's no block creation here, just fetching the value from the block that already exists, so no building is involved

Copy link
Member

@martijnvg martijnvg Mar 19, 2025

Choose a reason for hiding this comment

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

Sorry, this is comment doesn't make sense here. I think I was more wondering about whether in AggregateMetricDoubleBlockBuilder we should use build a specialized block instead of building CompositeBlock. ComposteBlock is very generic and using this here now I think is ok, I'm just wondering whether in the future this makes sense? (when there are more usages of CompositeBlock)

Copy link
Member

@nik9000 nik9000 Mar 19, 2025

Choose a reason for hiding this comment

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

We could if we wanted, but I'm not sure it's all that important. The entire compute engine is pretty ok with runtime types and treating blocks in specific ways based on the input. We don't have a special type for utf-8 text - just bytes - so I'm not sure we'd need a special type for these blocks.

If we want it we can have it for sure, but I don't think it's all that important.

var minBlock = (DoubleBlock) compositeBlock.getBlock(AggregateMetricDoubleBlockBuilder.Metric.MIN.getIndex());
var maxBlock = (DoubleBlock) compositeBlock.getBlock(AggregateMetricDoubleBlockBuilder.Metric.MAX.getIndex());
var sumBlock = (DoubleBlock) compositeBlock.getBlock(AggregateMetricDoubleBlockBuilder.Metric.SUM.getIndex());
var countBlock = (IntBlock) compositeBlock.getBlock(AggregateMetricDoubleBlockBuilder.Metric.COUNT.getIndex());
yield new AggregateMetricDoubleLiteral(
minBlock.getDouble(offset),
maxBlock.getDouble(offset),
sumBlock.getDouble(offset),
countBlock.getInt(offset)
);
}
case UNKNOWN -> throw new IllegalArgumentException("can't read values from [" + block + "]");
};
}
Expand Down
12 changes: 10 additions & 2 deletions ...plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.BigArrays;
import org.elasticsearch.compute.data.AggregateMetricDoubleBlockBuilder;
import org.elasticsearch.compute.data.BlockFactory;
import org.elasticsearch.compute.data.BlockUtils;
import org.elasticsearch.compute.data.BytesRefBlock;
Expand Down Expand Up @@ -788,6 +789,12 @@ public static Literal randomLiteral(DataType type) {
case CARTESIAN_POINT -> CARTESIAN.asWkb(ShapeTestUtils.randomPoint());
case GEO_SHAPE -> GEO.asWkb(GeometryTestUtils.randomGeometry(randomBoolean()));
case CARTESIAN_SHAPE -> CARTESIAN.asWkb(ShapeTestUtils.randomGeometry(randomBoolean()));
case AGGREGATE_METRIC_DOUBLE -> new AggregateMetricDoubleBlockBuilder.AggregateMetricDoubleLiteral(
randomDouble(),
randomDouble(),
randomDouble(),
randomInt()
);
case NULL -> null;
case SOURCE -> {
try {
Expand All @@ -798,8 +805,9 @@ public static Literal randomLiteral(DataType type) {
throw new UncheckedIOException(e);
}
}
case UNSUPPORTED, OBJECT, DOC_DATA_TYPE, TSID_DATA_TYPE, PARTIAL_AGG, AGGREGATE_METRIC_DOUBLE ->
throw new IllegalArgumentException("can't make random values for [" + type.typeName() + "]");
case UNSUPPORTED, OBJECT, DOC_DATA_TYPE, TSID_DATA_TYPE, PARTIAL_AGG -> throw new IllegalArgumentException(
"can't make random values for [" + type.typeName() + "]"
);
}, type);
}

Expand Down
7 changes: 6 additions & 1 deletion x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,12 @@ public enum Cap {
/**
* Do {@code TO_LOWER} and {@code TO_UPPER} process all field values?
*/
TO_LOWER_MV;
TO_LOWER_MV,

/**
* Support for to_aggregate_metric_double function
*/
AGGREGATE_METRIC_DOUBLE_CONVERT_TO(AGGREGATE_METRIC_DOUBLE_FEATURE_FLAG);

private final boolean enabled;

Expand Down
2 changes: 2 additions & 0 deletions ...lugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/ExpressionWritables.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.elasticsearch.xpack.esql.expression.function.fulltext.FullTextWritables;
import org.elasticsearch.xpack.esql.expression.function.scalar.ScalarFunctionWritables;
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.FromBase64;
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToAggregateMetricDouble;
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToBase64;
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToBoolean;
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToCartesianPoint;
Expand Down Expand Up @@ -180,6 +181,7 @@ public static List<NamedWriteableRegistry.Entry> unaryScalars() {
entries.add(StY.ENTRY);
entries.add(Tan.ENTRY);
entries.add(Tanh.ENTRY);
entries.add(ToAggregateMetricDouble.ENTRY);
entries.add(ToBase64.ENTRY);
entries.add(ToBoolean.ENTRY);
entries.add(ToCartesianPoint.ENTRY);
Expand Down
2 changes: 2 additions & 0 deletions .../src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.elasticsearch.xpack.esql.expression.function.scalar.conditional.Greatest;
import org.elasticsearch.xpack.esql.expression.function.scalar.conditional.Least;
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.FromBase64;
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToAggregateMetricDouble;
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToBase64;
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToBoolean;
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.ToCartesianPoint;
Expand Down Expand Up @@ -376,6 +377,7 @@ private static FunctionDefinition[][] functions() {
// conversion functions
new FunctionDefinition[] {
def(FromBase64.class, FromBase64::new, "from_base64"),
def(ToAggregateMetricDouble.class, ToAggregateMetricDouble::new, "to_aggregate_metric_double", "to_aggregatemetricdouble"),
def(ToBase64.class, ToBase64::new, "to_base64"),
def(ToBoolean.class, ToBoolean::new, "to_boolean", "to_bool"),
def(ToCartesianPoint.class, ToCartesianPoint::new, "to_cartesianpoint"),
Expand Down
Loading