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 varchar equality predicate pushdown for ClickHouse connector #21557

Merged
merged 3 commits into from
May 21, 2024

Conversation

sylph-eu
Copy link
Contributor

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

Description

This PR depends partially addresses #7100 (only textual types for now).

Type mapping unit tests were extended with more cases. In addition to Type and Nullable(Type) I'm testing whether LowCardinality modifier causes issues. In the past ClickHouse had some incompatibilities between regular and LowCardinality types, therefore it's worth putting them in the unit tests.

Only equality predicate is enabled for now as ClickHouse does not support collation. Respective unit tests were put into TestClickHouseConnectorTest.testTextualPredicatePushdown.

Additional context and related issues

I hope to continue working on #7100 to support pushdown for more more types (enums, decimals, etc).

Release notes

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

# ClickHouse
* Improve performance when pushing down equality predicate on textual types. ({issue}`7100`)

Copy link

cla-bot bot commented Apr 15, 2024

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@sylph-eu
Copy link
Contributor Author

Rebased.

@sylph-eu sylph-eu force-pushed the clickhouse_connector_2 branch 2 times, most recently from 48d7cd9 to 72ce208 Compare April 23, 2024 09:58
@sylph-eu sylph-eu force-pushed the clickhouse_connector_2 branch 2 times, most recently from 4681ef7 to c15a930 Compare April 29, 2024 08:19
@sylph-eu sylph-eu force-pushed the clickhouse_connector_2 branch from c15a930 to 36db7fa Compare May 7, 2024 09:11
@sylph-eu
Copy link
Contributor Author

sylph-eu commented May 7, 2024

Rebased

@nileshsinha1993
Copy link

nileshsinha1993 commented May 17, 2024

Hi @sylph-eu when are we planning to release this PR? Trino version.

@sylph-eu
Copy link
Contributor Author

Hi @sylph-eu when are we planning to release this PR? Trino version.

I'm not in control of merging/releasing this PR, I'm a side contributor. It's waiting for @ebyhr to complete the review and merge it. This, in turn, will unblock subsequent ones (please see #7100 for the current list).

@sylph-eu sylph-eu force-pushed the clickhouse_connector_2 branch from 36db7fa to 2ac3862 Compare May 17, 2024 07:40
@sylph-eu
Copy link
Contributor Author

Rebased once more.

@bhargav2427
Copy link

Hello @ebyhr, this is very good feature and our team is eagerly waiting for it. It will be really good if you can check.

@sylph-eu sylph-eu force-pushed the clickhouse_connector_2 branch 2 times, most recently from 65c976a to 105ef68 Compare May 20, 2024 08:44
@ebyhr ebyhr force-pushed the clickhouse_connector_2 branch from 105ef68 to cc8cea6 Compare May 21, 2024 01:44
@ebyhr ebyhr force-pushed the clickhouse_connector_2 branch from cc8cea6 to ba35000 Compare May 21, 2024 01:47
@ebyhr ebyhr merged commit d035094 into trinodb:master May 21, 2024
22 checks passed
@github-actions github-actions bot added this to the 449 milestone May 21, 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.

4 participants