maytasm commented on a change in pull request #10208:
URL: https://github.com/apache/druid/pull/10208#discussion_r461226723
##########
File path: docs/configuration/index.md
##########
@@ -1769,14 +1769,11 @@ If there is an L1 miss and L2 hit, it will also
populate L1.
This section describes configurations that control behavior of Druid's query
types, applicable to Broker, Historical, and MiddleManager processes.
-### Query vectorization config
+### Overriding default query context values
-The following configurations are to set the default behavior for query
vectorization.
-
-|Property|Description|Default|
-|--------|-----------|-------|
-|`druid.query.vectorize`|See [Vectorization
parameters](../querying/query-context.html#vectorization-parameters) for
details. This value can be overridden by `vectorize` in the query
contexts.|`true`|
-|`druid.query.vectorSize`|See [Vectorization
parameters](../querying/query-context.html#vectorization-parameters) for
details. This value can be overridden by `vectorSize` in the query
contexts.|`512`|
+Any [Query Context General
Parameter](../querying/query-context.html#general-parameters) default value
+can be overridden by setting runtime property in the format of
`druid.query.override.default.context.{query_context_key}`.
+Note that the runtime property value can be overridden if value for the same
key is explicitly specify in the query contexts.
Review comment:
Done
##########
File path:
processing/src/main/java/org/apache/druid/query/OverrideDefaultQueryContext.java
##########
@@ -19,43 +19,50 @@
package org.apache.druid.query;
-import com.fasterxml.jackson.annotation.JsonProperty;
-import org.apache.druid.query.QueryContexts.Vectorize;
-import org.apache.druid.segment.QueryableIndexStorageAdapter;
+import com.fasterxml.jackson.annotation.JsonAnyGetter;
+import com.fasterxml.jackson.annotation.JsonAnySetter;
+
+import java.util.HashMap;
+import java.util.Map;
/**
* A user configuration holder for all query types.
* Any query-specific configurations should go to their own configuration.
- *
* @see org.apache.druid.query.groupby.GroupByQueryConfig
* @see org.apache.druid.query.search.SearchQueryConfig
* @see org.apache.druid.query.topn.TopNQueryConfig
* @see org.apache.druid.query.metadata.SegmentMetadataQueryConfig
* @see org.apache.druid.query.scan.ScanQueryConfig
+ *
+ * This class config map is populated by any runtime property prefixed with
druid.query.override.default.context
+ * Note that config values should not be directly retrieved from this class
but instead should
+ * be read through {@link QueryContexts}. This class contains configs from
runtime property which is then merged with
+ * configs passed in query context. The result of the merge is subsequently
stored in the query context.
+ * The order of precedence in mergeing of the configs is as follow:
Review comment:
Done
##########
File path:
processing/src/main/java/org/apache/druid/query/OverrideDefaultQueryContext.java
##########
@@ -19,43 +19,50 @@
package org.apache.druid.query;
-import com.fasterxml.jackson.annotation.JsonProperty;
-import org.apache.druid.query.QueryContexts.Vectorize;
-import org.apache.druid.segment.QueryableIndexStorageAdapter;
+import com.fasterxml.jackson.annotation.JsonAnyGetter;
+import com.fasterxml.jackson.annotation.JsonAnySetter;
+
+import java.util.HashMap;
+import java.util.Map;
/**
* A user configuration holder for all query types.
* Any query-specific configurations should go to their own configuration.
- *
* @see org.apache.druid.query.groupby.GroupByQueryConfig
* @see org.apache.druid.query.search.SearchQueryConfig
* @see org.apache.druid.query.topn.TopNQueryConfig
* @see org.apache.druid.query.metadata.SegmentMetadataQueryConfig
* @see org.apache.druid.query.scan.ScanQueryConfig
+ *
+ * This class config map is populated by any runtime property prefixed with
druid.query.override.default.context
+ * Note that config values should not be directly retrieved from this class
but instead should
+ * be read through {@link QueryContexts}. This class contains configs from
runtime property which is then merged with
+ * configs passed in query context. The result of the merge is subsequently
stored in the query context.
+ * The order of precedence in mergeing of the configs is as follow:
+ * Hard codeded default values (from {@link QueryContexts} can be override by
Review comment:
Re-wrote this point
##########
File path: docs/querying/query-context.md
##########
@@ -100,5 +103,5 @@ vectorization. These query types will ignore the
"vectorize" parameter even if i
|property|default| description|
|--------|-------|------------|
-|vectorize|`true`|Enables or disables vectorized query execution. Possible
values are `false` (disabled), `true` (enabled if possible, disabled otherwise,
on a per-segment basis), and `force` (enabled, and groupBy or timeseries
queries that cannot be vectorized will fail). The `"force"` setting is meant to
aid in testing, and is not generally useful in production (since real-time
segments can never be processed with vectorized execution, any queries on
real-time data will fail). This will override `druid.query.vectorize` if it's
set.|
-|vectorSize|`512`|Sets the row batching size for a particular query. This will
override `druid.query.vectorSize` if it's set.|
+|vectorize|`true`|Enables or disables vectorized query execution. Possible
values are `false` (disabled), `true` (enabled if possible, disabled otherwise,
on a per-segment basis), and `force` (enabled, and groupBy or timeseries
queries that cannot be vectorized will fail). The `"force"` setting is meant to
aid in testing, and is not generally useful in production (since real-time
segments can never be processed with vectorized execution, any queries on
real-time data will fail). This will override runtime property value if it's
set.|
+|vectorSize|`512`|Sets the row batching size for a particular query. This will
override runtime property value if it's set.|
Review comment:
Done
##########
File path: server/src/main/java/org/apache/druid/guice/QueryToolChestModule.java
##########
@@ -97,7 +97,7 @@ public void configure(Binder binder)
binder.bind(QueryToolChestWarehouse.class).to(MapQueryToolChestWarehouse.class);
- JsonConfigProvider.bind(binder, "druid.query", QueryConfig.class);
+ JsonConfigProvider.bind(binder, "druid.query.override.default.context",
OverrideDefaultQueryContext.class);
Review comment:
Changed to `druid.query.default.context`
##########
File path: docs/configuration/index.md
##########
@@ -1769,14 +1769,23 @@ If there is an L1 miss and L2 hit, it will also
populate L1.
This section describes configurations that control behavior of Druid's query
types, applicable to Broker, Historical, and MiddleManager processes.
-### Query vectorization config
+### Overriding default query context values
Review comment:
Done
##########
File path:
processing/src/main/java/org/apache/druid/query/OverrideDefaultQueryContext.java
##########
@@ -19,43 +19,48 @@
package org.apache.druid.query;
-import com.fasterxml.jackson.annotation.JsonProperty;
-import org.apache.druid.query.QueryContexts.Vectorize;
-import org.apache.druid.segment.QueryableIndexStorageAdapter;
+import com.fasterxml.jackson.annotation.JsonAnyGetter;
+import com.fasterxml.jackson.annotation.JsonAnySetter;
+
+import java.util.HashMap;
+import java.util.Map;
/**
* A user configuration holder for all query types.
* Any query-specific configurations should go to their own configuration.
- *
* @see org.apache.druid.query.groupby.GroupByQueryConfig
* @see org.apache.druid.query.search.SearchQueryConfig
* @see org.apache.druid.query.topn.TopNQueryConfig
* @see org.apache.druid.query.metadata.SegmentMetadataQueryConfig
* @see org.apache.druid.query.scan.ScanQueryConfig
+ *
+ * This class config map is populated by any runtime property prefixed with
druid.query.override.default.context
+ * Note that config values should not be directly retrieved from this class
but instead should
+ * be read through {@link QueryContexts}. This class contains configs from
runtime property which is then merged with
+ * configs passed in query context. The result of the merge is subsequently
stored in the query context.
+ * The order of precedence in merging of the configs is as follow:
+ * runtime property values (store in this class) override by query context
parameter passed in with the query
+ *
+
*/
-public class QueryConfig
+public class OverrideDefaultQueryContext
{
- @JsonProperty
- private Vectorize vectorize = QueryContexts.DEFAULT_VECTORIZE;
-
- @JsonProperty
- private int vectorSize = QueryableIndexStorageAdapter.DEFAULT_VECTOR_SIZE;
+ private Map<String, Object> configs = new HashMap<>();
Review comment:
Yes. That is definitely possible. I think that will just be moving
things around from QueryContexts into this class. Since that does not impact
the end-user (meaning that the defaults will remains the same, the way they
specify the override are the same, etc.), I think we can do it in a separate
PR.
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
