clintropolis commented on code in PR #12855:
URL: https://github.com/apache/druid/pull/12855#discussion_r940888916
##########
processing/src/main/java/org/apache/druid/query/groupby/GroupByQueryQueryToolChest.java:
##########
@@ -533,6 +535,12 @@ public byte[] computeCacheKey(GroupByQuery query)
.appendCacheable(query.getVirtualColumns());
if (query.isApplyLimitPushDown()) {
builder.appendCacheable(query.getLimitSpec());
+ if (query.getLimitSpec() instanceof DefaultLimitSpec) {
+ DefaultLimitSpec limitSpec = (DefaultLimitSpec)
query.getLimitSpec();
+ for (OrderByColumnSpec column : limitSpec.getColumns()) {
+ appendOrderingColumnCacheable(query, builder,
column.getDimension());
Review Comment:
Yeah that makes sense, I hesitated on doing an approach involving passing
the `Query` in initially because these ordering/limiting containers currently
implement `Cacheable` and wasn't quite sure what that would look like, but
didn't consider also passing the builder in which is probably the best thing to
do here :+1:
##########
processing/src/main/java/org/apache/druid/query/topn/NumericTopNMetricSpec.java:
##########
@@ -135,12 +133,7 @@ public TopNResultBuilder getResultBuilder(
@Override
public byte[] getCacheKey()
{
- byte[] metricBytes = StringUtils.toUtf8(metric);
-
- return ByteBuffer.allocate(1 + metricBytes.length)
- .put(CACHE_TYPE_ID)
- .put(metricBytes)
- .array();
+ return new byte[] {CACHE_TYPE_ID};
Review Comment:
I removed this because it seemed redundant to have the name here after
modifying the topN query toolchest to be appending the information of the
ordering column immediately after, but I think your suggestion to push the
query into the cache key method makes sense so that it could lookup the
information itself to build the limit spec correctly would maybe make it more
obvious.
--
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]
