dsmiley commented on code in PR #1215:
URL: https://github.com/apache/solr/pull/1215#discussion_r1055099998


##########
solr/core/src/java/org/apache/solr/handler/admin/SplitOp.java:
##########
@@ -263,8 +263,9 @@ private void handleGetRanges(CoreAdminHandler.CallInfo it, 
String coreName) thro
         DocCollection collection = clusterState.getCollection(collectionName);
         String sliceName = 
parentCore.getCoreDescriptor().getCloudDescriptor().getShardId();
         Slice slice = collection.getSlice(sliceName);
-        DocRouter router =
-            collection.getRouter() != null ? collection.getRouter() : 
DocRouter.DEFAULT;
+        CompositeIdRouter router =

Review Comment:
   Correcting myself.  This method, `handleGetRanges`, is only called for 
splitByPrefix (as its javadocs say), which is what depends on CompositeIdRouter.
   
   I suppose if there was some weird/unexpected code path (maybe in the 
future), a ClassCastException wouldn't be particularly unfriendly?



##########
solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateCmd.java:
##########
@@ -253,7 +252,7 @@ private void migrateKey(
             SHARD_ID_PROP,
             sourceSlice.getName(),
             "routeKey",
-            SolrIndexSplitter.getRouteKey(splitKey) + "!",
+            sourceRouter.getRouteKeyNoSuffix(splitKey) + "!",

Review Comment:
   Line 108 checks for CompositeIdRouter and throws a friendly exception if it 
isn't.



##########
solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java:
##########
@@ -765,18 +766,11 @@ static FixedBitSet[] split(
     return docSets;
   }
 
-  public static String getRouteKey(String idString) {
-    int idx = idString.indexOf(CompositeIdRouter.SEPARATOR);
-    if (idx <= 0) return null;
-    String part1 = idString.substring(0, idx);
-    int commaIdx = part1.indexOf(CompositeIdRouter.bitsSeparator);
-    if (commaIdx > 0 && commaIdx + 1 < part1.length()) {
-      char ch = part1.charAt(commaIdx + 1);
-      if (ch >= '0' && ch <= '9') {
-        part1 = part1.substring(0, commaIdx);
-      }
+  private static void checkRouterSupportsSplitKey(HashBasedRouter hashRouter, 
String splitKey) {

Review Comment:
   SolrIndexSplitterTest tests the "plain" (hash) router.  The test continues 
to pass.  It's only the "splitKey" feature of shard splitting that requires 
CompositeIdRouter.  The exception message tries to clarify that the 
expectation/requirement is tied to splitKey.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org

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


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

Reply via email to