[ESQL] Makes LIMIT BY Tech Preview#145225
Conversation
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
| - 112918 | ||
| pr: 145225 | ||
| summary: Adds LIMIT BY ESQL command in Tech Preview | ||
| type: enhancement |
There was a problem hiding this comment.
Not sure whether I need feature here or whether I need a changelog at all
There was a problem hiding this comment.
Not sure whether I need feature here
I don't think it matters as features and enhancements get mushed together anyways 😄
https://www.elastic.co/docs/release-notes/elasticsearch#elasticsearch-9.3.2-features-enhancements
Co-authored-by: Iván Cea Fontenla <ivancea96@outlook.com>
5a14f9b to
ee40db4
Compare
|
run docs-build |
|
@ncordon docs eng had to some security work to harden the docs build pipelines, and a side effect is that when working from a fork, we need your profile to be public to verify elastic org membership and then enable automatic previews You can do that simply by flipping to public here: https://github.com/orgs/elastic/people?query=cordon The alternative is to build locally: https://www.elastic.co/docs/contribute-docs/locally#install-docs-builder |
Done! |
🔍 Preview links for changed docs⏳ Building and deploying preview... View progress This comment will be updated with preview links when the build is complete. |
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
alex-spies
left a comment
There was a problem hiding this comment.
Reviewed the technical change, not the docs.
LGTM, thanks @ncordon !
|
|
||
| ```esql | ||
| FROM employees | ||
| | SORT salary DESC |
There was a problem hiding this comment.
Nit: all examples use a SORT, it seems? Not wrong, though.
There was a problem hiding this comment.
This is an interesting limitation. With LIMIT BY alone it would be difficult to stabilize the output of the csv tests and I'd have to do artificial things like including a LIMIT between the SORT and LIMIT BY:
FROM employees
| SORT salary DESC
| LIMIT 1000
| LIMIT 1 BY gender
| KEEP first_name, last_name, salary, gender
There was a problem hiding this comment.
What we've done previously is place the tags such that the sort is not included :) a little bit fake, but should work.
But it's just a nit, not a must-have!
|
run docs-build |
|
I merged the pr. The CI passed, the jobs not reporting are successful but stuck on the reporting back. I'll address any feedback you leave here separately @leemthompo |
|
@ncordon according to this PR's labels, I need to update the changelog YAML, but I can't because the PR is closed. Please either update the changelog yourself on the appropriate branch, or adjust the labels. Specifically:
|
--------- Co-authored-by: Iván Cea Fontenla <ivancea96@outlook.com>
This PR gets the
LIMIT BYcommand out of snapshot and into Tech Preview. Also it adds docs for the new command.This should be merged after #145115.
Closes #112918, closes https://github.com/elastic/esql-planning/issues/424