Skip to content
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

EQL: [Test] Add a test for identifier as eventType #63227

Merged
merged 2 commits into from
Oct 5, 2020

Conversation

matriv
Copy link
Contributor

@matriv matriv commented Oct 5, 2020

Add a unit test to verify that an identifier surrounded with backquotes
is not a valid syntax for eventType value, as eventType is
schematically a string literal and not a field identifier.

Follows: #63169

Add a unit test to verify that an identifier surrounded with backquotes
is not a valid syntax for eventType value, as eventType is
schemantically a string literal and not a field identifier.

Follows: elastic#63169
@matriv matriv added >test Issues or PRs that are addressing/adding tests v8.0.0 :Analytics/EQL EQL querying v7.10.0 labels Oct 5, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-ql (:Query Languages/EQL)

@elasticmachine elasticmachine added the Team:QL (Deprecated) Meta label for query languages team label Oct 5, 2020
@matriv matriv requested a review from costin October 5, 2020 07:27
Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

LGTM

@matriv matriv merged commit ff12c13 into elastic:master Oct 5, 2020
@matriv matriv deleted the test-eventType branch October 5, 2020 09:40
@rw-access
Copy link
Contributor

as eventType is
schematically a string literal and not a field identifier.

This schematic conclusion seems more like an implementation detail of EQL in Elasticsearch, and less a property of EQL the language proper. For some historical context, we had a few discussions in the early implementation last year for EQL in ES about how this concept translated into ES most naturally, and the conclusion we arrived at for event.category wasn't immediately obvious. We were debating if the event type name should become an index pattern or event.category.

FWIW, in some other implementations it makes more sense to treat this as an identifier, since the event type lives outside of the event data, and is not simply used a shorthand for event.category == "<event type name>", and always matches the regex for identifiers [A-Za-z_][A-Za-z_0-9]+`.

So I agree that it's not a field identifer, but moreso an event type identifier. There are also other kinds of identifiers or rather "names" in the language than just field identifiers/names: function names, pipe names and relationship names/types.

matriv added a commit that referenced this pull request Oct 5, 2020
Add a unit test to verify that an identifier surrounded with backquotes
is not a valid syntax for eventType value, as eventType is
schemantically a string literal and not a field identifier.

Follows: #63169
(cherry picked from commit ff12c13)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/EQL EQL querying Team:QL (Deprecated) Meta label for query languages team >test Issues or PRs that are addressing/adding tests v7.10.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants