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: Fix bug in hashlookup #108191

Merged
merged 1 commit into from
May 2, 2024
Merged

ESQL: Fix bug in hashlookup #108191

merged 1 commit into from
May 2, 2024

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented May 2, 2024

It only worked properly when the keys were the first blocks in the input page - that's unlikely. But that's what the test was running! Surprise! Anyway, this fixes it and adds a test that'd catch the bug.

It only worked properly when the keys were the first blocks in the input
page - that's unlikely. But that's what the test was running! Surprise!
Anyway, this fixes it and adds a test that'd catch the bug.
@nik9000 nik9000 requested a review from dnhatn May 2, 2024 12:16
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label May 2, 2024
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Member

@dnhatn dnhatn left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks Nik!

@@ -122,8 +122,13 @@ public void add(int positionOffset, IntVector groupIds) {
@Override
protected ReleasableIterator<Page> receive(Page page) {
Page mapped = page.projectBlocks(blockMapping);
page.releaseBlocks();
return appendBlocks(mapped, hash.lookup(mapped, BlockFactory.DEFAULT_MAX_BLOCK_PRIMITIVE_ARRAY_SIZE));
// TODO maybe this should take an array of Blocks instead?
Copy link
Member

Choose a reason for hiding this comment

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

Either way is fine to me.

@nik9000 nik9000 merged commit f968534 into elastic:main May 2, 2024
15 checks passed
elasticsearchmachine pushed a commit that referenced this pull request Jun 7, 2024
This adds support for `LOOKUP`, a command that implements a sort of
inline `ENRICH`, using data that is passed in the request:

```
$ 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"]
        }
    },
    "version": "2024.04.01"
}'
      v1       |      v2       |       a       
---------------+---------------+---------------
10             |cat            |1
```

This required these PRs: * #107624 * #107634 * #107701 * #107762 *
#107923 * #107894 * #107982 * #108012 * #108020 * #108169 * #108191 *
#108334 * #108482 * #108696 * #109040 * #109045

Closes #107306
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL >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