[ 
https://issues.apache.org/jira/browse/LUCENE-6554?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14595036#comment-14595036
 ] 

Martijn van Groningen commented on LUCENE-6554:
-----------------------------------------------

I would like this feature to remain. I think implementing this via a selector 
is okay, its more code, but that shouldn't be too bad. I don't like the 
workaround, because the aggregated information relies on the child query and to 
aggregate it properly the child information needs to be aggregated multiple 
times. I think it is good that someone can choose to avoid duplication of data 
with block join sorting at the cost that this is linear to the number of 
matching child documents.

> ToBlockJoinFieldComparator wrapping is illegal
> ----------------------------------------------
>
>                 Key: LUCENE-6554
>                 URL: https://issues.apache.org/jira/browse/LUCENE-6554
>             Project: Lucene - Core
>          Issue Type: Improvement
>            Reporter: Adrien Grand
>            Assignee: Adrien Grand
>         Attachments: LUCENE-6554.patch
>
>
> The following test case triggers an AssertionError:
> {code}
>   public void testMissingValues() throws IOException {
>     final Directory dir = newDirectory();
>     final RandomIndexWriter w = new RandomIndexWriter(random(), dir, 
> newIndexWriterConfig(new MockAnalyzer(random()))
>         .setMergePolicy(NoMergePolicy.INSTANCE));
>     w.addDocument(new Document());
>     w.getReader().close();
>     w.addDocument(new Document());
>     IndexReader reader = w.getReader();
>     w.close();
>     IndexSearcher searcher = newSearcher(reader);
>     // all docs are parent
>     BitDocIdSetFilter parentFilter = new BitDocIdSetCachingWrapperFilter(new 
> QueryWrapperFilter(new MatchAllDocsQuery()));
>     BitDocIdSetFilter childFilter = new BitDocIdSetCachingWrapperFilter(new 
> QueryWrapperFilter(new MatchNoDocsQuery()));
>     ToParentBlockJoinSortField sortField = new ToParentBlockJoinSortField(
>         "some_random_field", SortField.Type.STRING, false, parentFilter, 
> childFilter
>     );
>     Sort sort = new Sort(sortField);
>     TopFieldDocs topDocs = searcher.search(new MatchAllDocsQuery(), 1, sort);
>     searcher.getIndexReader().close();
>     dir.close();
>   }
> {code}
> {code}
> java.lang.AssertionError
>       at 
> __randomizedtesting.SeedInfo.seed([E9D45D81F597AE4B:83490FC7D11D9ABA]:0)
>       at 
> org.apache.lucene.search.FieldComparator$TermOrdValComparator.setBottom(FieldComparator.java:800)
>       at 
> org.apache.lucene.search.FieldComparator$TermOrdValComparator.getLeafComparator(FieldComparator.java:783)
>       at 
> org.apache.lucene.search.join.ToParentBlockJoinFieldComparator.doSetNextReader(ToParentBlockJoinFieldComparator.java:83)
>       at 
> org.apache.lucene.search.SimpleFieldComparator.getLeafComparator(SimpleFieldComparator.java:36)
>       at 
> org.apache.lucene.search.FieldValueHitQueue.getComparators(FieldValueHitQueue.java:183)
>       at 
> org.apache.lucene.search.TopFieldCollector$NonScoringCollector.getLeafCollector(TopFieldCollector.java:141)
>       at 
> org.apache.lucene.search.FilterCollector.getLeafCollector(FilterCollector.java:40)
>       at 
> org.apache.lucene.search.AssertingCollector.getLeafCollector(AssertingCollector.java:48)
>       at org.apache.lucene.search.IndexSearcher.search(IndexSearcher.java:611)
>       at 
> org.apache.lucene.search.AssertingIndexSearcher.search(AssertingIndexSearcher.java:92)
>       at org.apache.lucene.search.IndexSearcher.search(IndexSearcher.java:424)
>       at org.apache.lucene.search.IndexSearcher.search(IndexSearcher.java:543)
>       at 
> org.apache.lucene.search.IndexSearcher.searchAfter(IndexSearcher.java:528)
>       at org.apache.lucene.search.IndexSearcher.search(IndexSearcher.java:455)
>       at 
> org.apache.lucene.search.join.TestBlockJoinSorting.testMissingValues(TestBlockJoinSorting.java:347)
>       at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>       at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>       at java.lang.reflect.Method.invoke(Method.java:483)
>       at 
> com.carrotsearch.randomizedtesting.RandomizedRunner.invoke(RandomizedRunner.java:1627)
>       at 
> com.carrotsearch.randomizedtesting.RandomizedRunner$6.evaluate(RandomizedRunner.java:836)
>       at 
> com.carrotsearch.randomizedtesting.RandomizedRunner$7.evaluate(RandomizedRunner.java:872)
>       at 
> com.carrotsearch.randomizedtesting.RandomizedRunner$8.evaluate(RandomizedRunner.java:886)
>       at 
> org.apache.lucene.util.TestRuleSetupTeardownChained$1.evaluate(TestRuleSetupTeardownChained.java:50)
>       at 
> org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:46)
>       at 
> org.apache.lucene.util.TestRuleThreadAndTestName$1.evaluate(TestRuleThreadAndTestName.java:49)
>       at 
> org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:65)
>       at 
> org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:48)
>       at 
> com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
>       at 
> com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:365)
>       at 
> com.carrotsearch.randomizedtesting.ThreadLeakControl.forkTimeoutingTask(ThreadLeakControl.java:798)
>       at 
> com.carrotsearch.randomizedtesting.ThreadLeakControl$3.evaluate(ThreadLeakControl.java:458)
>       at 
> com.carrotsearch.randomizedtesting.RandomizedRunner.runSingleTest(RandomizedRunner.java:845)
>       at 
> com.carrotsearch.randomizedtesting.RandomizedRunner$3.evaluate(RandomizedRunner.java:747)
>       at 
> com.carrotsearch.randomizedtesting.RandomizedRunner$4.evaluate(RandomizedRunner.java:781)
>       at 
> com.carrotsearch.randomizedtesting.RandomizedRunner$5.evaluate(RandomizedRunner.java:792)
>       at 
> org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:46)
>       at 
> com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
>       at 
> org.apache.lucene.util.TestRuleStoreClassName$1.evaluate(TestRuleStoreClassName.java:42)
>       at 
> com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:39)
>       at 
> com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:39)
>       at 
> com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
>       at 
> com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
>       at 
> com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
>       at 
> org.apache.lucene.util.TestRuleAssertionsRequired$1.evaluate(TestRuleAssertionsRequired.java:54)
>       at 
> org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:48)
>       at 
> org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:65)
>       at 
> org.apache.lucene.util.TestRuleIgnoreTestSuites$1.evaluate(TestRuleIgnoreTestSuites.java:55)
>       at 
> com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
>       at 
> com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:365)
>       at java.lang.Thread.run(Thread.java:745)
> {code}
> The reason is that when a parent document does not have children, 
> ToParentBlockJoinComparator simply omits to forward calls to {{copy}} to the 
> wrapped comparator. So the wrapped comparator ends up with allocated slots 
> that have 0 as an ordinal (the default value in an array) and a null value, 
> which is illegal since 0 is a legal ordinal which can't map to null.
> This can't be fixed without adding new methods to the already crazy 
> comparator API, so I think there is nothing we can do but remove this 
> comparator.
> It would be possible to achieve the same functionnality by implementing 
> something similar to SortedNumericSelector, except that it would have to 
> select across several docs instead of values.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to