github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-25 02:09:32 -04:00
Code Issues Projects Releases Packages Wiki Activity
kibana/x-pack/platform
History
Exact
Drew Tate 315e9b19ca
[8.x] [ES|QL] Remove command option definitions (#215425) (#215542) 
# Backport

This will backport the following commits from `main` to `8.x`:
- [[ES|QL] Remove command option definitions
(#215425)](https://github.com/elastic/kibana/pull/215425)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Drew
Tate","email":"drew.tate@elastic.co"},"sourceCommit":{"committedDate":"2025-03-21T11:04:33Z","message":"[ES|QL]
Remove command option definitions (#215425)\n\n## Summary\n\nThis PR
removes the declarative objects that were meant to describe
the\nbehavior of \"options\" (see details section below if you don't
know what\nI'm talking about). **It does not remove \"options\" as a
concept from our\nAST.** \"Option\" is probably the wrong name for the
subcommands in the\nAST but, at the moment, it is working fine how it
is.\n\nHere is a list of what these definitions were being used for and
where I\nended up.\n\n| Use | How it worked | What I did
|\n\n|---------------------------------------------------------------------|---------------|------------|\n|
To generate command declarations for display in suggestions menu |
It\nhad some complex logic to try to construct a declaration string from
the\ninformation in the `signature` property | I replaced this
with\nstatically declared declaration strings on the command
definitions. I\ntook most of them directly from our docs. They are a
better result than\nthe autogenerated stuff |\n| To build the `METADATA`
suggestion | the definition was passed into\n`buildOptionDefinition` | I
declared the `METADATA` suggestion\nstatically in the `FROM`
autocomplete code. |\n| To check for field correctness in `METADATA` |
This logic lived in the\noption definition's `validate` method | I moved
it to the `FROM`\ncommand's validate method |\n| To validate the type of
the value assigned to `APPEND_SEPARATOR` in\n`DISSECT` | This logic
lived in the option definition's `validate`\nmethod | I moved it to the
`DISSECT` command's validate method |\n| To check if the left side of
the equals sign in `DISSECT` is\n\"APPEND_SEPARATOR | In most cases, the
parser catches stuff like this,\nbut for some reason `DISSECT`'s grammar
is very loose so we have been\nstepping in with our own validation
(maybe we should suggest changing\nthis). This was the only case that
was triggering the \"Unknown option\"\nmessage. | I moved it to the
`DISSECT` command's validate method |\n| To prevent default column
validation for `METADATA` | This was the\nonly true use of the
`skipCommonValidation` property which would prevent\nthe validator
trying to find metadata fields in the standard field list\n| I inserted
an option name check directly into the validation code.\nIt's not a good
long-term solution, but it is actually an improvement\nsince the former
code pretended to be general but was actually just for\n`METADATA`. At
least now it is clear what the exception is for. |\n| To filter
functions and operators that are available after `BY` |\nFunction
definitions sometimes declare that they are supported in a
`by`\nstatement. The validator checks if the function does. | This
didn't\nchange. The option nodes in the AST are still there and we are
still\nrelying on the `supportedCommands` and `supportedOptions`
properties in\nthe function definitions. |\n\n#### Pictures\n\n<img
width=\"859\" alt=\"Screenshot 2025-03-20 at 1 47
36 PM\"\nsrc=\"https://github.com/user-attachments/assets/3bd3c3c6-6066-466e-b33b-9444ab58670a\"\n/>\n\n_New,
statically-defined declarations_\n\n<img width=\"783\" alt=\"Screenshot
2025-03-20 at 2 12
28 PM\"\nsrc=\"https://github.com/user-attachments/assets/94550b25-5da9-4c82-9586-11b3515debd7\"\n/>\n\n_In
cases besides `APPEND_SEPARATOR`, incorrect keywords produce
syntax\nerrors._\n\n<img width=\"700\" alt=\"Screenshot 2025-03-20 at 2
09
05 PM\"\nsrc=\"https://github.com/user-attachments/assets/de1a23f4-2509-4c6e-84ec-a807e96b65a5\"\n/>\n\n_Didn't
break the `APPEND_SEPARATOR` datatype validation_\n\n<img width=\"791\"
alt=\"Screenshot 2025-03-20 at 2 03
28 PM\"\nsrc=\"https://github.com/user-attachments/assets/169aaa15-52f3-4d22-ab77-26a560cd9359\"\n/>\n\n_Didn't
break `METADATA` fields validation_\n\n### Checklist\n\nCheck the PR
satisfies following conditions. \n\nReviewers should verify this PR
satisfies this list as well.\n\n- [x] Any text added follows [EUI's
writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\nsentence case text and includes
[i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n-
[x]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas
added for features that require explanation or tutorials\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common scenarios\n\n###
Background/details\n\nTill now, \"options\" have been a concept in our
code. Their definition\nisn't clear, but it essentially comes down to
any capitalized keyword\nafter the command name. For example `STATS...
>BY<`, `DISSECT...\n>APPEND_SEPARATOR<`. You could think of them as
roughly subcommands or\nsubstatements.\n\nThere was a hope that commands
would be uniform enough that these\n\"options\" would deserve to be
their own special first-class citizen. But\nthey break
conceptually...\n\nFor example `APPEND_SEPARATOR` is not a keyword with
an expression after\nit... it is a variable `APPEND_SEPARATOR=\":\"`...
or filtering in\nstats.... `STATS AVG(bytes) >WHERE<` .... so is WHERE
an option now?\n\n`FORK` will break this even further.\n\nSo, we are
moving the architecture to allow for complexity and variance\namong the
commands. Command-specific logic will have the final say in\nhow
autocomplete and validation work for anything with that
command.","sha":"b7854a8759ca91255fe318c8d7a33b91996bf990","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Feature:ES|QL","Team:ESQL","backport:version","v9.1.0","v8.19.0"],"title":"[ES|QL]
Remove command option
definitions","number":215425,"url":"https://github.com/elastic/kibana/pull/215425","mergeCommit":{"message":"[ES|QL]
Remove command option definitions (#215425)\n\n## Summary\n\nThis PR
removes the declarative objects that were meant to describe
the\nbehavior of \"options\" (see details section below if you don't
know what\nI'm talking about). **It does not remove \"options\" as a
concept from our\nAST.** \"Option\" is probably the wrong name for the
subcommands in the\nAST but, at the moment, it is working fine how it
is.\n\nHere is a list of what these definitions were being used for and
where I\nended up.\n\n| Use | How it worked | What I did
|\n\n|---------------------------------------------------------------------|---------------|------------|\n|
To generate command declarations for display in suggestions menu |
It\nhad some complex logic to try to construct a declaration string from
the\ninformation in the `signature` property | I replaced this
with\nstatically declared declaration strings on the command
definitions. I\ntook most of them directly from our docs. They are a
better result than\nthe autogenerated stuff |\n| To build the `METADATA`
suggestion | the definition was passed into\n`buildOptionDefinition` | I
declared the `METADATA` suggestion\nstatically in the `FROM`
autocomplete code. |\n| To check for field correctness in `METADATA` |
This logic lived in the\noption definition's `validate` method | I moved
it to the `FROM`\ncommand's validate method |\n| To validate the type of
the value assigned to `APPEND_SEPARATOR` in\n`DISSECT` | This logic
lived in the option definition's `validate`\nmethod | I moved it to the
`DISSECT` command's validate method |\n| To check if the left side of
the equals sign in `DISSECT` is\n\"APPEND_SEPARATOR | In most cases, the
parser catches stuff like this,\nbut for some reason `DISSECT`'s grammar
is very loose so we have been\nstepping in with our own validation
(maybe we should suggest changing\nthis). This was the only case that
was triggering the \"Unknown option\"\nmessage. | I moved it to the
`DISSECT` command's validate method |\n| To prevent default column
validation for `METADATA` | This was the\nonly true use of the
`skipCommonValidation` property which would prevent\nthe validator
trying to find metadata fields in the standard field list\n| I inserted
an option name check directly into the validation code.\nIt's not a good
long-term solution, but it is actually an improvement\nsince the former
code pretended to be general but was actually just for\n`METADATA`. At
least now it is clear what the exception is for. |\n| To filter
functions and operators that are available after `BY` |\nFunction
definitions sometimes declare that they are supported in a
`by`\nstatement. The validator checks if the function does. | This
didn't\nchange. The option nodes in the AST are still there and we are
still\nrelying on the `supportedCommands` and `supportedOptions`
properties in\nthe function definitions. |\n\n#### Pictures\n\n<img
width=\"859\" alt=\"Screenshot 2025-03-20 at 1 47
36 PM\"\nsrc=\"https://github.com/user-attachments/assets/3bd3c3c6-6066-466e-b33b-9444ab58670a\"\n/>\n\n_New,
statically-defined declarations_\n\n<img width=\"783\" alt=\"Screenshot
2025-03-20 at 2 12
28 PM\"\nsrc=\"https://github.com/user-attachments/assets/94550b25-5da9-4c82-9586-11b3515debd7\"\n/>\n\n_In
cases besides `APPEND_SEPARATOR`, incorrect keywords produce
syntax\nerrors._\n\n<img width=\"700\" alt=\"Screenshot 2025-03-20 at 2
09
05 PM\"\nsrc=\"https://github.com/user-attachments/assets/de1a23f4-2509-4c6e-84ec-a807e96b65a5\"\n/>\n\n_Didn't
break the `APPEND_SEPARATOR` datatype validation_\n\n<img width=\"791\"
alt=\"Screenshot 2025-03-20 at 2 03
28 PM\"\nsrc=\"https://github.com/user-attachments/assets/169aaa15-52f3-4d22-ab77-26a560cd9359\"\n/>\n\n_Didn't
break `METADATA` fields validation_\n\n### Checklist\n\nCheck the PR
satisfies following conditions. \n\nReviewers should verify this PR
satisfies this list as well.\n\n- [x] Any text added follows [EUI's
writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\nsentence case text and includes
[i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n-
[x]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas
added for features that require explanation or tutorials\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common scenarios\n\n###
Background/details\n\nTill now, \"options\" have been a concept in our
code. Their definition\nisn't clear, but it essentially comes down to
any capitalized keyword\nafter the command name. For example `STATS...
>BY<`, `DISSECT...\n>APPEND_SEPARATOR<`. You could think of them as
roughly subcommands or\nsubstatements.\n\nThere was a hope that commands
would be uniform enough that these\n\"options\" would deserve to be
their own special first-class citizen. But\nthey break
conceptually...\n\nFor example `APPEND_SEPARATOR` is not a keyword with
an expression after\nit... it is a variable `APPEND_SEPARATOR=\":\"`...
or filtering in\nstats.... `STATS AVG(bytes) >WHERE<` .... so is WHERE
an option now?\n\n`FORK` will break this even further.\n\nSo, we are
moving the architecture to allow for complexity and variance\namong the
commands. Command-specific logic will have the final say in\nhow
autocomplete and validation work for anything with that
command.","sha":"b7854a8759ca91255fe318c8d7a33b91996bf990"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/215425","number":215425,"mergeCommit":{"message":"[ES|QL]
Remove command option definitions (#215425)\n\n## Summary\n\nThis PR
removes the declarative objects that were meant to describe
the\nbehavior of \"options\" (see details section below if you don't
know what\nI'm talking about). **It does not remove \"options\" as a
concept from our\nAST.** \"Option\" is probably the wrong name for the
subcommands in the\nAST but, at the moment, it is working fine how it
is.\n\nHere is a list of what these definitions were being used for and
where I\nended up.\n\n| Use | How it worked | What I did
|\n\n|---------------------------------------------------------------------|---------------|------------|\n|
To generate command declarations for display in suggestions menu |
It\nhad some complex logic to try to construct a declaration string from
the\ninformation in the `signature` property | I replaced this
with\nstatically declared declaration strings on the command
definitions. I\ntook most of them directly from our docs. They are a
better result than\nthe autogenerated stuff |\n| To build the `METADATA`
suggestion | the definition was passed into\n`buildOptionDefinition` | I
declared the `METADATA` suggestion\nstatically in the `FROM`
autocomplete code. |\n| To check for field correctness in `METADATA` |
This logic lived in the\noption definition's `validate` method | I moved
it to the `FROM`\ncommand's validate method |\n| To validate the type of
the value assigned to `APPEND_SEPARATOR` in\n`DISSECT` | This logic
lived in the option definition's `validate`\nmethod | I moved it to the
`DISSECT` command's validate method |\n| To check if the left side of
the equals sign in `DISSECT` is\n\"APPEND_SEPARATOR | In most cases, the
parser catches stuff like this,\nbut for some reason `DISSECT`'s grammar
is very loose so we have been\nstepping in with our own validation
(maybe we should suggest changing\nthis). This was the only case that
was triggering the \"Unknown option\"\nmessage. | I moved it to the
`DISSECT` command's validate method |\n| To prevent default column
validation for `METADATA` | This was the\nonly true use of the
`skipCommonValidation` property which would prevent\nthe validator
trying to find metadata fields in the standard field list\n| I inserted
an option name check directly into the validation code.\nIt's not a good
long-term solution, but it is actually an improvement\nsince the former
code pretended to be general but was actually just for\n`METADATA`. At
least now it is clear what the exception is for. |\n| To filter
functions and operators that are available after `BY` |\nFunction
definitions sometimes declare that they are supported in a
`by`\nstatement. The validator checks if the function does. | This
didn't\nchange. The option nodes in the AST are still there and we are
still\nrelying on the `supportedCommands` and `supportedOptions`
properties in\nthe function definitions. |\n\n#### Pictures\n\n<img
width=\"859\" alt=\"Screenshot 2025-03-20 at 1 47
36 PM\"\nsrc=\"https://github.com/user-attachments/assets/3bd3c3c6-6066-466e-b33b-9444ab58670a\"\n/>\n\n_New,
statically-defined declarations_\n\n<img width=\"783\" alt=\"Screenshot
2025-03-20 at 2 12
28 PM\"\nsrc=\"https://github.com/user-attachments/assets/94550b25-5da9-4c82-9586-11b3515debd7\"\n/>\n\n_In
cases besides `APPEND_SEPARATOR`, incorrect keywords produce
syntax\nerrors._\n\n<img width=\"700\" alt=\"Screenshot 2025-03-20 at 2
09
05 PM\"\nsrc=\"https://github.com/user-attachments/assets/de1a23f4-2509-4c6e-84ec-a807e96b65a5\"\n/>\n\n_Didn't
break the `APPEND_SEPARATOR` datatype validation_\n\n<img width=\"791\"
alt=\"Screenshot 2025-03-20 at 2 03
28 PM\"\nsrc=\"https://github.com/user-attachments/assets/169aaa15-52f3-4d22-ab77-26a560cd9359\"\n/>\n\n_Didn't
break `METADATA` fields validation_\n\n### Checklist\n\nCheck the PR
satisfies following conditions. \n\nReviewers should verify this PR
satisfies this list as well.\n\n- [x] Any text added follows [EUI's
writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\nsentence case text and includes
[i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n-
[x]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas
added for features that require explanation or tutorials\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common scenarios\n\n###
Background/details\n\nTill now, \"options\" have been a concept in our
code. Their definition\nisn't clear, but it essentially comes down to
any capitalized keyword\nafter the command name. For example `STATS...
>BY<`, `DISSECT...\n>APPEND_SEPARATOR<`. You could think of them as
roughly subcommands or\nsubstatements.\n\nThere was a hope that commands
would be uniform enough that these\n\"options\" would deserve to be
their own special first-class citizen. But\nthey break
conceptually...\n\nFor example `APPEND_SEPARATOR` is not a keyword with
an expression after\nit... it is a variable `APPEND_SEPARATOR=\":\"`...
or filtering in\nstats.... `STATS AVG(bytes) >WHERE<` .... so is WHERE
an option now?\n\n`FORK` will break this even further.\n\nSo, we are
moving the architecture to allow for complexity and variance\namong the
commands. Command-specific logic will have the final say in\nhow
autocomplete and validation work for anything with that
command.","sha":"b7854a8759ca91255fe318c8d7a33b91996bf990"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
2025-03-24 09:29:55 +01:00
..
packages [8.x] [Streams 🌊] Enrichment - Add support for date processor (#213559) (#215359) 2025-03-20 20:24:06 +02:00
plugins [8.x] [ES|QL] Remove command option definitions (#215425) (#215542) 2025-03-24 09:29:55 +01:00