You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "jpountz (via GitHub)" <gi...@apache.org> on 2023/12/11 08:31:53 UTC
[PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
jpountz opened a new pull request, #12908:
URL: https://github.com/apache/lucene/pull/12908
This builds on top of #12904 and adds back the write logic for the terms
dictionary format we used between 9.0 and 9.8, so that we can more easily test
it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on PR #12908:
URL: https://github.com/apache/lucene/pull/12908#issuecomment-1849946150
The `precommit` failure looks unrelated -- good 'ol `TestRandomChains`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on code in PR #12908:
URL: https://github.com/apache/lucene/pull/12908#discussion_r1422599871
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_codecs/lucene90/Lucene90RWPostingsFormat.java:
##########
@@ -75,7 +75,11 @@ public FieldsConsumer fieldsConsumer(SegmentWriteState state) throws IOException
try {
FieldsConsumer ret =
new Lucene90BlockTreeTermsWriter(
- state, postingsWriter, minTermBlockSize, maxTermBlockSize);
+ state,
+ postingsWriter,
+ minTermBlockSize,
+ maxTermBlockSize,
+ Lucene90BlockTreeTermsReader.VERSION_START);
Review Comment:
Oh! Sorry, I was confusing the `VERSION_X` constants between `Lucene90BlockTreeTermsReader` vs `FST`, sigh.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on PR #12908:
URL: https://github.com/apache/lucene/pull/12908#issuecomment-1849961825
And in fact the first run's failure was NOT spurious! It failed with this root cause:
```
org.apache.lucene.analysis.tests.TestRandomChains > test suite's output saved to /home/runner/work/lucene/lucene/lucene/analysis.tests/build/test-results/test/outputs/OUTPUT-org.apache.lucene.analysis.tests.TestRandomChains.txt, copied below:
> java.lang.AssertionError: mismatch frozenHash=930432529 vs hash=3634519
> at __randomizedtesting.SeedInfo.seed([18C0E39CA25896EF:2521CAFDE54A8B2F]:0)
> at org.apache.lucene.core@10.0.0-SNAPSHOT/org.apache.lucene.util.fst.NodeHash.add(NodeHash.java:148)
> at org.apache.lucene.core@10.0.0-SNAPSHOT/org.apache.lucene.util.fst.FSTCompiler.compileNode(FSTCompiler.java:384)
> at org.apache.lucene.core@10.0.0-SNAPSHOT/org.apache.lucene.util.fst.FSTCompiler.freezeTail(FSTCompiler.java:819)
> at org.apache.lucene.core@10.0.0-SNAPSHOT/org.apache.lucene.util.fst.FSTCompiler.compile(FSTCompiler.java:967)
> at org.apache.lucene.analysis.common@10.0.0-SNAPSHOT/org.apache.lucene.analysis.charfilter.NormalizeCharMap$Builder.build(NormalizeCharMap.java:114)
> at org.apache.lucene.analysis.tests@10.0.0-SNAPSHOT/org.apache.lucene.analysis.tests.TestRandomChains$1.lambda$new$17(TestRandomChains.java:327)
> at org.apache.lucene.analysis.tests@10.0.0-SNAPSHOT/org.apache.lucene.analysis.tests.TestRandomChains.newRandomArg(TestRandomChains.java:681)
> at org.apache.lucene.analysis.tests@10.0.0-SNAPSHOT/org.apache.lucene.analysis.tests.TestRandomChains.newCharFilterArgs(TestRandomChains.java:700)
> at org.apache.lucene.analysis.tests@10.0.0-SNAPSHOT/org.apache.lucene.analysis.tests.TestRandomChains$MockRandomAnalyzer.newCharFilterChain(TestRandomChains.java:836)
> at org.apache.lucene.analysis.tests@10.0.0-SNAPSHOT/org.apache.lucene.analysis.tests.TestRandomChains$MockRandomAnalyzer.toString(TestRandomChains.java:750)
> at java.base/java.lang.StringConcatHelper.stringOf(StringConcatHelper.java:453)
> at java.base/java.lang.StringConcatHelper.simpleConcat(StringConcatHelper.java:408)
> at org.apache.lucene.analysis.tests@10.0.0-SNAPSHOT/org.apache.lucene.analysis.tests.TestRandomChains.testRandomChains(TestRandomChains.java:954)
```
`NeedMoreCaffeineException`!!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on PR #12908:
URL: https://github.com/apache/lucene/pull/12908#issuecomment-1850050936
I think I found the bug, the FST version was mistakenly initialized to 0 instead of Version.CURRENT in some cases, it should be good now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on PR #12908:
URL: https://github.com/apache/lucene/pull/12908#issuecomment-1850129755
Hmmm... I agree we can't expect `BasePostingsFormatTestCase` to catch all bw compat problems, but the `TestLucene90PostingsFormat` from this PR writes data in the 9.8 format of the terms dictionary, and then reads the data with the 9.9 logic, only relying on version checks to read outputs with regular vlongs. So it should be exposing the bug?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on code in PR #12908:
URL: https://github.com/apache/lucene/pull/12908#discussion_r1422541990
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_codecs/lucene90/Lucene90RWPostingsFormat.java:
##########
@@ -75,7 +75,11 @@ public FieldsConsumer fieldsConsumer(SegmentWriteState state) throws IOException
try {
FieldsConsumer ret =
new Lucene90BlockTreeTermsWriter(
- state, postingsWriter, minTermBlockSize, maxTermBlockSize);
+ state,
+ postingsWriter,
+ minTermBlockSize,
+ maxTermBlockSize,
+ Lucene90BlockTreeTermsReader.VERSION_START);
Review Comment:
Hmm test should now be angry about this unsupported (too early) version?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on PR #12908:
URL: https://github.com/apache/lucene/pull/12908#issuecomment-1849963592
Don't worry @mikemccand, I'll look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on PR #12908:
URL: https://github.com/apache/lucene/pull/12908#issuecomment-1850117651
> It's actually a bad news that all tests pass here, as this means that our `BasePostingsFormatTestCase` is not good enough to uncover the recent problem with `Terms#intersect`... So we could add a similar bug in the future, and we'd only learn about it after releasing when @mikemccand 's new bw compat test from #12901 starts running?
Well, at least for this particular `Terms.intersect` bug (#12895), the bug only exists when 9.9.0 reads <= 9.8 written indices, i.e. the bug is sidestepped when 9.9.0 reads 9.9.0 written indices because of the separate LSB -> MSB `writeVLong` change. It masks/prevents the read-time bug entirely. So there is / was no hope for `BasePostingsFormatTestCase` to catch this hidden / lurking bug, I fear.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on PR #12908:
URL: https://github.com/apache/lucene/pull/12908#issuecomment-1849958671
Hmm the retried job failed with something scary and clearly FST related:
```
java.lang.AssertionError: mismatch frozenHash=1634033861501336 vs hash=6385840662311
> at org.apache.lucene.core@10.0.0-SNAPSHOT/org.apache.lucene.util.fst.NodeHash.add(NodeHash.java:148)
> at org.apache.lucene.core@10.0.0-SNAPSHOT/org.apache.lucene.util.fst.FSTCompiler.compileNode(FSTCompiler.java:384)
> at org.apache.lucene.core@10.0.0-SNAPSHOT/org.apache.lucene.util.fst.FSTCompiler.compile(FSTCompiler.java:976)
> at org.apache.lucene.analysis.common@10.0.0-SNAPSHOT/org.apache.lucene.analysis.charfilter.NormalizeCharMap$Builder.build(NormalizeCharMap.java:114)
> at org.apache.lucene.analysis.common@10.0.0-SNAPSHOT/org.apache.lucene.analysis.charfilter.MappingCharFilterFactory.inform(MappingCharFilterFactory.java:78)
> at org.apache.lucene.benchmark.byTask.tasks.AnalyzerFactoryTask.createAnalysisPipelineComponent(AnalyzerFactoryTask.java:443)
> at org.apache.lucene.benchmark.byTask.tasks.AnalyzerFactoryTask.setParams(AnalyzerFactoryTask.java:274)
> at org.apache.lucene.benchmark.byTask.utils.Algorithm.<init>(Algorithm.java:149)
> at org.apache.lucene.benchmark.byTask.Benchmark.<init>(Benchmark.java:65)
> at org.apache.lucene.benchmark.BenchmarkTestCase.execBenchmark(BenchmarkTestCase.java:72)
> at org.apache.lucene.benchmark.byTask.TestPerfTasksLogic.testAnalyzerFactory(TestPerfTasksLogic.java:983)
```
I'll try to understand what is happening here ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on code in PR #12908:
URL: https://github.com/apache/lucene/pull/12908#discussion_r1422609478
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_codecs/lucene90/Lucene90RWPostingsFormat.java:
##########
@@ -75,7 +75,11 @@ public FieldsConsumer fieldsConsumer(SegmentWriteState state) throws IOException
try {
FieldsConsumer ret =
new Lucene90BlockTreeTermsWriter(
- state, postingsWriter, minTermBlockSize, maxTermBlockSize);
+ state,
+ postingsWriter,
+ minTermBlockSize,
+ maxTermBlockSize,
+ Lucene90BlockTreeTermsReader.VERSION_START);
Review Comment:
No worries.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on code in PR #12908:
URL: https://github.com/apache/lucene/pull/12908#discussion_r1422487100
##########
lucene/core/src/java/org/apache/lucene/util/fst/FSTCompiler.java:
##########
@@ -325,6 +328,21 @@ public Builder<T> directAddressingMaxOversizingFactor(float factor) {
return this;
}
+ /** Expert: Set the codec version. * */
+ public Builder<T> setVersion(int version) {
+ if (version < FST.VERSION_START || version > FST.VERSION_CURRENT) {
Review Comment:
I was thinking of adding support for pre-little-endian in a follow-up, but no need to be lenient in the meantime indeed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on code in PR #12908:
URL: https://github.com/apache/lucene/pull/12908#discussion_r1422523889
##########
lucene/core/src/java/org/apache/lucene/util/fst/FST.java:
##########
@@ -109,10 +109,20 @@ public enum INPUT_TYPE {
// Increment version to change it
private static final String FILE_FORMAT_NAME = "FST";
- private static final int VERSION_START = 6;
+
+ /** First supported version, this is the version that was used when releasing Lucene 7.0. */
+ public static final int VERSION_START = 6;
+
private static final int VERSION_LITTLE_ENDIAN = 8;
Review Comment:
So version 7 was introduced by https://github.com/apache/lucene/commit/5dd9c4c04bcab9911b9a9f0b092eadc50262ee2c but it was later removed because it did not need version checks on the read side for bw compat. I'll add a comment explaining the gap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on PR #12908:
URL: https://github.com/apache/lucene/pull/12908#issuecomment-1850181921
> Hmmm... I agree we can't expect `BasePostingsFormatTestCase` to catch all bw compat problems, but the `TestLucene90PostingsFormat` from this PR writes data in the 9.8 format of the terms dictionary, and then reads the data with the 9.9 logic, only relying on version checks to read outputs with regular vlongs. So it should be exposing the bug?
Oh! I didn't realize it did this, OK. Then indeed it has a chance of uncovering the bug, but failed to do so :(
Note that the bug was quite hard to repro -- I started with various random simple strings, than realistic unicode strings, varying counts, term lengths, etc.. (using one Java tool to write the index using 9.8.0, and another to randomly search the index using 9.9.0), but bug would not repro after a great many iterations.
So then I switched to contiguous slices of `wikibigall` terms, and had to also randomize the `WildcardQuery` substring, and finally it repros quite readily.
Too many of our tests use unrealistic random terms ... realistic term distributions tickle the many `if` statements better. Does `BasePostingsFormatTestCase` index from `LineFileDocs`? If so, the nightly line docs file is quite a bit beefier and may have a better shot at repro? (And they are somewhat extracted from `enwiki` text...).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on code in PR #12908:
URL: https://github.com/apache/lucene/pull/12908#discussion_r1422538272
##########
lucene/core/src/java/org/apache/lucene/util/fst/FST.java:
##########
@@ -109,10 +109,20 @@ public enum INPUT_TYPE {
// Increment version to change it
private static final String FILE_FORMAT_NAME = "FST";
- private static final int VERSION_START = 6;
+
+ /** First supported version, this is the version that was used when releasing Lucene 7.0. */
+ public static final int VERSION_START = 6;
+
private static final int VERSION_LITTLE_ENDIAN = 8;
Review Comment:
Thanks @jpountz for the digital archaeology and explanation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on PR #12908:
URL: https://github.com/apache/lucene/pull/12908#issuecomment-1850104246
> I think I found the bug, the FST version was mistakenly initialized to 0 instead of Version.CURRENT in some cases, it should be good now.
Phew!!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on PR #12908:
URL: https://github.com/apache/lucene/pull/12908#issuecomment-1850255060
> OK, let me try feeding LineFileDocs into this test case.
FTR I will look into it, but it's probably best done in a follow-up PR rather than this one, let's merge this PR first?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz merged PR #12908:
URL: https://github.com/apache/lucene/pull/12908
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on PR #12908:
URL: https://github.com/apache/lucene/pull/12908#issuecomment-1849947295
I restarted that failed job.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on PR #12908:
URL: https://github.com/apache/lucene/pull/12908#issuecomment-1850094072
It's actually a bad news that all tests pass here, as this means that our `BasePostingsFormatTestCase` is not good enough to uncover the recent problem with `Terms#intersect`... So we could add a similar bug in the future, and we'd only learn about it after releasing when @mikemccand 's new bw compat test from #12901 starts running?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on code in PR #12908:
URL: https://github.com/apache/lucene/pull/12908#discussion_r1422549348
##########
lucene/backward-codecs/src/test/org/apache/lucene/backward_codecs/lucene90/Lucene90RWPostingsFormat.java:
##########
@@ -75,7 +75,11 @@ public FieldsConsumer fieldsConsumer(SegmentWriteState state) throws IOException
try {
FieldsConsumer ret =
new Lucene90BlockTreeTermsWriter(
- state, postingsWriter, minTermBlockSize, maxTermBlockSize);
+ state,
+ postingsWriter,
+ minTermBlockSize,
+ maxTermBlockSize,
+ Lucene90BlockTreeTermsReader.VERSION_START);
Review Comment:
No, because `Lucene90BlockTreeTermsReader.VERSION_START` maps to `FST.VERSION_90`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on PR #12908:
URL: https://github.com/apache/lucene/pull/12908#issuecomment-1850184681
OK, let me try feeding LineFileDocs into this test case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on code in PR #12908:
URL: https://github.com/apache/lucene/pull/12908#discussion_r1422449061
##########
lucene/core/src/java/org/apache/lucene/util/fst/FSTCompiler.java:
##########
@@ -325,6 +328,21 @@ public Builder<T> directAddressingMaxOversizingFactor(float factor) {
return this;
}
+ /** Expert: Set the codec version. * */
+ public Builder<T> setVersion(int version) {
+ if (version < FST.VERSION_START || version > FST.VERSION_CURRENT) {
Review Comment:
Hmm should we make this tighter, to `version < FST.VERSION_LITTLE_ENDIAN`? I.e. this code cannot write in the really old format before that? Just the pre and post continuous arcs format?
##########
lucene/core/src/java/org/apache/lucene/util/fst/FST.java:
##########
@@ -109,10 +109,20 @@ public enum INPUT_TYPE {
// Increment version to change it
private static final String FILE_FORMAT_NAME = "FST";
- private static final int VERSION_START = 6;
+
+ /** First supported version, this is the version that was used when releasing Lucene 7.0. */
+ public static final int VERSION_START = 6;
+
private static final int VERSION_LITTLE_ENDIAN = 8;
Review Comment:
Hmm what happened to `7`? Are we unable to count properly lol :) Or more likely we hard-removed this version because `main` no longer supports it? Or it was only used during SNAPSHOT builds, never released?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add tests for the 9.0->9.8 block tree terms dict format back. [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on code in PR #12908:
URL: https://github.com/apache/lucene/pull/12908#discussion_r1422487388
##########
lucene/core/src/java/org/apache/lucene/util/fst/FST.java:
##########
@@ -109,10 +109,20 @@ public enum INPUT_TYPE {
// Increment version to change it
private static final String FILE_FORMAT_NAME = "FST";
- private static final int VERSION_START = 6;
+
+ /** First supported version, this is the version that was used when releasing Lucene 7.0. */
+ public static final int VERSION_START = 6;
+
private static final int VERSION_LITTLE_ENDIAN = 8;
Review Comment:
Let me do some spelunking.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org