-
Notifications
You must be signed in to change notification settings - Fork 25.2k
EQL: Add implicit ordering on timestamp #53004
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
Conversation
QL: Move Sort base class from SQL to QL
Pinging @elastic/es-search (:Search/EQL) |
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.
LGTM. Left two comments.
@@ -3,6 +3,9 @@ | |||
"event_type" : { | |||
"type" : "keyword" | |||
}, | |||
"timestamp" : { | |||
"type" : "keyword" |
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.
keyword
?
@@ -3,6 +3,9 @@ | |||
"event_type" : { | |||
"type" : "keyword" | |||
}, | |||
"timestamp" : { | |||
"type" : "keyword" |
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.
keyword
?
docs/reference/eql/search.asciidoc
Outdated
@@ -27,7 +27,7 @@ PUT sec_logs/_bulk?refresh | |||
You can now use the EQL search API to search this index using an EQL query. | |||
|
|||
The following request searches the `sec_logs` index using the EQL query | |||
specified in the `query` parameter. The EQL query matches events with an | |||
specified in the `rule` parameter. The EQL query matches events with an |
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.
something didn't merge right, should be query
@elasticmachine run elasticsearch-ci/2 |
@@ -74,7 +75,8 @@ The API returns the following response containing the matching event: | |||
"name": "cmd.exe", | |||
"path": "C:\\Windows\\System32\\cmd.exe" | |||
} | |||
} | |||
}, | |||
"sort" : [1607339167000] |
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.
@jrodewig Not sure whether there's a better way to match the document - essentially the score has been replaced by sort and I'd like to remove both from the result.
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.
@costin No worries. As long as the CI tests pass, I would not consider this a blocker. I can work on the additional doc changes as a separate PR.
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.
Related PR: #53027
QL: Move Sort base class from SQL to QL (cherry picked from commit 798015b)
QL: Move Sort base class from SQL to QL