FrankChen021 commented on code in PR #20314:
URL: https://github.com/apache/druid/pull/20314#discussion_r4023015151
##########
docs/querying/sql-query-context.md:
##########
@@ -56,6 +56,7 @@ The table below lists the query context parameters you can
use with Druid SQL.
|`inFunctionThreshold`| At or beyond this threshold number of values, Druid
converts SQL `IN` to [`SCALAR_IN_ARRAY`](sql-functions.md#scalar_in_array). A
threshold of 0 forces this conversion in all cases. A threshold of
`Integer.MAX_VALUE` disables this conversion. The converted function is
eligible for fewer planning-time optimizations, which speeds up planning, but
may prevent certain planning-time optimizations.| `100`|
|`inFunctionExprThreshold`|At or beyond this threshold number of values, SQL
`IN` is eligible for execution using the native function `scalar_in_array`
rather than an <code>||</code> of `==`, even if the number of values
is below `inFunctionThreshold`. This property only affects translation of SQL
`IN` to a [native expression](math-expr.md). It doesn't affect translation of
SQL `IN` to a [native filter](filters.md). This property is provided for
backwards compatibility purposes, and may be removed in a future release.|`2`|
|`inSubQueryThreshold`|At or beyond this threshold number of values, Druid
converts SQL `IN` to `JOIN` on an inline table. `inFunctionThreshold` takes
priority over this setting. A threshold of 0 forces usage of an inline table in
all cases where the size of a SQL `IN` is larger than `inFunctionThreshold`. A
threshold of `2147483647` disables the rewrite of SQL `IN` to `JOIN`.
|`2147483647`|
+|`maxPlanningTimeMs`|Maximum wall-clock time, in milliseconds, allowed for
planning this query on the Broker. When planning exceeds this budget, planning
is aborted and the query fails with an HTTP 504 `Query timeout` error. Use this
to guard against pathological queries, such as one with a very large `IN`
filter, whose planning time can spike. A value of `0` disables the timeout.
Defaults to the value of the `druid.sql.planner.maxPlanningTimeMs` runtime
property.|`0` (disabled)|
Review Comment:
what if the value is negative?
##########
sql/src/test/java/org/apache/druid/sql/SqlStatementTest.java:
##########
@@ -568,15 +570,109 @@ private SqlStatementFactory buildSqlStatementFactory()
new DruidHookDispatcher()
);
- return new SqlStatementFactory(
- new SqlToolbox(
- CalciteTests.createMockSqlEngine(walker, conglomerate),
- plannerFactory,
- new NoopServiceEmitter(),
- testRequestLogger,
- QueryStackTests.DEFAULT_NOOP_SCHEDULER,
- new SqlLifecycleManager()
- )
+ this.sqlToolbox = new SqlToolbox(
+ CalciteTests.createMockSqlEngine(walker, conglomerate),
+ plannerFactory,
+ new NoopServiceEmitter(),
+ testRequestLogger,
+ QueryStackTests.DEFAULT_NOOP_SCHEDULER,
+ new SqlLifecycleManager()
);
+ return new SqlStatementFactory(sqlToolbox);
+ }
+
+ /**
+ * A query whose planning exceeds the configured {@code maxPlanningTimeMs}
should fail with a
+ * {@link QueryTimeoutException} rather than occupying the planning thread
indefinitely. Planning is simulated as a
+ * CPU-bound loop that honours the Calcite cancel flag (as Calcite's planner
does), so we verify the watchdog trips
+ * that flag and the failure is surfaced as a timeout.
+ */
+ @Test
+ @org.junit.jupiter.api.Timeout(30)
Review Comment:
update the code to use non full qualified class name
##########
docs/configuration/index.md:
##########
@@ -1920,6 +1920,7 @@ The Druid SQL server is configured through the following
properties on the Broke
|`druid.sql.planner.maxNumericInFilters`|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 to an [OR](../querying/filters.md#or) of
[Bound filter](../querying/filters.md#bound-filter). By default, Druid does not
restrict the amount 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’)`. If this value is disabled, `maxNumericInFilters` set through
query context is ignored.|`-1` (disabled)|
|`druid.sql.approxCountDistinct.function`|Implementation to use for the
[`APPROX_COUNT_DISTINCT` function](../querying/sql-aggregations.md). Without
extensions loaded, the only valid value is `APPROX_COUNT_DISTINCT_BUILTIN` (a
HyperLogLog, or HLL, based implementation). If the [DataSketches
extension](../development/extensions-core/datasketches-extension.md) is loaded,
this can also be `APPROX_COUNT_DISTINCT_DS_HLL` (alternative HLL
implementation) or `APPROX_COUNT_DISTINCT_DS_THETA`.<br /><br />Theta sketches
use significantly more memory than HLL sketches, so you should prefer one of
the two HLL implementations.|`APPROX_COUNT_DISTINCT_BUILTIN`|
|`druid.sql.planner.enableSysQueriesTable`|**Experimental.** Whether to enable
the [`sys.queries` table](../querying/sql-metadata-tables.md#queries-table),
which provides information about currently running and recently completed SQL
queries. Currently only queries from the Dart (MSQ) engine are shown.|false|
+|`druid.sql.planner.maxPlanningTimeMs`|Maximum wall-clock time, in
milliseconds, allowed for planning a SQL query on the Broker (the phase
measured by the `sqlQuery/planningTimeMs` metric). When planning exceeds this
budget, the in-progress Calcite planning is aborted and the query fails with an
HTTP 504 `Query timeout` error. This guards the Broker against pathological
queries, such as one with a very large `IN` filter, whose planning time can
spike to tens of seconds and, under load, freeze the process. A value of `0`
disables the timeout. Can be overridden per query with the `maxPlanningTimeMs`
context parameter.|`0` (disabled)|
Review Comment:
I think there's no need to define a configuration for this one. because this
one is a query context level parameter, operators can use the broker side
dynamic configuration to update the query context default value gobally.
--
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]
