kgyrtkirk commented on code in PR #15465:
URL: https://github.com/apache/druid/pull/15465#discussion_r1446196180
##########
processing/src/main/java/org/apache/druid/query/CacheStrategy.java:
##########
@@ -155,10 +157,16 @@ static void fetchAggregatorsFromCache(
throw new ISE("Ran out of objects while reading aggregators from
cache!");
}
- if (isResultLevelCache) {
- addToResultFunction.apply(aggregator.getName(), i, resultIter.next());
- } else {
+ ColumnType resultType = aggregator.getResultType();
+ ColumnType intermediateType = aggregator.getIntermediateType();
+
+ boolean needsDeserialize = !isResultLevelCache
+ || (!resultType.isPrimitive() &&
resultType.equals(intermediateType));
Review Comment:
if `resultType.isPrimitive() && resultType.equals(intermediateType)` is
*true*; then
both `resultType.isPrimitive()` and `intermediateType.isPrimitive()` is true
- so there might be not much to deserialize....
I've removed it - as I don't think it will make a big difference
##########
server/src/main/java/org/apache/druid/segment/metadata/AbstractSegmentMetadataCache.java:
##########
@@ -881,11 +882,16 @@ public Sequence<SegmentAnalysis> runSegmentMetadataQuery(
querySegmentSpec,
new AllColumnIncluderator(),
false,
- // disable the parallel merge because we don't care about the merge
and don't want to consume its resources
QueryContexts.override(
internalQueryConfig.getContext(),
- QueryContexts.BROKER_PARALLEL_MERGE_KEY,
- false
+ ImmutableMap.of(
+ // disable the parallel merge because we don't care about the
merge and don't want to consume its resources
+ QueryContexts.BROKER_PARALLEL_MERGE_KEY,
+ false,
+ // dont use result cache for metadata
+ QueryContexts.POPULATE_RESULT_LEVEL_CACHE_KEY,
Review Comment:
I've seen that these queries were excercising the resultlevel cache during
the exploration of running `CalciteQueryWithResultCacheTest`
I think this change is not about fix this issue - I've removed it for now as
`CalciteQueryWithResultCacheTest` will not be added in this patch it might have
been possible that it have hit the `ResultCache` due to the fact how testing is
being set up - and will not happen in real use cases
--
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]
