Skip to content

Commit de098fe

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

3 files changed

Lines changed: 91 additions & 11 deletions

File tree

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

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,21 @@ 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+
* As {@link #runSimple(Query, AuthenticationResult, AuthorizationResult)}, but takes the user-provided context keys.
174+
* See {@link #initialize(Query, Set)}.
175+
*/
176+
public <T> QueryResponse<T> runSimple(
177+
final Query<T> query,
178+
final AuthenticationResult authenticationResult,
179+
final AuthorizationResult authorizationResult,
180+
@Nullable final Set<String> userProvidedContextKeys
181+
)
182+
{
183+
initialize(query, userProvidedContextKeys);
170184

171185
final Sequence<T> results;
172186

@@ -212,43 +226,64 @@ public void after(final boolean isDone, final Throwable thrown)
212226
* @throws DruidException if the current state is not NEW, which indicates a bug
213227
*/
214228
public void initialize(final Query<?> baseQuery)
229+
{
230+
initialize(baseQuery, null);
231+
}
232+
233+
/**
234+
* As {@link #initialize(Query)}, but takes the keys the caller set in the context. Pass {@code null} to treat the
235+
* whole context as caller-set (native queries). The SQL layer merges static defaults into the context, so it must
236+
* pass the real user-provided keys.
237+
*
238+
* @throws DruidException if the current state is not NEW, which indicates a bug
239+
*/
240+
public void initialize(final Query<?> baseQuery, @Nullable final Set<String> userProvidedContextKeys)
215241
{
216242
transition(State.NEW, State.INITIALIZED);
217243

218244
userContextKeys = new HashSet<>(baseQuery.getContext().keySet());
245+
final Set<String> effectiveUserKeys =
246+
userProvidedContextKeys != null ? userProvidedContextKeys : baseQuery.getContext().keySet();
247+
219248
String queryId = baseQuery.getId();
220249
if (Strings.isNullOrEmpty(queryId)) {
221250
queryId = UUID.randomUUID().toString();
222251
}
223252

224-
// Start with system defaults, apply per-datasource override, then user context wins
253+
// Precedence, high to low: user context > per-datasource per-segment timeout > static defaults > code default.
225254
Map<String, Object> contextWithDefaults = new HashMap<>(queryConfigProvider.getContext());
226-
applyPerDatasourcePerSegmentTimeout(baseQuery, contextWithDefaults, queryId);
227255
Map<String, Object> finalContext = QueryContexts.override(contextWithDefaults, baseQuery.getContext());
256+
applyPerDatasourcePerSegmentTimeout(baseQuery, finalContext, effectiveUserKeys, queryId);
228257
finalContext.put(BaseQuery.QUERY_ID, queryId);
229258

230259
this.baseQuery = baseQuery.withOverriddenContext(finalContext);
231260
this.toolChest = conglomerate.getToolChest(this.baseQuery);
232261
}
233262

234263
/**
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.
264+
* If a per-datasource per-segment timeout is configured, injects it into {@code finalContext}, overriding a value
265+
* merged in from static defaults. A perSegmentTimeout the caller set (per {@code userProvidedContextKeys}) is left
266+
* untouched. In monitorOnly mode, logs the configured timeout but does not inject it.
238267
*
239-
* For queries involving multiple datasources (e.g., joins or unions), the timeout from the first matching datasource is applied
240-
* since getTableNames() returns a Set, the match order is non-deterministic.
268+
* For multi-datasource queries (joins/unions), the first matching datasource wins; getTableNames() returns a Set, so
269+
* the match order is non-deterministic.
241270
*/
242271
private void applyPerDatasourcePerSegmentTimeout(
243272
final Query<?> query,
244-
final Map<String, Object> contextWithDefaults,
273+
final Map<String, Object> finalContext,
274+
final Set<String> userProvidedContextKeys,
245275
final String queryId
246276
)
247277
{
248278
if (perSegmentTimeoutConfig.isEmpty()) {
249279
return;
250280
}
251281

282+
// Caller set perSegmentTimeout: respect it.
283+
if (userProvidedContextKeys.contains(QueryContexts.PER_SEGMENT_TIMEOUT_KEY)) {
284+
return;
285+
}
286+
252287
for (String tableName : query.getDataSource().getTableNames()) {
253288
PerSegmentTimeoutConfig dsConfig = perSegmentTimeoutConfig.get(tableName);
254289
if (dsConfig != null) {
@@ -260,7 +295,7 @@ private void applyPerDatasourcePerSegmentTimeout(
260295
queryId
261296
);
262297
} else {
263-
contextWithDefaults.put(QueryContexts.PER_SEGMENT_TIMEOUT_KEY, dsConfig.getPerSegmentTimeoutMs());
298+
finalContext.put(QueryContexts.PER_SEGMENT_TIMEOUT_KEY, dsConfig.getPerSegmentTimeoutMs());
264299
}
265300
return;
266301
}

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

Lines changed: 42 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,47 @@ 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+
// SQL path: a static default is merged into the context but the caller did not set perSegmentTimeout, so the
203+
// dynamic config overrides it.
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+
lifecycle.initialize(queryWithMergedDefault, Collections.emptySet());
216+
217+
Assert.assertEquals(5000L, lifecycle.getQuery().context().getPerSegmentTimeout());
218+
}
219+
220+
@Test
221+
public void testPerDatasourceTimeout_userProvidedViaProvenance_userWins()
222+
{
223+
// Same context value, but the caller set perSegmentTimeout, so it wins over the dynamic config.
224+
Map<String, PerSegmentTimeoutConfig> config = Map.of(
225+
DATASOURCE, new PerSegmentTimeoutConfig(5000, false)
226+
);
227+
228+
expectDefaults();
229+
230+
TimeseriesQuery queryWithUserTimeout = baseQuery.withOverriddenContext(
231+
Map.of(QueryContexts.PER_SEGMENT_TIMEOUT_KEY, 2000L)
232+
);
233+
234+
QueryLifecycle lifecycle = createLifecycle(config);
235+
lifecycle.initialize(queryWithUserTimeout, Set.of(QueryContexts.PER_SEGMENT_TIMEOUT_KEY));
236+
237+
Assert.assertEquals(2000L, lifecycle.getQuery().context().getPerSegmentTimeout());
238+
}
239+
198240
private void expectDefaults()
199241
{
200242
EasyMock.expect(queryConfig.getContext()).andReturn(Map.of()).anyTimes();

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,10 @@ private <T> QueryResponse<Object[]> execute(
186186
final QueryResponse<T> results = queryLifecycle.runSimple(
187187
(Query<T>) query,
188188
authenticationResult,
189-
authorizationResult
189+
authorizationResult,
190+
// SQL merges static defaults into the context; pass the user-set keys so dynamic config can override a
191+
// merged-in default but not a value the caller set.
192+
plannerContext.authContextKeys()
190193
);
191194

192195
return mapResultSequence(

0 commit comments

Comments
 (0)