Skip to content

Commit 1b93d75

Browse files
committed
fix: fix query context precedence layer
1 parent 4e51761 commit 1b93d75

3 files changed

Lines changed: 106 additions & 9 deletions

File tree

server/src/main/java/org/apache/druid/server/QueryLifecycle.java

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,24 @@ public <T> QueryResponse<T> runSimple(
166166
final AuthorizationResult authorizationResult
167167
)
168168
{
169-
initialize(query);
169+
return runSimple(query, authenticationResult, authorizationResult, null);
170+
}
171+
172+
/**
173+
* Variant of {@link #runSimple(Query, AuthenticationResult, AuthorizationResult)} that also accepts the set of
174+
* context keys the caller genuinely supplied (as opposed to values merged in from static/system defaults). The SQL
175+
* layer merges static defaults into the query context before it reaches this class, so it must pass the real
176+
* user-provided keys; otherwise a merged-in default is indistinguishable from a user-supplied value. See
177+
* {@link #initialize(Query, Set)}.
178+
*/
179+
public <T> QueryResponse<T> runSimple(
180+
final Query<T> query,
181+
final AuthenticationResult authenticationResult,
182+
final AuthorizationResult authorizationResult,
183+
@Nullable final Set<String> userProvidedContextKeys
184+
)
185+
{
186+
initialize(query, userProvidedContextKeys);
170187

171188
final Sequence<T> results;
172189

@@ -212,43 +229,76 @@ public void after(final boolean isDone, final Throwable thrown)
212229
* @throws DruidException if the current state is not NEW, which indicates a bug
213230
*/
214231
public void initialize(final Query<?> baseQuery)
232+
{
233+
initialize(baseQuery, null);
234+
}
235+
236+
/**
237+
* Initializes this object to execute a specific query. Does not actually execute the query.
238+
* <p>
239+
* The {@code state} transitions from NEW, to INITIALIZED.
240+
*
241+
* @param baseQuery the query
242+
* @param userProvidedContextKeys the context keys the caller genuinely supplied, or {@code null} to treat the whole
243+
* query context as caller-supplied. Native queries can pass {@code null} since their
244+
* context is exactly the caller's context. The SQL layer merges static/system defaults
245+
* into the query context upstream, so it must pass the real user-provided keys so a
246+
* merged-in default is not mistaken for an explicit user value.
247+
* @throws DruidException if the current state is not NEW, which indicates a bug
248+
*/
249+
public void initialize(final Query<?> baseQuery, @Nullable final Set<String> userProvidedContextKeys)
215250
{
216251
transition(State.NEW, State.INITIALIZED);
217252

218253
userContextKeys = new HashSet<>(baseQuery.getContext().keySet());
254+
// For native queries the query context is exactly the caller's context, so its key set is authoritative. For SQL
255+
// queries, static/system defaults are merged into the query context upstream, so the caller passes the real
256+
// user-provided keys (otherwise a merged-in default looks user-supplied).
257+
final Set<String> effectiveUserKeys =
258+
userProvidedContextKeys != null ? userProvidedContextKeys : baseQuery.getContext().keySet();
259+
219260
String queryId = baseQuery.getId();
220261
if (Strings.isNullOrEmpty(queryId)) {
221262
queryId = UUID.randomUUID().toString();
222263
}
223264

224-
// Start with system defaults, apply per-datasource override, then user context wins
265+
// Precedence, highest to lowest: user-supplied context > per-datasource per-segment timeout dynamic config >
266+
// static/system defaults > code default. User context is merged last so it wins; the per-datasource timeout is then
267+
// applied over the merged result, but only when the user did not supply perSegmentTimeout themselves.
225268
Map<String, Object> contextWithDefaults = new HashMap<>(queryConfigProvider.getContext());
226-
applyPerDatasourcePerSegmentTimeout(baseQuery, contextWithDefaults, queryId);
227269
Map<String, Object> finalContext = QueryContexts.override(contextWithDefaults, baseQuery.getContext());
270+
applyPerDatasourcePerSegmentTimeout(baseQuery, finalContext, effectiveUserKeys, queryId);
228271
finalContext.put(BaseQuery.QUERY_ID, queryId);
229272

230273
this.baseQuery = baseQuery.withOverriddenContext(finalContext);
231274
this.toolChest = conglomerate.getToolChest(this.baseQuery);
232275
}
233276

234277
/**
235-
* If a per-datasource per-segment timeout is configured, injects it into the context defaults.
236-
* User context (applied later via {@link QueryContexts#override}) will override this if set explicitly.
237-
* In monitorOnly mode, logs the configured timeout but does not inject it.
278+
* If a per-datasource per-segment timeout is configured, injects it into {@code finalContext}, overriding any value
279+
* that was merged in from static/system defaults. A perSegmentTimeout the caller supplied explicitly wins and is left
280+
* untouched (identified via {@code userProvidedContextKeys}). In monitorOnly mode, logs the configured timeout but
281+
* does not inject it.
238282
*
239283
* For queries involving multiple datasources (e.g., joins or unions), the timeout from the first matching datasource is applied
240284
* since getTableNames() returns a Set, the match order is non-deterministic.
241285
*/
242286
private void applyPerDatasourcePerSegmentTimeout(
243287
final Query<?> query,
244-
final Map<String, Object> contextWithDefaults,
288+
final Map<String, Object> finalContext,
289+
final Set<String> userProvidedContextKeys,
245290
final String queryId
246291
)
247292
{
248293
if (perSegmentTimeoutConfig.isEmpty()) {
249294
return;
250295
}
251296

297+
// The caller explicitly set perSegmentTimeout: respect it (highest precedence).
298+
if (userProvidedContextKeys.contains(QueryContexts.PER_SEGMENT_TIMEOUT_KEY)) {
299+
return;
300+
}
301+
252302
for (String tableName : query.getDataSource().getTableNames()) {
253303
PerSegmentTimeoutConfig dsConfig = perSegmentTimeoutConfig.get(tableName);
254304
if (dsConfig != null) {
@@ -260,7 +310,7 @@ private void applyPerDatasourcePerSegmentTimeout(
260310
queryId
261311
);
262312
} else {
263-
contextWithDefaults.put(QueryContexts.PER_SEGMENT_TIMEOUT_KEY, dsConfig.getPerSegmentTimeoutMs());
313+
finalContext.put(QueryContexts.PER_SEGMENT_TIMEOUT_KEY, dsConfig.getPerSegmentTimeoutMs());
264314
}
265315
return;
266316
}

server/src/test/java/org/apache/druid/server/PerSegmentTimeoutInjectionTest.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import java.util.Collections;
4545
import java.util.List;
4646
import java.util.Map;
47+
import java.util.Set;
4748

4849
public class PerSegmentTimeoutInjectionTest
4950
{
@@ -195,6 +196,48 @@ DATASOURCE, new PerSegmentTimeoutConfig(5000, false)
195196
Assert.assertEquals(2000L, lifecycle.getQuery().context().getPerSegmentTimeout());
196197
}
197198

199+
@Test
200+
public void testPerDatasourceTimeout_staticDefaultInContextNotUserProvided_dynamicWins()
201+
{
202+
// Simulates the SQL path: a static/system default was merged into the query context upstream, but the user did not
203+
// actually set perSegmentTimeout. The per-datasource dynamic config must override the merged-in default.
204+
Map<String, PerSegmentTimeoutConfig> config = Map.of(
205+
DATASOURCE, new PerSegmentTimeoutConfig(5000, false)
206+
);
207+
208+
expectDefaults();
209+
210+
TimeseriesQuery queryWithMergedDefault = baseQuery.withOverriddenContext(
211+
Map.of(QueryContexts.PER_SEGMENT_TIMEOUT_KEY, "0")
212+
);
213+
214+
QueryLifecycle lifecycle = createLifecycle(config);
215+
// Empty user-provided key set: perSegmentTimeout in the context came from a default, not the caller.
216+
lifecycle.initialize(queryWithMergedDefault, Collections.emptySet());
217+
218+
Assert.assertEquals(5000L, lifecycle.getQuery().context().getPerSegmentTimeout());
219+
}
220+
221+
@Test
222+
public void testPerDatasourceTimeout_userProvidedViaProvenance_userWins()
223+
{
224+
// Same context value as above, but the caller genuinely set perSegmentTimeout; it must win over the dynamic config.
225+
Map<String, PerSegmentTimeoutConfig> config = Map.of(
226+
DATASOURCE, new PerSegmentTimeoutConfig(5000, false)
227+
);
228+
229+
expectDefaults();
230+
231+
TimeseriesQuery queryWithUserTimeout = baseQuery.withOverriddenContext(
232+
Map.of(QueryContexts.PER_SEGMENT_TIMEOUT_KEY, 2000L)
233+
);
234+
235+
QueryLifecycle lifecycle = createLifecycle(config);
236+
lifecycle.initialize(queryWithUserTimeout, Set.of(QueryContexts.PER_SEGMENT_TIMEOUT_KEY));
237+
238+
Assert.assertEquals(2000L, lifecycle.getQuery().context().getPerSegmentTimeout());
239+
}
240+
198241
private void expectDefaults()
199242
{
200243
EasyMock.expect(queryConfig.getContext()).andReturn(Map.of()).anyTimes();

sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,11 @@ private <T> QueryResponse<Object[]> execute(
186186
final QueryResponse<T> results = queryLifecycle.runSimple(
187187
(Query<T>) query,
188188
authenticationResult,
189-
authorizationResult
189+
authorizationResult,
190+
// The SQL layer merges static/system defaults into the query context; pass the genuinely user-provided keys so
191+
// per-datasource per-segment timeout config can override a merged-in default while still yielding to an
192+
// explicit user value.
193+
plannerContext.authContextKeys()
190194
);
191195

192196
return mapResultSequence(

0 commit comments

Comments
 (0)