Skip to content

[ES|QL] fix: completion column unknown validation#230589

Merged
sddonne merged 1 commit intoelastic:mainfrom
sddonne:fix-completion-validation
Aug 6, 2025
Merged

[ES|QL] fix: completion column unknown validation#230589
sddonne merged 1 commit intoelastic:mainfrom
sddonne:fix-completion-validation

Conversation

@sddonne
Copy link
Contributor

@sddonne sddonne commented Aug 5, 2025

Summary

Problem: When the completion command was using a target column, the column was marked as unknown.

image

@sddonne sddonne added release_note:skip Skip the PR/issue when compiling release notes Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana t// backport:version Backport to applied version labels backport:skip This PR does not require backporting v9.2.0 and removed backport:version Backport to applied version labels labels Aug 5, 2025
});

describe('a custom targetField is provided', () => {
it('targetField is not available before COMPLETION', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can't be tested here now, as the general validation routine is still in validation-autocomplete package.

Copy link
Contributor

@stratoula stratoula Aug 5, 2025

Choose a reason for hiding this comment

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

So this test used to run and stopped working due to this change? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It made sense when the validation routine was run from validation-autocomplete, now it was passing by chance, because we were checking the unknown columns before pushing the new one.

@sddonne sddonne marked this pull request as ready for review August 5, 2025 13:53
@sddonne sddonne requested a review from a team as a code owner August 5, 2025 13:53
@elasticmachine
Copy link
Contributor

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

},
]);

messages.push(...validateCommandArguments(command, ast, context, callbacks));
Copy link
Contributor

Choose a reason for hiding this comment

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

This fix makes sense yes 👍

@stratoula stratoula added backport:version Backport to applied version labels v9.1.1 v8.19.1 and removed backport:skip This PR does not require backporting labels Aug 5, 2025
Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

Thanx for fixing this Seb! LGTM! Although completion is on tech preview let's backport it to the previous patches. I updated the labels

Update: No need actually, Seb reminded me that the registry got introduced in 9.2

@stratoula stratoula added backport:skip This PR does not require backporting and removed v9.1.1 v8.19.1 backport:version Backport to applied version labels labels Aug 5, 2025
@sddonne sddonne merged commit 102f7b5 into elastic:main Aug 6, 2025
39 checks passed
@wildemat wildemat mentioned this pull request Aug 7, 2025
10 tasks
NicholasPeretti pushed a commit to NicholasPeretti/kibana that referenced this pull request Aug 18, 2025
## Summary
Problem: When the completion command was using a target column, the
column was marked as unknown.

<img width="1636" height="454" alt="image"
src="https://github.com/user-attachments/assets/f39bdf32-80df-4922-a63d-c2631cfcb82b"
/>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:ES|QL ES|QL related features in Kibana release_note:skip Skip the PR/issue when compiling release notes Team:ESQL ES|QL related features in Kibana t// v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants