-
Notifications
You must be signed in to change notification settings - Fork 25.7k
ESQL: Add support for exponential_histogram in code generation #137459
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.
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
Changes from 3 commits
b5a27e0
5466107
1e7f784
00d6d78
504f48d
1e7c489
6aca146
7e0e697
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,7 @@ | |
| import javax.lang.model.util.Elements; | ||
|
|
||
| import static java.util.stream.Collectors.joining; | ||
| import static org.elasticsearch.compute.gen.Methods.getMethod; | ||
| import static org.elasticsearch.compute.gen.Methods.optionalStaticMethod; | ||
| import static org.elasticsearch.compute.gen.Methods.requireAnyArgs; | ||
| import static org.elasticsearch.compute.gen.Methods.requireAnyType; | ||
|
|
@@ -53,7 +54,6 @@ | |
| import static org.elasticsearch.compute.gen.Types.BLOCK; | ||
| import static org.elasticsearch.compute.gen.Types.BLOCK_ARRAY; | ||
| import static org.elasticsearch.compute.gen.Types.BOOLEAN_VECTOR; | ||
| import static org.elasticsearch.compute.gen.Types.BYTES_REF; | ||
| import static org.elasticsearch.compute.gen.Types.DRIVER_CONTEXT; | ||
| import static org.elasticsearch.compute.gen.Types.ELEMENT_TYPE; | ||
| import static org.elasticsearch.compute.gen.Types.INTERMEDIATE_STATE_DESC; | ||
|
|
@@ -62,6 +62,8 @@ | |
| import static org.elasticsearch.compute.gen.Types.PAGE; | ||
| import static org.elasticsearch.compute.gen.Types.WARNINGS; | ||
| import static org.elasticsearch.compute.gen.Types.blockType; | ||
| import static org.elasticsearch.compute.gen.Types.fromString; | ||
| import static org.elasticsearch.compute.gen.Types.scratchType; | ||
| import static org.elasticsearch.compute.gen.Types.vectorType; | ||
|
|
||
| /** | ||
|
|
@@ -85,7 +87,7 @@ public class AggregatorImplementer { | |
|
|
||
| private final AggregationState aggState; | ||
| private final List<Argument> aggParams; | ||
| private final boolean hasOnlyBlockArguments; | ||
| private final boolean tryToUseVectors; | ||
|
|
||
| public AggregatorImplementer( | ||
| Elements elements, | ||
|
|
@@ -119,7 +121,8 @@ public AggregatorImplementer( | |
| return a; | ||
| }).filter(a -> a instanceof PositionArgument == false).toList(); | ||
|
|
||
| this.hasOnlyBlockArguments = this.aggParams.stream().allMatch(a -> a instanceof BlockArgument); | ||
| this.tryToUseVectors = aggParams.stream().anyMatch(a -> (a instanceof BlockArgument) == false) | ||
| && aggParams.stream().noneMatch(a -> a instanceof StandardArgument && a.dataType(false) == null); | ||
|
|
||
| this.createParameters = init.getParameters() | ||
| .stream() | ||
|
|
@@ -199,7 +202,7 @@ private TypeSpec type() { | |
| builder.addMethod(addRawInput()); | ||
| builder.addMethod(addRawInputExploded(true)); | ||
| builder.addMethod(addRawInputExploded(false)); | ||
| if (hasOnlyBlockArguments == false) { | ||
| if (tryToUseVectors) { | ||
| builder.addMethod(addRawVector(false)); | ||
| builder.addMethod(addRawVector(true)); | ||
| } | ||
|
|
@@ -340,16 +343,18 @@ private MethodSpec addRawInputExploded(boolean hasMask) { | |
| builder.addStatement("$T $L = page.getBlock(channels.get($L))", a.dataType(true), a.blockName(), i); | ||
| } | ||
|
|
||
| for (Argument a : aggParams) { | ||
| String rawBlock = "addRawBlock(" | ||
| + aggParams.stream().map(arg -> arg.blockName()).collect(joining(", ")) | ||
| + (hasMask ? ", mask" : "") | ||
| + ")"; | ||
| if (tryToUseVectors) { | ||
| for (Argument a : aggParams) { | ||
| String rawBlock = "addRawBlock(" | ||
| + aggParams.stream().map(arg -> arg.blockName()).collect(joining(", ")) | ||
| + (hasMask ? ", mask" : "") | ||
| + ")"; | ||
|
|
||
| a.resolveVectors(builder, rawBlock, "return"); | ||
| a.resolveVectors(builder, rawBlock, "return"); | ||
| } | ||
| } | ||
|
|
||
| builder.addStatement(invokeAddRaw(hasOnlyBlockArguments, hasMask)); | ||
| builder.addStatement(invokeAddRaw(tryToUseVectors == false, hasMask)); | ||
| return builder.build(); | ||
| } | ||
|
|
||
|
|
@@ -499,9 +504,9 @@ private MethodSpec.Builder initAddRaw(boolean blockStyle, boolean masked) { | |
| builder.addParameter(BOOLEAN_VECTOR, "mask"); | ||
| } | ||
| for (Argument a : aggParams) { | ||
| if (a.isBytesRef()) { | ||
| // Add bytes_ref scratch var that will be used for bytes_ref blocks/vectors | ||
| builder.addStatement("$T $L = new $T()", BYTES_REF, a.scratchName(), BYTES_REF); | ||
| if (a.scratchType() != null) { | ||
| // Add scratch var that will be used for some blocks/vectors, e.g. for bytes_ref | ||
| builder.addStatement("$T $L = new $T()", a.scratchType(), a.scratchName(), a.scratchType()); | ||
| } | ||
| } | ||
| return builder; | ||
|
|
@@ -610,8 +615,8 @@ private MethodSpec addIntermediateInput() { | |
| ).map(Methods::requireType).toArray(TypeMatcher[]::new) | ||
| ) | ||
| ); | ||
| if (intermediateState.stream().map(IntermediateStateDesc::elementType).anyMatch(n -> n.equals("BYTES_REF"))) { | ||
| builder.addStatement("$T scratch = new $T()", BYTES_REF, BYTES_REF); | ||
| for (IntermediateStateDesc interState : intermediateState) { | ||
| interState.addScratchDeclaration(builder); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this was buggy prior to my change, but without causing a defect? If there were multiple
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That seems possible. I imagine we don't have any intermediate states with two strings. |
||
| } | ||
| builder.addStatement("$T.combineIntermediate(state, " + intermediateStateRowAccess() + ")", declarationType); | ||
| } | ||
|
|
@@ -706,13 +711,25 @@ public String access(String position) { | |
| if (block) { | ||
| return name(); | ||
| } | ||
| String s = name() + "." + vectorAccessorName(elementType()) + "(" + position; | ||
| if (elementType().equals("BYTES_REF")) { | ||
| s += ", scratch"; | ||
| String s = name() + "."; | ||
| if (vectorType(elementType) != null) { | ||
| s += vectorAccessorName(elementType()) + "(" + position; | ||
| } else { | ||
| s += getMethod(fromString(elementType())) + "(" + name() + ".getFirstValueIndex(" + position + ")"; | ||
| } | ||
| if (scratchType(elementType()) != null) { | ||
| s += ", " + name() + "Scratch"; | ||
| } | ||
| return s + ")"; | ||
| } | ||
|
|
||
| public void addScratchDeclaration(MethodSpec.Builder builder) { | ||
| ClassName scratchType = scratchType(elementType()); | ||
| if (scratchType != null) { | ||
| builder.addStatement("$T $L = new $T()", scratchType, name() + "Scratch", scratchType); | ||
| } | ||
| } | ||
|
|
||
| public void assignToVariable(MethodSpec.Builder builder, int offset) { | ||
| builder.addStatement("Block $L = page.getBlock(channels.get($L))", name + "Uncast", offset); | ||
| ClassName blockType = blockType(elementType()); | ||
|
|
@@ -721,7 +738,7 @@ public void assignToVariable(MethodSpec.Builder builder, int offset) { | |
| builder.addStatement("return"); | ||
| builder.endControlFlow(); | ||
| } | ||
| if (block) { | ||
| if (block || vectorType(elementType) == null) { | ||
| builder.addStatement("$T $L = ($T) $L", blockType, name, blockType, name + "Uncast"); | ||
| } else { | ||
| builder.addStatement("$T $L = (($T) $L).asVector()", vectorType(elementType), name, blockType, name + "Uncast"); | ||
|
|
@@ -732,6 +749,7 @@ public TypeName combineArgType() { | |
| var type = Types.fromString(elementType); | ||
| return block ? blockType(type) : type; | ||
| } | ||
|
|
||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer making a method in
Argumentthat's, like,hasVectoror something.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 00d6d78.