Add feature flag for sql planning of TimeBoundary queries #12491
Conversation
| @@ -70,6 +70,7 @@ | |||
| public static final String BY_SEGMENT_KEY = "bySegment"; | |||
| public static final String BROKER_SERVICE_NAME = "brokerService"; | |||
| public static final String IN_SUB_QUERY_THRESHOLD_KEY = "inSubQueryThreshold"; | |||
| public static final String PLAN_TIME_BOUNDARY_SQL = "planTimeBoundarySql"; | |||
There was a problem hiding this comment.
can you add documentation for this context? Also can this be renamed to
enableTimeBoundaryPlanning or enableTimeBoundaryConversion ? I am suggesting it since other boolean flags follow similar convention. Also sql word in there is not needed.
There was a problem hiding this comment.
I had added sql to point that this config is only applicable to sql planning - the native query runs are independent of this flag.
But seems right as you said that no other property also has this distinction - which upon rereading of the query context flags made me realise that there's no obvious way to in the name to know that whether a property is applicable only to SQL or both.
There's PlannerConfig which has a druid.sql.planner prefix for all properties but that doesn't allow a direct query level override.
Anyways, I will rename to enableTimeBoundaryPlanning to adhere to the current convention - thanks for the suggestion
docs/querying/query-context.md
Outdated
| @@ -64,6 +64,7 @@ Unless otherwise noted, the following parameters apply to all query types. | |||
| |debug| `false` | Flag indicating whether to enable debugging outputs for the query. When set to false, no additional logs will be produced (logs produced will be entirely dependent on your logging level). When set to true, the following addition logs will be produced:<br />- Log the stack trace of the exception (if any) produced by the query | | |||
| |maxNumericInFilters|`-1`|Max limit for the amount of numeric values that can be compared for a string type dimension when the entire SQL WHERE clause of a query translates only to an [OR](../querying/filters.md#or) of [Bound filter](../querying/filters.md#bound-filter). By default, Druid does not restrict the amount of of numeric Bound Filters on String columns, although this situation may block other queries from running. Set this property to a smaller value to prevent Druid from running queries that have prohibitively long segment processing times. The optimal limit requires some trial and error; we recommend starting with 100. Users who submit a query that exceeds the limit of `maxNumericInFilters` should instead rewrite their queries to use strings in the `WHERE` clause instead of numbers. For example, `WHERE someString IN (‘123’, ‘456’)`. This value cannot exceed the set system configuration `druid.sql.planner.maxNumericInFilters`. This value is ignored if `druid.sql.planner.maxNumericInFilters` is not set explicitly.| | |||
| |inSubQueryThreshold|`2147483647`| Threshold for minimum number of values in an IN clause to convert the query to a JOIN operation on an inlined table rather than a predicate. A threshold of 0 forces usage of an inline table in all cases; a threshold of [Integer.MAX_VALUE] forces usage of OR in all cases. | | |||
| |enableTimeBoundaryPlanning|`false`| If true, SQL queries will get converted to TimeBoundary queries wherever possible. TimeBoundary queries are very efficient for min-max calculation on __time column in a datasource. | | |||
There was a problem hiding this comment.
I think the default should be true here. It's better for query performance and the results are more compliant to what a user would expect if they wrote a SQL query asking for MIN(__time) that matched nothing.
There was a problem hiding this comment.
it is somewhat similar to useStrictBooleans flag which is off by default in code. That flag also enables more SQL compliant behavior but has the potential to break existing queries, just like this flag.
Adding a flag for converting SQL queries to time boundary queries. This will help in reverting back to old behavior of planning timeseries queries instead, if needed.
This PR has: