Skip to content
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

ESQL: Improve grammar to allow identifiers with . #100740

Merged
merged 14 commits into from
Dec 12, 2023
6 changes: 6 additions & 0 deletions docs/changelog/100740.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 100740
summary: "ESQL: Improve grammar to allow identifiers with"
Copy link
Contributor

Choose a reason for hiding this comment

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

Changelog didn't like the period.

area: ES|QL
type: bug
issues:
- 100312
11 changes: 9 additions & 2 deletions x-pack/plugin/esql/src/main/antlr/EsqlBaseLexer.g4
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ fragment EXPONENT
: [Ee] [+-]? DIGIT+
;

fragment UNDERSCORE
: '_'
;

STRING
: '"' (ESCAPE_SEQUENCE | UNESCAPED_CHARS)* '"'
| '"""' (~[\r\n])*? '"""' '"'? '"'?
Expand Down Expand Up @@ -127,12 +131,15 @@ PERCENT : '%';
OPENING_BRACKET : '[' -> pushMode(EXPRESSION), pushMode(EXPRESSION);
CLOSING_BRACKET : ']' -> popMode, popMode;

fragment UNQUOTED_ID_BODY
: (LETTER | DIGIT | DOT | UNDERSCORE)
;

UNQUOTED_IDENTIFIER
: LETTER (LETTER | DIGIT | '_')*
: LETTER UNQUOTED_ID_BODY*
Copy link
Contributor

Choose a reason for hiding this comment

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

This will allow digits inside "subfields", but not on the root field. For example 123elasticsearch.node.stats.os.cpu.load_avg.1m

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to allow unquoted identifiers that start with digits, though? If so, we'd at least have to disallow identifiers being all digits, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

we'd at least have to disallow identifiers being all digits, no?

ES accepts both fields "123" and "123.456".

Copy link
Member Author

Choose a reason for hiding this comment

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

Which we accept as long as the fields are quoted "123" - same with field above "a.1m.4321" is accepted; the problem is handling fields which are NOT quoted.

// only allow @ at beginning of identifier to keep the option to allow @ as infix operator in the future
// also, single `_` and `@` characters are not valid identifiers
| ('_' | '@') (LETTER | DIGIT | '_')+
| (UNDERSCORE | '@') UNQUOTED_ID_BODY+
;

QUOTED_IDENTIFIER
Expand Down

Large diffs are not rendered by default.

Loading