Skip to content

Conversation

@luigidellaquila
Copy link
Contributor

Updating ES|QL spec after changes introduced with elastic/elasticsearch#137641

@luigidellaquila luigidellaquila requested a review from a team as a code owner November 12, 2025 09:16
@luigidellaquila luigidellaquila added the skip-backport This pull request should not be backported label Nov 12, 2025
@github-actions
Copy link
Contributor

Following you can find the validation changes against the target branch for the APIs.

No changes detected.

You can validate these APIs yourself by using the make validate target.

Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM.

"resolved": "https://registry.npmjs.org/@types/node/-/node-22.15.31.tgz",
"integrity": "sha512-jnVe5ULKl6tijxUhvQeNbQG/84fHfg+yMak02cT8QVhBx/F05rAVxCGBYYTh2EKz22D6JF5ktXuNwdx7b9iEGw==",
"license": "MIT",
"peer": true,

Choose a reason for hiding this comment

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

This seems unrelated to include_execution_metadata.
Could you please confirm this is intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea what it does, it was generated by make contrib.

@pquentin can you confirm that it's expected?

Copy link
Member

Choose a reason for hiding this comment

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

Good catch. Might be related to the local NPM version.

Since you didn't change anything on the JS tooling itself, we should probably remove all the package-lock changes from this PR to be on the safe side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks!

Copy link
Member

Choose a reason for hiding this comment

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

@pquentin can you confirm that it's expected?

As long as CI passes, I don't think that was harmful! We often have small random changes like this that don't make any difference in practice.

Copy link

@idegtiarenko idegtiarenko left a comment

Choose a reason for hiding this comment

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

Changes around include_execution_metadata looks good to me 👍

@luigidellaquila luigidellaquila merged commit b591f49 into main Nov 12, 2025
10 checks passed
@luigidellaquila luigidellaquila deleted the esql/include_execution_metadata_2 branch November 12, 2025 14:07
@pquentin
Copy link
Member

Thank you for the contribution! Appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants