Skip to content

remove isDescending from Query interface, move to TimeseriesQuery#16917

Merged
clintropolis merged 2 commits intoapache:masterfrom
clintropolis:move-query-descending-to-timeseries
Aug 20, 2024
Merged

remove isDescending from Query interface, move to TimeseriesQuery#16917
clintropolis merged 2 commits intoapache:masterfrom
clintropolis:move-query-descending-to-timeseries

Conversation

@clintropolis
Copy link
Copy Markdown
Member

Description

Follow-up to #16533, Query.isDescending() can only be set by TimeseriesQuery, everything else hard-coded this parameter to the value false, and it is only used by TimeseriesQuery to control the direction of time ordering in results.

Release note

(for developers)

Query.isDescending has been moved from the base Query interface to TimeseriesQuery since it was the only built-in query type which could set or use this parameter. If you have developed a custom query type or query engine extension that utilized this method on the Query interface, it will need to be moved to the custom query type, replaced with false if referencing any built-in query type that is not timeseries, or handle the query as a TimeseriesQuery directly if it is referencing a timeseries query.

Copy link
Copy Markdown
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@clintropolis clintropolis merged commit 518f642 into apache:master Aug 20, 2024
@clintropolis clintropolis deleted the move-query-descending-to-timeseries branch August 20, 2024 06:02
hevansDev pushed a commit to hevansDev/druid that referenced this pull request Aug 29, 2024
…ache#16917)

* remove isDescending from Query interface, since it is only actually settable and usable by TimeseriesQuery
MDC.put("hasFilters", Boolean.toString(query.hasFilters()));
MDC.put("remoteAddr", requestLogLine.getRemoteAddr());
MDC.put("duration", query.getDuration().toString());
MDC.put("descending", Boolean.toString(query.isDescending()));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we want to log this if the query is TImeseriesQuery?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, i guess we could, not sure how interesting it really is though which is why i removed it

@kfaraz kfaraz added this to the 31.0.0 milestone Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants