From 1336a43928a9cf2b4bfcdac0ab0874f3ff4b7317 Mon Sep 17 00:00:00 2001 From: Michael Gibney Date: Wed, 20 Sep 2023 13:23:47 -0400 Subject: [PATCH] port test from upstream PR; fix multivalued reuse bug (#146) --- .../solr/search/SolrDocumentFetcher.java | 4 + .../search/TestDocValuesIteratorCache.java | 141 ++++++++++++++++++ 2 files changed, 145 insertions(+) create mode 100644 solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java diff --git a/solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java b/solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java index a04a0d0d59b..7693ee4ac75 100644 --- a/solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java +++ b/solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java @@ -899,6 +899,8 @@ public SortedNumericDocValues getSortedNumericDocValues( dv = sndvs[leafOrd]; int currentDoc = dv.docID(); if (localId == currentDoc && !consumed[leafOrd]) { + consumed[leafOrd] = true; + noMatchSince[leafOrd] = DocIdSetIterator.NO_MORE_DOCS; return dv; } else if (localId <= currentDoc) { if (localId >= noMatchSince[leafOrd]) { @@ -949,6 +951,8 @@ public SortedSetDocValues getSortedSetDocValues(int localId, LeafReader leafRead dv = ssdvs[leafOrd]; int currentDoc = dv.docID(); if (localId == currentDoc && !consumed[leafOrd]) { + consumed[leafOrd] = true; + noMatchSince[leafOrd] = DocIdSetIterator.NO_MORE_DOCS; return dv; } else if (localId <= currentDoc) { if (localId >= noMatchSince[leafOrd]) { diff --git a/solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java b/solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java new file mode 100644 index 00000000000..38f163833a5 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java @@ -0,0 +1,141 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.search; + +import java.io.Closeable; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; +import java.util.Random; +import java.util.Set; +import org.apache.lucene.tests.util.LuceneTestCase; +import org.apache.lucene.tests.util.TestUtil; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer; +import org.apache.solr.common.SolrDocument; +import org.apache.solr.core.SolrCore; +import org.apache.solr.index.NoMergePolicyFactory; +import org.apache.solr.util.EmbeddedSolrServerTestRule; +import org.apache.solr.util.RefCounted; +import org.apache.solr.util.SolrClientTestRule; +import org.junit.ClassRule; + +public class TestDocValuesIteratorCache extends SolrTestCaseJ4 { + + private static final int DOC_COUNT = 1000; + + @ClassRule + public static final SolrClientTestRule solrClientTestRule = + new EmbeddedSolrServerTestRule() { + @Override + protected void before() throws Throwable { + // must set NoMergePolicyFactory, because OrdinalMap building depends on the predictable + // existence of multiple segments; if the merge policy happens to combine into a single + // segment, no OrdinalMap will be built, throwing off our tests + systemSetPropertySolrTestsMergePolicyFactory(NoMergePolicyFactory.class.getName()); + startSolr(LuceneTestCase.createTempDir()); + } + + @Override + protected void after() { + systemClearPropertySolrTestsMergePolicyFactory(); + super.after(); + } + }; + + @SuppressWarnings("try") + public void test() throws Exception { + Path configSet = LuceneTestCase.createTempDir(); + SolrTestCaseJ4.copyMinConf(configSet.toFile()); + Files.copy( + TEST_PATH().resolve("collection1/conf/schema-docValuesMulti.xml"), + configSet.resolve("conf/schema.xml"), + StandardCopyOption.REPLACE_EXISTING); + + solrClientTestRule.newCollection().withConfigSet(configSet.toString()).create(); + + SolrClient client = solrClientTestRule.getSolrClient(); + + Random r = random(); + String[][] expectVals = indexDocs(client, r); + + try (SolrCore core = + ((EmbeddedSolrServer) client).getCoreContainer().getCore(DEFAULT_TEST_CORENAME)) { + RefCounted sref = core.getSearcher(); + try (Closeable c = sref::decref) { + SolrIndexSearcher s = sref.get(); + assertEquals(DOC_COUNT, s.maxDoc()); + SolrDocumentFetcher docFetcher = s.getDocFetcher(); + Map dvIterCache = new HashMap<>(); + final Set getFields = Set.of("stringdv"); + final SolrDocument doc = new SolrDocument(); + for (int i = DOC_COUNT * 10; i >= 0; i--) { + int checkId = r.nextInt(DOC_COUNT); + doc.clear(); + docFetcher.decorateDocValueFields(doc, checkId, getFields, dvIterCache); + String[] expected = expectVals[checkId]; + if (expected == null) { + assertTrue(doc.isEmpty()); + } else { + assertEquals(1, doc.size()); + Collection actualVals = doc.getFieldValues("stringdv"); + assertEquals(expected.length, actualVals.size()); + int j = 0; + for (Object o : actualVals) { + assertEquals(expected[j++], o); + } + } + } + } + } + } + + private String[][] indexDocs(SolrClient client, Random r) + throws SolrServerException, IOException { + String[][] ret = new String[DOC_COUNT][]; + int pct = r.nextInt(100); + for (int i = 0; i < DOC_COUNT; i++) { + if (r.nextInt(100) > pct) { + client.add(sdoc("id", Integer.toString(i))); + } else { + String str1 = TestUtil.randomSimpleString(r); + String str2 = TestUtil.randomSimpleString(r); + client.add(sdoc("id", Integer.toString(i), "stringdv", str1, "stringdv", str2)); + int cmp = str1.compareTo(str2); + if (cmp == 0) { + ret[i] = new String[] {str1}; + } else if (cmp < 0) { + ret[i] = new String[] {str1, str2}; + } else { + ret[i] = new String[] {str2, str1}; + } + } + if (r.nextInt(DOC_COUNT / 5) == 0) { + // aim for 5 segments + client.commit(); + } + } + client.commit(); + return ret; + } +}