-
Notifications
You must be signed in to change notification settings - Fork 181
Support refering to implicit @timestamp field in span
#4138
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
Changes from 6 commits
c637bb2
e3f904b
4c20af1
2d09610
599b6db
c0d50fa
111b095
7d7108e
1d4de5a
4a9193f
1907947
3bf6a69
9d47720
edcb72f
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 |
|---|---|---|
|
|
@@ -729,9 +729,13 @@ public String visitAggregateFunction(AggregateFunction node, String context) { | |
|
|
||
| @Override | ||
| public String visitSpan(Span node, String context) { | ||
| String field = analyze(node.getField(), context); | ||
| String field = node.getField() != null ? analyze(node.getField(), context) : null; | ||
| String value = analyze(node.getValue(), context); | ||
| return StringUtils.format("span(%s, %s %s)", field, value, node.getUnit().getName()); | ||
| if (field != null) { | ||
| return StringUtils.format("span(%s, %s %s)", field, value, node.getUnit().getName()); | ||
| } else { | ||
| return StringUtils.format("span(%s %s)", value, node.getUnit().getName()); | ||
|
||
| } | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
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.
Will it be better to set
fieldin span to benew Field(new QualifiedName(OpenSearchConstants.IMPLICIT_FIELD_TIMESTAMP))if field is null? Then we don't need any change here.And I'm wondering if v2 works well since we only make change for calcite visitor.
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 referred it here because I thought the AST level should be consistent with users' input. E.g.
BY span(1h)should be parsed toSpan(value=1, unit=hour, field=null). Subsequent implicit reference can be added later because this semantic of how absent field should be interpreted is in a higher level than syntax tree construction.But it's also not bizarre to refer it while building AST tree. I'll modify the implementation accordingly.