Skip to content

Support older postings formats#85303

Merged
ywelsch merged 23 commits intoelastic:masterfrom
ywelsch:old-postings
Apr 28, 2022
Merged

Support older postings formats#85303
ywelsch merged 23 commits intoelastic:masterfrom
ywelsch:old-postings

Conversation

@ywelsch
Copy link
Copy Markdown
Contributor

@ywelsch ywelsch commented Mar 24, 2022

Adds support for the Lucene 5 postings format (used by Lucene 6 and 7).

Relates #81210

@jpountz

This comment was marked as outdated.

@ywelsch ywelsch added >non-issue :Search/Search Search-related issues that do not fall into other categories labels Mar 24, 2022
@ywelsch ywelsch requested a review from dnhatn March 24, 2022 15:43
@ywelsch ywelsch marked this pull request as ready for review March 24, 2022 15:43
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Mar 24, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-search (Team:Search)

Copy link
Copy Markdown
Member

@dnhatn dnhatn left a comment

Choose a reason for hiding this comment

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

I left some minor comments. This looks great. Thanks Yannick!

if (suffix == null) {
throw new IllegalStateException("missing attribute: " + PER_FIELD_SUFFIX_KEY + " for field: " + fieldName);
}
PostingsFormat format = legacyAdaptingPerFieldPostingsFormat.getPostingsFormat(formatName);
Copy link
Copy Markdown
Member

@dnhatn dnhatn Mar 28, 2022

Choose a reason for hiding this comment

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

I think we introduced this adapter because of line 125? I tried to remove this class by converting the PerFieldPostingsFormat.format attribute from Lucene50 to BWCLucene50 in FieldInfos, but it didn't work because of line 126. I think it's fine to have this class.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Another motivation for this class is that it keeps the information in these older indices as much "as-is" as possible, not requiring a conversion step before reading. Furthermore, future versions won't have to be aware that that conversion step happened in some prior versions of archive, if we were to remove it e.g. in the future. The nice bit is that this conversion can go away in ES 9 as we won't have a conflicting Lucene50PostingsFormat shipped anymore as part of bwc jars.

@ywelsch ywelsch requested a review from javanna April 28, 2022 10:24
Copy link
Copy Markdown
Contributor

@javanna javanna left a comment

Choose a reason for hiding this comment

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

LGTM

@ywelsch ywelsch merged commit c082858 into elastic:master Apr 28, 2022
@ywelsch
Copy link
Copy Markdown
Contributor Author

ywelsch commented Apr 28, 2022

Thanks @dnhatn and @javanna. I will work on text field support in a follow-up.

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

Labels

>non-issue :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v8.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants