[GitHub] [druid] clintropolis commented on a diff in pull request #12428: Fix indexMerger to respect the includeAllDimensions flag

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

clintropolis commented on code in PR #12428:
URL: https://github.com/apache/druid/pull/12428#discussion_r847883718


##########
processing/src/main/java/org/apache/druid/segment/IndexMergerV9.java:
##########
@@ -1460,4 +1456,33 @@ private static <T extends TimeAndDimsPointer> T reorderRowPointerColumns(
       );
     }
   }
+
+  private static class DimensionMergerUtil

Review Comment:
   super nit, this doesn't really seem like a 'util', but does sort of remind me of some other thingswe call `Inspector` though not sure what exactly would call this, naming is hard.
   
   Also, it seems like only one place creates this, maybe should push `dimensionsSpec` into the constructor of this thing and do the logic there.



-- 
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: commits-unsubscribe@druid.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org