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: Rename AUTO_BUCKET to just BUCKET #107197

Merged
merged 7 commits into from
Apr 10, 2024

Conversation

bpintea
Copy link
Contributor

@bpintea bpintea commented Apr 8, 2024

This renames the function AUTO_BUCKET to just BUCKET.

It also removes the experimental tagging of the function in the docs, making it generally available.

Copy link
Contributor

github-actions bot commented Apr 8, 2024

Documentation preview:

@bpintea bpintea marked this pull request as ready for review April 8, 2024 15:10
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Apr 8, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@bpintea bpintea added the ES|QL-ui Impacts ES|QL UI label Apr 8, 2024
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.

Looks good - just one, potentially important remark :)

* experimental:[] <<esql-auto_bucket>>
* <<esql-bucket>>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it intended that we declare BUCKET as released in this PR?

(If so, let's add this to the PR description as it's important IMHO.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it intended that we declare BUCKET as released in this PR?

Yes, I believe we wanted to decide on the naming, also considering the non-auto version.

@costin costin requested a review from astefan April 10, 2024 05:50
Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

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

LGTM however please wait for Andrei's review.

Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

LGTM

@bpintea
Copy link
Contributor Author

bpintea commented Apr 10, 2024

Thanks, Alex, Costin, Andrei.

@bpintea bpintea merged commit d6f9d1e into elastic:main Apr 10, 2024
14 checks passed
@bpintea bpintea deleted the esql/rename_auto_bucket branch April 10, 2024 10:21
drewdaemon added a commit to elastic/kibana that referenced this pull request Apr 16, 2024
## Summary

Close #180061

Key fixes
- `auto_bucket` 
- now just called `bucket` as of
elastic/elasticsearch#107197
  - now accepts dates for the 3rd and 4th parameters
  - now accepts a number field for the first parameter
- Constant-only parameters
- We no longer suggest fields or variables for `auto-bucket`'s 2nd, 3rd,
and 4th parameters, all of which must be constants
- Validator catches non-constants even if they are nested in layers of
functions (e.g. `auto_bucket(@timestamp, abs(length(bytes)), "", "")`)

Outstanding issues
- [x] Validation catches fields in a constant-only parameter spot, but
not if they are nested in a function
<img width="600" alt="Screenshot 2024-04-10 at 11 31 38 AM"
src="https://github.com/elastic/kibana/assets/315764/3b73419a-f9d1-4d59-a128-b0135eb50a90">

_Validator sees the problem_

<img width="600" alt="Screenshot 2024-04-10 at 11 31 14 AM"
src="https://github.com/elastic/kibana/assets/315764/1b5b0999-a697-4625-a8a3-15415054200b">

_Validator doesn't see the problem_

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Stratoula Kalafateli <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL ES|QL-ui Impacts ES|QL UI >refactoring Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants