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.

Sign up for GitHub

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

Jump to bottom

Fix compile errors #101874

Merged
merged 9 commits into from
Nov 9, 2023
+16 −11
Merged

Fix compile errors #101874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8272824
Fix compile error
javanna Nov 7, 2023
a7019c0
iter
javanna Nov 7, 2023
331dd92
iter
javanna Nov 7, 2023
e3e5004
iter
javanna Nov 7, 2023
fd5d711
iter
javanna Nov 7, 2023
943de8b
iter
javanna Nov 7, 2023
a1035ac
iter
javanna Nov 8, 2023
d13c8c1
Merge branch 'lucene_snapshot' into fix/lucene_90_postings_format
javanna Nov 8, 2023
120f35c
iter
javanna Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ji/src/main/java/org/elasticsearch/plugin/analysis/kuromoji/KuromojiTokenizerFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import org.apache.lucene.analysis.ja.JapaneseTokenizer;
import org.apache.lucene.analysis.ja.JapaneseTokenizer.Mode;
import org.apache.lucene.analysis.ja.dict.UserDictionary;
import org.apache.lucene.analysis.ja.util.CSVUtil;
import org.apache.lucene.analysis.util.CSVUtil;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
Expand Down
7 changes: 6 additions & 1 deletion ...rc/main/java/org/elasticsearch/action/admin/indices/diskusage/IndexDiskUsageAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@
import org.apache.logging.log4j.Logger;
import org.apache.lucene.backward_codecs.lucene50.Lucene50PostingsFormat;
import org.apache.lucene.backward_codecs.lucene84.Lucene84PostingsFormat;
import org.apache.lucene.backward_codecs.lucene90.Lucene90PostingsFormat;
import org.apache.lucene.codecs.DocValuesProducer;
import org.apache.lucene.codecs.FieldsProducer;
import org.apache.lucene.codecs.KnnVectorsReader;
import org.apache.lucene.codecs.NormsProducer;
import org.apache.lucene.codecs.PointsReader;
import org.apache.lucene.codecs.StoredFieldsReader;
import org.apache.lucene.codecs.TermVectorsReader;
import org.apache.lucene.codecs.lucene90.Lucene90PostingsFormat;
Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure if besides updating the import for 90 we need an additional conditional for 99 here

Copy link
Member

Choose a reason for hiding this comment

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

It seems like we should have one. I honestly don't know, we have one for other older formats :/

Copy link
Member

Choose a reason for hiding this comment

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

So we don't forget, could you make that change in this PR or add a TODO?

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed the change, I don't feel good about no failing tests with or without it though.

Copy link
Member

Choose a reason for hiding this comment

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

I pushed the change, I don't feel good about no failing tests with or without it though.

No test fails because we miss an inverted index unit test, and the assertion in the IT test is too relaxed. There's a strict assertion in a unit test, but it compares the results between the per-field format and the regular format, both of which silently skip a term state if the format is missing. Can you add an assertion at the end of these format checks to ensure we won't forget to add a new format here? I can add an inverted index unit test next week.

It seems like we should have one. I honestly don't know, we have one for other older formats :/

Yes, and we should have BWC tests for this API too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @dnhatn I pushed 120f35c .

