Skip to content

[ES|QL] fix column name detection#234788

Closed
drewdaemon wants to merge 3 commits intoelastic:mainfrom
drewdaemon:fix-expression-column-name-detection
Closed

[ES|QL] fix column name detection#234788
drewdaemon wants to merge 3 commits intoelastic:mainfrom
drewdaemon:fix-expression-column-name-detection

Conversation

@drewdaemon
Copy link
Copy Markdown
Contributor

@drewdaemon drewdaemon commented Sep 11, 2025

Summary

  1. Fixes the column name detection (bug introduced in [ES|QL] improve available column lists #233221)

Before
Screenshot 2025-09-11 at 9 00 34 AM

After
Screenshot 2025-09-11 at 9 42 13 AM

  1. Makes case changes bust the columns cache. To do this I had to make the cache keys sections of the original query, since there's no way to preserve casing through a parse-serialize process. The side effect is that there will be more cache misses.

This fixes this case (pre-existing bug)

Screen.Recording.2025-09-11.at.2.28.15.PM.mov

Checklist

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Sep 11, 2025

🤖 Jobs for this PR can be triggered through checkboxes. 🚧

ℹ️ To trigger the CI, please tick the checkbox below 👇

  • Click to trigger kibana-pull-request for this PR!
  • Click to trigger kibana-deploy-project-from-pr for this PR!
  • Click to trigger kibana-deploy-cloud-from-pr for this PR!

@drewdaemon
Copy link
Copy Markdown
Contributor Author

/ci

@drewdaemon drewdaemon force-pushed the fix-expression-column-name-detection branch from ac79f9f to 3fcbed0 Compare September 11, 2025 17:48
@drewdaemon
Copy link
Copy Markdown
Contributor Author

/ci

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Sep 11, 2025

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #6 / column escaping recognizes escaped fields
  • [job] [logs] Jest Tests #6 / column escaping recognizes escaped fields
  • [job] [logs] Jest Tests #6 / getQueryForFields should convert FORK branches into vanilla queries
  • [job] [logs] Jest Tests #6 / getQueryForFields should convert FORK branches into vanilla queries
  • [job] [logs] Jest Tests #6 / getQueryForFields should convert multiple EVAL expressions into separate EVAL commands
  • [job] [logs] Jest Tests #6 / getQueryForFields should convert multiple EVAL expressions into separate EVAL commands
  • [job] [logs] Jest Tests #6 / getQueryForFields should return everything up till the last command
  • [job] [logs] Jest Tests #6 / getQueryForFields should return everything up till the last command
  • [job] [logs] FTR Configs #136 / InfraOps App feature controls infrastructure security global infrastructure all privileges infrastructure landing page with data shows Wafflemap
  • [job] [logs] FTR Configs #136 / InfraOps App feature controls infrastructure security global infrastructure all privileges infrastructure landing page with data shows Wafflemap

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
onechat 248.6KB 248.5KB -102.0B

History

@drewdaemon
Copy link
Copy Markdown
Contributor Author

@stratoula before I go to the effort of debugging tests and stuff, can you confirm that you agree with the overall decisions I made here? They are described in the PR description.

@stratoula
Copy link
Copy Markdown
Contributor

stratoula commented Sep 12, 2025

the 2nd is a good catch, it makes sense yes (unfortunately...). Let's create 2 PRs though. The one to solve quickly the regression (it is very bad, I dont want to land in serverless) and the other the existing bug. I want to test the latter well so it might take more time to get merged in main

@drewdaemon
Copy link
Copy Markdown
Contributor Author

Makes sense, I'll split them out and we'll get the first fix in

@drewdaemon
Copy link
Copy Markdown
Contributor Author

drewdaemon commented Sep 15, 2025

Waiting on #234986

@drewdaemon drewdaemon closed this Oct 7, 2025
@drewdaemon drewdaemon deleted the fix-expression-column-name-detection branch October 7, 2025 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants