You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/01/15 14:56:56 UTC

[GitHub] [lucene-solr] gerlowskija opened a new pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

gerlowskija opened a new pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171

# Description

Some join usecases are better served (performance wise) by using top-level docValues structures, because they avoid some wasteful seeking and iteration that the per-segment alternatives do.

# Solution
This PR introduces a "TopLevelJoinQuery" which can be run on join's where the fields in question have docValues enabled. This scales much better with the number of "from" query matches than existing alternatives. Since it's implemented as a Two-Phase Iterator it scales less well by the number of overall matches. See SOLR-13892 for more in-depth performance discussion.

# Tests

Beefed up tests in `TestJoin` to prove correctness. Wrote a performance test driver `TestJoinQueryPerformance` that allows reproduction of the perf results. (Though this will not be committed in the final version)

# Checklist

Please review the following and check all that apply:

- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `master` branch.
- [x] I have run `ant precommit` and the appropriate test suite.
- [x] I have added tests for my changes.
- [x] I have added documentation for the [Ref Guide](https://github.com/apache/lucene-solr/tree/master/solr/solr-ref-guide) (for Solr changes only).

----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#issuecomment-580284541
 
 
   >  I'm not familiar enough with how rewrites work to speak to it.
   
   I recommend you become familiar with that, as it's a step in overall query processing and you are here writing a new Query, so....    Just saying.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#issuecomment-576700003
 
 
   See Lucene's SingletonSortedSetDocValues

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija commented on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija commented on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#issuecomment-577188503
 
 
   That's exactly what I was looking for.  I knew it had to exist already.  Thanks David!

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370910102
 
 

 ##########
 File path: solr/core/src/test/org/apache/solr/search/TestJoinQueryPerformance.java
 ##########
 @@ -0,0 +1,235 @@
+/*
+ * 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.BufferedReader;
+import java.io.FileReader;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.UUID;
+
+import com.google.common.collect.Lists;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrInputDocument;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+
+public class TestJoinQueryPerformance extends SolrTestCaseJ4 {
+  // Dictionary used to load String data
+  private static final String DICT_PATH = "/usr/share/dict/words";
+  private static final int NUM_DICT_TERMS = 235886;
+  private static final String[] LOADED_DICTIONARY = new String[NUM_DICT_TERMS];
+
+  // Performance run parameters: Indexing
+  private static final String FROM_COLLECTION_NAME = "user_acls";
+  private static final int NUM_FROM_DOCS = 5050; // 1 + 2 + 3 + 4 + ...  + 100
+  private static final String TO_COLLECTION_NAME = "products";
+  private static final int NUM_TO_DOCS = 500000;
+  private static final int PERMISSION_CARDINALITY = 50000; // 50K unique groups/roles/whatever
+  private static int BATCH_SIZE = 500;
+  private static int NUM_COMMITS = 500;
+  private static final int VAL_MAX = 1000;
+  private static final int USER_MAX = 100;
+
+  private static String COLLECTION_NAME= "foo";
+
+  /*
+   * As I start out here, I think I'll want a few different axes.
+   *  - "from" collection matches (with "to" matches held constant)
+   *  - "to" collection matches (with "from" matches held constant)
+   *
+   * So I think I should index a finite number of docs
+   */
+
+  @BeforeClass
+  public static void setUpCluster() throws Exception {
+    loadDictionary();
+    //loadCollectionData(DType.USER);
+    //loadCollectionData(DType.DATA);
+  }
+
+  private static void loadDictionary() throws Exception {
+    try (BufferedReader reader = new BufferedReader(new FileReader(DICT_PATH))) {
+      for (int i = 0; i < NUM_DICT_TERMS; i++) {
+        LOADED_DICTIONARY[i] = reader.readLine();
+      }
+    }
+  }
+
+  public enum DType {
+    USER(NUM_FROM_DOCS, FROM_COLLECTION_NAME) {
+      // id - unique string
+      // userid_s - username (user# from 1-100)...each user appears in # entries
+      // permissions_ss - set of 300 string permissions (cardinality 50K)
+      @Override
+      SolrInputDocument buildDoc() {
+        if (userRecordCounts[currentUser - 1] == currentUser) {
+          currentUser++;
+        } else {
+          userRecordCounts[currentUser -1]++;
+        }
+
+        final SolrInputDocument newDoc = new SolrInputDocument("id", UUID.randomUUID().toString());
+        final String userString = "user" + currentUser;
+        final String[] permissions = getAFewDictionaryWords(300, PERMISSION_CARDINALITY);
+
+        newDoc.addField("userid_s", userString);
+        newDoc.addField("permissions_ss", permissions);
+
+        return newDoc;
+      }
+    },
+    DATA(NUM_TO_DOCS, TO_COLLECTION_NAME) {
+      // id - unique string
+      // val_i - random int between 1-1000
+      // cost_d - random cost between 1-1000
+      // body_txt - random text string between 100 - 10000 words
+      // acl_ss - set of 100-3000 string permissions (cardinality 50K)
+      @Override
+      SolrInputDocument buildDoc() {
+        final SolrInputDocument newDoc = new SolrInputDocument("id", UUID.randomUUID().toString());
+        final int val = random().nextInt(1000) + 1;
+        final double cost = random().nextDouble() * 1000d;
+        final String body = String.join(" ", getAFewDictionaryWords(random().nextInt(9900) + 100));
+        final String[] acls = getAFewDictionaryWords(random().nextInt(2900) + 100, PERMISSION_CARDINALITY);
+
+        newDoc.addField("val_i", val);
+        newDoc.addField("cost_d", cost);
+        newDoc.addField("body_txt", body);
+        newDoc.addField("acl_ss", acls);
+
+        return newDoc;
+      }
+    };
+
+    private int numDocs;
+    private String collName;
+    private static int[] userRecordCounts = new int[100];
+    private static int currentUser = 1;
+
+    private DType(int numDocs, String collectionName) {
+      this.numDocs = numDocs;
+      this.collName = collectionName;
+    }
+
+    abstract SolrInputDocument buildDoc();
+  }
+
+  private static void loadCollectionData(DType type) throws Exception {
+    int numDocs = type.numDocs;
+    String collectionName = type.collName;
+    int numLoaded = 0;
+    try (HttpSolrClient client = new HttpSolrClient.Builder("http://localhost:8983/solr").build()) {
+      final int numBatches = numDocs / BATCH_SIZE + 1;
+      final int commitEveryBatches = NUM_COMMITS > 0 ? numBatches / NUM_COMMITS : Integer.MAX_VALUE;
+      int batchCount = 0;
+      while (numLoaded < numDocs) {
+        final int sizeOfBatch = numLoaded + BATCH_SIZE > numDocs ? numDocs - numLoaded : BATCH_SIZE;
+        final Collection<SolrInputDocument> batch = buildBatch(type, sizeOfBatch);
+        client.add(collectionName, batch);
+        batchCount++;
+        numLoaded+=sizeOfBatch;
+
+        if (batchCount == commitEveryBatches) {
+          client.commit(collectionName);
+          batchCount = 0;
+        }
+      }
+      client.commit(collectionName);
+    }
+
+  }
+
+  private static Collection<SolrInputDocument> buildBatch(DType type, int sizeOfBatch) {
+    final List<SolrInputDocument> batch = Lists.newArrayList();
+    for (int i = 0; i < sizeOfBatch; i++) {
+      batch.add(type.buildDoc());
+    }
+    return batch;
+  }
+
+  private static String[] getAFewDictionaryWords(int numWords) {
+    return getAFewDictionaryWords(numWords, NUM_DICT_TERMS);
+  }
+
+  private static String[] getAFewDictionaryWords(int numWords, int onlyFirstN) {
+    final String[] words = new String[numWords];
+    for (int i = 0; i < numWords; i++) {
+      words[i] = LOADED_DICTIONARY[random().nextInt(onlyFirstN)];
+    }
+
+    return words;
+  }
+
+
+  @Test
+  public void testJoinPerformanceAsMainQueryHitsIncrease() throws Exception {
+    final String joinQueryBase = "{!join fromIndex=" + FROM_COLLECTION_NAME + " from=permissions_ss to=acl_ss cache=false";
+    final String fromQuery = "userid_s:user25"; // The higher the user number, the more permissions he has attached to his name (1-100)
+    final String standardJoin = joinQueryBase + "}" + fromQuery;
+    final String noScoreJoin = joinQueryBase + " score=none}" + fromQuery;
+    final String tpiJoin = joinQueryBase + " toplevel=true}" + fromQuery;
+
+    try (HttpSolrClient client = new HttpSolrClient.Builder("http://localhost:8983/solr").build()) {
+      for ( int i = 0; i < VAL_MAX; i+=20) {
+        final String mainQuery = "val_i:[1 TO " + (i+1) + "]";
+        final QueryResponse standardJoinRsp = client.query(TO_COLLECTION_NAME, new SolrQuery("q", mainQuery, "fq", standardJoin), SolrRequest.METHOD.POST);
+        final QueryResponse tpiJoinRsp = client.query(TO_COLLECTION_NAME, new SolrQuery("q", mainQuery, "fq", tpiJoin), SolrRequest.METHOD.POST);
+        final QueryResponse noScoreJoinRsp = client.query(TO_COLLECTION_NAME, new SolrQuery("q", mainQuery, "fq", noScoreJoin), SolrRequest.METHOD.POST);
+        final long numFound = tpiJoinRsp.getResults().getNumFound();
+
+        System.out.println(i + "," + numFound + "," + standardJoinRsp.getQTime() + "," + noScoreJoinRsp.getQTime() + "," + tpiJoinRsp.getQTime());
+      }
+    }
+  }
+
+  @Test
+  public void testJoinPerformanceAsFromQueryHitsIncrease() throws Exception {
+    final String mainQuery = "val_i:[1 TO 250]"; // Half the docs match the query (250K)
+
+    final String joinQueryBase = "{!join fromIndex=" + FROM_COLLECTION_NAME + " from=permissions_ss to=acl_ss cache=false";
+
+
+    try (HttpSolrClient client = new HttpSolrClient.Builder("http://localhost:8983/solr").build()) {
+      for ( int i = 1; i <= USER_MAX; i++) {
+        final String fromQuery = "userid_s:user" + i;
+        final String standardJoin = joinQueryBase + "}" + fromQuery;
+        final String noScoreJoin = joinQueryBase + " score=none}" + fromQuery;
+        final String tpiJoin = joinQueryBase + " toplevel=true}" + fromQuery;
+
+        final QueryResponse standardJoinRsp = client.query(TO_COLLECTION_NAME, new SolrQuery("q", mainQuery, "fq", standardJoin), SolrRequest.METHOD.POST);
+        final QueryResponse tpiJoinRsp = client.query(TO_COLLECTION_NAME, new SolrQuery("q", mainQuery, "fq", tpiJoin), SolrRequest.METHOD.POST);
+        final QueryResponse noScoreJoinRsp = client.query(TO_COLLECTION_NAME, new SolrQuery("q", mainQuery, "fq", noScoreJoin), SolrRequest.METHOD.POST);
+        final long numFound = tpiJoinRsp.getResults().getNumFound();
+
+        System.out.println(i + "," + numFound + "," + standardJoinRsp.getQTime() + "," + noScoreJoinRsp.getQTime() + "," + tpiJoinRsp.getQTime());
 
 Review comment:
   Hey Mikhail, thanks for the review.  I have no intentions of committing this file.  I kept it in the PR up to this point so that people could reproduce/tweak the perf test I ran.  But I'm going to remove it prior to commit.
   
   (In fact, I thought I already had removed it, but there was one last "cleanup" commit I'd neglected to push until right now.  You should see this file gone from the PR 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija merged pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija merged pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija edited a comment on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija edited a comment on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#issuecomment-577188503
 
 
   That's 100% what I was looking for.  I knew it had to exist somewhere.  Thanks David!
   
   ~It's package scoped, but I'll file a LUCENE jira making the argument for wider visibility and see where that goes.~ Nevermind, I see there's a static accessor method on DocValues

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370304900
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java
 ##########
 @@ -139,11 +197,16 @@ public static Query createJoinQuery(Query subQuery, String fromField, String toF
 
 
 class JoinQuery extends Query {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
 Review comment:
   Not used?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370311943
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/search/join/MultiValueTermOrdinalCollector.java
 ##########
 @@ -0,0 +1,72 @@
+/*
+ * 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.join;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.index.SortedSetDocValues;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.util.LongBitSet;
+import org.apache.solr.search.DelegatingCollector;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Populates a bitset of (top-level) ordinals based on field values in a multi-valued field.
+ */
+public class MultiValueTermOrdinalCollector extends DelegatingCollector {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
 Review comment:
   Nothing to log

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370281149
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java
 ##########
 @@ -59,67 +60,124 @@
 import org.apache.solr.search.join.ScoreJoinQParserPlugin;
 import org.apache.solr.util.RTimer;
 import org.apache.solr.util.RefCounted;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class JoinQParserPlugin extends QParserPlugin {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
   public static final String NAME = "join";
+  /** Choose the internal algorithm */
+  private static final String METHOD = "method";
+
+  private static class JoinParams {
+    final String fromField;
+    final String fromCore;
+    final Query fromQuery;
+    final long fromCoreOpenTime;
+    final String toField;
+
+    public JoinParams(String fromField, String fromCore, Query fromQuery, long fromCoreOpenTime, String toField) {
+      this.fromField = fromField;
+      this.fromCore = fromCore;
+      this.fromQuery = fromQuery;
+      this.fromCoreOpenTime = fromCoreOpenTime;
+      this.toField = toField;
+    }
+  }
+
+  private enum Method {
+    index {
+      @Override
+      Query makeFilter(QParser qparser) throws SyntaxError {
+        final JoinParams jParams = parseJoin(qparser);
+        final JoinQuery q = new JoinQuery(jParams.fromField, jParams.toField, jParams.fromCore, jParams.fromQuery);
+        q.fromCoreOpenTime = jParams.fromCoreOpenTime;
+        return q;
+      }
+    },
+    dvWithScore {
+      @Override
+      Query makeFilter(QParser qparser) throws SyntaxError {
+        return new ScoreJoinQParserPlugin().createParser(qparser.qstr, qparser.localParams, qparser.params, qparser.req).parse();
+      }
+    },
+    topLevelDV {
+      @Override
+      Query makeFilter(QParser qparser) throws SyntaxError {
+        final JoinParams jParams = parseJoin(qparser);
+        final JoinQuery q = new TopLevelJoinQuery(jParams.fromField, jParams.toField, jParams.fromCore, jParams.fromQuery);
+        q.fromCoreOpenTime = jParams.fromCoreOpenTime;
+        return q;
+      }
+    };
+
+    abstract Query makeFilter(QParser qparser) throws SyntaxError;
+
+    JoinParams parseJoin(QParser qparser) throws SyntaxError {
+      final String fromField = qparser.getParam("from");
+      final String fromIndex = qparser.getParam("fromIndex");
+      final String toField = qparser.getParam("to");
+      final String v = qparser.localParams.get("v");
 
 Review comment:
   Use `QueryParsing.V`  thus also signals this isn't special

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r371376748
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/search/TopLevelJoinQuery.java
 ##########
 @@ -0,0 +1,218 @@
+/*
+ * 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.IOException;
+import java.lang.invoke.MethodHandles;
+
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.index.LeafReader;
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.index.SortedSetDocValues;
+import org.apache.lucene.search.Collector;
+import org.apache.lucene.search.ConstantScoreScorer;
+import org.apache.lucene.search.ConstantScoreWeight;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Scorer;
+import org.apache.lucene.search.TwoPhaseIterator;
+import org.apache.lucene.search.Weight;
+import org.apache.lucene.util.BytesRef;
+import org.apache.lucene.util.LongBitSet;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.join.MultiValueTermOrdinalCollector;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TopLevelJoinQuery extends JoinQuery {
 
 Review comment:
   Always add at least one sentence javadoc for a class

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r371286866
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java
 ##########
 @@ -59,67 +60,124 @@
 import org.apache.solr.search.join.ScoreJoinQParserPlugin;
 import org.apache.solr.util.RTimer;
 import org.apache.solr.util.RefCounted;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class JoinQParserPlugin extends QParserPlugin {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
   public static final String NAME = "join";
+  /** Choose the internal algorithm */
+  private static final String METHOD = "method";
+
+  private static class JoinParams {
+    final String fromField;
+    final String fromCore;
+    final Query fromQuery;
+    final long fromCoreOpenTime;
+    final String toField;
+
+    public JoinParams(String fromField, String fromCore, Query fromQuery, long fromCoreOpenTime, String toField) {
+      this.fromField = fromField;
+      this.fromCore = fromCore;
+      this.fromQuery = fromQuery;
+      this.fromCoreOpenTime = fromCoreOpenTime;
+      this.toField = toField;
+    }
+  }
+
+  private enum Method {
+    index {
+      @Override
+      Query makeFilter(QParser qparser) throws SyntaxError {
+        final JoinParams jParams = parseJoin(qparser);
+        final JoinQuery q = new JoinQuery(jParams.fromField, jParams.toField, jParams.fromCore, jParams.fromQuery);
+        q.fromCoreOpenTime = jParams.fromCoreOpenTime;
+        return q;
+      }
+    },
+    dvWithScore {
+      @Override
+      Query makeFilter(QParser qparser) throws SyntaxError {
+        return new ScoreJoinQParserPlugin().createParser(qparser.qstr, qparser.localParams, qparser.params, qparser.req).parse();
+      }
+    },
+    topLevelDV {
+      @Override
+      Query makeFilter(QParser qparser) throws SyntaxError {
+        final JoinParams jParams = parseJoin(qparser);
+        final JoinQuery q = new TopLevelJoinQuery(jParams.fromField, jParams.toField, jParams.fromCore, jParams.fromQuery);
+        q.fromCoreOpenTime = jParams.fromCoreOpenTime;
+        return q;
+      }
+    };
+
+    abstract Query makeFilter(QParser qparser) throws SyntaxError;
+
+    JoinParams parseJoin(QParser qparser) throws SyntaxError {
+      final String fromField = qparser.getParam("from");
+      final String fromIndex = qparser.getParam("fromIndex");
+      final String toField = qparser.getParam("to");
+      final String v = qparser.localParams.get("v");
+      final String coreName;
+
+      Query fromQuery;
+      long fromCoreOpenTime = 0;
+
+      if (fromIndex != null && !fromIndex.equals(qparser.req.getCore().getCoreDescriptor().getName()) ) {
+        CoreContainer container = qparser.req.getCore().getCoreContainer();
+
+        // if in SolrCloud mode, fromIndex should be the name of a single-sharded collection
+        coreName = ScoreJoinQParserPlugin.getCoreName(fromIndex, container);
+
+        final SolrCore fromCore = container.getCore(coreName);
+        if (fromCore == null) {
+          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+              "Cross-core join: no such core " + coreName);
+        }
+
+        RefCounted<SolrIndexSearcher> fromHolder = null;
+        LocalSolrQueryRequest otherReq = new LocalSolrQueryRequest(fromCore, qparser.params);
+        try {
 
 Review comment:
   Totally agree - should be using try-with-resources here.
   
   But I'm reluctant to introduce changes here that aren't strictly necessary.  (Github shows this section as "added", but really it was just moved from elsewhere in the file.)
   
   My opinion on this changes, but I've been burned too many times recently by adding a "harmless" refactor into a related commit, only for that to cause issues later that force a revert.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija commented on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija commented on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#issuecomment-578801824
 
 
   > In JoinQuery.rewrite I see it says explicitly "don't rewrite the subQuery" but why not? If it never gets rewritten then that's a bug.
   
   That's a question for the original Join author maybe?  I'm not familiar enough with how rewrites work to speak to 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] mkhludnev commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

mkhludnev commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370547212
 
 

 ##########
 File path: solr/core/src/test/org/apache/solr/search/TestJoinQueryPerformance.java
 ##########
 @@ -0,0 +1,235 @@
+/*
+ * 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.BufferedReader;
+import java.io.FileReader;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.UUID;
+
+import com.google.common.collect.Lists;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrInputDocument;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+
+public class TestJoinQueryPerformance extends SolrTestCaseJ4 {
 
 Review comment:
   How long does it take to run? Souldn't it be @Slow? 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370320580

##########
File path: solr/solr-ref-guide/src/other-parsers.adoc
##########
@@ -591,36 +591,95 @@ The hash range query parser uses a special cache to improve the speedup of the q

== Join Query Parser

-`JoinQParser` extends the `QParserPlugin`. It allows normalizing relationships between documents with a join operation. This is different from the concept of a join in a relational database because no information is being truly joined. An appropriate SQL analogy would be an "inner query".
+The Join query parser allows users to run queries that normalize relationships between documents.
+Solr runs a subquery of the user's choosing (the `v` param), identifies all the values that matching documents have in a field of interest (the `from` param), and then returns documents where those values are contained in a second field of interest (the `to` param).

-Examples:
-
-Find all products containing the word "ipod", join them against manufacturer docs and return the list of manufacturers:
+In practice, these semantics are much like "inner queries" in a SQL engine.
+As an example, consider the Solr query below:

[source,text]
----
-{!join from=manu_id_s to=id}ipod
+/solr/techproducts/select?q={!join from=manu_id_s to=id}title:ipod
----

-Find all manufacturer docs named "belkin", join them against product docs, and filter the list to only products with a price less than $12:
+This query, which returns a document for each manufacturer that makes a product with "ipod" in the title, is semantically identical to the SQL query below:

[source,text]
----
-q = {!join from=id to=manu_id_s}compName_s:Belkin
-fq = price:[* TO 12]
+SELECT *
+FROM techproducts
+WHERE id IN (
+ SELECT manu_id_s
+ FROM techproducts
+ WHERE title='ipod'
+ )
----

-The join operation is done on a term basis, so the "from" and "to" fields must use compatible field types. For example: joining between a `StrField` and a `IntPointField` will not work, likewise joining between a `StrField` and a `TextField` that uses `LowerCaseFilterFactory` will only work for values that are already lower cased in the string field.
+The join operation is done on a term basis, so the `from` and `to` fields must use compatible field types.
+For example: joining between a `StrField` and a `IntPointField` will not work.
+Likewise joining between a `StrField` and a `TextField` that uses `LowerCaseFilterFactory` will only work for values that are already lower cased in the string field.
+
+=== Parameters
+
+This query parser takes the following parameters:
+
+`from`::
+The name of a field which contains values to look for in the "to" field.
+Can be single or multi-valued, but must have a field type compatible with the field represented in the "to" field.
+This parameter is required.
+
+`to`::
+The name of a field whose value(s) will be checked against those found in the "from" field.
+Can be single or multi-valued, but must have a field type compatible with the "from" field.
+This parameter is required.
+
+`fromIndex`::
+The name of the index to run the "from" query (`v` parameter) on and where "from" values are gathered.
+Must be located on the same node as the core processing the request.
+This parameter is optional; it defaults to the value of the processing core if not specified.
+See <<Joining Across Collections,Joining Across Collections>> below for more information.
+
+`score`::
+An optional parameter that instructs Solr to return information about the "from" query scores.
+The value of this parameter controls what type of aggregation information is returned.
+Options include `avg` (average), `max` (maximum), `min` (minimum), `total` (total), or `none`.
++
+If `method` is not specified but `score` is, then the `dvWithScore` method is used.
+If `method` is specified and is not `dvWithScore`, then the `score` value is ignored.
+See the `method` parameter documentation below for more details.

-=== Join Parser Scoring

-You can optionally use the `score` parameter to return scores of the subordinate query. The values to use for this parameter define the type of aggregation, which are `avg` (average), `max` (maximum), `min` (minimum) `total`, or `none`.
+`method`::
+An optional parameter used to determine which of several query implementations should be used by Solr.
+Options are restricted to: `index`, `dvWithScore`, and `topLevelDV`.
+If unspecified the default value is `index`, unless the `score` parameter is present which overrides it to `dvWithScore`.
+Each implementation has its own performance characteristics, and users are encouraged to experiment to determine which implementation is most performant for their use-case.
+Details and performance heuristics are given below.
++
+`index` the default `method` unless the `score` parameter is specified.
+Uses inverted index structures to process the request.
+Performance scales linearly with the number of values matched in the "from" field.
+Consider this method when the "from" query matches few documents, when the "to" side returns a large number of documents, or when sporadic post-commit slowdowns cannot be tolerated (this a disadvantage of other methods that `index` avoids).

-.Score parameter and single value numerics
++
+`dvWithScore` returns an optional "score" statistic alongside result documents.
+Uses docValues structures if available, but falls back to uninverted values when necessary.
+Uninversion slows down the initial requests following a commit and takes up additional space on the JVM heap, so docValues are recommended in most situations.

Review comment:
"Uninversion" -> or maybe "First access to the field cache" ?

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370318149
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/search/join/TopLevelDVTermsCollector.java
 ##########
 @@ -0,0 +1,78 @@
+/*
+ * 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.join;
+
+import java.io.IOException;
+
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.index.SortedSetDocValues;
+import org.apache.lucene.search.LeafCollector;
+import org.apache.lucene.search.Scorable;
+import org.apache.lucene.util.LongBitSet;
+import org.apache.solr.search.DelegatingCollector;
+
+/**
+ * Collects all documents with a field value matching a set value in an ordinal bitset.
+ *
+ * Implementation is similar to {@link org.apache.lucene.search.join.TermsCollector}, but uses top-level ordinals
+ * explicitly and has wider visibility.
+ */
+public class TopLevelDVTermsCollector extends DelegatingCollector {
 
 Review comment:
   This class seems unused; no?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r373111544

##########
File path: solr/solr-ref-guide/src/other-parsers.adoc
##########
@@ -591,36 +591,95 @@ The hash range query parser uses a special cache to improve the speedup of the q

== Join Query Parser

[source,text]
----
-{!join from=manu_id_s to=id}ipod
+/solr/techproducts/select?q={!join from=manu_id_s to=id}title:ipod
----

-=== Join Parser Scoring

-You can optionally use the `score` parameter to return scores of the subordinate query. The values to use for this parameter define the type of aggregation, which are `avg` (average), `max` (maximum), `min` (minimum) `total`, or `none`.
+`method`::
+An optional parameter used to determine which of several query implementations should be used by Solr.
+Options are restricted to: `index`, `dvWithScore`, and `topLevelDV`.
+If unspecified the default value is `index`, unless the `score` parameter is present which overrides it to `dvWithScore`.
+Each implementation has its own performance characteristics, and users are encouraged to experiment to determine which implementation is most performant for their use-case.
+Details and performance heuristics are given below.
++
+`index` the default `method` unless the `score` parameter is specified.
+Uses the terms index structures to process the request.
+Performance scales linearly with the number of values matched in the "from" field.
+Consider this method when the "from" query matches few documents, when the "to" side returns a large number of documents, or when sporadic post-commit slowdowns cannot be tolerated (this is a disadvantage of other methods that `index` avoids).

Review comment:
I was surprised in that thread that the user got no results without docValues. The comments and code itself on ScoreJoinQParser make it look like docValues aren't strictly required, but you're right - it's totally possible this hasn't been working. Let me know if that's necessary.

I'll leave this wording as-is for now, and wait for clarification as you investigate SOLR-14230. If you don't have time to address 14230 soon, I'd rather not leave this blocked, so I'll figure out some intermediate wording.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r373111544

##########
File path: solr/solr-ref-guide/src/other-parsers.adoc
##########
@@ -591,36 +591,95 @@ The hash range query parser uses a special cache to improve the speedup of the q

== Join Query Parser

[source,text]
----
-{!join from=manu_id_s to=id}ipod
+/solr/techproducts/select?q={!join from=manu_id_s to=id}title:ipod
----

-=== Join Parser Scoring

-You can optionally use the `score` parameter to return scores of the subordinate query. The values to use for this parameter define the type of aggregation, which are `avg` (average), `max` (maximum), `min` (minimum) `total`, or `none`.
+`method`::
+An optional parameter used to determine which of several query implementations should be used by Solr.
+Options are restricted to: `index`, `dvWithScore`, and `topLevelDV`.
+If unspecified the default value is `index`, unless the `score` parameter is present which overrides it to `dvWithScore`.
+Each implementation has its own performance characteristics, and users are encouraged to experiment to determine which implementation is most performant for their use-case.
+Details and performance heuristics are given below.
++
+`index` the default `method` unless the `score` parameter is specified.
+Uses the terms index structures to process the request.
+Performance scales linearly with the number of values matched in the "from" field.
+Consider this method when the "from" query matches few documents, when the "to" side returns a large number of documents, or when sporadic post-commit slowdowns cannot be tolerated (this is a disadvantage of other methods that `index` avoids).

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] mkhludnev commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

mkhludnev commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r372784319

##########
File path: solr/solr-ref-guide/src/other-parsers.adoc
##########
@@ -591,36 +591,95 @@ The hash range query parser uses a special cache to improve the speedup of the q

== Join Query Parser

[source,text]
----
-{!join from=manu_id_s to=id}ipod
+/solr/techproducts/select?q={!join from=manu_id_s to=id}title:ipod
----

-=== Join Parser Scoring

-You can optionally use the `score` parameter to return scores of the subordinate query. The values to use for this parameter define the type of aggregation, which are `avg` (average), `max` (maximum), `min` (minimum) `total`, or `none`.
+`method`::
+An optional parameter used to determine which of several query implementations should be used by Solr.
+Options are restricted to: `index`, `dvWithScore`, and `topLevelDV`.
+If unspecified the default value is `index`, unless the `score` parameter is present which overrides it to `dvWithScore`.
+Each implementation has its own performance characteristics, and users are encouraged to experiment to determine which implementation is most performant for their use-case.
+Details and performance heuristics are given below.
++
+`index` the default `method` unless the `score` parameter is specified.
+Uses the terms index structures to process the request.
+Performance scales linearly with the number of values matched in the "from" field.
+Consider this method when the "from" query matches few documents, when the "to" side returns a large number of documents, or when sporadic post-commit slowdowns cannot be tolerated (this is a disadvantage of other methods that `index` avoids).

Review comment:
That what I always think, but giving the mail linked to SOLR-14230 there might be something broken in _fallback_. I'll clarify.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370309875
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/search/join/MultiValueTermOrdinalCollector.java
 ##########
 @@ -0,0 +1,72 @@
+/*
+ * 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.join;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.index.SortedSetDocValues;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.util.LongBitSet;
+import org.apache.solr.search.DelegatingCollector;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Populates a bitset of (top-level) ordinals based on field values in a multi-valued field.
+ */
+public class MultiValueTermOrdinalCollector extends DelegatingCollector {
 
 Review comment:
   Why extend DelegatingCollector?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] mkhludnev commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

mkhludnev commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370547761
 
 

 ##########
 File path: solr/core/src/test/org/apache/solr/search/TestJoinQueryPerformance.java
 ##########
 @@ -0,0 +1,235 @@
+/*
+ * 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.BufferedReader;
+import java.io.FileReader;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.UUID;
+
+import com.google.common.collect.Lists;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrInputDocument;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+
+public class TestJoinQueryPerformance extends SolrTestCaseJ4 {
+  // Dictionary used to load String data
+  private static final String DICT_PATH = "/usr/share/dict/words";
+  private static final int NUM_DICT_TERMS = 235886;
+  private static final String[] LOADED_DICTIONARY = new String[NUM_DICT_TERMS];
+
+  // Performance run parameters: Indexing
+  private static final String FROM_COLLECTION_NAME = "user_acls";
+  private static final int NUM_FROM_DOCS = 5050; // 1 + 2 + 3 + 4 + ...  + 100
+  private static final String TO_COLLECTION_NAME = "products";
+  private static final int NUM_TO_DOCS = 500000;
+  private static final int PERMISSION_CARDINALITY = 50000; // 50K unique groups/roles/whatever
+  private static int BATCH_SIZE = 500;
+  private static int NUM_COMMITS = 500;
+  private static final int VAL_MAX = 1000;
+  private static final int USER_MAX = 100;
+
+  private static String COLLECTION_NAME= "foo";
+
+  /*
+   * As I start out here, I think I'll want a few different axes.
+   *  - "from" collection matches (with "to" matches held constant)
+   *  - "to" collection matches (with "from" matches held constant)
+   *
+   * So I think I should index a finite number of docs
+   */
+
+  @BeforeClass
+  public static void setUpCluster() throws Exception {
+    loadDictionary();
+    //loadCollectionData(DType.USER);
+    //loadCollectionData(DType.DATA);
+  }
+
+  private static void loadDictionary() throws Exception {
+    try (BufferedReader reader = new BufferedReader(new FileReader(DICT_PATH))) {
+      for (int i = 0; i < NUM_DICT_TERMS; i++) {
+        LOADED_DICTIONARY[i] = reader.readLine();
+      }
+    }
+  }
+
+  public enum DType {
+    USER(NUM_FROM_DOCS, FROM_COLLECTION_NAME) {
+      // id - unique string
+      // userid_s - username (user# from 1-100)...each user appears in # entries
+      // permissions_ss - set of 300 string permissions (cardinality 50K)
+      @Override
+      SolrInputDocument buildDoc() {
+        if (userRecordCounts[currentUser - 1] == currentUser) {
+          currentUser++;
+        } else {
+          userRecordCounts[currentUser -1]++;
+        }
+
+        final SolrInputDocument newDoc = new SolrInputDocument("id", UUID.randomUUID().toString());
+        final String userString = "user" + currentUser;
+        final String[] permissions = getAFewDictionaryWords(300, PERMISSION_CARDINALITY);
+
+        newDoc.addField("userid_s", userString);
+        newDoc.addField("permissions_ss", permissions);
+
+        return newDoc;
+      }
+    },
+    DATA(NUM_TO_DOCS, TO_COLLECTION_NAME) {
+      // id - unique string
+      // val_i - random int between 1-1000
+      // cost_d - random cost between 1-1000
+      // body_txt - random text string between 100 - 10000 words
+      // acl_ss - set of 100-3000 string permissions (cardinality 50K)
+      @Override
+      SolrInputDocument buildDoc() {
+        final SolrInputDocument newDoc = new SolrInputDocument("id", UUID.randomUUID().toString());
+        final int val = random().nextInt(1000) + 1;
+        final double cost = random().nextDouble() * 1000d;
+        final String body = String.join(" ", getAFewDictionaryWords(random().nextInt(9900) + 100));
+        final String[] acls = getAFewDictionaryWords(random().nextInt(2900) + 100, PERMISSION_CARDINALITY);
+
+        newDoc.addField("val_i", val);
+        newDoc.addField("cost_d", cost);
+        newDoc.addField("body_txt", body);
+        newDoc.addField("acl_ss", acls);
+
+        return newDoc;
+      }
+    };
+
+    private int numDocs;
+    private String collName;
+    private static int[] userRecordCounts = new int[100];
+    private static int currentUser = 1;
+
+    private DType(int numDocs, String collectionName) {
+      this.numDocs = numDocs;
+      this.collName = collectionName;
+    }
+
+    abstract SolrInputDocument buildDoc();
+  }
+
+  private static void loadCollectionData(DType type) throws Exception {
+    int numDocs = type.numDocs;
+    String collectionName = type.collName;
+    int numLoaded = 0;
+    try (HttpSolrClient client = new HttpSolrClient.Builder("http://localhost:8983/solr").build()) {
+      final int numBatches = numDocs / BATCH_SIZE + 1;
+      final int commitEveryBatches = NUM_COMMITS > 0 ? numBatches / NUM_COMMITS : Integer.MAX_VALUE;
+      int batchCount = 0;
+      while (numLoaded < numDocs) {
+        final int sizeOfBatch = numLoaded + BATCH_SIZE > numDocs ? numDocs - numLoaded : BATCH_SIZE;
+        final Collection<SolrInputDocument> batch = buildBatch(type, sizeOfBatch);
+        client.add(collectionName, batch);
+        batchCount++;
+        numLoaded+=sizeOfBatch;
+
+        if (batchCount == commitEveryBatches) {
+          client.commit(collectionName);
+          batchCount = 0;
+        }
+      }
+      client.commit(collectionName);
+    }
+
+  }
+
+  private static Collection<SolrInputDocument> buildBatch(DType type, int sizeOfBatch) {
+    final List<SolrInputDocument> batch = Lists.newArrayList();
+    for (int i = 0; i < sizeOfBatch; i++) {
+      batch.add(type.buildDoc());
+    }
+    return batch;
+  }
+
+  private static String[] getAFewDictionaryWords(int numWords) {
+    return getAFewDictionaryWords(numWords, NUM_DICT_TERMS);
+  }
+
+  private static String[] getAFewDictionaryWords(int numWords, int onlyFirstN) {
+    final String[] words = new String[numWords];
+    for (int i = 0; i < numWords; i++) {
+      words[i] = LOADED_DICTIONARY[random().nextInt(onlyFirstN)];
+    }
+
+    return words;
+  }
+
+
+  @Test
+  public void testJoinPerformanceAsMainQueryHitsIncrease() throws Exception {
+    final String joinQueryBase = "{!join fromIndex=" + FROM_COLLECTION_NAME + " from=permissions_ss to=acl_ss cache=false";
+    final String fromQuery = "userid_s:user25"; // The higher the user number, the more permissions he has attached to his name (1-100)
+    final String standardJoin = joinQueryBase + "}" + fromQuery;
+    final String noScoreJoin = joinQueryBase + " score=none}" + fromQuery;
+    final String tpiJoin = joinQueryBase + " toplevel=true}" + fromQuery;
+
+    try (HttpSolrClient client = new HttpSolrClient.Builder("http://localhost:8983/solr").build()) {
+      for ( int i = 0; i < VAL_MAX; i+=20) {
+        final String mainQuery = "val_i:[1 TO " + (i+1) + "]";
+        final QueryResponse standardJoinRsp = client.query(TO_COLLECTION_NAME, new SolrQuery("q", mainQuery, "fq", standardJoin), SolrRequest.METHOD.POST);
+        final QueryResponse tpiJoinRsp = client.query(TO_COLLECTION_NAME, new SolrQuery("q", mainQuery, "fq", tpiJoin), SolrRequest.METHOD.POST);
+        final QueryResponse noScoreJoinRsp = client.query(TO_COLLECTION_NAME, new SolrQuery("q", mainQuery, "fq", noScoreJoin), SolrRequest.METHOD.POST);
+        final long numFound = tpiJoinRsp.getResults().getNumFound();
+
+        System.out.println(i + "," + numFound + "," + standardJoinRsp.getQTime() + "," + noScoreJoinRsp.getQTime() + "," + tpiJoinRsp.getQTime());
+      }
+    }
+  }
+
+  @Test
+  public void testJoinPerformanceAsFromQueryHitsIncrease() throws Exception {
+    final String mainQuery = "val_i:[1 TO 250]"; // Half the docs match the query (250K)
+
+    final String joinQueryBase = "{!join fromIndex=" + FROM_COLLECTION_NAME + " from=permissions_ss to=acl_ss cache=false";
+
+
+    try (HttpSolrClient client = new HttpSolrClient.Builder("http://localhost:8983/solr").build()) {
+      for ( int i = 1; i <= USER_MAX; i++) {
+        final String fromQuery = "userid_s:user" + i;
+        final String standardJoin = joinQueryBase + "}" + fromQuery;
+        final String noScoreJoin = joinQueryBase + " score=none}" + fromQuery;
+        final String tpiJoin = joinQueryBase + " toplevel=true}" + fromQuery;
+
+        final QueryResponse standardJoinRsp = client.query(TO_COLLECTION_NAME, new SolrQuery("q", mainQuery, "fq", standardJoin), SolrRequest.METHOD.POST);
+        final QueryResponse tpiJoinRsp = client.query(TO_COLLECTION_NAME, new SolrQuery("q", mainQuery, "fq", tpiJoin), SolrRequest.METHOD.POST);
+        final QueryResponse noScoreJoinRsp = client.query(TO_COLLECTION_NAME, new SolrQuery("q", mainQuery, "fq", noScoreJoin), SolrRequest.METHOD.POST);
+        final long numFound = tpiJoinRsp.getResults().getNumFound();
+
+        System.out.println(i + "," + numFound + "," + standardJoinRsp.getQTime() + "," + noScoreJoinRsp.getQTime() + "," + tpiJoinRsp.getQTime());
 
 Review comment:
   Printing to stdout is prosecuted. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370282592
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java
 ##########
 @@ -59,67 +60,124 @@
 import org.apache.solr.search.join.ScoreJoinQParserPlugin;
 import org.apache.solr.util.RTimer;
 import org.apache.solr.util.RefCounted;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class JoinQParserPlugin extends QParserPlugin {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
   public static final String NAME = "join";
+  /** Choose the internal algorithm */
+  private static final String METHOD = "method";
+
+  private static class JoinParams {
+    final String fromField;
+    final String fromCore;
+    final Query fromQuery;
+    final long fromCoreOpenTime;
+    final String toField;
+
+    public JoinParams(String fromField, String fromCore, Query fromQuery, long fromCoreOpenTime, String toField) {
+      this.fromField = fromField;
+      this.fromCore = fromCore;
+      this.fromQuery = fromQuery;
+      this.fromCoreOpenTime = fromCoreOpenTime;
+      this.toField = toField;
+    }
+  }
+
+  private enum Method {
+    index {
+      @Override
+      Query makeFilter(QParser qparser) throws SyntaxError {
+        final JoinParams jParams = parseJoin(qparser);
+        final JoinQuery q = new JoinQuery(jParams.fromField, jParams.toField, jParams.fromCore, jParams.fromQuery);
+        q.fromCoreOpenTime = jParams.fromCoreOpenTime;
+        return q;
+      }
+    },
+    dvWithScore {
+      @Override
+      Query makeFilter(QParser qparser) throws SyntaxError {
+        return new ScoreJoinQParserPlugin().createParser(qparser.qstr, qparser.localParams, qparser.params, qparser.req).parse();
+      }
+    },
+    topLevelDV {
+      @Override
+      Query makeFilter(QParser qparser) throws SyntaxError {
+        final JoinParams jParams = parseJoin(qparser);
+        final JoinQuery q = new TopLevelJoinQuery(jParams.fromField, jParams.toField, jParams.fromCore, jParams.fromQuery);
+        q.fromCoreOpenTime = jParams.fromCoreOpenTime;
+        return q;
+      }
+    };
+
+    abstract Query makeFilter(QParser qparser) throws SyntaxError;
+
+    JoinParams parseJoin(QParser qparser) throws SyntaxError {
+      final String fromField = qparser.getParam("from");
+      final String fromIndex = qparser.getParam("fromIndex");
+      final String toField = qparser.getParam("to");
+      final String v = qparser.localParams.get("v");
+      final String coreName;
+
+      Query fromQuery;
+      long fromCoreOpenTime = 0;
+
+      if (fromIndex != null && !fromIndex.equals(qparser.req.getCore().getCoreDescriptor().getName()) ) {
+        CoreContainer container = qparser.req.getCore().getCoreContainer();
+
+        // if in SolrCloud mode, fromIndex should be the name of a single-sharded collection
+        coreName = ScoreJoinQParserPlugin.getCoreName(fromIndex, container);
+
+        final SolrCore fromCore = container.getCore(coreName);
+        if (fromCore == null) {
+          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+              "Cross-core join: no such core " + coreName);
+        }
+
+        RefCounted<SolrIndexSearcher> fromHolder = null;
+        LocalSolrQueryRequest otherReq = new LocalSolrQueryRequest(fromCore, qparser.params);
+        try {
 
 Review comment:
   Can you use try-with-resources style for SolrCore and otherReq?  You might need to create a static method that gets the core and throws if non-null so that you are able to have a one-liner fetch of the core in the "try".  I generally like to use try-with-resources when possible as it's easier to reason to guarantee things get closed, and it's sometimes less code.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija commented on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija commented on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#issuecomment-580801355
 
 
   Fair enough - I'll add it to my list of things to brush up on.  Always plenty to learn.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r373111544

##########
File path: solr/solr-ref-guide/src/other-parsers.adoc
##########
@@ -591,36 +591,95 @@ The hash range query parser uses a special cache to improve the speedup of the q

== Join Query Parser

[source,text]
----
-{!join from=manu_id_s to=id}ipod
+/solr/techproducts/select?q={!join from=manu_id_s to=id}title:ipod
----

-=== Join Parser Scoring

-You can optionally use the `score` parameter to return scores of the subordinate query. The values to use for this parameter define the type of aggregation, which are `avg` (average), `max` (maximum), `min` (minimum) `total`, or `none`.
+`method`::
+An optional parameter used to determine which of several query implementations should be used by Solr.
+Options are restricted to: `index`, `dvWithScore`, and `topLevelDV`.
+If unspecified the default value is `index`, unless the `score` parameter is present which overrides it to `dvWithScore`.
+Each implementation has its own performance characteristics, and users are encouraged to experiment to determine which implementation is most performant for their use-case.
+Details and performance heuristics are given below.
++
+`index` the default `method` unless the `score` parameter is specified.
+Uses the terms index structures to process the request.
+Performance scales linearly with the number of values matched in the "from" field.
+Consider this method when the "from" query matches few documents, when the "to" side returns a large number of documents, or when sporadic post-commit slowdowns cannot be tolerated (this is a disadvantage of other methods that `index` avoids).

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370311726
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/search/join/MultiValueTermOrdinalCollector.java
 ##########
 @@ -0,0 +1,72 @@
+/*
+ * 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.join;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.index.SortedSetDocValues;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.util.LongBitSet;
+import org.apache.solr.search.DelegatingCollector;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Populates a bitset of (top-level) ordinals based on field values in a multi-valued field.
+ */
+public class MultiValueTermOrdinalCollector extends DelegatingCollector {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  private int docBase;
+  private SortedSetDocValues topLevelDocValues;
+  private final String fieldName;
+  private final LongBitSet topLevelDocValuesBitSet;
+
+  public MultiValueTermOrdinalCollector(String fieldName, SortedSetDocValues topLevelDocValues, LongBitSet topLevelDocValuesBitSet) {
+    this.fieldName = fieldName;
+    this.topLevelDocValues = topLevelDocValues;
+    this.topLevelDocValuesBitSet = topLevelDocValuesBitSet;
+  }
+
+  public ScoreMode scoreMode() {
+    return ScoreMode.COMPLETE_NO_SCORES;
+  }
+
+  public boolean needsScores(){
 
 Review comment:
   What's this for?  Looks old/legacy before needsScores became scoreMode yet you just wrote this code.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija edited a comment on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija edited a comment on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#issuecomment-577188503
 
 
   That's 100% what I was looking for.  I knew it had to exist somewhere.  Thanks David!
   
   It's package scoped, but I'll file a LUCENE jira making the argument for wider visibility and see where that goes.  

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370319920

##########
File path: solr/solr-ref-guide/src/other-parsers.adoc
##########
@@ -591,36 +591,95 @@ The hash range query parser uses a special cache to improve the speedup of the q

== Join Query Parser

[source,text]
----
-{!join from=manu_id_s to=id}ipod
+/solr/techproducts/select?q={!join from=manu_id_s to=id}title:ipod
----

-=== Join Parser Scoring

Review comment:
"uninverted values" -> "the field cache"
I believe "uninvert" is more of an internal word whereas publicly we say "field cache"?

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] mkhludnev commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

mkhludnev commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r373345476

##########
File path: solr/solr-ref-guide/src/other-parsers.adoc
##########
@@ -591,36 +591,95 @@ The hash range query parser uses a special cache to improve the speedup of the q

== Join Query Parser

[source,text]
----
-{!join from=manu_id_s to=id}ipod
+/solr/techproducts/select?q={!join from=manu_id_s to=id}title:ipod
----

-=== Join Parser Scoring

-You can optionally use the `score` parameter to return scores of the subordinate query. The values to use for this parameter define the type of aggregation, which are `avg` (average), `max` (maximum), `min` (minimum) `total`, or `none`.
+`method`::
+An optional parameter used to determine which of several query implementations should be used by Solr.
+Options are restricted to: `index`, `dvWithScore`, and `topLevelDV`.
+If unspecified the default value is `index`, unless the `score` parameter is present which overrides it to `dvWithScore`.
+Each implementation has its own performance characteristics, and users are encouraged to experiment to determine which implementation is most performant for their use-case.
+Details and performance heuristics are given below.
++
+`index` the default `method` unless the `score` parameter is specified.
+Uses the terms index structures to process the request.
+Performance scales linearly with the number of values matched in the "from" field.
+Consider this method when the "from" query matches few documents, when the "to" side returns a large number of documents, or when sporadic post-commit slowdowns cannot be tolerated (this is a disadvantage of other methods that `index` avoids).

Review comment:
Sure, @gerlowskija please go ahead with this. It's fine. Sadly, I hardly check 14230 any time soon.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r373109412

##########
File path: solr/solr-ref-guide/src/other-parsers.adoc
##########
@@ -591,36 +591,95 @@ The hash range query parser uses a special cache to improve the speedup of the q

== Join Query Parser

[source,text]
----
-{!join from=manu_id_s to=id}ipod
+/solr/techproducts/select?q={!join from=manu_id_s to=id}title:ipod
----

-=== Join Parser Scoring

-You can optionally use the `score` parameter to return scores of the subordinate query. The values to use for this parameter define the type of aggregation, which are `avg` (average), `max` (maximum), `min` (minimum) `total`, or `none`.
+`method`::
+An optional parameter used to determine which of several query implementations should be used by Solr.
+Options are restricted to: `index`, `dvWithScore`, and `topLevelDV`.
+If unspecified the default value is `index`, unless the `score` parameter is present which overrides it to `dvWithScore`.
+Each implementation has its own performance characteristics, and users are encouraged to experiment to determine which implementation is most performant for their use-case.
+Details and performance heuristics are given below.
++
+`index` the default `method` unless the `score` parameter is specified.
+Uses the terms index structures to process the request.
+Performance scales linearly with the number of values matched in the "from" field.

Review comment:
Thanks for the clarification. Let me know if you're ok with the new wording.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370307365
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/search/TopLevelJoinQuery.java
 ##########
 @@ -0,0 +1,218 @@
+/*
+ * 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.IOException;
+import java.lang.invoke.MethodHandles;
+
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.index.LeafReader;
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.index.SortedSetDocValues;
+import org.apache.lucene.search.Collector;
+import org.apache.lucene.search.ConstantScoreScorer;
+import org.apache.lucene.search.ConstantScoreWeight;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Scorer;
+import org.apache.lucene.search.TwoPhaseIterator;
+import org.apache.lucene.search.Weight;
+import org.apache.lucene.util.BytesRef;
+import org.apache.lucene.util.LongBitSet;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.join.MultiValueTermOrdinalCollector;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TopLevelJoinQuery extends JoinQuery {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public TopLevelJoinQuery(String fromField, String toField, String coreName, Query subQuery) {
+    super(fromField, toField, coreName, subQuery);
+  }
+
+  @Override
+  public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException {
+    if (! (searcher instanceof SolrIndexSearcher)) {
+      log.debug("Falling back to JoinQueryWeight because searcher [{}] is not the required SolrIndexSearcher", searcher);
+      return super.createWeight(searcher, scoreMode, boost);
+    }
+
+    final SolrIndexSearcher solrSearcher = (SolrIndexSearcher) searcher;
+    final JoinQueryWeight weight = new JoinQueryWeight(solrSearcher, ScoreMode.COMPLETE_NO_SCORES, 1.0f);
+    final SolrIndexSearcher fromSearcher = weight.fromSearcher;
+    final SolrIndexSearcher toSearcher = weight.toSearcher;
+
+    try {
+      final SortedSetDocValues topLevelFromDocValues = validateAndFetchDocValues(fromSearcher, fromField, "from");
+      final SortedSetDocValues topLevelToDocValues = validateAndFetchDocValues(toSearcher, toField, "to");
+      if (topLevelFromDocValues.getValueCount() == 0 || topLevelToDocValues.getValueCount() == 0) {
+        return createNoMatchesWeight(boost);
+      }
+
+      final LongBitSet fromOrdBitSet = findOrdinalsMatchingFromQuery(fromSearcher, topLevelFromDocValues);
+      final LongBitSet toOrdBitSet = new LongBitSet(topLevelToDocValues.getValueCount());
+      final BitsetBounds toBitsetBounds = convertFromOrdinalsIntoToField(fromOrdBitSet, topLevelFromDocValues, toOrdBitSet, topLevelToDocValues);
+
+      final boolean toMultivalued = toSearcher.getSchema().getFieldOrNull(toField).multiValued();
+      return new ConstantScoreWeight(this, boost) {
+        public Scorer scorer(LeafReaderContext context) throws IOException {
+          if (toBitsetBounds.lower == BitsetBounds.NO_MATCHES) {
+            return null;
+          }
+
+          final DocIdSetIterator toApproximation = (toMultivalued) ? context.reader().getSortedSetDocValues(toField) :
+              context.reader().getSortedDocValues(toField);
+          if (toApproximation == null) {
+            return null;
+          }
+
+          final int docBase = context.docBase;
+          return new ConstantScoreScorer(this, this.score(), scoreMode, new TwoPhaseIterator(toApproximation) {
+            public boolean matches() throws IOException {
+              final boolean hasDoc = topLevelToDocValues.advanceExact(docBase + approximation.docID());
+              if (hasDoc) {
+                for (long ord = topLevelToDocValues.nextOrd(); ord != -1L; ord = topLevelToDocValues.nextOrd()) {
+                  if (toOrdBitSet.get(ord)) {
+                    return true;
+                  }
+                }
+              }
+              return false;
+            }
+
+            public float matchCost() {
+              return 10.0F;
+            }
+          });
+
+        }
+
+        public boolean isCacheable(LeafReaderContext ctx) {
+          return false;
+        }
+      };
+    } catch (IOException e) {
+      throw new RuntimeException(e);
+    }
+  }
+
+  private Weight createNoMatchesWeight(float boost) {
+    return new ConstantScoreWeight(this, boost) {
+      @Override
+      public Scorer scorer(LeafReaderContext context) throws IOException {
+        return null;
+      }
+
+      @Override
+      public boolean isCacheable(LeafReaderContext ctx) {
+        return false;
+      }
+    };
+  }
+
+  private SortedSetDocValues validateAndFetchDocValues(SolrIndexSearcher solrSearcher, String fieldName, String querySide) throws IOException {
+    final IndexSchema schema = solrSearcher.getSchema();
+    final SchemaField field = schema.getFieldOrNull(fieldName);
+    if (field == null) {
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, querySide + " field '" + fieldName + "' does not exist");
+    }
+
+    if (!field.hasDocValues()) {
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+          "'top-level' join queries require both 'from' and 'to' fields to have docValues, but " + querySide +
+              " field [" + fieldName +  "] does not.");
+    }
+
+    final LeafReader leafReader = solrSearcher.getSlowAtomicReader();
+    if (field.multiValued()) {
+      return DocValues.getSortedSet(leafReader, fieldName);
+    }
+    return DocValues.singleton(DocValues.getSorted(leafReader, fieldName));
+  }
+
+  private LongBitSet findOrdinalsMatchingFromQuery(SolrIndexSearcher fromSearcher, SortedSetDocValues fromDocValues) throws IOException {
 
 Review comment:
   Couldn't you rename the method and it's variables to remove "From" and it would be just as valid?  Okay I see why not; this is an instance method that refers to 'fromField' but if that's the only distinction then maybe it should be a static method and take that argument?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] mkhludnev commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

mkhludnev commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r372783880

##########
File path: solr/solr-ref-guide/src/other-parsers.adoc
##########
@@ -591,36 +591,95 @@ The hash range query parser uses a special cache to improve the speedup of the q

== Join Query Parser

[source,text]
----
-{!join from=manu_id_s to=id}ipod
+/solr/techproducts/select?q={!join from=manu_id_s to=id}title:ipod
----

-=== Join Parser Scoring

-You can optionally use the `score` parameter to return scores of the subordinate query. The values to use for this parameter define the type of aggregation, which are `avg` (average), `max` (maximum), `min` (minimum) `total`, or `none`.
+`method`::
+An optional parameter used to determine which of several query implementations should be used by Solr.
+Options are restricted to: `index`, `dvWithScore`, and `topLevelDV`.
+If unspecified the default value is `index`, unless the `score` parameter is present which overrides it to `dvWithScore`.
+Each implementation has its own performance characteristics, and users are encouraged to experiment to determine which implementation is most performant for their use-case.
+Details and performance heuristics are given below.
++
+`index` the default `method` unless the `score` parameter is specified.
+Uses the terms index structures to process the request.
+Performance scales linearly with the number of values matched in the "from" field.

Review comment:
Here I would disagree. Old solr's join loops term enums. So, number of terms and posings in `from` dominates over number of `from` side matches in the overall complexity. see the thread linked at SOLR-14230, and also something like this is explained in LW's blogpost.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370304007
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java
 ##########
 @@ -59,67 +60,124 @@
 import org.apache.solr.search.join.ScoreJoinQParserPlugin;
 import org.apache.solr.util.RTimer;
 import org.apache.solr.util.RefCounted;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class JoinQParserPlugin extends QParserPlugin {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
   public static final String NAME = "join";
+  /** Choose the internal algorithm */
+  private static final String METHOD = "method";
+
+  private static class JoinParams {
+    final String fromField;
+    final String fromCore;
+    final Query fromQuery;
+    final long fromCoreOpenTime;
+    final String toField;
+
+    public JoinParams(String fromField, String fromCore, Query fromQuery, long fromCoreOpenTime, String toField) {
+      this.fromField = fromField;
+      this.fromCore = fromCore;
+      this.fromQuery = fromQuery;
+      this.fromCoreOpenTime = fromCoreOpenTime;
+      this.toField = toField;
+    }
+  }
+
+  private enum Method {
+    index {
+      @Override
+      Query makeFilter(QParser qparser) throws SyntaxError {
+        final JoinParams jParams = parseJoin(qparser);
+        final JoinQuery q = new JoinQuery(jParams.fromField, jParams.toField, jParams.fromCore, jParams.fromQuery);
+        q.fromCoreOpenTime = jParams.fromCoreOpenTime;
+        return q;
+      }
+    },
+    dvWithScore {
+      @Override
+      Query makeFilter(QParser qparser) throws SyntaxError {
+        return new ScoreJoinQParserPlugin().createParser(qparser.qstr, qparser.localParams, qparser.params, qparser.req).parse();
+      }
+    },
+    topLevelDV {
+      @Override
+      Query makeFilter(QParser qparser) throws SyntaxError {
+        final JoinParams jParams = parseJoin(qparser);
+        final JoinQuery q = new TopLevelJoinQuery(jParams.fromField, jParams.toField, jParams.fromCore, jParams.fromQuery);
+        q.fromCoreOpenTime = jParams.fromCoreOpenTime;
+        return q;
+      }
+    };
+
+    abstract Query makeFilter(QParser qparser) throws SyntaxError;
+
+    JoinParams parseJoin(QParser qparser) throws SyntaxError {
+      final String fromField = qparser.getParam("from");
+      final String fromIndex = qparser.getParam("fromIndex");
+      final String toField = qparser.getParam("to");
+      final String v = qparser.localParams.get("v");
+      final String coreName;
+
+      Query fromQuery;
+      long fromCoreOpenTime = 0;
+
+      if (fromIndex != null && !fromIndex.equals(qparser.req.getCore().getCoreDescriptor().getName()) ) {
+        CoreContainer container = qparser.req.getCore().getCoreContainer();
+
+        // if in SolrCloud mode, fromIndex should be the name of a single-sharded collection
+        coreName = ScoreJoinQParserPlugin.getCoreName(fromIndex, container);
+
+        final SolrCore fromCore = container.getCore(coreName);
+        if (fromCore == null) {
+          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+              "Cross-core join: no such core " + coreName);
+        }
+
+        RefCounted<SolrIndexSearcher> fromHolder = null;
+        LocalSolrQueryRequest otherReq = new LocalSolrQueryRequest(fromCore, qparser.params);
+        try {
+          QParser parser = QParser.getParser(v, otherReq);
+          fromQuery = parser.getQuery();
+          fromHolder = fromCore.getRegisteredSearcher();
+          if (fromHolder != null) fromCoreOpenTime = fromHolder.get().getOpenNanoTime();
+        } finally {
+          otherReq.close();
+          fromCore.close();
+          if (fromHolder != null) fromHolder.decref();
+        }
+      } else {
+        coreName = null;
+        QParser fromQueryParser = qparser.subQuery(v, null);
+        fromQueryParser.setIsFilter(true);
 
 Review comment:
   Perhaps these two lines, setting is filter on the QParser and calling getQuery should be pulled out of the block as it is (or should be) in common with the positive side of the if-else?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija commented on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija commented on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#issuecomment-576666800
 
 
   I have mixed feelings about the last commit in the series I just pushed up (`Remove single-/multi-value duplication`).  It helps get rid of a lot of duplication across SortedDocValues and SortedSetDocValues codepaths, but it also addresses a problem that I imagine people are running into all over the place.  Is there some established way for handling this in Solr?  I imagine tons of query implementations must run into this...
   
   Need to think it over a bit...

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370319600

##########
File path: solr/solr-ref-guide/src/other-parsers.adoc
##########
@@ -591,36 +591,95 @@ The hash range query parser uses a special cache to improve the speedup of the q

== Join Query Parser

[source,text]
----
-{!join from=manu_id_s to=id}ipod
+/solr/techproducts/select?q={!join from=manu_id_s to=id}title:ipod
----

-=== Join Parser Scoring

Review comment:
"this a" -> "this is a"

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370313149
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/search/join/MultiValueTermOrdinalCollector.java
 ##########
 @@ -0,0 +1,72 @@
+/*
+ * 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.join;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.index.SortedSetDocValues;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.util.LongBitSet;
+import org.apache.solr.search.DelegatingCollector;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Populates a bitset of (top-level) ordinals based on field values in a multi-valued field.
+ */
+public class MultiValueTermOrdinalCollector extends DelegatingCollector {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  private int docBase;
+  private SortedSetDocValues topLevelDocValues;
+  private final String fieldName;
+  private final LongBitSet topLevelDocValuesBitSet;
 
 Review comment:
   document this is the outgoing data structure expressing the results.  Could be a trivial comment

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370304976
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java
 ##########
 @@ -139,11 +197,16 @@ public static Query createJoinQuery(Query subQuery, String fromField, String toF
 
 
 class JoinQuery extends Query {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
   String fromField;
   String toField;
   String fromIndex; // TODO: name is missleading here compared to JoinQParserPlugin usage - here it must be a core name
   Query q;
   long fromCoreOpenTime;
+  private boolean cache;
 
 Review comment:
   Not used?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija edited a comment on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija edited a comment on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#issuecomment-577188503
 
 
   That's 100% what I was looking for.  I knew it had to exist somewhere.  Thanks David!

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r373109412

##########
File path: solr/solr-ref-guide/src/other-parsers.adoc
##########
@@ -591,36 +591,95 @@ The hash range query parser uses a special cache to improve the speedup of the q

== Join Query Parser

[source,text]
----
-{!join from=manu_id_s to=id}ipod
+/solr/techproducts/select?q={!join from=manu_id_s to=id}title:ipod
----

-=== Join Parser Scoring

-You can optionally use the `score` parameter to return scores of the subordinate query. The values to use for this parameter define the type of aggregation, which are `avg` (average), `max` (maximum), `min` (minimum) `total`, or `none`.
+`method`::
+An optional parameter used to determine which of several query implementations should be used by Solr.
+Options are restricted to: `index`, `dvWithScore`, and `topLevelDV`.
+If unspecified the default value is `index`, unless the `score` parameter is present which overrides it to `dvWithScore`.
+Each implementation has its own performance characteristics, and users are encouraged to experiment to determine which implementation is most performant for their use-case.
+Details and performance heuristics are given below.
++
+`index` the default `method` unless the `score` parameter is specified.
+Uses the terms index structures to process the request.
+Performance scales linearly with the number of values matched in the "from" field.

Review comment:
Thanks for the clarification. Let me know if you're ok with the new wording. (Or feel free to make changes yourself, if there's specific language you like.)

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] mkhludnev commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

mkhludnev commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370548790
 
 

 ##########
 File path: solr/core/src/test/org/apache/solr/search/TestJoinQueryPerformance.java
 ##########
 @@ -0,0 +1,235 @@
+/*
+ * 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.BufferedReader;
+import java.io.FileReader;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.UUID;
+
+import com.google.common.collect.Lists;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrInputDocument;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+
+public class TestJoinQueryPerformance extends SolrTestCaseJ4 {
+  // Dictionary used to load String data
+  private static final String DICT_PATH = "/usr/share/dict/words";
+  private static final int NUM_DICT_TERMS = 235886;
+  private static final String[] LOADED_DICTIONARY = new String[NUM_DICT_TERMS];
+
+  // Performance run parameters: Indexing
+  private static final String FROM_COLLECTION_NAME = "user_acls";
+  private static final int NUM_FROM_DOCS = 5050; // 1 + 2 + 3 + 4 + ...  + 100
+  private static final String TO_COLLECTION_NAME = "products";
+  private static final int NUM_TO_DOCS = 500000;
+  private static final int PERMISSION_CARDINALITY = 50000; // 50K unique groups/roles/whatever
+  private static int BATCH_SIZE = 500;
+  private static int NUM_COMMITS = 500;
+  private static final int VAL_MAX = 1000;
+  private static final int USER_MAX = 100;
+
+  private static String COLLECTION_NAME= "foo";
+
+  /*
+   * As I start out here, I think I'll want a few different axes.
+   *  - "from" collection matches (with "to" matches held constant)
+   *  - "to" collection matches (with "from" matches held constant)
+   *
+   * So I think I should index a finite number of docs
+   */
+
+  @BeforeClass
+  public static void setUpCluster() throws Exception {
+    loadDictionary();
+    //loadCollectionData(DType.USER);
 
 Review comment:
   Shouldn't it load at least anything? 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija commented on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija commented on issue #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#issuecomment-579763577
 
 
   Ready for review again when you get the chance @dsmiley 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

gerlowskija commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r371296171
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/search/join/MultiValueTermOrdinalCollector.java
 ##########
 @@ -0,0 +1,72 @@
+/*
+ * 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.join;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.index.SortedSetDocValues;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.util.LongBitSet;
+import org.apache.solr.search.DelegatingCollector;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Populates a bitset of (top-level) ordinals based on field values in a multi-valued field.
+ */
+public class MultiValueTermOrdinalCollector extends DelegatingCollector {
 
 Review comment:
   Because it saves me from reimplementing `getLeafCollector()` and some other methods.  If you're wondering why DelegatingCollector as opposed to SimpleCollector or other options, there's not a great answer - DelegatingCollector was needed in some earlier revision when things were postfilter based.  I've changed it to use SimpleCollector; hopefully that addresses your concern.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation

Posted by GitBox <gi...@apache.org>.

dsmiley commented on a change in pull request #1171: SOLR-13892: Add 'top-level' docValues Join implementation
URL: https://github.com/apache/lucene-solr/pull/1171#discussion_r370319378

##########
File path: solr/solr-ref-guide/src/other-parsers.adoc
##########
@@ -591,36 +591,95 @@ The hash range query parser uses a special cache to improve the speedup of the q

== Join Query Parser

[source,text]
----
-{!join from=manu_id_s to=id}ipod
+/solr/techproducts/select?q={!join from=manu_id_s to=id}title:ipod
----

-=== Join Parser Scoring

Review comment:
Maybe instead of "inverted index" (which might confuse people) simply say terms index structure?

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org