diff --git a/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java b/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java index 4a844ae9557ba..e540a1764e4cf 100644 --- a/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java +++ b/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java @@ -41,7 +41,6 @@ import io.trino.plugin.jdbc.LongReadFunction; import io.trino.plugin.jdbc.LongWriteFunction; import io.trino.plugin.jdbc.ObjectWriteFunction; -import io.trino.plugin.jdbc.PredicatePushdownController; import io.trino.plugin.jdbc.QueryBuilder; import io.trino.plugin.jdbc.RemoteTableName; import io.trino.plugin.jdbc.SliceWriteFunction; @@ -64,7 +63,6 @@ import io.trino.spi.connector.ColumnMetadata; import io.trino.spi.connector.ConnectorSession; import io.trino.spi.connector.ConnectorTableMetadata; -import io.trino.spi.predicate.Domain; import io.trino.spi.type.CharType; import io.trino.spi.type.DecimalType; import io.trino.spi.type.Decimals; @@ -128,7 +126,6 @@ import static io.trino.plugin.jdbc.DecimalSessionSessionProperties.getDecimalRounding; import static io.trino.plugin.jdbc.DecimalSessionSessionProperties.getDecimalRoundingMode; import static io.trino.plugin.jdbc.JdbcErrorCode.JDBC_ERROR; -import static io.trino.plugin.jdbc.JdbcMetadataSessionProperties.getDomainCompactionThreshold; import static io.trino.plugin.jdbc.PredicatePushdownController.DISABLE_PUSHDOWN; import static io.trino.plugin.jdbc.PredicatePushdownController.FULL_PUSHDOWN; import static io.trino.plugin.jdbc.StandardColumnMappings.bigintColumnMapping; @@ -206,20 +203,6 @@ public class ClickHouseClient public static final int DEFAULT_DOMAIN_COMPACTION_THRESHOLD = 1_000; - private static final PredicatePushdownController CLICKHOUSE_PUSHDOWN_CONTROLLER = (session, domain) -> { - if (domain.isOnlyNull()) { - return FULL_PUSHDOWN.apply(session, domain); - } - - Domain simplifiedDomain = domain.simplify(getDomainCompactionThreshold(session)); - if (!simplifiedDomain.getValues().isDiscreteSet()) { - // Domain#simplify can turn a discrete set into a range predicate - return DISABLE_PUSHDOWN.apply(session, domain); - } - - return FULL_PUSHDOWN.apply(session, simplifiedDomain); - }; - private final ConnectorExpressionRewriter connectorExpressionRewriter; private final AggregateFunctionRewriter aggregateFunctionRewriter; private final Type uuidType; @@ -247,8 +230,8 @@ public ClickHouseClient( ImmutableSet.>builder() .add(new ImplementCountAll(bigintTypeHandle)) .add(new ImplementCount(bigintTypeHandle)) - .add(new ImplementCountDistinct(bigintTypeHandle, false)) - .add(new ImplementMinMax(false)) // TODO: Revisit once https://github.com/trinodb/trino/issues/7100 is resolved + .add(new ImplementCountDistinct(bigintTypeHandle, true)) + .add(new ImplementMinMax(true)) .add(new ImplementSum(ClickHouseClient::toTypeHandle)) .add(new ImplementAvgFloatingPoint()) .add(new ImplementAvgBigint()) @@ -265,23 +248,9 @@ public Optional implementAggregation(ConnectorSession session, A return aggregateFunctionRewriter.rewrite(session, aggregate, assignments); } - @Override - public boolean supportsAggregationPushdown(ConnectorSession session, JdbcTableHandle table, List aggregates, Map assignments, List> groupingSets) - { - // 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. - return preventTextualTypeAggregationPushdown(groupingSets); - } - @Override public boolean supportsTopN(ConnectorSession session, JdbcTableHandle handle, List sortOrder) { - for (JdbcSortItem sortItem : sortOrder) { - Type sortItemType = sortItem.column().getColumnType(); - checkArgument(!(sortItemType instanceof CharType), "Unexpected char type: %s", sortItem.column().getColumnName()); - if (sortItemType instanceof VarcharType) { - return false; - } - } return true; } @@ -672,7 +641,7 @@ public Optional toColumnMapping(ConnectorSession session, Connect createUnboundedVarcharType(), varcharReadFunction(createUnboundedVarcharType()), varcharWriteFunction(), - CLICKHOUSE_PUSHDOWN_CONTROLLER)); + FULL_PUSHDOWN)); } return Optional.of(varbinaryColumnMapping()); case UUID: diff --git a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java b/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java index 18b204a9c7b39..dffd29bda90bf 100644 --- a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java +++ b/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java @@ -17,8 +17,6 @@ import com.google.common.collect.ImmutableMap; import io.trino.Session; import io.trino.plugin.jdbc.BaseJdbcConnectorTest; -import io.trino.plugin.jdbc.JdbcTableHandle; -import io.trino.spi.predicate.TupleDomain; import io.trino.sql.planner.plan.AggregationNode; import io.trino.sql.planner.plan.FilterNode; import io.trino.testing.MaterializedResult; @@ -47,8 +45,6 @@ import static io.trino.plugin.clickhouse.TestingClickHouseServer.CLICKHOUSE_LATEST_IMAGE; import static io.trino.plugin.jdbc.JdbcMetadataSessionProperties.DOMAIN_COMPACTION_THRESHOLD; import static io.trino.spi.type.VarcharType.VARCHAR; -import static io.trino.sql.planner.assertions.PlanMatchPattern.node; -import static io.trino.sql.planner.assertions.PlanMatchPattern.tableScan; import static io.trino.testing.MaterializedResult.resultBuilder; import static io.trino.testing.TestingNames.randomNameSuffix; import static java.lang.String.format; @@ -78,7 +74,6 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior) SUPPORTS_DELETE, SUPPORTS_DROP_NOT_NULL_CONSTRAINT, SUPPORTS_NEGATIVE_DATE, - SUPPORTS_PREDICATE_PUSHDOWN_WITH_VARCHAR_INEQUALITY, SUPPORTS_ROW_TYPE, SUPPORTS_SET_COLUMN_TYPE, SUPPORTS_UPDATE -> false; @@ -879,7 +874,7 @@ public void testTextualPredicatePushdown() // varchar range assertThat(query("SELECT regionkey, nationkey, name FROM nation WHERE name BETWEEN 'POLAND' AND 'RPA'")) .matches("VALUES (BIGINT '3', BIGINT '19', CAST('ROMANIA' AS varchar))") - .isNotFullyPushedDown(FilterNode.class); + .isFullyPushedDown(); // varchar IN without domain compaction assertThat(query("SELECT regionkey, nationkey, name FROM nation WHERE name IN ('POLAND', 'ROMANIA', 'VIETNAM')")) @@ -897,17 +892,10 @@ public void testTextualPredicatePushdown() .matches("VALUES " + "(BIGINT '3', BIGINT '19', CAST('ROMANIA' AS varchar)), " + "(BIGINT '2', BIGINT '21', CAST('VIETNAM' AS varchar))") - // Filter node is retained as no constraint is pushed into connector. + // Filter node is retained as constraint is pushed into connector is simplified, and // The compacted domain is a range predicate which can give wrong results - // if pushed down as ClickHouse has different sort ordering for letters from Trino - .isNotFullyPushedDown( - node( - FilterNode.class, - // verify that no constraint is applied by the connector - tableScan( - tableHandle -> ((JdbcTableHandle) tableHandle).getConstraint().isAll(), - TupleDomain.all(), - ImmutableMap.of()))); + // so has to be filtered by trino too to ensure correct predicate. + .isNotFullyPushedDown(FilterNode.class); // varchar different case assertThat(query("SELECT regionkey, nationkey, name FROM nation WHERE name = 'romania'"))