Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Expressions] Fix bug with expression reference extraction #107309

Merged
merged 2 commits into from
Aug 3, 2021

Conversation

crob611
Copy link
Contributor

@crob611 crob611 commented Jul 30, 2021

Summary

There seems to be a mismatch in how extract and inject were expecting references to be prefixed.

Extract was increasing the "step" counter for every reference that gets extracted.

Inject was increasing the counter for every function it encounters.

So, an expression like foo | ref refKey="a", extract would prefix the ref with l0 since it was the first ref that it sees.

But extract would search through all the references for l0 prefixes for the foo function and l1 for the ref function.

This changes extract to match the way inject does it. Increase the prefix count for every function it encounters.

Includes a test to make sure the extract is functioning as expected and that doing an extract and then immediately injecting it back provides the original expression.

@crob611 crob611 added Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) loe:small Small Level of Effort v8.0.0 Team:AppServices release_note:skip Skip the PR/issue when compiling release notes impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. v7.15.0 labels Jul 30, 2021
@crob611 crob611 requested a review from a team as a code owner July 30, 2021 16:47
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

code LGTM

@crob611
Copy link
Contributor Author

crob611 commented Aug 3, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
expressions 214.1KB 214.1KB +8.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@crob611 crob611 merged commit 91e64e0 into elastic:master Aug 3, 2021
@crob611 crob611 added the auto-backport Deprecated - use backport:version if exact versions are needed label Aug 3, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Aug 3, 2021
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Aug 3, 2021
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Corey Robertson <corey.robertson@elastic.co>
streamich pushed a commit to vadimkibana/kibana that referenced this pull request Aug 8, 2021
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes v7.15.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants