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