zachjsh commented on code in PR #14319:
URL: https://github.com/apache/druid/pull/14319#discussion_r1202957170
##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/SegmentMetadataCache.java:
##########
@@ -995,4 +986,118 @@ void doInLock(Runnable runnable)
runnable.run();
}
}
+
+
+ /**
+ * ColumnTypeMergePolicy defines the rules of which type to use when faced
with the possibility of different types
+ * for the same column from segment to segment. It is used to help compute a
{@link RowSignature} for a table in
+ * Druid based on the segment metadata of all segments, merging the types of
each column encountered to end up with
+ * a single type to represent it globally.
+ */
+ @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type", defaultImpl =
LeastRestrictiveTypeMergePolicy.class)
+ @JsonSubTypes(value = {
+ @JsonSubTypes.Type(name = FirstTypeMergePolicy.NAME, value =
FirstTypeMergePolicy.class),
+ @JsonSubTypes.Type(name = LeastRestrictiveTypeMergePolicy.NAME, value =
LeastRestrictiveTypeMergePolicy.class)
+ })
+ @FunctionalInterface
+ public interface ColumnTypeMergePolicy
+ {
+ ColumnType merge(ColumnType existingType, ColumnType newType);
+ }
+
+ /**
+ * Classic logic, we use the first type we encounter. This policy is
effectively 'newest first' because we iterated
+ * segments starting from the most recent time chunk, so this typically
results in the most recently used type being
+ * chosen, at least for systems that are continuously updated with 'current'
data.
+ *
+ * Since {@link ColumnTypeMergePolicy} are used to compute the SQL schema,
at least in systems using SQL schemas which
+ * are partially or fully computed by this cache, this merge policy can
result in query time errors if incompatible
+ * types are mixed if the chosen type is more restrictive than the types of
some segments. If data is likely to vary
+ * in type across segments, consider using {@link
LeastRestrictiveTypeMergePolicy} instead.
+ */
+ public static class FirstTypeMergePolicy implements ColumnTypeMergePolicy
+ {
+ public static final String NAME = "latestInterval";
+ private static final FirstTypeMergePolicy INSTANCE = new
FirstTypeMergePolicy();
+
+ @Override
+ public ColumnType merge(ColumnType existingType, ColumnType newType)
+ {
+ if (existingType == null) {
+ return newType;
+ }
+ if (newType == null) {
+ return existingType;
+ }
+ // if any are json, are all json
+ if (ColumnType.NESTED_DATA.equals(newType) ||
ColumnType.NESTED_DATA.equals(existingType)) {
+ return ColumnType.NESTED_DATA;
+ }
+ // "existing type" is the 'newest' type, since we iterate the segments
list by newest start time
+ return existingType;
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return Objects.hash(NAME);
+ }
+
+ @Override
+ public boolean equals(Object o)
+ {
+ if (this == o) {
+ return true;
+ }
+ return o != null && getClass() == o.getClass();
+ }
+
+ @Override
+ public String toString()
+ {
+ return NAME;
+ }
+ }
+
+ /**
+ * Resolves types using {@link ColumnType#leastRestrictiveType(ColumnType,
ColumnType)} to find the ColumnType that
+ * can best represent all data contained across all segments.
+ */
+ public static class LeastRestrictiveTypeMergePolicy implements
ColumnTypeMergePolicy
+ {
+ public static final String NAME = "leastRestrictive";
+
+ @Override
+ public ColumnType merge(ColumnType existingType, ColumnType newType)
+ {
+ try {
+ return ColumnType.leastRestrictiveType(existingType, newType);
+ }
+ catch (Types.IncompatibleTypeException incompatibleTypeException) {
+ // fall back to first encountered type if they are not compatible for
some reason
+ return FirstTypeMergePolicy.INSTANCE.merge(existingType, newType);
Review Comment:
what would happen if we just allowed the exception to propagate through here
instead of reverting back to the newest first strategy? With this reversion, it
feels like this policy may be hard to define, maybe? In this case, how is the
user expected to query the data if the types are not expressible from one base
type?
--
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]
