gianm commented on code in PR #15920:
URL: https://github.com/apache/druid/pull/15920#discussion_r1509434075
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/MSQTaskSqlEngine.java:
##########
@@ -307,6 +337,124 @@ private static void validateLimitAndOffset(final RelNode
topRel, final boolean l
}
}
+ /**
+ * Validate that the query does not include any type changes from string to
array or vice versa.
+ *
+ * These type changes tend to cause problems due to mixing of multi-value
strings and string arrays. In particular,
+ * many queries written in the "classic MVD" style (treating MVDs as if they
were regular strings) will fail when
+ * MVDs and arrays are mixed. So, we detect them as invalid.
+ *
+ * @param rootRel root rel
+ * @param fieldMappings field mappings from {@link #validateInsert(RelNode,
List, Table, PlannerContext)}
+ * @param targetTable table we are inserting (or replacing) into, if any
+ * @param plannerContext planner context
+ */
+ private static void validateTypeChanges(
+ final RelNode rootRel,
+ final List<Pair<Integer, String>> fieldMappings,
+ @Nullable final Table targetTable,
+ final PlannerContext plannerContext
+ )
+ {
+ if (targetTable == null) {
+ return;
Review Comment:
I was thinking more about the validations that are in effect when
`arrayIngestMode: array`, since those validations would help us change the
default.
Maybe when we change the default, we can also consider making it so when
people explicitly set `arrayIngestMode: mvd`, they need to use `ARRAY_TO_MV`.
(I think that's what you're suggesting here.)
--
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]
