clintropolis commented on a change in pull request #8089: add
CachingClusteredClient benchmark, refactor some stuff
URL: https://github.com/apache/incubator-druid/pull/8089#discussion_r304614898
##########
File path: processing/src/main/java/org/apache/druid/query/QueryToolChest.java
##########
@@ -77,11 +80,38 @@ public final JavaType getBySegmentResultType()
* ResultType objects in time order (ascending or descending). This method
should return a new QueryRunner that
* potentially merges the stream of ordered ResultType objects.
*
+ * A default implementation constructs a {@link ResultMergeQueryRunner}
which creates a
+ * {@link org.apache.druid.common.guava.CombiningSequence} using the
supplied {@link QueryRunner} with
+ * {@link QueryToolChest#createOrderingFn(Query)} and {@link
QueryToolChest#createMergeFn(Query)}} supplied by this
+ * toolchest.
+ *
* @param runner A QueryRunner that provides a series of ResultType objects
in time order (ascending or descending)
*
* @return a QueryRunner that potentially merges the stream of ordered
ResultType objects
*/
- public abstract QueryRunner<ResultType> mergeResults(QueryRunner<ResultType>
runner);
+ public QueryRunner<ResultType> mergeResults(QueryRunner<ResultType> runner)
+ {
+ return new ResultMergeQueryRunner<>(runner, this::createOrderingFn,
this::createMergeFn);
+ }
+
+ /**
+ * Creates a merge function that is used to merge intermediate aggregates
from historicals in broker. This merge
+ * function is used in the default {@link ResultMergeQueryRunner} provided by
+ * {@link QueryToolChest#mergeResults(QueryRunner)} and can be used in
additional future merge implementations
+ */
+ public CombiningFunction<ResultType> createMergeFn(Query<ResultType> query)
+ {
+ throw new UOE("%s doesn't support merge function",
query.getClass().getName());
+ }
+
+ /**
+ * Creates an ordering comparator that is used to order results. This
ordering function is used in the defaul
+ * {@link ResultMergeQueryRunner} provided by {@link
QueryToolChest#mergeResults(QueryRunner)}
+ */
+ public Ordering<ResultType> createOrderingFn(Query<ResultType> query)
Review comment:
It's currently producing an `Ordering` because the `CombiningSequence`
created by `ResultMergeQueryRunner` takes that instead of a `Comparator`.
However, it looks like the `Ordering` in `CombiningSequence` can be swapped to
use a regular `Comparator`, so I _think_ this could likely be safely changed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
