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

Add support for avg aggregation pushdown on bigint type in Snowflake #21148

Merged
merged 1 commit into from
Mar 22, 2024
Merged

Add support for avg aggregation pushdown on bigint type in Snowflake #21148

merged 1 commit into from
Mar 22, 2024

Conversation

chenjian2664
Copy link
Contributor

@chenjian2664 chenjian2664 commented Mar 19, 2024 •
edited by ebyhr
Loading

Release notes

(X) Release notes are required, with the following suggested text:

# Snowflake
* Add support for `avg` aggregation pushdown on `bigint` type.

@cla-bot cla-bot bot added the cla-signed label Mar 19, 2024
@chenjian2664 chenjian2664 marked this pull request as draft March 19, 2024 01:58
@ebyhr ebyhr added the no-release-notes This pull request does not require release notes entry label Mar 19, 2024
@ebyhr
Copy link
Member

ebyhr commented Mar 19, 2024

/test-with-secrets sha=ff0058d9f211d8ac8c670450eacc5878dfe97603

@github-actions GitHub Actions
Copy link

The CI workflow run with tests that require additional secrets has been started: https://github.com/trinodb/trino/actions/runs/8336732488

@ebyhr ebyhr changed the title Fix Snowflake aggregation pushdown Fix test for Snowflake aggregation pushdown Mar 19, 2024
@ebyhr
Copy link
Member

ebyhr commented Mar 19, 2024 •
edited
Loading

Please fix checkstyle violation.

Fix Snowflake aggregation pushdown

Also, the current commit title seems misleading. Please change to Enable test for Snowflake aggregation pushdown or something.

@ebyhr
Copy link
Member

ebyhr commented Mar 19, 2024

/test-with-secrets sha=18b2559f92f7d08572382f6756e249f0f2524788

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 19, 2024 •
edited
Loading

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

@ebyhr
Copy link
Member

ebyhr commented Mar 19, 2024

/test-with-secrets sha=18b2559f92f7d08572382f6756e249f0f2524788

@github-actions GitHub Actions
Copy link

The CI workflow run with tests that require additional secrets has been started: https://github.com/trinodb/trino/actions/runs/8342523680

@ebyhr
Copy link
Member

ebyhr commented Mar 19, 2024

/test-with-secrets sha=ae0f595057c6ab9ec94d2b40434453827bd8e5d5

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 19, 2024 •
edited
Loading

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

@chenjian2664 chenjian2664 self-assigned this Mar 21, 2024
@chenjian2664 chenjian2664 marked this pull request as ready for review March 21, 2024 08:52
@ebyhr
Copy link
Member

ebyhr commented Mar 22, 2024

/test-with-secrets sha=c94d35488e6457fc581a7034199e9fec14762088

@ebyhr ebyhr removed the no-release-notes This pull request does not require release notes entry label Mar 22, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 22, 2024 •
edited
Loading

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

@ebyhr ebyhr changed the title Fix test for Snowflake aggregation pushdown Add support for avg aggregation pushdown on bigint type in Snowflake Mar 22, 2024
@ebyhr
Copy link
Member

ebyhr commented Mar 22, 2024

Please fix CI failure. https://github.com/trinodb/trino/actions/runs/8384568924/job/22962120552

@ebyhr
Copy link
Member

ebyhr commented Mar 22, 2024

/test-with-secrets sha=767e9f24dfd10728f396ac5f9eec4a5ff224ec32

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 22, 2024 •
edited
Loading

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

@ebyhr
Copy link
Member

ebyhr commented Mar 22, 2024

Please fix CI failure. https://github.com/trinodb/trino/actions/runs/8385212710/job/22963774006

@ebyhr
Copy link
Member

ebyhr commented Mar 22, 2024

/test-with-secrets sha=e293844c28a47418525228cbd3ab8efc2865f602

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 22, 2024 •
edited
Loading

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

@ebyhr
Copy link
Member

ebyhr commented Mar 22, 2024

CI is still failing. https://github.com/trinodb/trino/actions/runs/8386772565/job/22967776829

@chenjian2664
Copy link
Contributor Author

chenjian2664 commented Mar 22, 2024 •
edited
Loading

I think maybe we need to open limit push down test first, currently the snowflake client implemented the limit pushdown, but the test is disabled. WDYT?
The second choice is that we remove the implementation for the limit pushdown part(SnowflakeClient#limitFunction, Snowflake#isLimitGuaranteed) in this pr.

@chenjian2664
Copy link
Contributor Author

chenjian2664 commented Mar 22, 2024 •
edited
Loading

Removed the connector behavior SUPPORTS_LIMIT_PUSHDOWN(change it to the default behavior, which is true) since the test is disabled but the client has it's implementation. Filed #21198 as the separate work

@ebyhr
Copy link
Member

ebyhr commented Mar 22, 2024

/test-with-secrets sha=0c1b7e94a88b7024b694a3fa032f4588d469a4f6

@github-actions GitHub Actions
Copy link

The CI workflow run with tests that require additional secrets has been started: https://github.com/trinodb/trino/actions/runs/8389683192

@chenjian2664 @ebyhr
Support pushdown avg function with bigint in Snowflake
5afaa2a 
Also, enable aggregation pushdown tests.

Co-Authored-By: Yuya Ebihara <[email protected]>
@ebyhr ebyhr merged commit fd12d10 into trinodb:master Mar 22, 2024
4 of 13 checks passed
@github-actions github-actions bot added this to the 444 milestone Mar 22, 2024
This was referenced Mar 22, 2024
Merged
Merged
@colebow colebow mentioned this pull request Mar 27, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebyhr ebyhr ebyhr left review comments

Assignees

@chenjian2664 chenjian2664

Labels
cla-signed
Milestone
444
Development

Successfully merging this pull request may close these issues.
2 participants
@chenjian2664 @ebyhr