You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/04/28 08:51:45 UTC
[GitHub] [druid] rohangarg opened a new pull request, #12491: Add feature flag for sql planning of TimeBoundary queries
rohangarg opened a new pull request, #12491:
URL: https://github.com/apache/druid/pull/12491
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:
- [x] been self-reviewed.
- [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
--
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: commits-unsubscribe@druid.apache.org
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
[GitHub] [druid] suneet-s commented on a diff in pull request #12491: Add feature flag for sql planning of TimeBoundary queries
Posted by GitBox <gi...@apache.org>.
suneet-s commented on code in PR #12491:
URL: https://github.com/apache/druid/pull/12491#discussion_r861937986
##########
docs/querying/query-context.md:
##########
@@ -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. |
Review 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.
--
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: commits-unsubscribe@druid.apache.org
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
[GitHub] [druid] abhishekagarwal87 merged pull request #12491: Add feature flag for sql planning of TimeBoundary queries
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 merged PR #12491:
URL: https://github.com/apache/druid/pull/12491
--
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: commits-unsubscribe@druid.apache.org
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
[GitHub] [druid] rohangarg commented on a diff in pull request #12491: Add feature flag for sql planning of TimeBoundary queries
Posted by GitBox <gi...@apache.org>.
rohangarg commented on code in PR #12491:
URL: https://github.com/apache/druid/pull/12491#discussion_r861576564
##########
processing/src/main/java/org/apache/druid/query/QueryContexts.java:
##########
@@ -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";
Review 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
--
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: commits-unsubscribe@druid.apache.org
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
[GitHub] [druid] abhishekagarwal87 commented on a diff in pull request #12491: Add feature flag for sql planning of TimeBoundary queries
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on code in PR #12491:
URL: https://github.com/apache/druid/pull/12491#discussion_r860655411
##########
processing/src/main/java/org/apache/druid/query/QueryContexts.java:
##########
@@ -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";
Review 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.
--
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: commits-unsubscribe@druid.apache.org
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
[GitHub] [druid] abhishekagarwal87 commented on a diff in pull request #12491: Add feature flag for sql planning of TimeBoundary queries
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on code in PR #12491:
URL: https://github.com/apache/druid/pull/12491#discussion_r861990761
##########
docs/querying/query-context.md:
##########
@@ -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. |
Review 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.
--
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: commits-unsubscribe@druid.apache.org
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