EQL: Use the implicit tiebreaker sort value in the search queries #72089
Conversation
elasticmachine
added
the
Team:QL (Deprecated)
|
Pinging @elastic/es-ql (Team:QL) |
|
@elasticmachine update branch |
| } | ||
| // this tiebreaker is null | ||
| else { | ||
| // nulls are last so unless both are null (equal) | ||
| // this ordinal is greater (after) then the other tiebreaker | ||
| // so fall through to 1 | ||
| if (o.tiebreaker == null) { | ||
| return 0; | ||
| return Long.valueOf(implicitTiebreaker).compareTo(Long.valueOf(o.implicitTiebreaker)); |
There was a problem hiding this comment.
Long.compare(implicitTieBreaker, o.implicitTiebreaker)
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return 35; |
There was a problem hiding this comment.
? Ideally should be a prime such as 31 or a relative constant such as ImplicitTiebreakerHitExtractor.class.hashCode()
|
|
||
| Object implicitTbreaker = implicitTiebreaker.extract(hit); | ||
| if (implicitTbreaker instanceof Number == false) { | ||
| throw new EqlIllegalArgumentException("Expected _shard_doc/implicit tiebreaker as long but got [{}]", implicitTbreaker); |
There was a problem hiding this comment.
Regarding the error message, could the value be a non-long integer?
There was a problem hiding this comment.
@bpintea no. Always long.
There was a problem hiding this comment.
Would then the instance check agains a Long be also appropriate? (just FMI, its prolly slightly safer the way it is now)
There was a problem hiding this comment.
I'd leave it as is, for the reason you mentioned but also to be consistent with the approach throughout that method.
| return o.tiebreaker != null ? tiebreaker.compareTo(o.tiebreaker) : -1; | ||
| if (o.tiebreaker != null) { | ||
| if (tiebreaker.compareTo(o.tiebreaker) == 0) { | ||
| return Long.valueOf(implicitTiebreaker).compareTo(Long.valueOf(o.implicitTiebreaker)); |
There was a problem hiding this comment.
I guess Costin's longs comparison applies here as well.
…o eql_pit_tiebreaker
…astic#72089) (cherry picked from commit fa92fc4)
…astic#72089) (elastic#72288) (cherry picked from commit fa92fc4)
Following the recent addition of a default tiebreaker for PIT requests, EQL sequence queries that use
search_aftershould also include in the list of its values (a timestamp and an optional EQL tiebreaker) the additional default tiebreaker value.