Skip to content

MSQ window functions: Minor cleanup for empty over clause related flows + Exhaustive tests#16754

Merged
cryptoe merged 4 commits intoapache:masterfrom
Akshat-Jain:fix-msq-wf-empty-over
Jul 23, 2024
Merged

MSQ window functions: Minor cleanup for empty over clause related flows + Exhaustive tests#16754
cryptoe merged 4 commits intoapache:masterfrom
Akshat-Jain:fix-msq-wf-empty-over

Conversation

@Akshat-Jain
Copy link
Copy Markdown
Contributor

@Akshat-Jain Akshat-Jain commented Jul 18, 2024 •
edited
Loading

Description

This PR does minor refactoring/cleanup for empty over clause related flows for MSQ window functions.

This PR makes the following changes:

  • Get rid of unnecessary isEmptyOver boolean in favor of partitionColumnNames
  • Change logic of populating operator list to club window functions together in the same stage.
  • Add a bunch of test cases for queries having empty over(), as well as a mix of empty and non-empty over() clauses - which run for both sql-native and MSQ engine.
Key changed/added classes in this PR
  • WindowOperatorQueryFrameProcessor
  • WindowOperatorQueryFrameProcessorFactory
  • WindowOperatorQueryKit

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • 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 code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

@github-actions github-actions Bot added Area - Batch Ingestion Area - Querying Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 labels Jul 18, 2024
@Akshat-Jain Akshat-Jain changed the title MSQ window functions: Revamp logic to create separate window stages when empty over() clause is present MSQ window functions: Minor cleanup for empty over clause related flows + Exhaustive tests Jul 18, 2024
@Akshat-Jain Akshat-Jain requested a review from kgyrtkirk July 18, 2024 14:46
@Akshat-Jain Akshat-Jain mentioned this pull request Jul 19, 2024
Merged
9 tasks
kgyrtkirk
kgyrtkirk reviewed Jul 22, 2024
View reviewed changes
Comment thread ...age-query/src/main/java/org/apache/druid/msq/querykit/WindowOperatorQueryFrameProcessor.java Outdated
Comment thread ...re/multi-stage-query/src/main/java/org/apache/druid/msq/querykit/WindowOperatorQueryKit.java Outdated
@Akshat-Jain Akshat-Jain requested a review from kgyrtkirk July 22, 2024 15:05
@cryptoe cryptoe merged commit c45d4fd into apache:master Jul 23, 2024
sreemanamala pushed a commit to sreemanamala/druid that referenced this pull request Aug 6, 2024
@Akshat-Jain @sreemanamala
MSQ window functions: Minor cleanup for empty over clause related flo…
cb44a39 
…ws + Exhaustive tests (apache#16754)

* MSQ window functions: Revamp logic to create separate window stages when empty over() clause is present

* Fix tests

* Revert changes of creating separate stages for empty over clause

* Address review comments
@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

Reviewers

@kgyrtkirk kgyrtkirk kgyrtkirk approved these changes

@cryptoe cryptoe cryptoe approved these changes

Assignees

No one assigned

Labels

Area - Batch Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 Area - Querying

Projects

None yet

Milestone

31.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@Akshat-Jain @kgyrtkirk @cryptoe @kfaraz