[GitHub] [incubator-druid] suneet-s commented on a change in pull request #9058: Fail superbatch range partition multi dim values

[GitBox <gi...@apache.org>]

suneet-s commented on a change in pull request #9058: Fail superbatch range partition multi dim values
URL: https://github.com/apache/incubator-druid/pull/9058#discussion_r359052983
 
 

 ##########
 File path: indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/iterator/RangePartitionIndexTaskInputRowIteratorBuilder.java
 ##########
 @@ -114,9 +114,18 @@ public HandlingInputRowIterator build()
   )
   {
     return inputRow -> {
-      List<String> dimensionValues = inputRow.getDimension(partitionDimension);
-      return dimensionValues.size() > 1;  // Rows.objectToStrings() returns an empty list for a single null value
+      int dimensionValueCount = getDimensionValueCount(inputRow, partitionDimension);
+      return dimensionValueCount > 1;  // Rows.objectToStrings() returns an empty list for a single null value
     };
   }
 
+  private static int getDimensionValueCount(InputRow inputRow, String partitionDimension)
 
 Review comment:
   Nothing in this function indicates that it will throw an exception if `dimensionValueCount > 1`
   
   Maybe rename the function to `getSingleOrNullDimensionValueCount` so that it's a little more obvious the scope of this function is very narrow. As written, I'd definitely use it just expecting it to return the number of dimensions

----------------------------------------------------------------
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org