diff --git a/docs/appendices/release-notes/6.6.0.rst b/docs/appendices/release-notes/6.6.0.rst index a54aad31c349..268c403d052e 100644 --- a/docs/appendices/release-notes/6.6.0.rst +++ b/docs/appendices/release-notes/6.6.0.rst @@ -109,6 +109,11 @@ Performance and Resilience Improvements Previously, the resulting logical plan would contain 1 inner join and 2 cross joins. Now, the logical plan has 2 inner joins and 1 cross join. +- Optimized queries with ``DISTINCT`` when the grouping columns include all + columns of an explicit primary key. In these cases, we can skip + unnecessary duplicate elimination because the primary key guarantees that + each row is unique. + Administration and Operations ----------------------------- diff --git a/server/src/main/java/io/crate/planner/operators/HashAggregate.java b/server/src/main/java/io/crate/planner/operators/HashAggregate.java index b87a261e435d..329b5eda16ec 100644 --- a/server/src/main/java/io/crate/planner/operators/HashAggregate.java +++ b/server/src/main/java/io/crate/planner/operators/HashAggregate.java @@ -194,6 +194,10 @@ public List aggregates() { return aggregates; } + public boolean rewriteDistinct() { + return rewriteDistinct; + } + @Override public List outputs() { return new ArrayList<>(aggregates); diff --git a/server/src/main/java/io/crate/planner/operators/LogicalPlanner.java b/server/src/main/java/io/crate/planner/operators/LogicalPlanner.java index 1feb15e9c754..80330f5d3832 100644 --- a/server/src/main/java/io/crate/planner/operators/LogicalPlanner.java +++ b/server/src/main/java/io/crate/planner/operators/LogicalPlanner.java @@ -128,6 +128,7 @@ import io.crate.planner.optimizer.rule.MoveOrderBeneathUnion; import io.crate.planner.optimizer.rule.OptimizeCollectWhereClauseAccess; import io.crate.planner.optimizer.rule.RemoveOrderBeneathInsert; +import io.crate.planner.optimizer.rule.RemoveRedundantDistinct; import io.crate.planner.optimizer.rule.RemoveRedundantEval; import io.crate.planner.optimizer.rule.ReorderHashJoin; import io.crate.planner.optimizer.rule.ReorderNestedLoopJoin; @@ -185,6 +186,7 @@ public class LogicalPlanner { new MoveOrderBeneathRename(), new DeduplicateOrder(), new OptimizeCollectWhereClauseAccess(), + new RemoveRedundantDistinct(), new RewriteGroupByKeysLimitToLimitDistinct(), new RewriteDistinctAggToGroupBy(), new MoveConstantJoinConditionsBeneathJoin(), diff --git a/server/src/main/java/io/crate/planner/optimizer/rule/RemoveRedundantDistinct.java b/server/src/main/java/io/crate/planner/optimizer/rule/RemoveRedundantDistinct.java new file mode 100644 index 000000000000..96038db1635f --- /dev/null +++ b/server/src/main/java/io/crate/planner/optimizer/rule/RemoveRedundantDistinct.java @@ -0,0 +1,223 @@ +/* + * Licensed to Crate.io GmbH ("Crate") under one or more contributor + * license agreements. See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. Crate licenses + * this file to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may + * obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + * However, if you have executed another commercial license agreement + * with Crate these terms will supersede the license and you may use the + * software solely pursuant to the terms of the relevant commercial agreement. + */ + +package io.crate.planner.optimizer.rule; + +import static io.crate.planner.optimizer.matcher.Pattern.typeOf; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.function.UnaryOperator; + +import io.crate.expression.symbol.AliasSymbol; +import io.crate.expression.symbol.ScopedColumn; +import io.crate.expression.symbol.Symbol; +import io.crate.metadata.ColumnIdent; +import io.crate.metadata.DocTableInfo; +import io.crate.metadata.RelationName; +import io.crate.metadata.doc.SysColumns; +import io.crate.planner.operators.Collect; +import io.crate.planner.operators.Eval; +import io.crate.planner.operators.Filter; +import io.crate.planner.operators.GroupHashAggregate; +import io.crate.planner.operators.Limit; +import io.crate.planner.operators.LogicalPlan; +import io.crate.planner.operators.Order; +import io.crate.planner.operators.Rename; +import io.crate.planner.optimizer.Rule; +import io.crate.planner.optimizer.matcher.Captures; +import io.crate.planner.optimizer.matcher.Pattern; + +/** + * Removes a redundant DISTINCT operation when its grouping keys + * contain every column of the source table's primary key. + */ +public final class RemoveRedundantDistinct implements Rule { + + private final Pattern pattern; + + public RemoveRedundantDistinct() { + this.pattern = typeOf(GroupHashAggregate.class) + .with(groupBy -> groupBy.aggregates().isEmpty()); + } + + @Override + public Pattern pattern() { + return pattern; + } + + @Override + public LogicalPlan apply( + GroupHashAggregate groupBy, + Captures captures, + Rule.Context context + ) { + if (!isUniqueByPrimaryKey(groupBy.source(), groupBy.groupKeys(), context.resolvePlan())) { + return null; + } + + // The grouping cannot eliminate any rows because the keys already + // contain the complete primary key. Preserve the requested outputs + // without performing the redundant grouping operation. + return Eval.create(groupBy.source(), groupBy.outputs()); + } + + public static boolean isUniqueByPrimaryKey( + LogicalPlan plan, + List groupKeys, + UnaryOperator resolvePlan + ) { + if (plan == null || groupKeys == null || groupKeys.isEmpty()) { + return false; + } + + LogicalPlan current = resolvePlan.apply(plan); + List currentKeys = groupKeys; + + while (current != null) { + if (current instanceof Filter || current instanceof Limit || current instanceof Order) { + // These operators do not alter the column identities or duplicate rows in a way + // that breaks uniqueness, so we just pass through to their source. + if (current.sources().size() != 1) { + return false; + } + current = resolvePlan.apply(current.sources().getFirst()); + } else if (current instanceof Rename rename) { + // A Rename operator alters the symbol names. We map our current group keys + // back to the original symbols produced by the Rename's source. + if (rename.sources().size() != 1) { + return false; + } + LogicalPlan source = resolvePlan.apply(rename.source()); + List renameOutputs = rename.outputs(); + List sourceOutputs = source.outputs(); + if (renameOutputs.size() != sourceOutputs.size()) { + return false; + } + + List mappedKeys = new ArrayList<>(currentKeys.size()); + for (Symbol key : currentKeys) { + // Match the unwrapped expression so aliases do not prevent identifying + // a grouping key that originates from the source plan. + Symbol unwrappedKey = unwrapAlias(key); + int idx = -1; + for (int i = 0; i < renameOutputs.size(); i++) { + if (unwrapAlias(renameOutputs.get(i)).equals(unwrappedKey)) { + idx = i; + break; + } + } + if (idx < 0) { + return false; + } + mappedKeys.add(sourceOutputs.get(idx)); + } + currentKeys = mappedKeys; + current = source; + } else if (current instanceof Eval eval) { + if (eval.sources().size() != 1) { + return false; + } + LogicalPlan source = resolvePlan.apply(eval.source()); + List evalOutputs = eval.outputs(); + List sourceOutputs = source.outputs(); + + // Only propagate keys that are preserved by Eval as direct source + // outputs. Computed expressions are not considered when proving + // primary-key uniqueness. + List mappedKeys = new ArrayList<>(currentKeys.size()); + for (Symbol key : currentKeys) { + Symbol unwrappedKey = unwrapAlias(key); + boolean foundInEval = false; + for (Symbol evalOut : evalOutputs) { + if (unwrapAlias(evalOut).equals(unwrappedKey)) { + foundInEval = true; + break; + } + } + if (!foundInEval) { + return false; + } + + // Ensure the key can also be traced to an actual source output. + // Otherwise, do not assume that the expression preserves uniqueness. + Symbol matchingSourceOutput = null; + for (Symbol srcOut : sourceOutputs) { + if (unwrapAlias(srcOut).equals(unwrappedKey)) { + matchingSourceOutput = srcOut; + break; + } + } + if (matchingSourceOutput == null) { + return false; + } + mappedKeys.add(matchingSourceOutput); + } + currentKeys = mappedKeys; + current = source; + } else if (current instanceof Collect collect) { + if (!(collect.relation().tableInfo() instanceof DocTableInfo tableInfo)) { + return false; + } + + // If the PK is auto-generated (like _id), user columns can't guarantee uniqueness + if (tableInfo.hasAutoGeneratedPrimaryKey()) { + return false; + } + + var primaryKey = tableInfo.primaryKey(); + if (primaryKey.isEmpty() || primaryKey.contains(SysColumns.ID.COLUMN)) { + return false; + } + RelationName tableIdent = tableInfo.ident(); + Set groupedColumns = new HashSet<>(); + + for (Symbol key : currentKeys) { + Symbol unwrapped = unwrapAlias(key); + if (unwrapped instanceof ScopedColumn scopedColumn) { + if (scopedColumn.relation().equals(tableIdent)) { + groupedColumns.add(scopedColumn.column()); + } + } + } + + // The rows are uniquely identifiable if every column of the primary key + // is present in our grouping columns. + return groupedColumns.containsAll(primaryKey); + } else { + // Joins, unions, aggregates and other unsupported operators may change + // row multiplicity or key identity. Retain DISTINCT unless uniqueness + // can be proven + return false; + } + } + return false; + } + + private static Symbol unwrapAlias(Symbol symbol) { + while (symbol instanceof AliasSymbol alias) { + symbol = alias.symbol(); + } + return symbol; + } +} diff --git a/server/src/main/java/io/crate/planner/optimizer/rule/RewriteDistinctAggToGroupBy.java b/server/src/main/java/io/crate/planner/optimizer/rule/RewriteDistinctAggToGroupBy.java index db4b82918f40..d67232c42a07 100644 --- a/server/src/main/java/io/crate/planner/optimizer/rule/RewriteDistinctAggToGroupBy.java +++ b/server/src/main/java/io/crate/planner/optimizer/rule/RewriteDistinctAggToGroupBy.java @@ -22,12 +22,14 @@ package io.crate.planner.optimizer.rule; import static io.crate.planner.optimizer.matcher.Pattern.typeOf; +import static io.crate.planner.optimizer.rule.RemoveRedundantDistinct.isUniqueByPrimaryKey; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.UnaryOperator; import io.crate.common.annotations.VisibleForTesting; import io.crate.execution.engine.aggregation.impl.CountAggregation; @@ -131,7 +133,8 @@ public RewriteDistinctAggToGroupBy() { } private static boolean matches(HashAggregate agg) { - return agg.aggregates().stream() + return agg.rewriteDistinct() && + agg.aggregates().stream() .allMatch(fn -> fn.distinct() && hasNoFilter(fn) && @@ -187,6 +190,10 @@ private static LogicalPlan applyToSameColumn(HashAggregate aggregate, Rule.Conte return null; } + if (isPrimaryKeyColumn(resolvedSource, groupKey, context.resolvePlan())) { + return null; + } + GroupHashAggregate dedup = new GroupHashAggregate(source, List.of(groupKey), List.of()); return new HashAggregate(dedup, aggregate.aggregates(), false); } @@ -236,4 +243,8 @@ private static boolean meetsNumRowsRequirement(Stats stats) { return stats.numDocs() <= MAX_TABLE_SIZE; } + + private static boolean isPrimaryKeyColumn(LogicalPlan plan, Symbol groupKey, UnaryOperator resolvePlan) { + return isUniqueByPrimaryKey(plan, List.of(groupKey), resolvePlan); + } } diff --git a/server/src/main/java/io/crate/planner/optimizer/rule/RewriteGroupByKeysLimitToLimitDistinct.java b/server/src/main/java/io/crate/planner/optimizer/rule/RewriteGroupByKeysLimitToLimitDistinct.java index 257aaeda0344..2c5391670252 100644 --- a/server/src/main/java/io/crate/planner/optimizer/rule/RewriteGroupByKeysLimitToLimitDistinct.java +++ b/server/src/main/java/io/crate/planner/optimizer/rule/RewriteGroupByKeysLimitToLimitDistinct.java @@ -27,6 +27,7 @@ import org.elasticsearch.Version; import io.crate.expression.symbol.Literal; +import io.crate.metadata.settings.CoordinatorSessionSettings; import io.crate.planner.operators.GroupHashAggregate; import io.crate.planner.operators.Limit; import io.crate.planner.operators.LimitDistinct; @@ -166,6 +167,15 @@ public LogicalPlan apply(Limit limit, Captures captures, Rule.Context context) { GroupHashAggregate groupBy = captures.get(groupCapture); + // Defer to RemoveRedundantDistinct when it is enabled and PK uniqueness + // makes the grouping redundant. Otherwise, allow LimitDistinct to act + // as a fallback optimization. + boolean removeDistinctExcluded = context.txnCtx().sessionSettings() instanceof CoordinatorSessionSettings cs + && cs.excludedOptimizerRules().contains(RemoveRedundantDistinct.class); + if (!removeDistinctExcluded + && RemoveRedundantDistinct.isUniqueByPrimaryKey(groupBy.source(), groupBy.groupKeys(), context.resolvePlan())) { + return null; + } if (!eagerTerminateIsLikely(limit, groupBy, context.planStats())) { return null; } diff --git a/server/src/test/java/io/crate/integrationtests/GroupByAggregateTest.java b/server/src/test/java/io/crate/integrationtests/GroupByAggregateTest.java index b1dd2ace214f..ccf4dfa95632 100644 --- a/server/src/test/java/io/crate/integrationtests/GroupByAggregateTest.java +++ b/server/src/test/java/io/crate/integrationtests/GroupByAggregateTest.java @@ -1435,7 +1435,7 @@ public void testSelectCollectSetAndGroupBy() { @UseRandomizedOptimizerRules(0) @Test public void test_optimized_limit_distinct_returns_2_unique_items() throws Throwable { - execute("create table m.tbl (id int primary key)"); + execute("create table m.tbl (id int)"); Object[][] ids = new Object[100][]; for (int i = 0; i < ids.length; i++) { ids[i] = new Object[] { i }; diff --git a/server/src/test/java/io/crate/integrationtests/PgCatalogITest.java b/server/src/test/java/io/crate/integrationtests/PgCatalogITest.java index 26755bdbb3d4..0ef4db048491 100644 --- a/server/src/test/java/io/crate/integrationtests/PgCatalogITest.java +++ b/server/src/test/java/io/crate/integrationtests/PgCatalogITest.java @@ -295,6 +295,7 @@ public void testPgSettingsTable() throws Exception { "optimizer_move_order_beneath_union| true| Indicates if the optimizer rule MoveOrderBeneathUnion is activated.| NULL| NULL", "optimizer_optimize_collect_where_clause_access| true| Indicates if the optimizer rule OptimizeCollectWhereClauseAccess is activated.| NULL| NULL", "optimizer_remove_order_beneath_insert| true| Indicates if the optimizer rule RemoveOrderBeneathInsert is activated.| NULL| NULL", + "optimizer_remove_redundant_distinct| true| Indicates if the optimizer rule RemoveRedundantDistinct is activated.| NULL| NULL", "optimizer_remove_redundant_eval| true| Indicates if the optimizer rule RemoveRedundantEval is activated.| NULL| NULL", "optimizer_reorder_hash_join| true| Indicates if the optimizer rule ReorderHashJoin is activated.| NULL| NULL", "optimizer_reorder_nested_loop_join| false| Indicates if the optimizer rule ReorderNestedLoopJoin is activated.| NULL| NULL", diff --git a/server/src/test/java/io/crate/integrationtests/ShowIntegrationTest.java b/server/src/test/java/io/crate/integrationtests/ShowIntegrationTest.java index 32cea1aa9f51..ba5195d10f72 100644 --- a/server/src/test/java/io/crate/integrationtests/ShowIntegrationTest.java +++ b/server/src/test/java/io/crate/integrationtests/ShowIntegrationTest.java @@ -439,6 +439,7 @@ public void testShowAll() { "optimizer_move_order_beneath_union| true| Indicates if the optimizer rule MoveOrderBeneathUnion is activated.", "optimizer_optimize_collect_where_clause_access| true| Indicates if the optimizer rule OptimizeCollectWhereClauseAccess is activated.", "optimizer_remove_order_beneath_insert| true| Indicates if the optimizer rule RemoveOrderBeneathInsert is activated.", + "optimizer_remove_redundant_distinct| true| Indicates if the optimizer rule RemoveRedundantDistinct is activated.", "optimizer_remove_redundant_eval| true| Indicates if the optimizer rule RemoveRedundantEval is activated.", "optimizer_reorder_hash_join| true| Indicates if the optimizer rule ReorderHashJoin is activated.", "optimizer_reorder_nested_loop_join| false| Indicates if the optimizer rule ReorderNestedLoopJoin is activated.", diff --git a/server/src/test/java/io/crate/planner/GroupByScalarPlannerTest.java b/server/src/test/java/io/crate/planner/GroupByScalarPlannerTest.java index 35bd8c6dace0..7f04d6d427fe 100644 --- a/server/src/test/java/io/crate/planner/GroupByScalarPlannerTest.java +++ b/server/src/test/java/io/crate/planner/GroupByScalarPlannerTest.java @@ -38,6 +38,7 @@ import io.crate.expression.symbol.Function; import io.crate.metadata.RowGranularity; import io.crate.planner.node.dql.Collect; +import io.crate.planner.optimizer.rule.RemoveRedundantDistinct; import io.crate.test.integration.CrateDummyClusterServiceUnitTest; import io.crate.testing.SQLExecutor; import io.crate.types.DataTypes; @@ -52,6 +53,7 @@ public void prepare() throws IOException { .setNumNodes(2) .build() .addTable(TableDefinitions.USER_TABLE_DEFINITION); + e.getSessionSettings().excludedOptimizerRules().add(RemoveRedundantDistinct.class); } @Test diff --git a/server/src/test/java/io/crate/planner/InsertPlannerTest.java b/server/src/test/java/io/crate/planner/InsertPlannerTest.java index 89084cffe1ec..785db6a4cb54 100644 --- a/server/src/test/java/io/crate/planner/InsertPlannerTest.java +++ b/server/src/test/java/io/crate/planner/InsertPlannerTest.java @@ -61,6 +61,7 @@ import io.crate.planner.node.dql.join.Join; import io.crate.planner.operators.InsertFromValues; import io.crate.planner.operators.LogicalPlan; +import io.crate.planner.optimizer.rule.RemoveRedundantDistinct; import io.crate.test.integration.CrateDummyClusterServiceUnitTest; import io.crate.testing.SQLExecutor; import io.crate.types.DataTypes; @@ -94,6 +95,7 @@ public void prepare() throws IOException { ") clustered into 4 shards") .addTable("create table source (id int primary key, name string)") .addTable("CREATE TABLE double_parted(x int, y int) PARTITIONED BY (x, y)"); + e.getSessionSettings().excludedOptimizerRules().add(RemoveRedundantDistinct.class); } @Test diff --git a/server/src/test/java/io/crate/planner/SelectPlannerTest.java b/server/src/test/java/io/crate/planner/SelectPlannerTest.java index a5f7025490ca..21e594c0491d 100644 --- a/server/src/test/java/io/crate/planner/SelectPlannerTest.java +++ b/server/src/test/java/io/crate/planner/SelectPlannerTest.java @@ -83,6 +83,7 @@ import io.crate.planner.node.dql.join.Join; import io.crate.planner.operators.LogicalPlan; import io.crate.planner.operators.PKAndVersion; +import io.crate.planner.optimizer.rule.RemoveRedundantDistinct; import io.crate.sql.tree.JoinType; import io.crate.statistics.Stats; import io.crate.test.integration.CrateDummyClusterServiceUnitTest; @@ -1140,6 +1141,7 @@ public void test_group_by_without_aggregates_and_with_limit_is_optimized_to_limi .setNumNodes(2) .build() .addTable(TableDefinitions.USER_TABLE_DEFINITION); + e.getSessionSettings().excludedOptimizerRules().add(RemoveRedundantDistinct.class); String stmt = "select id, name from users group by id, name limit 1"; LogicalPlan plan = e.logicalPlan(stmt); @@ -1154,6 +1156,7 @@ public void test_distinct_with_limit_and_offset_keeps_offset() throws Exception .setNumNodes(2) .build() .addTable(TableDefinitions.USER_TABLE_DEFINITION); + e.getSessionSettings().excludedOptimizerRules().add(RemoveRedundantDistinct.class); String stmt = "select id, name from users group by id, name limit 1 offset 3"; LogicalPlan plan = e.logicalPlan(stmt); diff --git a/server/src/test/java/io/crate/planner/consumer/GroupByPlannerTest.java b/server/src/test/java/io/crate/planner/consumer/GroupByPlannerTest.java index 8defb79bf50d..3dab79554616 100644 --- a/server/src/test/java/io/crate/planner/consumer/GroupByPlannerTest.java +++ b/server/src/test/java/io/crate/planner/consumer/GroupByPlannerTest.java @@ -55,6 +55,7 @@ import io.crate.planner.PositionalOrderBy; import io.crate.planner.node.dql.Collect; import io.crate.planner.operators.LogicalPlan; +import io.crate.planner.optimizer.rule.RemoveRedundantDistinct; import io.crate.test.integration.CrateDummyClusterServiceUnitTest; import io.crate.testing.Asserts; import io.crate.testing.SQLExecutor; @@ -387,6 +388,8 @@ public void testGroupByHavingNonDistributed() throws Exception { .setNumNodes(2) .build() .addTable(TableDefinitions.USER_TABLE_DEFINITION); + e.getSessionSettings().excludedOptimizerRules().add(RemoveRedundantDistinct.class); + Merge merge = e.plan( "select id from users group by id having id > 0"); Collect collect = (Collect) merge.subPlan(); diff --git a/server/src/test/java/io/crate/planner/operators/LogicalPlannerTest.java b/server/src/test/java/io/crate/planner/operators/LogicalPlannerTest.java index 10702b29f7d7..c52f1e76323a 100644 --- a/server/src/test/java/io/crate/planner/operators/LogicalPlannerTest.java +++ b/server/src/test/java/io/crate/planner/operators/LogicalPlannerTest.java @@ -88,6 +88,9 @@ public void prepare() throws IOException { .addTable(T3.T1_DEFINITION) .addTable(T3.T2_DEFINITION) .addTable(T3.T3_DEFINITION) + .addTable(T3.DISTINCT_PK_DEFINITION) + .addTable(T3.COMPOSITE_PK_DEFINITION) + .addTable(T3.DISTINCT_TEXT_PK_DEFINITION) .addView(new RelationName("doc", "v2"), "SELECT a, x FROM doc.t1") .addView(new RelationName("doc", "v3"), "SELECT a, x FROM doc.t1"); if (WARM.compareAndSet(false, true)) { @@ -1090,4 +1093,192 @@ public void test_correlated_subqueries_in_where_and_in_select_with_group_by() th """ ); } + + @Test + public void test_distinct_is_removed_when_projection_contains_primary_key() { + LogicalPlan plan = sqlExecutor.logicalPlan( + "SELECT DISTINCT id, name FROM distinct_pk" + ); + + assertThat(plan).isEqualTo( + "Collect[doc.distinct_pk | [id, name] | true]" + ); + } + + @Test + public void test_distinct_is_kept_when_table_has_no_primary_key() { + LogicalPlan plan = sqlExecutor.logicalPlan( + "SELECT DISTINCT x, a FROM t1" + ); + + assertThat(plan).isEqualTo(""" + GroupHashAggregate[x, a] + └ Collect[doc.t1 | [x, a] | true] + \s"""); + } + + @Test + public void test_removes_redundant_distinct_on_composite_pk() { + LogicalPlan plan = sqlExecutor.logicalPlan( + "SELECT DISTINCT id1, id2, name FROM composite_pk" + ); + + assertThat(plan).isEqualTo( + "Collect[doc.composite_pk | [id1, id2, name] | true]" + ); + } + + @Test + public void test_distinct_is_kept_when_projection_contains_only_part_of_composite_pk() { + LogicalPlan plan = sqlExecutor.logicalPlan( + "SELECT DISTINCT id1, name FROM composite_pk" + ); + + assertThat(plan).isEqualTo(""" + GroupHashAggregate[id1, name] + └ Collect[doc.composite_pk | [id1, name] | true] + \s"""); + } + + @Test + public void test_distinct_is_removed_when_primary_key_column_is_aliased() { + LogicalPlan plan = sqlExecutor.logicalPlan( + "SELECT DISTINCT id AS alias_id, name FROM distinct_pk" + ); + + assertThat(plan).isEqualTo( + "Collect[doc.distinct_pk | [id AS alias_id, name] | true]" + ); + } + + @Test + public void test_distinct_is_removed_when_primary_key_column_is_reordered() { + LogicalPlan plan = sqlExecutor.logicalPlan( + "SELECT DISTINCT name, id FROM distinct_pk" + ); + + assertThat(plan).isEqualTo( + "Collect[doc.distinct_pk | [name, id] | true]" + ); + } + + @Test + public void test_count_distinct_on_primary_key_does_not_add_group_by() { + LogicalPlan plan = plan( + "SELECT COUNT(DISTINCT x) FROM distinct_text_pk" + ); + + assertThat(plan).isEqualTo( + """ + HashAggregate[count(DISTINCT x)] + └ Collect[doc.distinct_text_pk | [x] | true] + """ + ); + } + + @Test + public void test_distinct_is_removed_when_projection_contains_primary_key_with_limit() { + LogicalPlan plan = sqlExecutor.logicalPlan( + "SELECT DISTINCT id, name FROM distinct_pk LIMIT 10" + ); + + assertThat(plan).hasOperators( + "Fetch[id, name]", + " └ Limit[10;0]", + " └ Collect[doc.distinct_pk | [_fetchid] | true]" + ); + } + + @Test + public void test_distinct_is_removed_when_projection_contains_primary_key_with_filter() { + LogicalPlan plan = sqlExecutor.logicalPlan( + "SELECT DISTINCT id, name FROM distinct_pk WHERE name IS NOT NULL" + ); + + assertThat(plan).hasOperators( + "Collect[doc.distinct_pk | [id, name] | (NOT (name IS NULL))]" + ); + } + + @Test + public void test_distinct_is_removed_when_projection_contains_primary_key_with_order_by() { + LogicalPlan plan = sqlExecutor.logicalPlan( + "SELECT DISTINCT id, name FROM distinct_pk ORDER BY name" + ); + + assertThat(plan).hasOperators( + "Fetch[id, name]", + " └ OrderBy[name ASC]", + " └ Collect[doc.distinct_pk | [_fetchid, name] | true]" + ); + } + + @Test + public void test_distinct_is_kept_when_projection_contains_auto_generated_id() { + LogicalPlan plan = sqlExecutor.logicalPlan( + "SELECT DISTINCT _id FROM t1" + ); + + assertThat(plan).isEqualTo(""" + GroupHashAggregate[_id] + └ Collect[doc.t1 | [_id] | true] + \s"""); + } + + @Test + public void test_distinct_is_removed_when_projection_contains_primary_key_in_subquery() { + LogicalPlan plan = sqlExecutor.logicalPlan( + "SELECT DISTINCT id, name FROM (SELECT id, name FROM distinct_pk) t" + ); + + assertThat(plan).hasOperators( + "Rename[id, name] AS t", + " └ Collect[doc.distinct_pk | [id, name] | true]" + ); + } + + @Test + public void test_distinct_is_kept_for_union() { + LogicalPlan plan = sqlExecutor.logicalPlan(""" + SELECT DISTINCT id + FROM ( + SELECT id FROM distinct_pk + UNION ALL + SELECT id FROM distinct_pk + ) t + """); + + assertThat(plan).hasOperators( + "GroupHashAggregate[id]", + " └ Rename[id] AS t", + " └ Union[id]", + " ├ Collect[doc.distinct_pk | [id] | true]", + " └ Collect[doc.distinct_pk | [id] | true]" + ); + } + + @Test + public void test_distinct_is_removed_when_projection_contains_primary_key_with_limit_and_offset() { + LogicalPlan plan = sqlExecutor.logicalPlan( + "SELECT DISTINCT id, name FROM distinct_pk LIMIT 10 OFFSET 5" + ); + + assertThat(plan).hasOperators( + "Fetch[id, name]", + " └ Limit[10;5]", + " └ Collect[doc.distinct_pk | [_fetchid] | true]" + ); + } + + @Test + public void test_limit_distinct_is_used_when_group_keys_do_not_contain_primary_key() { + LogicalPlan plan = sqlExecutor.logicalPlan( + "SELECT DISTINCT id1, name FROM composite_pk LIMIT 10 OFFSET 5" + ); + + assertThat(plan).hasOperators( + "LimitDistinct[10;5 | [id1, name]]", + " └ Collect[doc.composite_pk | [id1, name] | true]" + ); + } } diff --git a/server/src/testFixtures/java/io/crate/analyze/TableDefinitions.java b/server/src/testFixtures/java/io/crate/analyze/TableDefinitions.java index ff69503bbb9d..a328ef60236a 100644 --- a/server/src/testFixtures/java/io/crate/analyze/TableDefinitions.java +++ b/server/src/testFixtures/java/io/crate/analyze/TableDefinitions.java @@ -76,7 +76,8 @@ private TableDefinitions() {} " name text," + " details object," + " awesome boolean," + - " friends array(object)" + + " friends array(object)," + + " id2 bigint" + ")" + " clustered by (id)"; static final RelationName TEST_PARTITIONED_TABLE_IDENT = new RelationName(DocSchemaInfo.NAME, "parted"); diff --git a/server/src/testFixtures/java/io/crate/testing/T3.java b/server/src/testFixtures/java/io/crate/testing/T3.java index b5e371dbcb44..53dcc5e167cc 100644 --- a/server/src/testFixtures/java/io/crate/testing/T3.java +++ b/server/src/testFixtures/java/io/crate/testing/T3.java @@ -74,6 +74,30 @@ public class T3 { " ts timestamp without time zone" + ")"; + public static final String DISTINCT_PK_DEFINITION = """ + CREATE TABLE doc.distinct_pk ( + id INTEGER PRIMARY KEY, + name TEXT + ) + """; + + public static final String COMPOSITE_PK_DEFINITION = """ + CREATE TABLE doc.composite_pk ( + id1 INTEGER, + id2 INTEGER, + name TEXT, + PRIMARY KEY (id1, id2) + ) + """; + + public static final String DISTINCT_TEXT_PK_DEFINITION = """ + CREATE TABLE doc.distinct_text_pk ( + x TEXT PRIMARY KEY, + y TEXT, + z TEXT + ) + """; + public static final RelationName T1 = new RelationName(DocSchemaInfo.NAME, "t1"); public static final RelationName T2 = new RelationName(DocSchemaInfo.NAME, "t2"); public static final RelationName T3 = new RelationName(DocSchemaInfo.NAME, "t3");