Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support sum(distinct) pushdown for jdbc connectors #16460

Merged
merged 1 commit into from
Apr 5, 2023
Merged

Support sum(distinct) pushdown for jdbc connectors #16460

merged 1 commit into from
Apr 5, 2023

Conversation

chenjian2664
Copy link
Contributor

@chenjian2664 chenjian2664 commented Mar 9, 2023 •
edited
Loading

Description

Fix #16452

Main changes:

  • Update the ImplementSum to support both sum(x) and sum(DISTINCT x)
  • Added test for sum(DISTINCT x) pushdown in BaseJdbcConnectorTest, I added those tests closes to the SUPPORTS_AGGREGATION_PUSHDOWN_COUNT_DISTINCT tests.
  • Adjust some connectors client tests.

Additional context and related issues

Release notes

( ) This is not user-visible or docs only and no release notes are required.
(x) Release notes are required, please propose a release note for me.
() Release notes are required, with the following suggested text:

# Section
* Support sum(distinct) pushdown for jdbc connectors ({issue}`16452`)

@cla-bot cla-bot bot added the cla-signed label Mar 9, 2023
@chenjian2664 chenjian2664 marked this pull request as draft March 9, 2023 05:27
@chenjian2664 chenjian2664 self-assigned this Mar 13, 2023
@chenjian2664 chenjian2664 added the enhancement New feature or request label Mar 13, 2023
@chenjian2664 chenjian2664 marked this pull request as ready for review March 27, 2023 00:52
@chenjian2664 chenjian2664 requested review from findepi and hashhar March 27, 2023 00:52
hashhar
hashhar reviewed Mar 31, 2023
View reviewed changes
Copy link
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

some comments, looks good otherwise

@hashhar

This comment was marked as outdated.

Sign in to view
@hashhar
Copy link
Member

hashhar commented Mar 31, 2023

/test-with-secrets sha=3f8c31e76b858e0eb356bcb37fdb88060b9bf854

@github-actions
Copy link

github-actions bot commented Apr 1, 2023

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/4578289581

@chenjian2664 chenjian2664 requested a review from hashhar April 3, 2023 07:55
@kokosing kokosing merged commit e7d560f into trinodb:master Apr 5, 2023
@kokosing
Copy link
Member

kokosing commented Apr 5, 2023

Thanks!

@github-actions github-actions bot added this to the 412 milestone Apr 5, 2023
@colebow colebow mentioned this pull request Apr 5, 2023
Merged
@chenjian2664 chenjian2664 deleted the sum_distinct branch April 7, 2023 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@findepi findepi findepi approved these changes

@kokosing kokosing kokosing approved these changes

@hashhar hashhar Awaiting requested review from hashhar

Assignees

@chenjian2664 chenjian2664

Labels
cla-signed enhancement New feature or request
Milestone
412
Development

Successfully merging this pull request may close these issues.

Support sum(DISTINCT) pushdown for jdbc connectors
4 participants
@chenjian2664 @hashhar @kokosing @findepi