elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.71k stars 8.13k forks source link

[ES|QL] Less prominent warning for LIMIT setting #190728

Open ryankeairns opened 3 weeks ago

ryankeairns commented 3 weeks ago

Describe the feature: Today, the text editor shows a warning if no limit is explicitly set in the query itself. That said, a LIMIT is in fact set 'under the hood' (default 1000). While this accurate in that it serves the response from ES, it feels a bit heavy-handed to present this as a warning in the UI.

CleanShot 2024-08-19 at 10 16 56@2x

Describe a specific use case for the feature: Typing a LIMIT is optional, and it seems the point of the default is to provide a backstop anyhow, so is the warning necessary in this case? This might need to be an issue in the ES repo where we suggest something like changing the level to 'info' versus 'warn', if possible.

elasticmachine commented 3 weeks ago

Pinging @elastic/kibana-esql (Team:ESQL)

stratoula commented 3 weeks ago

@ryankeairns your concern is that this is a warning? You would suggest to be an info instead? I think is something the users should know because it can create a lot of confusion especially taking under consideration the non default sorting of ES|QL queries. Also the search api has a 10K default limit so it is important at this point to be very clear and very prominent to the users that this works differently.

If ES|QL becomes performant I think we could ask ES to raise the limit to 10K and then we can ask them to remove the warning too.

If we want to change the icon to information instead, then we need to discuss with ES. We can't do anything on the kibana repo (unless manually identifying the message). I am marking this as blocked and I will discuss with ES

ryankeairns commented 2 weeks ago

Ok, thanks for explaining.

Mainly, I wanted to track this point you make - that we ultimately, hopefully, could remove this warning.

If ES|QL becomes performant I think we could ask ES to raise the limit to 10K and then we can ask them to remove the warning too.

That said, it is a little confusing that it warns about a limit not being set but isn't a default limit set? This was always the case, but with the recent addition to the footer (which is great!) it just became more apparent. In other words, it warns you that a limit is not set then right next to the warning the footer says LIMIT 1000

stratoula commented 2 weeks ago

That said, it is a little confusing that it warns about a limit not being set but isn't a default limit set?

Just to clarify it warns when the limit is not set in the query but is set in ES side