Skip to content

Minor: Move median test#10611

Merged
alamb merged 2 commits intoapache:mainfrom
jayzhan211:median-test
May 23, 2024
Merged

Minor: Move median test#10611
alamb merged 2 commits intoapache:mainfrom
jayzhan211:median-test

Conversation

@jayzhan211
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Part of #10384 .

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
@github-actions github-actions bot added physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt) labels May 22, 2024
@jayzhan211 jayzhan211 marked this pull request as ready for review May 23, 2024 05:07
Copy link
Copy Markdown
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you for this PR @jayzhan211

I think the actual values and their order is important in these tests as it covers various corner cases and thus the new tests in sql I think may not retain the same coverage

Comment thread datafusion/physical-expr/src/aggregate/median.rs
Comment thread datafusion/physical-expr/src/aggregate/median.rs
@jayzhan211 jayzhan211 marked this pull request as draft May 23, 2024 13:02
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
@jayzhan211 jayzhan211 marked this pull request as ready for review May 23, 2024 13:57
@jayzhan211 jayzhan211 requested a review from alamb May 23, 2024 13:58
Copy link
Copy Markdown
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

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

lgtm thanks @jayzhan211
Extra kudos for adding comments to the tests

Copy link
Copy Markdown
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @jayzhan211

@alamb alamb merged commit c75a957 into apache:main May 23, 2024
findepi pushed a commit to findepi/datafusion that referenced this pull request Jul 16, 2024
* move median test

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

* add more test

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

---------

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants