Skip to content

EQL: Add optional fields #78769

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions x-pack/plugin/eql/qa/common/src/main/resources/data/extra.data
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,22 @@
"event_type": "STAT",
"transID": 1235,
"sequence": 3
},
{
"@timestamp": "100",
"event_type": "OPTIONAL",
"default_null_value_field": null,
"sequence": 4
},
{
"@timestamp": "101",
"event_type": "OPTIONAL",
"sequence": 5
},
{
"@timestamp": "102",
"event_type": "OPTIONAL",
"default_null_value_field": null,
"sequence": 6
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@
"path": "sequence"
}
}
},
"no_docs_field": {
Copy link
Member

Choose a reason for hiding this comment

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

Improved name would be - "optional_field_mapping_only". Potentially drop optional.

"type": "keyword"
},
"default_null_value_field": {
Copy link
Member

Choose a reason for hiding this comment

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

"optional_field_default_null"

"type": "keyword",
"null_value": "NULL"
}
}
}
42 changes: 42 additions & 0 deletions x-pack/plugin/eql/qa/common/src/main/resources/test_extra.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,45 @@ query = '''
[ STAT where transID == 1234 ]
'''
expected_event_ids = [1,2,3]

[[queries]]
name = "optionalNoDocsFieldNullEquality"
query = '''
OPTIONAL where ?no_docs_field == null
'''
expected_event_ids = [4,5,6]

[[queries]]
name = "optionalNoDocsFieldNumericEquality"
query = '''
OPTIONAL where ?no_docs_field == 123
'''
expected_event_ids = []

[[queries]]
name = "optionalDefaultNullValueFieldEqualNull"
query = '''
OPTIONAL where ?default_null_value_field == null
'''
expected_event_ids = [5]

[[queries]]
name = "optionalDefaultNullValueFieldEqualDefaultValue"
query = '''
OPTIONAL where ?default_null_value_field == "NULL"
'''
expected_event_ids = [4,6]

[[queries]]
name = "optionalMissingFieldNullEquality"
query = '''
OPTIONAL where ?inexistent_field == null
'''
expected_event_ids = [4,5,6]

[[queries]]
name = "optionalMissingFieldImplicitBooleanEquality"
query = '''
OPTIONAL where ?inexistent_field
'''
expected_event_ids = []
3 changes: 2 additions & 1 deletion x-pack/plugin/eql/src/main/antlr/EqlBase.g4
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ booleanValue
;

qualifiedName
: identifier (DOT identifier | LB INTEGER_VALUE+ RB)*
: OPTIONAL? identifier (DOT identifier | LB INTEGER_VALUE+ RB)*
;

identifier
Expand Down Expand Up @@ -204,6 +204,7 @@ RB: ']';
LP: '(';
RP: ')';
PIPE: '|';
OPTIONAL: '?';

