-
Notifications
You must be signed in to change notification settings - Fork 25.2k
EQL: Introduce ~ grammar for case-insensitive functions #67869
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 all commits
df95fff
26fd397
419902b
8733f70
3917f37
0e65475
d8011c3
f4c7abf
e75f2a4
d1c2f11
3495163
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 |
---|---|---|
@@ -0,0 +1,39 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
package org.elasticsearch.xpack.eql.expression.function; | ||
|
||
import org.elasticsearch.xpack.ql.expression.function.Function; | ||
import org.elasticsearch.xpack.ql.expression.function.FunctionDefinition; | ||
|
||
import java.util.List; | ||
|
||
/** | ||
* Dedicated wrapper for EQL specific function definitions. | ||
* Used mainly for validating case sensitivity of wrapping functions. | ||
*/ | ||
class EqlFunctionDefinition extends FunctionDefinition { | ||
|
||
private final boolean caseAware; | ||
|
||
protected EqlFunctionDefinition(String name, | ||
List<String> aliases, | ||
Class<? extends Function> clazz, | ||
boolean caseAware, | ||
Builder builder) { | ||
super(name, aliases, clazz, builder); | ||
this.caseAware = caseAware; | ||
} | ||
|
||
public boolean isCaseAware() { | ||
return caseAware; | ||
} | ||
|
||
@Override | ||
protected Builder builder() { | ||
return super.builder(); | ||
} | ||
Comment on lines
+35
to
+38
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. Is this needed? 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. Yes, the parent method is protected and is not visible outside classes in the same package - that is nobody outside the package in |
||
} |
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.
Nit: I expected that the
functionName
will beIDENTIFIER '~'?
. Why are function names allowed to start with_
and@
only if the function is not suffixed with~
?I guess you don't want to allow
~
for@timestamp
, but what about functions starting with_
?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.
No. Defining a new token just for function names would overlap with the IDENTIIFER and the grammar ends up applying whoever is declared first due to ambiguity leading to rules not matching and thus broken queries.