-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Add support for 'size' in EQL Sample queries #87846
Changes from 10 commits
f6003b8
872cdfa
b690f64
8571f04
7299858
a2d7d31
8dab37d
21111f7
7c38041
c4bd4d3
259d2e0
c81f907
36c4a17
ee33383
226b8a2
7602015
92dc846
8bf0df8
eed422b
cfad420
24e367f
81265d2
ac25477
2429dbc
f3659a6
398b014
cdbd7ad
e6cfd9c
254e6bc
c6c05bb
80eeca7
9dd47d8
c9d4892
08fb6ed
cd359b3
e63bcb5
5233229
7b615ac
6a91f97
de281b5
264f09f
895baf0
d663231
72e24d3
546a2e2
ceffaf9
a278594
341f3b7
399a8ac
c0019a3
841ac8e
7cc275d
453b5b1
0bf31b7
616fd07
2d3bcc4
7caa242
12a1290
5cbf4fb
892ad01
993e467
88a0f6f
e4a19d4
6c12fe0
89ff87d
9ad91f2
e063ce8
96febb7
8dfbcd5
5a19729
da3e4e8
654f31d
ed940b6
f9055b5
e4c7feb
0502139
5a26455
ca11e82
dcc87dd
4779893
104ad7f
51f89f4
621c38c
745947e
8d37d48
c4c1802
9b24b41
10b8047
60016c8
098f518
8360bf9
3496dd5
0811850
2569d1f
00d4953
e4ff839
f87ce07
e7a84b1
80796fb
914e216
fd9473a
ac9f12f
b327b17
d248fa4
5af8ec5
82ad45f
dc672b0
acf9a67
2841bf7
c4dfc66
03f3c81
f2257ca
af8ac50
2c37c59
59c745c
27061a5
e2bf861
189f279
3c30674
5d6af58
f1071ca
09d0025
c038a91
695d1a8
837a8d7
2a08258
79a8979
ad61274
a1056f1
fe8e586
f849847
cbea639
1aa43ec
f31b1f6
825c354
63b850c
b46d95b
3bde177
725367e
3bb13e2
1403ab3
c238aa1
18328b0
c541610
58ddca3
22e1150
058ea45
9f29241
3c2fc5a
20ed7e3
f0df4b7
a1015ce
e949dff
acf70cd
51c6e6b
7d7332a
b15f6dd
58fafe2
824bfd0
e9ea463
5b3d51d
6b962ef
5b999ee
91d2db2
4b92e1d
2800957
dc0cfd9
64ccf04
c9b2cc4
8f52a55
327b8f8
2b6fdfd
af4421d
fb4adda
ac71b52
0cf3dc9
b557d20
74d694e
3a78f80
5e797c3
1a57652
c32e850
e276fd9
32d5122
d506aa4
0aa0477
5422860
365be4d
caafd4c
773aeab
061e643
114955f
875164c
7f91884
ffcf0ea
bdbfcb3
8cac490
16e4cb1
862c885
8b3293d
65b05f8
2fad061
6616746
3fca120
dc4debc
95a9b35
98475ee
ff7291e
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 |
---|---|---|
|
@@ -286,3 +286,61 @@ expected_event_ids = [18,11] | |
join_keys = ["doom","win10"] | ||
|
||
|
||
[[queries]] | ||
name = "size0" | ||
size = 0 | ||
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. Other eql queries use 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'm not sure I got this comment: About "size" in the tests, we have unit tests (mostly for the execution planning, plus some specific tests for the algorithms) in Java, but they do not fully execute any query (mocking the full execution is extremely convoluted due to the usage of internal - Server - components in the execution, that cannot be instantiated in our tests). |
||
query = ''' | ||
sample by host, ?os | ||
[success where true] | ||
[alert where true and id == 21 or id == 24 or id == 11] | ||
''' | ||
expected_event_ids = [] | ||
join_keys = [] | ||
|
||
|
||
[[queries]] | ||
name = "size2" | ||
size = 2 | ||
query = ''' | ||
sample by host, ?os | ||
[success where true] | ||
[alert where true and id == 21 or id == 24 or id == 11] | ||
''' | ||
expected_event_ids = [29,24, | ||
28,21] | ||
join_keys = ["GTA","null", | ||
"doom","null"] | ||
|
||
|
||
[[queries]] | ||
name = "size3" | ||
size = 3 | ||
query = ''' | ||
sample by host, ?os | ||
[success where true] | ||
[alert where true and id == 21 or id == 24 or id == 11] | ||
''' | ||
expected_event_ids = [29,24, | ||
28,21, | ||
18,11] | ||
join_keys = ["GTA","null", | ||
"doom","null", | ||
"doom","win10"] | ||
|
||
|
||
[[queries]] | ||
name = "sizeBig" | ||
size = 500 | ||
query = ''' | ||
sample by host, ?os | ||
[success where true] | ||
[alert where true and id == 21 or id == 24 or id == 11] | ||
''' | ||
expected_event_ids = [29,24, | ||
28,21, | ||
18,11] | ||
join_keys = ["GTA","null", | ||
"doom","null", | ||
"doom","win10"] | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,7 +102,7 @@ public Object visitStatement(StatementContext ctx) { | |
if (ctx.pipe().size() > 0) { | ||
throw new ParsingException(source(ctx.pipe().get(0)), "Samples do not support pipes yet"); | ||
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. Shouldn't support for pipes now also be possible since limit and offset are both supported? 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. No, because the limit/size comes from a request parameter, not from the query itself (through the query parser). 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. but it works with 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. With 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. if you can do 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 see HEAD and TAIL as very natural operators for sequences, that have well defined semantics defined by their order (eg. 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. @Luegg, as @luigidellaquila well put this, re-using 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 don't see it as reusing 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. @Luegg I agree with you, tail is definitely not straight forward at this stage. |
||
} | ||
return plan; | ||
return new LimitWithOffset(plan.source(), new Literal(Source.EMPTY, params.size(), DataTypes.INTEGER), 0, plan); | ||
} | ||
// | ||
// Add implicit blocks | ||
|
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 have only one nitpick around the usage of Integer (object) instead of a primitive.
The sign doesn't seem to be used (head and tail take care of where the limit is applied) hence why not use the negative value as indicator that the size is out of band and thus unspecified?
As Integer, size has 3 states - valid if >=0, unspecified if null and invalid if <0.
What I'm saying is using only 2 through a primitive (int) - valid if >=0, unspecified if <0.
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.
Do you mean something like
... | tail n
with n different from size?For samples, at this stage, pipes are not supported yet, so it cannot happen.
For sequences, both conditions are applied, so the smaller one prevails; this seems a reasonable behavior, so I'd say we could have the same for samples in the future