GWphua commented on code in PR #20198:
URL: https://github.com/apache/druid/pull/20198#discussion_r3951652577
##########
processing/src/main/java/org/apache/druid/query/QueryContext.java:
##########
@@ -514,15 +692,6 @@ public long getTimeout(long defaultTimeout)
);
}
- @Nullable
- public Duration getTimeoutDuration()
- {
- if (hasTimeout()) {
- return Duration.ofMillis(getTimeout());
- }
- return null;
- }
-
Review Comment:
This removal seem out of scope of this PR
##########
processing/src/main/java/org/apache/druid/query/QueryContext.java:
##########
@@ -199,18 +389,6 @@ public long getLong(final String key, final long
defaultValue)
return QueryContexts.parseLong(context, key, defaultValue);
}
- /**
- * Return a value as an {@code Float}, returning {@link null} if the
- * context value is not set.
- *
- * @throws BadQueryContextException for an invalid value
- */
- @SuppressWarnings("unused")
- public Float getFloat(final String key)
- {
- return QueryContexts.getAsFloat(key, get(key));
- }
-
Review Comment:
This removal seem out of scope of this PR
##########
docs/querying/query-context-reference.md:
##########
@@ -51,7 +51,7 @@ Unless otherwise noted, the following parameters apply to all
query types, and t
|`brokerService` | `null` | Broker service
to which this query should be routed. This parameter is honored only by a
broker selector strategy of type *manual*. See [Router
strategies](../design/router.md#router-strategies) for more details.|
|`useCache` | `true` | Flag indicating
whether to leverage the query cache for this query. When set to false, it
disables reading from the query cache for this query. When set to true, Apache
Druid uses `druid.broker.cache.useCache` or `druid.historical.cache.useCache`
to determine whether or not to read from the query cache |
|`populateCache` | `true` | Flag indicating
whether to save the results of the query to the query cache. Primarily used for
debugging. When set to false, it disables saving the results of this query to
the query cache. When set to true, Druid uses
`druid.broker.cache.populateCache` or `druid.historical.cache.populateCache` to
determine whether or not to save the results of this query to the query cache |
-|`useResultLevelCache`| `true` | Flag indicating whether
to leverage the result level cache for this query. When set to false, it
disables reading from the query cache for this query. When set to true, Druid
uses `druid.broker.cache.useResultLevelCache` to determine whether or not to
read from the result-level query cache |
Review Comment:
Is this comment an intended addition?
##########
docs/querying/scan-query.md:
##########
@@ -197,7 +197,7 @@ Configuration properties:
|property|description|values|default|
|--------|-----------|------|-------|
-|maxRowsQueuedForOrdering|The maximum number of rows returned when time
ordering is used. Overrides the identically named config.|An integer in [1,
2147483647]|`druid.query.scan.maxRowsQueuedForOrdering`|
+|maxRowsQueuedForOrdering|The maximum number of rows returned when time
ordering is used. Overrides the identically named config.|An integer in [1,
2147483647]|`druid.query.scan.maxRowsQueuedForOrdering`| <!-- GENERATED QUERY
CONTEXT PARAMETER: maxRowsQueuedForOrdering -->
Review Comment:
Same as above
##########
processing/src/main/java/org/apache/druid/query/Query.java:
##########
@@ -132,7 +134,7 @@ default QueryContext context()
* {@link QueryContext#getString(String)} <br/>
* {@link QueryContext#getInt(String)} <br/>
* {@link QueryContext#getLong(String)} <br/>
- * {@link QueryContext#getFloat(String)} <br/>
+ * {@link QueryContext#getFloat(String, float)} <br/>
Review Comment:
Indentation + Should revert if we choose not to delete
QueryContext#getFloat(String)
##########
processing/src/main/java/org/apache/druid/query/QueryContext.java:
##########
@@ -818,12 +987,4 @@ public RealtimeSegmentsMode getRealtimeSegmentsMode()
return QueryContexts.DEFAULT_REALTIME_SEGMENTS_MODE;
}
- /**
- * @deprecated Use {@link #getRealtimeSegmentsMode()} instead.
- */
- @Deprecated
- public boolean isRealtimeSegmentsOnly()
- {
- return getRealtimeSegmentsMode() == RealtimeSegmentsMode.EXCLUSIVE;
- }
Review Comment:
This removal seem out of scope of this PR
##########
server/src/main/java/org/apache/druid/client/CachingClusteredClient.java:
##########
@@ -297,24 +298,24 @@ private class SpecificQueryRunnable<T>
);
}
- private ImmutableMap<String, Object> makeDownstreamQueryContext()
+ private Map<String, Object> makeDownstreamQueryContext()
{
- final ImmutableMap.Builder<String, Object> contextBuilder = new
ImmutableMap.Builder<>();
+ final QueryContextBuilder contextBuilder = QueryContext.builder();
final QueryContext queryContext = query.context();
final int priority = queryContext.getPriority();
- contextBuilder.put(QueryContexts.PRIORITY_KEY, priority);
+ contextBuilder.putRaw(QueryContexts.PRIORITY_KEY, priority);
final String lane = queryContext.getLane();
if (lane != null) {
- contextBuilder.put(QueryContexts.LANE_KEY, lane);
+ contextBuilder.putRaw(QueryContexts.LANE_KEY, lane);
}
if (populateCache) {
// prevent down-stream nodes from caching results as well if we are
populating the cache
- contextBuilder.put(CacheConfig.POPULATE_CACHE, false);
- contextBuilder.put(QueryContexts.BY_SEGMENT_KEY, true);
+ contextBuilder.putRaw(CacheConfig.POPULATE_CACHE, false);
+ contextBuilder.putRaw(QueryContexts.BY_SEGMENT_KEY, true);
}
- return contextBuilder.build();
+ return contextBuilder.toMap();
Review Comment:
Is this change necessary?
##########
processing/src/main/java/org/apache/druid/query/QueryContext.java:
##########
@@ -81,11 +84,152 @@ public static QueryContext empty()
return EMPTY;
}
+ /**
+ * Creates a builder for a query context map.
+ */
+ public static QueryContextBuilder builder()
+ {
+ return new QueryContextBuilder();
+ }
+
public static QueryContext of(Map<String, Object> context)
{
return new QueryContext(context);
}
+ /**
+ * Creates a query context from one declared query context parameter.
+ */
+ public static <T> QueryContext of(
+ final QueryContextParameter<T> parameter,
+ @Nullable final T value
+ )
+ {
+ return new QueryContext(ofMap(parameter, value));
+ }
+
+ /**
+ * Creates a query context map from one declared query context parameter.
+ */
+ public static <T> Map<String, Object> ofMap(
+ final QueryContextParameter<T> parameter,
+ @Nullable final T value
+ )
+ {
+ return builder().put(parameter, value).toMap();
+ }
+
+ /**
+ * Creates a query context from two declared query context parameters.
+ */
+ public static <T1, T2> QueryContext of(
+ final QueryContextParameter<T1> parameter1,
+ @Nullable final T1 value1,
+ final QueryContextParameter<T2> parameter2,
+ @Nullable final T2 value2
+ )
+ {
+ return new QueryContext(ofMap(parameter1, value1, parameter2, value2));
+ }
+
+ /**
+ * Creates a query context map from two declared query context parameters.
+ */
+ public static <T1, T2> Map<String, Object> ofMap(
+ final QueryContextParameter<T1> parameter1,
+ @Nullable final T1 value1,
+ final QueryContextParameter<T2> parameter2,
+ @Nullable final T2 value2
+ )
+ {
+ return builder()
+ .put(parameter1, value1)
+ .put(parameter2, value2)
+ .toMap();
+ }
+
+ /**
+ * Creates a query context from three declared query context parameters.
+ */
+ public static <T1, T2, T3> QueryContext of(
+ final QueryContextParameter<T1> parameter1,
+ @Nullable final T1 value1,
+ final QueryContextParameter<T2> parameter2,
+ @Nullable final T2 value2,
+ final QueryContextParameter<T3> parameter3,
+ @Nullable final T3 value3
+ )
+ {
+ return new QueryContext(ofMap(parameter1, value1, parameter2, value2,
parameter3, value3));
+ }
+
+ /**
+ * Creates a query context map from three declared query context parameters.
+ */
+ public static <T1, T2, T3> Map<String, Object> ofMap(
+ final QueryContextParameter<T1> parameter1,
+ @Nullable final T1 value1,
+ final QueryContextParameter<T2> parameter2,
+ @Nullable final T2 value2,
+ final QueryContextParameter<T3> parameter3,
+ @Nullable final T3 value3
+ )
+ {
+ return builder()
+ .put(parameter1, value1)
+ .put(parameter2, value2)
+ .put(parameter3, value3)
+ .toMap();
+ }
+
+ /**
+ * Creates a query context from four declared query context parameters.
+ */
+ public static <T1, T2, T3, T4> QueryContext of(
+ final QueryContextParameter<T1> parameter1,
+ @Nullable final T1 value1,
+ final QueryContextParameter<T2> parameter2,
+ @Nullable final T2 value2,
+ final QueryContextParameter<T3> parameter3,
+ @Nullable final T3 value3,
+ final QueryContextParameter<T4> parameter4,
+ @Nullable final T4 value4
+ )
+ {
+ return new QueryContext(ofMap(
+ parameter1,
+ value1,
+ parameter2,
+ value2,
+ parameter3,
+ value3,
+ parameter4,
+ value4
+ ));
+ }
+
+ /**
+ * Creates a query context map from four declared query context parameters.
+ */
+ public static <T1, T2, T3, T4> Map<String, Object> ofMap(
+ final QueryContextParameter<T1> parameter1,
+ @Nullable final T1 value1,
+ final QueryContextParameter<T2> parameter2,
+ @Nullable final T2 value2,
+ final QueryContextParameter<T3> parameter3,
+ @Nullable final T3 value3,
+ final QueryContextParameter<T4> parameter4,
+ @Nullable final T4 value4
+ )
+ {
+ return builder()
+ .put(parameter1, value1)
+ .put(parameter2, value2)
+ .put(parameter3, value3)
+ .put(parameter4, value4)
+ .toMap();
+ }
+
Review Comment:
These methods should be unnecessary. We can simply use the following instead:
QueryContext context = QueryContext.builder()
.put(...)
.put(...)
.build();
--
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]