fragment STRING_ESCAPE
: '\\' [btnfr"'\\]
Expand Down
20 changes: 11 additions & 9 deletions x-pack/plugin/eql/src/main/antlr/EqlBase.tokens
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,16 @@ RB=38
LP=39
RP=40
PIPE=41
STRING=42
INTEGER_VALUE=43
DECIMAL_VALUE=44
IDENTIFIER=45
QUOTED_IDENTIFIER=46
TILDE_IDENTIFIER=47
LINE_COMMENT=48
BRACKETED_COMMENT=49
WS=50
OPTIONAL=42
STRING=43
INTEGER_VALUE=44
DECIMAL_VALUE=45
IDENTIFIER=46
QUOTED_IDENTIFIER=47
TILDE_IDENTIFIER=48
LINE_COMMENT=49
BRACKETED_COMMENT=50
WS=51
'and'=1
'any'=2
'by'=3
Expand Down Expand Up @@ -89,3 +90,4 @@ WS=50
'('=39
')'=40
'|'=41
'?'=42
20 changes: 11 additions & 9 deletions x-pack/plugin/eql/src/main/antlr/EqlBaseLexer.tokens
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,16 @@ RB=38
LP=39
RP=40
PIPE=41
STRING=42
INTEGER_VALUE=43
DECIMAL_VALUE=44
IDENTIFIER=45
QUOTED_IDENTIFIER=46
TILDE_IDENTIFIER=47
LINE_COMMENT=48
BRACKETED_COMMENT=49
WS=50
OPTIONAL=42
STRING=43
INTEGER_VALUE=44
DECIMAL_VALUE=45
IDENTIFIER=46
QUOTED_IDENTIFIER=47
TILDE_IDENTIFIER=48
LINE_COMMENT=49
BRACKETED_COMMENT=50
WS=51
'and'=1
'any'=2
'by'=3
Expand Down Expand Up @@ -89,3 +90,4 @@ WS=50
'('=39
')'=40
'|'=41
'?'=42
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@

import org.elasticsearch.xpack.ql.common.Failure;
import org.elasticsearch.xpack.ql.expression.Attribute;
import org.elasticsearch.xpack.ql.expression.NamedExpression;
import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.expression.Literal;
import org.elasticsearch.xpack.ql.expression.UnresolvedAttribute;
import org.elasticsearch.xpack.ql.expression.function.Function;
import org.elasticsearch.xpack.ql.expression.function.FunctionDefinition;
Expand All @@ -21,21 +22,24 @@

import java.util.Collection;
import java.util.LinkedHashSet;
import java.util.Set;

import static java.util.Arrays.asList;
import static org.elasticsearch.xpack.eql.analysis.AnalysisUtils.resolveAgainstList;
import static org.elasticsearch.xpack.ql.analyzer.AnalyzerRules.AddMissingEqualsToBoolField;
import org.elasticsearch.xpack.ql.analyzer.AnalyzerRules.AddMissingEqualsToBoolField;

public class Analyzer extends RuleExecutor<LogicalPlan> {

private final Configuration configuration;
private final FunctionRegistry functionRegistry;
private final Verifier verifier;
private final Set<UnresolvedAttribute> optionals;

public Analyzer(Configuration configuration, FunctionRegistry functionRegistry, Verifier verifier) {
public Analyzer(Configuration configuration, FunctionRegistry functionRegistry, Verifier verifier, Set<UnresolvedAttribute> optionals) {
this.configuration = configuration;
this.functionRegistry = functionRegistry;
this.verifier = verifier;
this.optionals = optionals;
}

@Override
Expand All @@ -62,7 +66,7 @@ private LogicalPlan verify(LogicalPlan plan) {
return plan;
}

private static class ResolveRefs extends AnalyzerRule<LogicalPlan> {
private class ResolveRefs extends AnalyzerRule<LogicalPlan> {

@Override
protected LogicalPlan rule(LogicalPlan plan) {
Expand All @@ -81,7 +85,11 @@ protected LogicalPlan rule(LogicalPlan plan) {
for (LogicalPlan child : plan.children()) {
childrenOutput.addAll(child.output());
}
NamedExpression named = resolveAgainstList(u, childrenOutput);
Expression named = resolveAgainstList(u, childrenOutput);
// if it's not resolved (it doesn't exist in mappings) and it's an optional field, replace it with "null"
if (named == null && optionals.contains(u)) {
named = Literal.NULL;
Copy link
Member

Choose a reason for hiding this comment

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

you could preserve the source (for better error messaging):

new Literal(named.source(), null, DataTypes.NULL)

}
// if resolved, return it; otherwise keep it in place to be resolved later
if (named != null) {
if (log.isTraceEnabled()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@
package org.elasticsearch.xpack.eql.parser;

import org.elasticsearch.xpack.eql.parser.EqlBaseParser.SingleStatementContext;
import org.elasticsearch.xpack.ql.expression.UnresolvedAttribute;
import org.elasticsearch.xpack.ql.plan.logical.LogicalPlan;

import java.util.Set;

public class AstBuilder extends LogicalPlanBuilder {

AstBuilder(ParserParams params) {
Copy link
Member

Choose a reason for hiding this comment

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

You could keep the old constructor around and make it a shortcut to this(params, emptySet())

super(params);
AstBuilder(ParserParams params, Set<UnresolvedAttribute> optionals) {
super(params, optionals);
}

@Override
Expand Down
Loading