Skip to content

Fix split package org.apache.lucene.document#78186

Merged
ChrisHegarty merged 4 commits intoelastic:masterfrom
ChrisHegarty:relocate_BinaryRange
Sep 23, 2021
Merged

Fix split package org.apache.lucene.document#78186
ChrisHegarty merged 4 commits intoelastic:masterfrom
ChrisHegarty:relocate_BinaryRange

Conversation

@ChrisHegarty
Copy link
Copy Markdown
Contributor

@ChrisHegarty ChrisHegarty commented Sep 22, 2021

To allow for the future modularization of Elasticsearch with Java Modules, there are a number preparatory tasks that need be completed. This is one part of one such task: eliminate split packages.

The ES class BinaryRange is located in the org.apache.lucene.document package, which is shared (thus split at runtime) with Lucene-core. A split package, in this case between Elasticsearch-core and Lucene-core, will result in a configuration error at runtime when Elasticsearch-core is deployed as a module.

The BinaryRange class accesses the previously package-private RangeFieldQuery class, thus requiring it to be in the same runtime package. In a recent Lucene 9.0 change, RangeFieldQuery has been made public, therefore BinaryRange no longer requires to be in the same runtime package as RangeFieldQuery. Moving BinaryRange to a non-lucene package will eliminate the split package issue mentioned above.

This PR proposes to keep BinaryRange in sever, but moved it to org.elasticsearch.lucene.document. Alternatively, since it appears that BinaryRange is used exclusively by code in org.elasticsearch.percolator, moving it to the percolator module could be reasonable.

relates #78166

@ChrisHegarty ChrisHegarty added v8.0.0 modularization Java Modules related labels Sep 22, 2021
@ChrisHegarty ChrisHegarty mentioned this pull request Sep 22, 2021
58 tasks
@rjernst
Copy link
Copy Markdown
Member

rjernst commented Sep 22, 2021

I think moving it to percolator would be good, the less stuff in server the better, but I would like @martijnvg to comment on whether there is another reason BinaryRange should be in server.

@martijnvg
Copy link
Copy Markdown
Member

I'm +1 to moving BinaryRange to the percolator module. I'm not aware of any other usages of this class nor can find any usages of the class in the codebase.

@ChrisHegarty
Copy link
Copy Markdown
Contributor Author

Thanks for the comments - moved to percolator.

@ChrisHegarty
Copy link
Copy Markdown
Contributor Author

@elasticmachine test this please

@ChrisHegarty
Copy link
Copy Markdown
Contributor Author

@elasticmachine update branch

@ChrisHegarty ChrisHegarty added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Sep 23, 2021
@ChrisHegarty
Copy link
Copy Markdown
Contributor Author

@elasticmachine update branch

@ChrisHegarty ChrisHegarty removed the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Sep 23, 2021
@ChrisHegarty ChrisHegarty merged commit d1f5d61 into elastic:master Sep 23, 2021
@ChrisHegarty ChrisHegarty deleted the relocate_BinaryRange branch September 23, 2021 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants