-
Notifications
You must be signed in to change notification settings - Fork 141
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 patterns and grok command #813
Conversation
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 2.x #813 +/- ##
============================================
+ Coverage 94.87% 94.97% +0.09%
- Complexity 2956 3004 +48
============================================
Files 291 294 +3
Lines 7869 8017 +148
Branches 572 586 +14
============================================
+ Hits 7466 7614 +148
Misses 349 349
Partials 54 54
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@joshuali925 as comments in #814. I think we could add grok as a method in parse command for now. |
if (match != null) { | ||
return new ExprStringValue(match.toString()); | ||
} | ||
log.warn("failed to extract pattern {} from input {}", grok.getOriginalGrokPattern(), |
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.
we should sanitize rawString when logging
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.
is it warn? or debug?
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.
changed to debug. rawString is unstructured text not sure how to sanitize, let me know if i should remove it from log
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.
we should replace rawString as ***
.
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.
hard coded it to ***
core/src/main/java/org/opensearch/sql/expression/ExpressionNodeVisitor.java
Show resolved
Hide resolved
core/src/main/java/org/opensearch/sql/expression/parse/GrokExpression.java
Show resolved
Hide resolved
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
common/src/main/java/org/opensearch/sql/common/grok/Discovery.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java
Show resolved
Hide resolved
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.
files in common/test/resources/nasa is required? seems like split file
yes it's split file, used here
|
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
core/src/main/java/org/opensearch/sql/expression/parse/ParseExpression.java
Show resolved
Hide resolved
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
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.
Thanks for the changes!
|
||
patternsParameter | ||
: (NEW_FIELD EQUAL new_field=stringLiteral) | ||
| (PATTERN EQUAL pattern=stringLiteral) |
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.
What if the pattern itself contains single quotes and double quotes.
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.
user should escape quotes in quotes
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Description
see #814 , #815
grok
command, but in the latest releaseGrok
is not serializable (and SQL grok expression cannot be pushed down). After discussing with peng I'm tentatively moving the source code into sql.common module.grok
PPL command, similar to howparse
currently works with regexpatterns
command, which removes unwanted characters from a log line to create a new field as the pattern of that log linepatterns
currently supports default pattern (remove all alphanumerical characters) or user can define what characters should be removedIssues Resolved
closes #814
closes #815
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.