druid icon indicating copy to clipboard operation
druid copied to clipboard

Remove redundant TopNQueryConfig#minTopNThreshold

Open zhan7236 opened this issue 1 month ago • 0 comments

Description

This PR removes the redundant minTopNThreshold field from TopNQueryConfig as requested in #16817.

The minTopNThreshold setting can already be overridden by the query context variable minTopNThreshold, making the config-based approach redundant. The default value (1000) is now available as a constant TopNQueryConfig.DEFAULT_MIN_TOPN_THRESHOLD.

Changes

Core Changes

  • Removed minTopNThreshold field and getMinTopNThreshold() method from TopNQueryConfig
  • Updated TopNQueryQueryToolChest.ThresholdAdjustingQueryRunner to use the default constant directly when query context doesn't specify the value

Test Framework Changes

  • Removed @MinTopNThreshold annotation from SqlTestFrameworkConfig
  • Updated SqlTestFramework to remove minTopNThreshold builder method
  • Updated tests in CalciteJoinQueryTest to use query context (QueryContexts.MIN_TOP_N_THRESHOLD) instead of the annotation
  • Updated QueryStackTests methods to deprecate the minTopNThreshold parameter (kept for backward compatibility)
  • Updated quidem test file to use !set minTopNThreshold 1 instead of URL parameter

Documentation Updates

  • Updated docs/configuration/index.md to mark druid.query.topN.minTopNThreshold as deprecated
  • Updated docs/querying/topnquery.md to remove the server parameter reference

Release Note

Deprecated: The server configuration druid.query.topN.minTopNThreshold is now deprecated. Use the minTopNThreshold query context parameter instead to control the TopN threshold per query.

This PR has:

  • [x] been self-reviewed
  • [x] added documentation for new or modified features or behaviors
  • [ ] a release note entry in the PR description (for user-impacting changes)
  • [ ] added Javadocs for most classes and all non-trivial methods
  • [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader
  • [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for)}coverage is met
  • [ ] been tested in a test Druid cluster

Fixes #16817

zhan7236 avatar Nov 29 '25 15:11 zhan7236