clintropolis commented on code in PR #16533:
URL: https://github.com/apache/druid/pull/16533#discussion_r1717805127
##########
processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQueryEngine.java:
##########
@@ -94,64 +103,46 @@ public Sequence<Result<TimeseriesResultValue>> process(
);
}
- final Filter filter = Filters.convertToCNFFromQueryContext(query,
Filters.toFilter(query.getFilter()));
final Interval interval = Iterables.getOnlyElement(query.getIntervals());
final Granularity gran = query.getGranularity();
- final boolean descending = query.isDescending();
- final ColumnInspector inspector =
query.getVirtualColumns().wrapInspector(adapter);
-
- final boolean doVectorize = query.context().getVectorize().shouldVectorize(
- adapter.canVectorize(filter, query.getVirtualColumns(), descending)
- && VirtualColumns.shouldVectorize(query, query.getVirtualColumns(),
adapter)
- && VectorGroupByEngine.canVectorizeAggregators(inspector,
query.getAggregatorSpecs())
- );
+ final CursorHolder cursorHolder =
adapter.makeCursorHolder(makeCursorBuildSpec(query, timeseriesQueryMetrics));
+ Cursors.requireTimeOrdering(cursorHolder, query.isDescending() ?
Order.DESCENDING : Order.ASCENDING);
final Sequence<Result<TimeseriesResultValue>> result;
- if (doVectorize) {
- result = processVectorized(query, adapter, filter, interval, gran,
descending, timeseriesQueryMetrics);
+ if
(query.context().getVectorize().shouldVectorize(cursorHolder.canVectorize(),
cursorHolder::close)) {
+ result = processVectorized(query, adapter, cursorHolder, interval, gran);
Review Comment:
good call, i meant to do this anyway to dump the funny shouldVectorize thing
##########
processing/src/main/java/org/apache/druid/query/topn/TopNQueryEngine.java:
##########
@@ -193,13 +205,74 @@ private static boolean canUsePooledAlgorithm(
// non-string output cannot use the pooled algorith, even if the
underlying selector supports it
return false;
}
- if (Types.is(capabilities, ValueType.STRING)) {
- // string columns must use the on heap algorithm unless they have the
following capabilites
- return capabilities.isDictionaryEncoded().isTrue() &&
capabilities.areDictionaryValuesUnique().isTrue();
- } else {
+ if (!Types.is(capabilities, ValueType.STRING)) {
// non-strings are not eligible to use the pooled algorithm, and should
use a heap algorithm
return false;
}
+
+ // string columns must use the on heap algorithm unless they have the
following capabilites
+ if (!capabilities.isDictionaryEncoded().isTrue() ||
!capabilities.areDictionaryValuesUnique().isTrue()) {
+ return false;
+ }
+ if (Granularities.ALL.equals(query.getGranularity())) {
+ // all other requirements have been satisfied, ALL granularity can
always use the pooled algorithms
+ return true;
+ }
+ // if not using ALL granularity, we can still potentially use the pooled
algorithm if we are certain it doesn't
+ // need to make multiple passes (e.g. reset the cursor)
+ try (final ResourceHolder<ByteBuffer> resultsBufHolder =
bufferPool.take()) {
+ final ByteBuffer resultsBuf = resultsBufHolder.get();
+ resultsBuf.clear();
+
+ final int numBytesToWorkWith = resultsBuf.remaining();
Review Comment:
cool, i was just copying stuff from
https://github.com/apache/druid/blob/master/processing/src/main/java/org/apache/druid/query/topn/PooledTopNAlgorithm.java#L259
and didn't think too much about it
--
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: commits-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
