More predicate pushdown in ClickHouse connector

[findepi]

See TODOs in the code referring to this issue

These include (not an exhaustive list):

• Predicate pushdown for DECIMAL columns
• Predicate pushdown for textual types: Enable varchar equality predicate pushdown for ClickHouse connector #21557

[JamesRTaylor]

@findepi - would it be possible to elaborate on what "more pushdown" means? It's not clear which TODOs you mean. Ideally, if we could break this down into specific pushdown improvements that would correspond to individual PRs.

[hashhar]

@JamesRTaylor most of it is regarding untested code (pushdown for text types) and decimal predicates

A quick grep gives me:

$ grep -R -n -E 'TODO.*7100' plugin/trino-clickhouse
plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java:409:    // TODO: Remove override once decimal predicate pushdown is implemented (https://github.com/trinodb/trino/issues/7100)
plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java:176:                        .add(new ImplementMinMax(false)) // TODO: Revisit once https://github.com/trinodb/trino/issues/7100 is resolved
plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java:193:        // TODO: Remove override once https://github.com/trinodb/trino/issues/7100 is resolved. Currently pushdown for textual types is not tested and may lead to incorrect results.
plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java:419:                        // TODO (https://github.com/trinodb/trino/issues/7100) Currently pushdown would not work and may require a custom bind expression
plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java:431:                // TODO (https://github.com/trinodb/trino/issues/7100) test & enable predicate pushdown
plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java:479:                        // TODO (https://github.com/trinodb/trino/issues/7100) fix, enable and test decimal pushdown

I've edited the issue description accordingly to mention some of these.

[JamesRTaylor]

Is pushdown for TIMESTAMP types complete? See here for more context.

[ebyhr]

@JamesRTaylor Predicate pushdown happens on DateTime type, but it doesn't happen on other types (e.g. Datetime64) at this time.

[mathsteam]

Hi, any news for predicate pushdown for textual types?
What could be the effects of making textual types as FULL_PUSHDOWN?

[hashhar]

@mathsteam Tests will need to be added to verify the effect and fix any failures accordingly.

See TestPostgreSqlTypeMapping and PostgreSqlConnectorTest#testPredicatePushdown as some examples of what to add to ClickHouse.

[tangjiangling]

Let me do this.

[chenjian2664]

@tangjiangling Are you still working on textual types pushdown on ClickHouse? If not, I would like to take this

[tangjiangling]

Not at the moment, you can take over.

[klimmilk]

@tangjiangling Are you still working on textual types pushdown on ClickHouse? If not, I would like to take this

Any progress on textual types pushdown?
This feature can be also replaced with raw query support, like here https://trino.io/docs/current/connector/postgresql.html#query-varchar-table

[style1002]

Add ClickHouse automatic cost based JOIN pushdown support #18380
Have any suggestions for #18380

[sylph-eu]

@ebyhr, could you point me what needs to be checked to guarantee correctness of enabling predicate pushdown for textual types (when ClickHouse String is mapped to VARCHAR)?

It's clear how to check that expressions were actually pushed-down (for this I can refer to PostgreSqlConnectorTest#testPredicatePushdown), plus I see examples from TestPostgreSqlTypeMapping to make sure that types are correctly mapped. Given that ClickHouse doesn't support collation, anything else needs to be checked?