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.

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

Enable full predicate pushdown for decimal type in ClickHouse connector #21659

Closed
wants to merge 1 commit into from

Conversation

sylph-eu
Copy link
Contributor

@sylph-eu sylph-eu commented Apr 22, 2024

Description

Partially resolves #7100.

This PR reworks decimal type support by ClickHouse connector and makes implementation similar to the one by e.g. MySQL or Postgres connectors. As the result it enables full predicate pushdown.

Changes:

  • Columns are ignored (unless it's configured to treat it as varchar) if precision exceeds maximum value (38).
  • ClickHouse doesn't support negative scale, however it's coded nonetheless.

It's worth noting that ClickHouse implements Decimal in a slightly nuanced way (link):

  1. Excessive digits in a fraction are discarded (not rounded). Excessive digits in integer part will lead to an exception. There's a unit test for that.
  2. Overflow check is not implemented for Decimal128 and Decimal256. In case of overflow incorrect result is returned, no exception is thrown. Trino supports precision up to 38 digits which is therefore implemented by Decimal128.

Additional context and related issues

Please review previous PRs related to ClickHouse connector as the number of changes keep accumulating.

Next I'm going to focus predicate pushdown for time types and on aggregate functions.

Release notes

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

# Section
* Enable decimal full predicate pushdown for ClickHouse connector. ({issue}`7100`)

Copy link

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

@github-actions github-actions bot added the stale label May 13, 2024
@ebyhr ebyhr removed their request for review May 20, 2024 04:13
@github-actions github-actions bot removed the stale label May 20, 2024
Copy link

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

@github-actions github-actions bot added the stale label Jun 12, 2024
Copy link

github-actions bot commented Jul 4, 2024

Closing this pull request, as it has been stale for six weeks. Feel free to re-open at any time.

@github-actions github-actions bot closed this Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

More predicate pushdown in ClickHouse connector
1 participant