-
Notifications
You must be signed in to change notification settings - Fork 25.6k
ES|QL change point #119458
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
ES|QL change point #119458
Changes from all commits
6e2a3a1
d3cba2d
2c3225a
faf49e5
ccaed49
5e4076b
81f2c7e
f9f47d6
715bc66
5346535
7e9a3a5
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 |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| pr: 119458 | ||
| summary: ES|QL change point | ||
| area: Machine Learning | ||
| type: feature | ||
| issues: [] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,12 +87,14 @@ public class AggregatorImplementer { | |
| private final boolean valuesIsBytesRef; | ||
| private final List<IntermediateStateDesc> intermediateState; | ||
| private final List<Parameter> createParameters; | ||
| private final boolean includeTimestampVector; | ||
|
|
||
| public AggregatorImplementer( | ||
| Elements elements, | ||
| TypeElement declarationType, | ||
| IntermediateState[] interStateAnno, | ||
| List<TypeMirror> warnExceptions | ||
| List<TypeMirror> warnExceptions, | ||
| boolean includeTimestampVector | ||
| ) { | ||
| this.declarationType = declarationType; | ||
| this.warnExceptions = warnExceptions; | ||
|
|
@@ -128,6 +130,7 @@ public AggregatorImplementer( | |
| ); | ||
| this.valuesIsBytesRef = BYTES_REF.equals(TypeName.get(combine.getParameters().get(combine.getParameters().size() - 1).asType())); | ||
| intermediateState = Arrays.stream(interStateAnno).map(IntermediateStateDesc::newIntermediateStateDesc).toList(); | ||
| this.includeTimestampVector = includeTimestampVector; | ||
| } | ||
|
|
||
| ClassName implementation() { | ||
|
|
@@ -359,34 +362,44 @@ private MethodSpec addRawInput() { | |
| builder.addStatement("return"); | ||
| } | ||
| builder.endControlFlow(); | ||
| builder.addStatement("$T block = page.getBlock(channels.get(0))", valueBlockType(init, combine)); | ||
| builder.addStatement("$T vector = block.asVector()", valueVectorType(init, combine)); | ||
| if (includeTimestampVector) { | ||
| builder.addStatement("$T timestampsBlock = page.getBlock(channels.get(1))", LONG_BLOCK); | ||
| builder.addStatement("$T timestampsVector = timestampsBlock.asVector()", LONG_VECTOR); | ||
| builder.beginControlFlow("if (timestampsVector == null) "); | ||
| builder.addStatement("throw new IllegalStateException($S)", "expected @timestamp vector; but got a block"); | ||
| builder.endControlFlow(); | ||
| } | ||
|
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. First arity 2 aggregation function! I suppose it's ok to make it timestamp-specific at this point. At some point we'll rework this when we more correlation or something, but it's all good. 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. The |
||
|
|
||
| builder.beginControlFlow("if (mask.allTrue())"); | ||
| String extra = includeTimestampVector ? ", timestampsVector" : ""; | ||
| { | ||
| builder.addComment("No masking"); | ||
| builder.addStatement("$T block = page.getBlock(channels.get(0))", valueBlockType(init, combine)); | ||
| builder.addStatement("$T vector = block.asVector()", valueVectorType(init, combine)); | ||
| builder.beginControlFlow("if (vector != null)"); | ||
| builder.addStatement("addRawVector(vector)"); | ||
| builder.addStatement("addRawVector(vector$L)", extra); | ||
| builder.nextControlFlow("else"); | ||
| builder.addStatement("addRawBlock(block)"); | ||
| builder.addStatement("addRawBlock(block$L)", extra); | ||
| builder.endControlFlow(); | ||
| builder.addStatement("return"); | ||
| } | ||
| builder.endControlFlow(); | ||
|
|
||
| builder.nextControlFlow("else"); | ||
| builder.addComment("Some positions masked away, others kept"); | ||
| builder.addStatement("$T block = page.getBlock(channels.get(0))", valueBlockType(init, combine)); | ||
| builder.addStatement("$T vector = block.asVector()", valueVectorType(init, combine)); | ||
| builder.beginControlFlow("if (vector != null)"); | ||
| builder.addStatement("addRawVector(vector, mask)"); | ||
| builder.addStatement("addRawVector(vector$L, mask)", extra); | ||
| builder.nextControlFlow("else"); | ||
| builder.addStatement("addRawBlock(block, mask)"); | ||
| builder.addStatement("addRawBlock(block$L, mask)", extra); | ||
| builder.endControlFlow(); | ||
| builder.endControlFlow(); | ||
| return builder.build(); | ||
| } | ||
|
|
||
| private MethodSpec addRawVector(boolean masked) { | ||
| MethodSpec.Builder builder = MethodSpec.methodBuilder("addRawVector"); | ||
| builder.addModifiers(Modifier.PRIVATE).addParameter(valueVectorType(init, combine), "vector"); | ||
| if (includeTimestampVector) { | ||
| builder.addParameter(LONG_VECTOR, "timestamps"); | ||
| } | ||
| if (masked) { | ||
| builder.addParameter(BOOLEAN_VECTOR, "mask"); | ||
| } | ||
|
|
@@ -416,6 +429,9 @@ private MethodSpec addRawVector(boolean masked) { | |
| private MethodSpec addRawBlock(boolean masked) { | ||
| MethodSpec.Builder builder = MethodSpec.methodBuilder("addRawBlock"); | ||
| builder.addModifiers(Modifier.PRIVATE).addParameter(valueBlockType(init, combine), "block"); | ||
| if (includeTimestampVector) { | ||
| builder.addParameter(LONG_VECTOR, "timestamps"); | ||
| } | ||
| if (masked) { | ||
| builder.addParameter(BOOLEAN_VECTOR, "mask"); | ||
| } | ||
|
|
@@ -455,6 +471,8 @@ private void combineRawInput(MethodSpec.Builder builder, String blockVariable) { | |
| } | ||
| if (valuesIsBytesRef) { | ||
| combineRawInputForBytesRef(builder, blockVariable); | ||
| } else if (includeTimestampVector) { | ||
| combineRawInputWithTimestamp(builder, blockVariable); | ||
| } else if (returnType.isPrimitive()) { | ||
| combineRawInputForPrimitive(returnType, builder, blockVariable); | ||
| } else if (returnType == TypeName.VOID) { | ||
|
|
@@ -492,6 +510,12 @@ private void combineRawInputForVoid(MethodSpec.Builder builder, String blockVari | |
| ); | ||
| } | ||
|
|
||
| private void combineRawInputWithTimestamp(MethodSpec.Builder builder, String blockVariable) { | ||
| TypeName valueType = TypeName.get(combine.getParameters().get(combine.getParameters().size() - 1).asType()); | ||
| String blockType = valueType.toString().substring(0, 1).toUpperCase(Locale.ROOT) + valueType.toString().substring(1); | ||
| builder.addStatement("$T.combine(state, timestamps.getLong(i), $L.get$L(i))", declarationType, blockVariable, blockType); | ||
| } | ||
|
|
||
| private void combineRawInputForBytesRef(MethodSpec.Builder builder, String blockVariable) { | ||
| // scratch is a BytesRef var that must have been defined before the iteration starts | ||
| builder.addStatement("$T.combine(state, $L.getBytesRef(i, scratch))", declarationType, blockVariable); | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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 think this is fine. But it's worth saying out loud: Things like this usually should become
nulland a warning. They'd put the agg in an "I'm broken" state and produce a warning on output. Likesumshould do if it overflows. It doesn't do that now, but it should.Anyway, I think this is fine to just hard fail here - just like this - because we're going to want to build machinery around the agg to make sure that it's input is a time series. Which will have the constraint that the
timestampis always single valued. And, probably, descending.