import org.apache.lucene.codecs.lucene99.Lucene99PostingsFormat;
import org.apache.lucene.index.BinaryDocValues;
import org.apache.lucene.index.ByteVectorValues;
import org.apache.lucene.index.DirectoryReader;
Expand Down Expand Up @@ -301,6 +302,9 @@ private static void readProximity(Terms terms, PostingsEnum postings) throws IOE
private static BlockTermState getBlockTermState(TermsEnum termsEnum, BytesRef term) throws IOException {
if (term != null && termsEnum.seekExact(term)) {
final TermState termState = termsEnum.termState();
if (termState instanceof final Lucene99PostingsFormat.IntBlockTermState blockTermState) {
return new BlockTermState(blockTermState.docStartFP, blockTermState.posStartFP, blockTermState.payStartFP);
}
if (termState instanceof final Lucene90PostingsFormat.IntBlockTermState blockTermState) {
return new BlockTermState(blockTermState.docStartFP, blockTermState.posStartFP, blockTermState.payStartFP);
}
Expand All @@ -310,6 +314,7 @@ private static BlockTermState getBlockTermState(TermsEnum termsEnum, BytesRef te
if (termState instanceof final Lucene50PostingsFormat.IntBlockTermState blockTermState) {
return new BlockTermState(blockTermState.docStartFP, blockTermState.posStartFP, blockTermState.payStartFP);
}
assert false : "unsupported postings format: " + termState;
}
return null;
}
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ public CompletionFieldType fieldType() {
}

static PostingsFormat postingsFormat() {
return PostingsFormat.forName("Completion90");
return PostingsFormat.forName("Completion99");
}

@Override
Expand Down
8 changes: 4 additions & 4 deletions ...st/java/org/elasticsearch/action/admin/indices/diskusage/IndexDiskUsageAnalyzerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
import org.apache.lucene.codecs.KnnVectorsFormat;
import org.apache.lucene.codecs.PostingsFormat;
import org.apache.lucene.codecs.lucene90.Lucene90DocValuesFormat;
import org.apache.lucene.codecs.lucene90.Lucene90PostingsFormat;
import org.apache.lucene.codecs.lucene99.Lucene99Codec;
import org.apache.lucene.codecs.lucene99.Lucene99HnswVectorsFormat;
import org.apache.lucene.codecs.lucene99.Lucene99PostingsFormat;
import org.apache.lucene.codecs.perfield.PerFieldDocValuesFormat;
import org.apache.lucene.codecs.perfield.PerFieldKnnVectorsFormat;
import org.apache.lucene.codecs.perfield.PerFieldPostingsFormat;
Expand Down Expand Up @@ -54,7 +54,7 @@
import org.apache.lucene.search.ScoreMode;
import org.apache.lucene.search.Scorer;
import org.apache.lucene.search.Weight;
import org.apache.lucene.search.suggest.document.Completion90PostingsFormat;
import org.apache.lucene.search.suggest.document.Completion99PostingsFormat;
import org.apache.lucene.search.suggest.document.CompletionPostingsFormat;
import org.apache.lucene.search.suggest.document.SuggestField;
import org.apache.lucene.store.Directory;
Expand Down Expand Up @@ -330,7 +330,7 @@ public void testCompletionField() throws Exception {
@Override
public PostingsFormat getPostingsFormatForField(String field) {
if (field.startsWith("suggest_")) {
return new Completion90PostingsFormat(randomFrom(CompletionPostingsFormat.FSTLoadMode.values()));
return new Completion99PostingsFormat(randomFrom(CompletionPostingsFormat.FSTLoadMode.values()));
} else {
return super.postingsFormat();
}
Expand Down Expand Up @@ -642,7 +642,7 @@ static void rewriteIndexWithPerFieldCodec(Directory source, CodecMode mode, Dire
.setCodec(new Lucene99Codec(mode.mode()) {
@Override
public PostingsFormat getPostingsFormatForField(String field) {
return new Lucene90PostingsFormat();
return new Lucene99PostingsFormat();
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions server/src/test/java/org/elasticsearch/index/engine/CompletionStatsCacheTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.index.IndexWriterConfig;
import org.apache.lucene.search.suggest.document.Completion90PostingsFormat;
import org.apache.lucene.search.suggest.document.Completion99PostingsFormat;
import org.apache.lucene.search.suggest.document.SuggestField;
import org.apache.lucene.store.Directory;
import org.elasticsearch.ElasticsearchException;
Expand Down Expand Up @@ -43,7 +43,7 @@ public void testExceptionsAreNotCached() {

public void testCompletionStatsCache() throws IOException, InterruptedException {
final IndexWriterConfig indexWriterConfig = newIndexWriterConfig();
final PostingsFormat postingsFormat = new Completion90PostingsFormat();
final PostingsFormat postingsFormat = new Completion99PostingsFormat();
indexWriterConfig.setCodec(new Lucene99Codec() {
@Override
public PostingsFormat getPostingsFormatForField(String field) {
Expand Down
4 changes: 2 additions & 2 deletions server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.IndexableField;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.suggest.document.Completion90PostingsFormat;
import org.apache.lucene.search.suggest.document.Completion99PostingsFormat;
import org.apache.lucene.search.suggest.document.CompletionAnalyzer;
import org.apache.lucene.search.suggest.document.ContextSuggestField;
import org.apache.lucene.search.suggest.document.FuzzyCompletionQuery;
Expand Down Expand Up @@ -149,7 +149,7 @@ public void testPostingsFormat() throws IOException {
Codec codec = codecService.codec("default");
assertThat(codec, instanceOf(PerFieldMapperCodec.class));
PerFieldMapperCodec perFieldCodec = (PerFieldMapperCodec) codec;
assertThat(perFieldCodec.getPostingsFormatForField("field"), instanceOf(Completion90PostingsFormat.class));
assertThat(perFieldCodec.getPostingsFormatForField("field"), instanceOf(Completion99PostingsFormat.class));
}

public void testDefaultConfiguration() throws IOException {
Expand Down