Remove redundant TopNQueryConfig#minTopNThreshold #18790
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This removes the minTopNThreshold config from TopNQueryConfig as it can be overridden by the query context variable. The default value is still available as a constant TopNQueryConfig.DEFAULT_MIN_TOPN_THRESHOLD. Changes: - Remove minTopNThreshold field and getter from TopNQueryConfig - Update TopNQueryQueryToolChest to use the default constant directly - Remove @MinTopNThreshold annotation from SqlTestFrameworkConfig - Update tests to use query context for setting minTopNThreshold - Deprecate minTopNThreshold parameter in QueryStackTests methods - Update documentation to reflect the change Fixes apache#16817
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @Deprecated | ||
| public static QueryRunnerFactoryConglomerate createQueryRunnerFactoryConglomerate( | ||
| final Closer closer, | ||
| final Integer minTopNThreshold |
Check notice
Code scanning / CodeQL
Useless parameter Note test
| @Deprecated | ||
| public static QueryRunnerFactoryConglomerate createQueryRunnerFactoryConglomerate( | ||
| final DruidProcessingConfig processingConfig, | ||
| final Integer minTopNThreshold, |
Check notice
Code scanning / CodeQL
Useless parameter Note test
- Update quidem test file CRC hash to match new test code - Add @SuppressWarnings("unused") to deprecated methods in QueryStackTests.java to address security bot warnings about unused minTopNThreshold parameters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR removes the redundant
minTopNThresholdfield fromTopNQueryConfigas requested in #16817.The
minTopNThresholdsetting can already be overridden by the query context variableminTopNThreshold, making the config-based approach redundant. The default value (1000) is now available as a constantTopNQueryConfig.DEFAULT_MIN_TOPN_THRESHOLD.Changes
Core Changes
minTopNThresholdfield andgetMinTopNThreshold()method fromTopNQueryConfigTopNQueryQueryToolChest.ThresholdAdjustingQueryRunnerto use the default constant directly when query context doesn't specify the valueTest Framework Changes
@MinTopNThresholdannotation fromSqlTestFrameworkConfigSqlTestFrameworkto removeminTopNThresholdbuilder methodCalciteJoinQueryTestto use query context (QueryContexts.MIN_TOP_N_THRESHOLD) instead of the annotationQueryStackTestsmethods to deprecate theminTopNThresholdparameter (kept for backward compatibility)!set minTopNThreshold 1instead of URL parameterDocumentation Updates
docs/configuration/index.mdto markdruid.query.topN.minTopNThresholdas deprecateddocs/querying/topnquery.mdto remove the server parameter referenceRelease Note
Deprecated: The server configuration
druid.query.topN.minTopNThresholdis now deprecated. Use theminTopNThresholdquery context parameter instead to control the TopN threshold per query.This PR has:
Fixes #16817