Skip to content

ESQL: Improve syntax for LOOKUP tables#109489

Merged
elasticsearchmachine merged 4 commits intoelastic:mainfrom
nik9000:lookup_better_syntax
Jun 11, 2024
Merged

ESQL: Improve syntax for LOOKUP tables#109489
elasticsearchmachine merged 4 commits intoelastic:mainfrom
nik9000:lookup_better_syntax

Conversation

@nik9000
Copy link
Copy Markdown
Member

@nik9000 nik9000 commented Jun 7, 2024

Replace the syntax for tables with something a little more natural.

Now it is:

$ curl -uelastic:password -HContent-Type:application/json -XPOST \
    'localhost:9200/_query?error_trace&pretty&format=txt' \
-d'{
    "query": "ROW a=1::LONG | LOOKUP t ON a",
    "tables": {
        "t": {
            "a": {"long":     [    1,     4,     2]},
            "v1": {"integer": [   10,    11,    12]},
            "v2": {"keyword": ["cat", "dog", "wow"]}
        }
    }
}'
      v1       |      v2       |       a
---------------+---------------+---------------
10             |cat            |1

Replace the syntax for `tables` with something a little more natural.

Now it is:
```
$ curl -uelastic:password -HContent-Type:application/json -XPOST \
    'localhost:9200/_query?error_trace&pretty&format=txt' \
-d'{
    "query": "ROW a=1::LONG | LOOKUP t ON a",
    "tables": {
        "t": {
            "a": {"long":     [    1,     4,     2]},
            "v1": {"integer": [   10,    11,    12]},
            "v2": {"keyword": ["cat", "dog", "wow"]}
        }
    }
}'
      v1       |      v2       |       a
---------------+---------------+---------------
10             |cat            |1
```
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/kibana-esql (ES|QL-ui)

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jun 7, 2024
@nik9000 nik9000 mentioned this pull request Jun 7, 2024
10 tasks
Copy link
Copy Markdown
Contributor

@alex-spies alex-spies left a comment

Choose a reason for hiding this comment

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

LGTM

path: /_query
parameters: []
capabilities: [lookup_command]
capabilities: [lookup_command, tables_types]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hm, tables_types implies lookup_command, so maybe we can just replace the capability instead of adding to the list?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure if it's clearer to put both or only the latest one.

@nik9000
Copy link
Copy Markdown
Member Author

nik9000 commented Jun 7, 2024

Blocking this behind #109493

@nik9000 nik9000 added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jun 11, 2024
@elasticsearchmachine elasticsearchmachine merged commit c6fe3c3 into elastic:main Jun 11, 2024
@nik9000 nik9000 deleted the lookup_better_syntax branch June 11, 2024 13:26
elasticsearchmachine pushed a commit that referenced this pull request Jun 20, 2024
Registering `TABLES_TYPES` capability, that was created (but not
registered) in #109489

Also using it as a condition for `lookup.csv-spec` tests. These tests
failed badly in Serverless multi-cluster due to different output format
in two different nodes.

Probably (if I got it right) the YAML tests that were using this
capability were not running at all. This PR should re-enable them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) ES|QL-ui Impacts ES|QL UI >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.15.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants