Skip to content

Add a direct sub classes data structure to the Painless lookup#76955

Merged
jdconrad merged 7 commits into
elastic:masterfrom
jdconrad:inheritance
Aug 26, 2021
Merged

Add a direct sub classes data structure to the Painless lookup#76955
jdconrad merged 7 commits into
elastic:masterfrom
jdconrad:inheritance

Conversation

@jdconrad

Copy link
Copy Markdown
Contributor

This change has two main components.

The first is to have method/field resolution for compile-time and run-time use the same code path for now. This removes copying of member methods between super and sub classes and instead does a resolution through the class hierarchy. This allows us to correctly implement the next change.

The second is a data structure that allows for the lookup of direct sub classes for all allow listed classes/interfaces within Painless.

Given there can be gaps within sub classes due to how allow listing is specified, a subclass may propagate to the nearest allow listed subclass.

Example:

class A {} // allowed
class B extends A {} // disallowed
class C extends B {} // allowed

In this case A has C as a direct sub class.

Example:

interface A {} // allowed
class B implements A {} // disallowed
class C extends B {} // allowed

In this case A has C as a direct sub class.

Interfaces work the same way to propagate across any gaps in allow listing.

@jdconrad jdconrad added >enhancement :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v8.0.0 auto-backport Automatically create backport pull requests when merged v7.16.0 labels Aug 25, 2021
@jdconrad jdconrad requested a review from stu-elastic August 25, 2021 22:36
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Aug 25, 2021
@elasticmachine

Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@stu-elastic stu-elastic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, just a few comment requests.

@jdconrad

Copy link
Copy Markdown
Contributor Author

@stu-elastic Thanks for the review. I addressed your comments, and I will commit once CI passes.

@jdconrad jdconrad merged commit 718b163 into elastic:master Aug 26, 2021
@elasticsearchmachine

Copy link
Copy Markdown
Collaborator

💔 Backport failed

Status Branch Result
7.x Commit could not be cherrypicked due to conflicts

To backport manually run backport --upstream elastic/elasticsearch --pr 76955

@jdconrad jdconrad removed the auto-backport Automatically create backport pull requests when merged label Aug 26, 2021
jdconrad added a commit to jdconrad/elasticsearch that referenced this pull request Aug 26, 2021
…ic#76955)

This change has two main components.

The first is to have method/field resolution for compile-time and run-time use the same code path for
now. This removes copying of member methods between super and sub classes and instead does a
resolution through the class hierarchy. This allows us to correctly implement the next change.

The second is a data structure that allows for the lookup of direct sub classes for all allow listed
classes/interfaces within Painless.
jdconrad added a commit that referenced this pull request Dec 14, 2021
A recent change [1] to how we load our allow list changed the resolution for how Painless looks up 
methods of super classes. However, functional interface loading was not changed which caused a 
bug where a functional interface would not look at its super interfaces for the functional interface 
method [2].

This fixes the issue by going through each super interface until the functional interface method is 
found when the target interface doesn't have the functional interface method.

[1] #76955
[2] #81696

Also a big thanks to @megglos and @TheFireCookie for their help with this issue.
jdconrad added a commit to jdconrad/elasticsearch that referenced this pull request Dec 14, 2021
…1698)

A recent change [1] to how we load our allow list changed the resolution for how Painless looks up 
methods of super classes. However, functional interface loading was not changed which caused a 
bug where a functional interface would not look at its super interfaces for the functional interface 
method [2].

This fixes the issue by going through each super interface until the functional interface method is 
found when the target interface doesn't have the functional interface method.

[1] elastic#76955
[2] elastic#81696

Also a big thanks to @megglos and @TheFireCookie for their help with this issue.
jdconrad added a commit to jdconrad/elasticsearch that referenced this pull request Dec 14, 2021
…1698)

A recent change [1] to how we load our allow list changed the resolution for how Painless looks up 
methods of super classes. However, functional interface loading was not changed which caused a 
bug where a functional interface would not look at its super interfaces for the functional interface 
method [2].

This fixes the issue by going through each super interface until the functional interface method is 
found when the target interface doesn't have the functional interface method.

[1] elastic#76955
[2] elastic#81696

Also a big thanks to @megglos and @TheFireCookie for their help with this issue.
jdconrad added a commit that referenced this pull request Dec 14, 2021
…81737)

A recent change [1] to how we load our allow list changed the resolution for how Painless looks up 
methods of super classes. However, functional interface loading was not changed which caused a 
bug where a functional interface would not look at its super interfaces for the functional interface 
method [2].

This fixes the issue by going through each super interface until the functional interface method is 
found when the target interface doesn't have the functional interface method.

[1] #76955
[2] #81696

Also a big thanks to @megglos and @TheFireCookie for their help with this issue.
jdconrad added a commit that referenced this pull request Dec 14, 2021
…81738)

A recent change [1] to how we load our allow list changed the resolution for how Painless looks up 
methods of super classes. However, functional interface loading was not changed which caused a 
bug where a functional interface would not look at its super interfaces for the functional interface 
method [2].

This fixes the issue by going through each super interface until the functional interface method is 
found when the target interface doesn't have the functional interface method.

[1] #76955
[2] #81696

Also a big thanks to @megglos and @TheFireCookie for their help with this issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement Team:Core/Infra Meta label for core/infra team v7.16.0 v8.0.0-alpha2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants