[GitHub] [druid] jihoonson commented on a change in pull request #10208: Cluster wide default query context setting

[GitBox <gi...@apache.org>]

jihoonson commented on a change in pull request #10208:
URL: https://github.com/apache/druid/pull/10208#discussion_r461920418



##########
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:
       Please add some example in this section.

##########
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:
       I'm wondering if it's possible to initialize `configs` with the default values in `QueryContexts` if they are missing and remove those defaults from `QueryContexts`. In this way, we can keep all default values in a single place and compute query context values in a more structured way. I think it's possible by keeping all default values in a map somewhere and overriding them with user-provided default values when initializing this class.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org