LakshSingla commented on code in PR #16864:
URL: https://github.com/apache/druid/pull/16864#discussion_r1719314441
##########
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/indexing/destination/DataSourceMSQDestinationTest.java:
##########
@@ -20,17 +20,41 @@
package org.apache.druid.msq.indexing.destination;
+import com.google.common.collect.ImmutableMap;
import nl.jqno.equalsverifier.EqualsVerifier;
+import org.apache.druid.data.input.impl.DimensionSchema;
+import org.apache.druid.data.input.impl.StringDimensionSchema;
import org.junit.Test;
+import java.util.Map;
+
public class DataSourceMSQDestinationTest
{
@Test
public void testEquals()
{
EqualsVerifier.forClass(DataSourceMSQDestination.class)
- .withNonnullFields("dataSource", "segmentGranularity",
"segmentSortOrder")
+ .withNonnullFields("dataSource", "segmentGranularity",
"segmentSortOrder", "dimensionToSchemaMap")
+ .withPrefabValues(
+ Map.class,
+ ImmutableMap.of(
+ "language",
+ new StringDimensionSchema(
+ "language",
+ DimensionSchema.MultiValueHandling.SORTED_ARRAY,
+ false
+ )
+ ),
+ ImmutableMap.of(
+ "region",
+ new StringDimensionSchema(
+ "region",
+ DimensionSchema.MultiValueHandling.SORTED_ARRAY,
+ false
+ )
+ )
Review Comment:
Based on my limited understanding, it's only required if a class is
self-referential. `DataSourceMSQDestination` doesn't has a reference to
`DataSourceMSQDestination`, so I don't think we'd need this.
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java:
##########
@@ -2122,13 +2123,34 @@ private static StringTuple makeStringTuple(
return new StringTuple(array);
}
+ private static DimensionSchema getDimensionSchema(
+ final String outputColumnName,
+ @Nullable final ColumnType queryType,
+ QueryContext context,
+ @Nullable Map<String, DimensionSchema> dimensionToSchemaMap
+ )
+ {
+ if (dimensionToSchemaMap != null &&
dimensionToSchemaMap.containsKey(outputColumnName)) {
+ return dimensionToSchemaMap.get(outputColumnName);
+ }
+ // For regular ingestion, or for metrics moved to dimensions in case of
compaction, we won't have an entry in the
Review Comment:
nit: can clarify "metrics moved to dimensions" a bit
```suggestion
// For ingestion or when metrics are converted to dimensions when
compaction is performed without rollup (finalize: false), we won't have an
entry in the
```
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/MSQCompactionRunner.java:
##########
@@ -497,6 +502,7 @@ private Map<String, Object>
createMSQTaskContext(CompactionTask compactionTask,
context.put(QueryContexts.FINALIZE_KEY, false);
// Only scalar or array-type dimensions are allowed as grouping keys.
context.putIfAbsent(GroupByQueryConfig.CTX_KEY_ENABLE_MULTI_VALUE_UNNESTING,
false);
+ context.putIfAbsent(MultiStageQueryContext.CTX_ARRAY_INGEST_MODE, "array");
Review Comment:
Should we even allow ARRAY_INGEST_MODE as "mvd", i.e. in addition to this,
do we need to have a check flagging the queries with CTX_ARRAY_INGEST_MODE =
'mvd'
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
