LakshSingla commented on code in PR #16068:
URL: https://github.com/apache/druid/pull/16068#discussion_r1538847396


##########
processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/column/DimensionToIdConverter.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.druid.query.groupby.epinephelinae.column;
+
+import org.apache.druid.segment.ColumnValueSelector;
+
+import javax.annotation.Nullable;
+
+/**
+ * Interface for converters of dimension to dictionary id.
+ *
+ * This is a slightly convoluted interface because it also encapsulates the 
additional logic for handling multi-value
+ * dimensions. It has an additional step that converts the given dimensions to 
"dimension holders", which represent the
+ * multi-value holders for a given dimension.
+ * Therefore, the conversion goes from ColumnValueSelector -> DimensionHolder 
-> DictionaryID (for each dimension in the holder)
+ *
+ * The dimension holder is only applicable for multi-value strings.
+ * For other dimensions that cannot have multi-values the dimension holder is 
identical to the dimension. They can be
+ * defensively cast or homogenised, for example doubles to floats for float 
selectors or Long[] to Object[] for array
+ * selectors, so that the upstream callers can assume the class of the 
dimensions. The size of these dimensions is always 1,
+ * and only contain a value at index 0.
+ *
+ * Converting a value to its dictionary id might require building dictionaries 
on the fly while computing the id. The
+ * return type of the methods, except {@link #multiValueSize}, takes that into 
account.
+ *
+ * The implementations can pre-convert the value to the dictionaryId while 
extracting the dimensionHolder. Extracting
+ * dictionary id for a specific value from the (potentially multi-value 
dimension holder) can be done by calling
+ * {@link #getIndividualValueDictId} and passing the index to the multi-value.
+ *
+ * @see IdToDimensionConverter for converting the dictionary values back to 
dimensions
+ *
+ * @param <DimensionHolderType> Type of the dimension holder
+ */
+public interface DimensionToIdConverter<DimensionHolderType>

Review Comment:
   👍 
   Currently, I am going ahead with refactoring the keymapping strategy to 
   a. KeyMappingGroupByColumnSelectorStrategy - For arrays, and complex types
   b. KeyMappingMultiValueGroupByColumnSelectorStrategy - This would handle 
only strings



-- 
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-unsubscr...@druid.apache.org

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


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

Reply via email to