imply-cheddar commented on code in PR #14014:
URL: https://github.com/apache/druid/pull/14014#discussion_r1155545000
##########
processing/src/main/java/org/apache/druid/segment/IndexMergerV9.java:
##########
@@ -890,100 +818,30 @@ private void writeDimValuesAndSetupDimConversion(
progress.stopSection(section);
}
- private void mergeCapabilities(
+ private void mergeFormat(
final List<IndexableAdapter> adapters,
final List<String> mergedDimensions,
- final Map<String, TypeSignature<ValueType>> metricTypes,
- final List<ColumnCapabilities> dimCapabilities
+ final Map<String, ColumnFormat> metricTypes,
+ final List<ColumnFormat> dimFormats
)
{
- final Map<String, ColumnCapabilities> capabilitiesMap = new HashMap<>();
+ final Map<String, ColumnFormat> columnFormats = new HashMap<>();
for (IndexableAdapter adapter : adapters) {
for (String dimension : adapter.getDimensionNames()) {
- ColumnCapabilities capabilities = adapter.getCapabilities(dimension);
- capabilitiesMap.compute(dimension, (d, existingCapabilities) ->
- mergeCapabilities(capabilities, existingCapabilities,
DIMENSION_CAPABILITY_MERGE_LOGIC)
- );
+ ColumnFormat format = adapter.getFormat(dimension);
+ columnFormats.compute(dimension, (d, existingFormat) -> existingFormat
== null ? format : format.merge(existingFormat));
}
for (String metric : adapter.getMetricNames()) {
- final ColumnCapabilities capabilities =
adapter.getCapabilities(metric);
- final ColumnCapabilities merged = capabilitiesMap.compute(metric, (m,
existingCapabilities) ->
- mergeCapabilities(capabilities, existingCapabilities,
METRIC_CAPABILITY_MERGE_LOGIC)
+ final ColumnFormat format = adapter.getFormat(metric);
+ final ColumnFormat merged = columnFormats.compute(metric, (m,
existingFormat) ->
+ existingFormat == null ? format : format.merge(existingFormat)
);
+
metricTypes.put(metric, merged);
}
}
for (String dim : mergedDimensions) {
- dimCapabilities.add(capabilitiesMap.get(dim));
- }
- }
Review Comment:
Heh, TBH, I would've enjoyed reviewing the logic changes for this method
being cleaned up into just a map of columns. Would be a good change for the
code anyway and I don't think it would actually be that big of a change
elsewhere.
But, we can do it in another PR too if you would prefer.
--
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]
