Skip to content

Conversation

@nik9000
Copy link
Member

@nik9000 nik9000 commented Aug 29, 2024

This tightens the invariant on Block - namely that there are no 0 length positions, at least none tracked by the firstValueIndexes in ArrayBlock - instead, the only way a position can be 0 length is to have been created with appendNull.

The only cause of these 0 length positions was ordinals blocks made by BlockHash. This reworks that infrastructure to instead create those with appendNull.

This tightens the invariant on `Block` - namely that there are no `0`
length positions, at least none tracked by the `firstValueIndexes` in
`ArrayBlock` - instead, the only way a position can be `0` length is to
have been created with `appendNull`.

The only cause of these `0` length positions was ordinals blocks made by
`BlockHash`. This reworks that infrastructure to instead create those
with `appendNull`.
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Aug 29, 2024
* 2.0.
*/

package org.elasticsearch.compute.aggregation.blockhash;
Copy link
Member Author

Choose a reason for hiding this comment

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

This is really a rename and a rewrite, but it's enough of a rewrite git doesn't think of it that way.

Copy link
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, thanks @nik9000 !

@nik9000 nik9000 merged commit c1d9e7e into elastic:main Aug 30, 2024
@nik9000
Copy link
Member Author

nik9000 commented Aug 30, 2024

Thanks @alex-spies !

cbuescher pushed a commit to cbuescher/elasticsearch that referenced this pull request Sep 4, 2024
This tightens the invariant on `Block` - namely that there are no `0`
length positions, at least none tracked by the `firstValueIndexes` in
`ArrayBlock` - instead, the only way a position can be `0` length is to
have been created with `appendNull`.

The only cause of these `0` length positions was ordinals blocks made by
`BlockHash`. This reworks that infrastructure to instead create those
with `appendNull`.
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.16.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants