elastic / kibana

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

[ES|QL] Drop command doesn't erase previously added column name header #189909

Open bhavyarm opened 1 month ago

bhavyarm commented 1 month ago

Kibana version: 8.15.0 BC5

Browser version: chrome latest

Browser OS version: OS X

Original install method (e.g. download page, yum, from source, etc.): from staging

Describe the bug: If user has already added a field as a column in discover and then executes DROP command on that field in an ES|QL query, Kibana only drops the field values from discover but still retains the column name(field name) in the header with deadspace below it.

Steps to reproduce:

  1. In discover pick your favourite sample dataview
  2. Switch to ES|QL and drag and drop some columns from side nav to main discover table - for sample logs data, I selected machine.os.keyword and response.keyword. Kibana displays these field names as columns and values
  3. Now execute FROM kibana_sample_data_logs | LIMIT 1000 | DROP response.keyword
  4. Kibana removes the values for the field response.keyword but still displays the field name(column name as column header) drop
elasticmachine commented 1 month ago

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

elasticmachine commented 1 month ago

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

stratoula commented 1 month ago

I am passing this to the Discovery team as it is related to the Discover state updates but let me know if there is something that the team can help with

kertal commented 1 month ago

Thx @bhavyarm I can see this makes sense from UX side, sind when I drop a column in the ES|QL query, it's unlikely I want to have a look at an empty column, even if I added it before.

wouldn't label it as a bug, since it currently works like expected. @stratoula we change the columns in state if there's a transformational command given, right?

https://github.com/elastic/kibana/blob/250c729087569cea009692ef2120fcee6842fed1/src/plugins/discover/public/application/main/hooks/use_esql_mode.ts#L85

so for this case we could need to have the information that there's a Drop command and filter out the dropped column if it's part of the current column state.

stratoula commented 1 month ago

yes I agree it is not a bug. yes we could parse the query, check the DROP commands, take the fields and check if they exist in the columns state. If yes, then remove the dropped fields from the state and update.

I personally find it as a low impact enhancement. I think users would understand what is going on here. I changed it to enhancement with low impact

kertal commented 1 month ago

I personally find it as a low impact enhancement. I think users would understand what is going on here. I changed it to enhancement with low impact

yes, I do agree, it's a simple workaround to just remove the column, no bug, impact:low