From 03ae5ed37ece2ec0380b2bb0627ec036a97496f3 Mon Sep 17 00:00:00 2001 From: yaofly Date: Tue, 26 Nov 2024 11:07:04 +0800 Subject: [PATCH 01/31] Support GroupConcat sql for aggregating multiple shards(#33797) --- .../groupby/GroupByMemoryMergedResult.java | 2 +- .../groupby/GroupByStreamMergedResult.java | 3 +- .../aggregation/AggregationUnitFactory.java | 15 +++++ .../DistinctGroupConcatAggregationUnit.java | 49 ++++++++++++++++ .../GroupConcatAggregationUnit.java | 56 +++++++++++++++++++ .../AggregationUnitFactoryTest.java | 6 ++ .../GroupConcatAggregationUnitTest.java | 51 +++++++++++++++++ .../projection/engine/ProjectionEngine.java | 3 +- .../impl/AggregationProjection.java | 10 ++++ .../src/main/antlr4/imports/mysql/BaseRule.g4 | 6 +- .../statement/MySQLStatementVisitor.java | 3 + .../item/AggregationProjectionSegment.java | 3 + 12 files changed, 203 insertions(+), 4 deletions(-) create mode 100644 features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java create mode 100644 features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java create mode 100644 features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnitTest.java diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/GroupByMemoryMergedResult.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/GroupByMemoryMergedResult.java index 56505af0f4454..2b99f1c649c2f 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/GroupByMemoryMergedResult.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/GroupByMemoryMergedResult.java @@ -81,7 +81,7 @@ private void initForFirstGroupByValue(final SelectStatementContext selectStateme dataMap.put(groupByValue, new MemoryQueryResultRow(queryResult)); } aggregationMap.computeIfAbsent(groupByValue, unused -> selectStatementContext.getProjectionsContext().getAggregationProjections().stream() - .collect(Collectors.toMap(Function.identity(), input -> AggregationUnitFactory.create(input.getType(), input instanceof AggregationDistinctProjection)))); + .collect(Collectors.toMap(Function.identity(), input -> AggregationUnitFactory.create(input.getType(), input instanceof AggregationDistinctProjection, input.getSeparator())))); } private void aggregate(final SelectStatementContext selectStatementContext, final QueryResult queryResult, diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/GroupByStreamMergedResult.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/GroupByStreamMergedResult.java index 550c982af4ba1..5a87ad9c51899 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/GroupByStreamMergedResult.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/GroupByStreamMergedResult.java @@ -77,7 +77,8 @@ private boolean aggregateCurrentGroupByRowAndNext() throws SQLException { boolean result = false; boolean cachedRow = false; Map aggregationUnitMap = Maps.toMap( - selectStatementContext.getProjectionsContext().getAggregationProjections(), input -> AggregationUnitFactory.create(input.getType(), input instanceof AggregationDistinctProjection)); + selectStatementContext.getProjectionsContext().getAggregationProjections(), + input -> AggregationUnitFactory.create(input.getType(), input instanceof AggregationDistinctProjection, input.getSeparator())); while (currentGroupByValues.equals(new GroupByValue(getCurrentQueryResult(), selectStatementContext.getGroupByContext().getItems()).getGroupValues())) { aggregate(aggregationUnitMap); if (!cachedRow) { diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactory.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactory.java index c6709cbb4ed57..0b09f0fc2d341 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactory.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactory.java @@ -37,6 +37,19 @@ public final class AggregationUnitFactory { * @throws UnsupportedSQLOperationException unsupported SQL operation exception */ public static AggregationUnit create(final AggregationType type, final boolean isDistinct) { + return create(type, isDistinct, null); + } + + /** + * Create aggregation unit instance. + * + * @param type aggregation function type + * @param isDistinct is distinct + * @param separator is separator for group_concat + * @return aggregation unit instance + * @throws UnsupportedSQLOperationException unsupported SQL operation exception + */ + public static AggregationUnit create(final AggregationType type, final boolean isDistinct, final String separator) { switch (type) { case MAX: return new ComparableAggregationUnit(false); @@ -50,6 +63,8 @@ public static AggregationUnit create(final AggregationType type, final boolean i return isDistinct ? new DistinctAverageAggregationUnit() : new AverageAggregationUnit(); case BIT_XOR: return new BitXorAggregationUnit(); + case GROUP_CONCAT: + return isDistinct ? new GroupConcatAggregationUnit(separator) : new DistinctGroupConcatAggregationUnit(separator); default: throw new UnsupportedSQLOperationException(type.name()); } diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java new file mode 100644 index 0000000000000..bac21295c499d --- /dev/null +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF 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. + */ + +package org.apache.shardingsphere.sharding.merge.dql.groupby.aggregation; + +import lombok.NoArgsConstructor; + +import java.util.*; +import java.util.stream.Collectors; + +public class DistinctGroupConcatAggregationUnit implements AggregationUnit { + + private final Collection values = new HashSet<>(); + + private final static String DEFAULT_SEPARATOR = ","; + + private final String separator; + + public DistinctGroupConcatAggregationUnit(String separator) { + this.separator = null == separator ? DEFAULT_SEPARATOR : separator; + } + + @Override + public void merge(List> values) { + if (null == values || null == values.get(0)) { + return; + } + this.values.addAll(Arrays.asList(values.get(0).toString().split(separator))); + } + + @Override + public Comparable getResult() { + return String.join(separator, values); + } +} diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java new file mode 100644 index 0000000000000..3fbd7df7bb023 --- /dev/null +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF 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. + */ + +package org.apache.shardingsphere.sharding.merge.dql.groupby.aggregation; + +import lombok.NoArgsConstructor; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.stream.Collectors; + +@NoArgsConstructor +public class GroupConcatAggregationUnit implements AggregationUnit { + + private final Collection> values = new ArrayList<>(); + + private final static String DEFAULT_SEPARATOR = ","; + + private String separator; + + public GroupConcatAggregationUnit(String separator) { + this.separator = separator; + } + + @Override + public void merge(List> values) { + if (null == values || null == values.get(0)) { + return; + } + this.values.add(values.get(0)); + } + + @Override + public Comparable getResult() { + if (null == separator) { + separator = DEFAULT_SEPARATOR; + } + return values.stream().map(Object::toString).collect(Collectors.joining(separator)); + } +} diff --git a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactoryTest.java b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactoryTest.java index 6e14d251409de..3aa5efc70ff63 100644 --- a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactoryTest.java +++ b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactoryTest.java @@ -61,4 +61,10 @@ void assertCreateDistinctAverageAggregationUnit() { void assertCreateBitXorAggregationUnit() { assertThat(AggregationUnitFactory.create(AggregationType.BIT_XOR, false), instanceOf(BitXorAggregationUnit.class)); } + + @Test + void assertGroupConcatAggregationUnit() { + assertThat(AggregationUnitFactory.create(AggregationType.GROUP_CONCAT, true), instanceOf(DistinctGroupConcatAggregationUnit.class)); + assertThat(AggregationUnitFactory.create(AggregationType.GROUP_CONCAT, true, " "), instanceOf(DistinctGroupConcatAggregationUnit.class)); + } } diff --git a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnitTest.java b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnitTest.java new file mode 100644 index 0000000000000..7ba4d281ca742 --- /dev/null +++ b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnitTest.java @@ -0,0 +1,51 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF 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. + */ + +package org.apache.shardingsphere.sharding.merge.dql.groupby.aggregation; + +import org.junit.jupiter.api.Test; + +import java.util.Collections; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; + +class GroupConcatAggregationUnitTest { + + @Test + void assertGroupConcatAggregation() { + GroupConcatAggregationUnit comparableAggregation = new GroupConcatAggregationUnit(" "); + comparableAggregation.merge(null); + comparableAggregation.merge(Collections.singletonList(null)); + comparableAggregation.merge(Collections.singletonList("001")); + comparableAggregation.merge(Collections.singletonList("002")); + comparableAggregation.merge(Collections.singletonList("002 003")); + assertThat(comparableAggregation.getResult(), is("001 002 002 003")); + } + + @Test + void assertDistinctGroupConcatAggregation() { + GroupConcatAggregationUnit comparableAggregation = new GroupConcatAggregationUnit(" "); + comparableAggregation.merge(null); + comparableAggregation.merge(Collections.singletonList(null)); + comparableAggregation.merge(Collections.singletonList("")); + comparableAggregation.merge(Collections.singletonList("001")); + comparableAggregation.merge(Collections.singletonList("001")); + comparableAggregation.merge(Collections.singletonList("001 003")); + assertThat(comparableAggregation.getResult(), is(" 001 003")); + } +} diff --git a/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/engine/ProjectionEngine.java b/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/engine/ProjectionEngine.java index 46a24422a41d0..3202b22d6d0c3 100644 --- a/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/engine/ProjectionEngine.java +++ b/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/engine/ProjectionEngine.java @@ -131,7 +131,8 @@ private AggregationDistinctProjection createProjection(final AggregationDistinct } private AggregationProjection createProjection(final AggregationProjectionSegment projectionSegment) { - AggregationProjection result = new AggregationProjection(projectionSegment.getType(), projectionSegment.getExpression(), projectionSegment.getAlias().orElse(null), databaseType); + AggregationProjection result = + new AggregationProjection(projectionSegment.getType(), projectionSegment.getExpression(), projectionSegment.getAlias().orElse(null), databaseType, projectionSegment.getSeparator()); if (AggregationType.AVG == result.getType()) { appendAverageDerivedProjection(result); // TODO replace avg to constant, avoid calculate useless avg diff --git a/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationProjection.java b/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationProjection.java index 3034779f82978..69506b4243b77 100644 --- a/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationProjection.java +++ b/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationProjection.java @@ -50,8 +50,18 @@ public class AggregationProjection implements Projection { private final DatabaseType databaseType; + private final String separator; + private final List derivedAggregationProjections = new ArrayList<>(2); + public AggregationProjection(AggregationType type, String expression, IdentifierValue alias, DatabaseType databaseType) { + this.type = type; + this.expression = expression; + this.alias = alias; + this.databaseType = databaseType; + this.separator = null; + } + @Setter private int index = -1; diff --git a/parser/sql/dialect/mysql/src/main/antlr4/imports/mysql/BaseRule.g4 b/parser/sql/dialect/mysql/src/main/antlr4/imports/mysql/BaseRule.g4 index f0aee586d69a6..ccebcfa04bcf5 100644 --- a/parser/sql/dialect/mysql/src/main/antlr4/imports/mysql/BaseRule.g4 +++ b/parser/sql/dialect/mysql/src/main/antlr4/imports/mysql/BaseRule.g4 @@ -961,8 +961,12 @@ udfFunction : functionName LP_ (expr? | expr (COMMA_ expr)*) RP_ ; +separatorName + : SEPARATOR string_ + ; + aggregationFunction - : aggregationFunctionName LP_ distinct? (expr (COMMA_ expr)* | ASTERISK_)? collateClause? RP_ overClause? + : aggregationFunctionName LP_ distinct? (expr (COMMA_ expr)* | ASTERISK_)? collateClause? orderByClause? separatorName? RP_ overClause? ; jsonFunction diff --git a/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java b/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java index 56dfda8057462..80afb4d092360 100644 --- a/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java +++ b/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java @@ -964,6 +964,9 @@ private ASTNode createAggregationSegment(final AggregationFunctionContext ctx, f return result; } AggregationProjectionSegment result = new AggregationProjectionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), type, getOriginalText(ctx)); + if (null != ctx.separatorName()) { + result.setSeparator(ctx.separatorName().string_().getText()); + } result.getParameters().addAll(getExpressions(ctx.expr())); return result; } diff --git a/parser/sql/statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/dml/item/AggregationProjectionSegment.java b/parser/sql/statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/dml/item/AggregationProjectionSegment.java index 068f93b46c387..0b3a8116845f9 100644 --- a/parser/sql/statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/dml/item/AggregationProjectionSegment.java +++ b/parser/sql/statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/dml/item/AggregationProjectionSegment.java @@ -43,6 +43,9 @@ public class AggregationProjectionSegment implements ProjectionSegment, AliasAva private final String expression; + @Setter + private String separator; + private final Collection parameters = new LinkedList<>(); @Setter From c0552d1e470774b713056b3647b7d0d4a5d44045 Mon Sep 17 00:00:00 2001 From: yaofly Date: Tue, 26 Nov 2024 11:17:06 +0800 Subject: [PATCH 02/31] Check Style fix(#33797) --- .../DistinctGroupConcatAggregationUnit.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java index bac21295c499d..b1194052799e1 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java @@ -17,25 +17,25 @@ package org.apache.shardingsphere.sharding.merge.dql.groupby.aggregation; -import lombok.NoArgsConstructor; - -import java.util.*; -import java.util.stream.Collectors; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; public class DistinctGroupConcatAggregationUnit implements AggregationUnit { - + + private static final String DEFAULT_SEPARATOR = ","; + private final Collection values = new HashSet<>(); - - private final static String DEFAULT_SEPARATOR = ","; - + private final String separator; - public DistinctGroupConcatAggregationUnit(String separator) { + public DistinctGroupConcatAggregationUnit(final String separator) { this.separator = null == separator ? DEFAULT_SEPARATOR : separator; } @Override - public void merge(List> values) { + public void merge(final List> values) { if (null == values || null == values.get(0)) { return; } From c256c10b374d7ca6bc5c4908af7e6ccb2570b740 Mon Sep 17 00:00:00 2001 From: yaofly Date: Tue, 26 Nov 2024 11:28:12 +0800 Subject: [PATCH 03/31] Check Style fix(#33797) --- .../aggregation/GroupConcatAggregationUnit.java | 13 ++++++------- .../projection/impl/AggregationProjection.java | 8 ++++---- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java index 3fbd7df7bb023..431f1f3b56326 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java @@ -21,25 +21,24 @@ import java.util.ArrayList; import java.util.Collection; -import java.util.HashSet; import java.util.List; import java.util.stream.Collectors; @NoArgsConstructor public class GroupConcatAggregationUnit implements AggregationUnit { - + + private static final String DEFAULT_SEPARATOR = ","; + private final Collection> values = new ArrayList<>(); - - private final static String DEFAULT_SEPARATOR = ","; - + private String separator; - public GroupConcatAggregationUnit(String separator) { + public GroupConcatAggregationUnit(final String separator) { this.separator = separator; } @Override - public void merge(List> values) { + public void merge(final List> values) { if (null == values || null == values.get(0)) { return; } diff --git a/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationProjection.java b/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationProjection.java index 69506b4243b77..ff10d395ea50f 100644 --- a/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationProjection.java +++ b/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationProjection.java @@ -54,7 +54,10 @@ public class AggregationProjection implements Projection { private final List derivedAggregationProjections = new ArrayList<>(2); - public AggregationProjection(AggregationType type, String expression, IdentifierValue alias, DatabaseType databaseType) { + @Setter + private int index = -1; + + public AggregationProjection(final AggregationType type, final String expression, final IdentifierValue alias, final DatabaseType databaseType) { this.type = type; this.expression = expression; this.alias = alias; @@ -62,9 +65,6 @@ public AggregationProjection(AggregationType type, String expression, Identifier this.separator = null; } - @Setter - private int index = -1; - @Override public String getColumnName() { return getColumnLabel(); From 812f81361f37ef93e288b923498cb98caccd9984 Mon Sep 17 00:00:00 2001 From: yaofly Date: Tue, 26 Nov 2024 11:28:57 +0800 Subject: [PATCH 04/31] spotless fix (#33797) --- .../aggregation/DistinctGroupConcatAggregationUnit.java | 6 +++--- .../dql/groupby/aggregation/GroupConcatAggregationUnit.java | 6 +++--- .../select/projection/impl/AggregationProjection.java | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java index b1194052799e1..eb371da387ad5 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java @@ -23,11 +23,11 @@ import java.util.List; public class DistinctGroupConcatAggregationUnit implements AggregationUnit { - + private static final String DEFAULT_SEPARATOR = ","; - + private final Collection values = new HashSet<>(); - + private final String separator; public DistinctGroupConcatAggregationUnit(final String separator) { diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java index 431f1f3b56326..c2fcccfc23b4f 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java @@ -26,11 +26,11 @@ @NoArgsConstructor public class GroupConcatAggregationUnit implements AggregationUnit { - + private static final String DEFAULT_SEPARATOR = ","; - + private final Collection> values = new ArrayList<>(); - + private String separator; public GroupConcatAggregationUnit(final String separator) { diff --git a/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationProjection.java b/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationProjection.java index ff10d395ea50f..cb4510cbd364a 100644 --- a/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationProjection.java +++ b/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationProjection.java @@ -56,7 +56,7 @@ public class AggregationProjection implements Projection { @Setter private int index = -1; - + public AggregationProjection(final AggregationType type, final String expression, final IdentifierValue alias, final DatabaseType databaseType) { this.type = type; this.expression = expression; From 05ccc968d90623f9d9701765bc04c427c9caa1ef Mon Sep 17 00:00:00 2001 From: yaofly Date: Tue, 26 Nov 2024 23:17:06 +0800 Subject: [PATCH 05/31] unit test fix (#33797) --- .../aggregation/AggregationUnitFactory.java | 2 +- .../GroupConcatAggregationUnitTest.java | 30 +++++++++---------- .../impl/AggregationProjectionConverter.java | 12 ++++++-- .../statement/MySQLStatementVisitor.java | 10 +++++-- 4 files changed, 34 insertions(+), 20 deletions(-) diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactory.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactory.java index 0b09f0fc2d341..9e0a1e36720e9 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactory.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactory.java @@ -64,7 +64,7 @@ public static AggregationUnit create(final AggregationType type, final boolean i case BIT_XOR: return new BitXorAggregationUnit(); case GROUP_CONCAT: - return isDistinct ? new GroupConcatAggregationUnit(separator) : new DistinctGroupConcatAggregationUnit(separator); + return isDistinct ? new DistinctGroupConcatAggregationUnit(separator) : new GroupConcatAggregationUnit(separator); default: throw new UnsupportedSQLOperationException(type.name()); } diff --git a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnitTest.java b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnitTest.java index 7ba4d281ca742..0fc69c75dafea 100644 --- a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnitTest.java +++ b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnitTest.java @@ -28,24 +28,24 @@ class GroupConcatAggregationUnitTest { @Test void assertGroupConcatAggregation() { - GroupConcatAggregationUnit comparableAggregation = new GroupConcatAggregationUnit(" "); - comparableAggregation.merge(null); - comparableAggregation.merge(Collections.singletonList(null)); - comparableAggregation.merge(Collections.singletonList("001")); - comparableAggregation.merge(Collections.singletonList("002")); - comparableAggregation.merge(Collections.singletonList("002 003")); - assertThat(comparableAggregation.getResult(), is("001 002 002 003")); + GroupConcatAggregationUnit groupConcatAggregationUnit = new GroupConcatAggregationUnit(" "); + groupConcatAggregationUnit.merge(null); + groupConcatAggregationUnit.merge(Collections.singletonList(null)); + groupConcatAggregationUnit.merge(Collections.singletonList("001")); + groupConcatAggregationUnit.merge(Collections.singletonList("002")); + groupConcatAggregationUnit.merge(Collections.singletonList("002 003")); + assertThat(groupConcatAggregationUnit.getResult(), is("001 002 002 003")); } @Test void assertDistinctGroupConcatAggregation() { - GroupConcatAggregationUnit comparableAggregation = new GroupConcatAggregationUnit(" "); - comparableAggregation.merge(null); - comparableAggregation.merge(Collections.singletonList(null)); - comparableAggregation.merge(Collections.singletonList("")); - comparableAggregation.merge(Collections.singletonList("001")); - comparableAggregation.merge(Collections.singletonList("001")); - comparableAggregation.merge(Collections.singletonList("001 003")); - assertThat(comparableAggregation.getResult(), is(" 001 003")); + DistinctGroupConcatAggregationUnit distinctGroupConcatAggregationUnit = new DistinctGroupConcatAggregationUnit(" "); + distinctGroupConcatAggregationUnit.merge(null); + distinctGroupConcatAggregationUnit.merge(Collections.singletonList(null)); + distinctGroupConcatAggregationUnit.merge(Collections.singletonList("")); + distinctGroupConcatAggregationUnit.merge(Collections.singletonList("001")); + distinctGroupConcatAggregationUnit.merge(Collections.singletonList("001")); + distinctGroupConcatAggregationUnit.merge(Collections.singletonList("001 003")); + assertThat(distinctGroupConcatAggregationUnit.getResult(), is(" 001 003")); } } diff --git a/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/projection/impl/AggregationProjectionConverter.java b/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/projection/impl/AggregationProjectionConverter.java index dd62b33e6b2c1..4813ca0b54779 100644 --- a/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/projection/impl/AggregationProjectionConverter.java +++ b/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/projection/impl/AggregationProjectionConverter.java @@ -57,11 +57,16 @@ public final class AggregationProjectionConverter { register(SqlStdOperatorTable.COUNT); register(SqlStdOperatorTable.AVG); register(SqlStdOperatorTable.BIT_XOR); + register(SqlStdOperatorTable.LISTAGG, "GROUP_CONCAT"); } private static void register(final SqlAggFunction sqlAggFunction) { REGISTRY.put(sqlAggFunction.getName(), sqlAggFunction); } + + private static void register(final SqlAggFunction sqlAggFunction, final String alias) { + REGISTRY.put(alias, sqlAggFunction); + } /** * Convert aggregation projection segment to sql node. @@ -75,7 +80,7 @@ public static Optional convert(final AggregationProjectionSegment segme } SqlLiteral functionQuantifier = segment instanceof AggregationDistinctProjectionSegment ? SqlLiteral.createSymbol(SqlSelectKeyword.DISTINCT, SqlParserPos.ZERO) : null; SqlAggFunction operator = convertOperator(segment.getType().name()); - List params = convertParameters(segment.getParameters(), segment.getExpression()); + List params = convertParameters(segment.getParameters(), segment.getExpression(), segment.getSeparator()); SqlBasicCall sqlBasicCall = new SqlBasicCall(operator, params, SqlParserPos.ZERO, functionQuantifier); if (segment.getAliasName().isPresent()) { return Optional.of(new SqlBasicCall(SqlStdOperatorTable.AS, Arrays.asList(sqlBasicCall, @@ -89,7 +94,7 @@ private static SqlAggFunction convertOperator(final String operator) { return REGISTRY.get(operator); } - private static List convertParameters(final Collection params, final String expression) { + private static List convertParameters(final Collection params, final String expression, final String separator) { if (expression.contains("*")) { return Collections.singletonList(SqlIdentifier.star(SqlParserPos.ZERO)); } @@ -97,6 +102,9 @@ private static List convertParameters(final Collection Date: Wed, 27 Nov 2024 00:07:00 +0800 Subject: [PATCH 06/31] spotless fix (#33797) --- .../projection/impl/AggregationProjectionConverter.java | 2 +- .../parser/mysql/visitor/statement/MySQLStatementVisitor.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/projection/impl/AggregationProjectionConverter.java b/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/projection/impl/AggregationProjectionConverter.java index 4813ca0b54779..d56c2f9d714c2 100644 --- a/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/projection/impl/AggregationProjectionConverter.java +++ b/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/projection/impl/AggregationProjectionConverter.java @@ -63,7 +63,7 @@ public final class AggregationProjectionConverter { private static void register(final SqlAggFunction sqlAggFunction) { REGISTRY.put(sqlAggFunction.getName(), sqlAggFunction); } - + private static void register(final SqlAggFunction sqlAggFunction, final String alias) { REGISTRY.put(alias, sqlAggFunction); } diff --git a/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java b/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java index fbfaadbe2c837..348ec085ba03a 100644 --- a/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java +++ b/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java @@ -916,12 +916,12 @@ public final ASTNode visitAggregationFunction(final AggregationFunctionContext c ? createAggregationSegment(ctx, aggregationType) : new ExpressionProjectionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), getOriginalText(ctx)); } - + @Override public ASTNode visitSeparatorName(final MySQLStatementParser.SeparatorNameContext ctx) { return new StringLiteralValue(ctx.string_().getText()); } - + @Override public final ASTNode visitJsonFunction(final JsonFunctionContext ctx) { JsonFunctionNameContext functionNameContext = ctx.jsonFunctionName(); From 07209e2052fadbde99c7343a7ccdcce57c3e3ec4 Mon Sep 17 00:00:00 2001 From: yaofly Date: Wed, 27 Nov 2024 11:24:47 +0800 Subject: [PATCH 07/31] group_concat distinct compatible (#33797) --- .../projection/engine/ProjectionEngine.java | 2 +- .../impl/AggregationDistinctProjection.java | 12 ++++++++++++ .../src/main/antlr4/imports/mysql/BaseRule.g4 | 6 +++++- .../statement/MySQLStatementVisitor.java | 17 +++++++++-------- .../AggregationDistinctProjectionSegment.java | 6 ++++++ .../dml/item/AggregationProjectionSegment.java | 12 ++++++++++-- 6 files changed, 43 insertions(+), 12 deletions(-) diff --git a/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/engine/ProjectionEngine.java b/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/engine/ProjectionEngine.java index 3202b22d6d0c3..5696fec179d64 100644 --- a/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/engine/ProjectionEngine.java +++ b/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/engine/ProjectionEngine.java @@ -123,7 +123,7 @@ private AggregationDistinctProjection createProjection(final AggregationDistinct projectionSegment.getAlias().orElseGet(() -> new IdentifierValue(DerivedColumn.AGGREGATION_DISTINCT_DERIVED.getDerivedColumnAlias(aggregationDistinctDerivedColumnCount++))); AggregationDistinctProjection result = new AggregationDistinctProjection( projectionSegment.getStartIndex(), projectionSegment.getStopIndex(), projectionSegment.getType(), projectionSegment.getExpression(), alias, - projectionSegment.getDistinctInnerExpression(), databaseType); + projectionSegment.getDistinctInnerExpression(), databaseType, projectionSegment.getSeparator()); if (AggregationType.AVG == result.getType()) { appendAverageDistinctDerivedProjection(result); } diff --git a/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationDistinctProjection.java b/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationDistinctProjection.java index 6bb6241b00b58..03cd49b4b33d9 100644 --- a/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationDistinctProjection.java +++ b/infra/binder/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/impl/AggregationDistinctProjection.java @@ -36,11 +36,23 @@ public final class AggregationDistinctProjection extends AggregationProjection { private final String distinctInnerExpression; + private final String separator; + public AggregationDistinctProjection(final int startIndex, final int stopIndex, final AggregationType type, final String expression, final IdentifierValue alias, final String distinctInnerExpression, final DatabaseType databaseType) { super(type, expression, alias, databaseType); this.startIndex = startIndex; this.stopIndex = stopIndex; this.distinctInnerExpression = distinctInnerExpression; + this.separator = null; + } + + public AggregationDistinctProjection(final int startIndex, final int stopIndex, final AggregationType type, final String expression, + final IdentifierValue alias, final String distinctInnerExpression, final DatabaseType databaseType, final String separator) { + super(type, expression, alias, databaseType); + this.startIndex = startIndex; + this.stopIndex = stopIndex; + this.distinctInnerExpression = distinctInnerExpression; + this.separator = separator; } } diff --git a/parser/sql/dialect/mysql/src/main/antlr4/imports/mysql/BaseRule.g4 b/parser/sql/dialect/mysql/src/main/antlr4/imports/mysql/BaseRule.g4 index ccebcfa04bcf5..479dc2c9ae1e2 100644 --- a/parser/sql/dialect/mysql/src/main/antlr4/imports/mysql/BaseRule.g4 +++ b/parser/sql/dialect/mysql/src/main/antlr4/imports/mysql/BaseRule.g4 @@ -965,8 +965,12 @@ separatorName : SEPARATOR string_ ; +aggregationExpression + : expr (COMMA_ expr)* | ASTERISK_ + ; + aggregationFunction - : aggregationFunctionName LP_ distinct? (expr (COMMA_ expr)* | ASTERISK_)? collateClause? orderByClause? separatorName? RP_ overClause? + : aggregationFunctionName LP_ distinct? aggregationExpression? collateClause? orderByClause? separatorName? RP_ overClause? ; jsonFunction diff --git a/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java b/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java index 348ec085ba03a..8f2e7c36cb856 100644 --- a/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java +++ b/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java @@ -963,17 +963,18 @@ public final ASTNode visitJsonTableFunction(final JsonTableFunctionContext ctx) private ASTNode createAggregationSegment(final AggregationFunctionContext ctx, final String aggregationType) { AggregationType type = AggregationType.valueOf(aggregationType.toUpperCase()); + String separator = null; + if (null != ctx.separatorName()) { + separator = new StringLiteralValue(ctx.separatorName().string_().getText()).getValue(); + } if (null != ctx.distinct()) { AggregationDistinctProjectionSegment result = - new AggregationDistinctProjectionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), type, getOriginalText(ctx), getDistinctExpression(ctx)); - result.getParameters().addAll(getExpressions(ctx.expr())); + new AggregationDistinctProjectionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), type, getOriginalText(ctx), getDistinctExpression(ctx), separator); + result.getParameters().addAll(getExpressions(ctx.aggregationExpression().expr())); return result; } - AggregationProjectionSegment result = new AggregationProjectionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), type, getOriginalText(ctx)); - if (null != ctx.separatorName()) { - result.setSeparator(new StringLiteralValue(ctx.separatorName().string_().getText()).getValue()); - } - result.getParameters().addAll(getExpressions(ctx.expr())); + AggregationProjectionSegment result = new AggregationProjectionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), type, getOriginalText(ctx), separator); + result.getParameters().addAll(getExpressions(ctx.aggregationExpression().expr())); return result; } @@ -993,7 +994,7 @@ private String getDistinctExpression(final AggregationFunctionContext ctx) { for (int i = 3; i < ctx.getChildCount() - 1; i++) { result.append(ctx.getChild(i).getText()); } - return result.toString(); + return ctx.aggregationExpression().getText(); } @Override diff --git a/parser/sql/statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/dml/item/AggregationDistinctProjectionSegment.java b/parser/sql/statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/dml/item/AggregationDistinctProjectionSegment.java index e01efd80f4a42..2b82bd1209693 100644 --- a/parser/sql/statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/dml/item/AggregationDistinctProjectionSegment.java +++ b/parser/sql/statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/dml/item/AggregationDistinctProjectionSegment.java @@ -33,4 +33,10 @@ public AggregationDistinctProjectionSegment(final int startIndex, final int stop super(startIndex, stopIndex, type, expression); distinctInnerExpression = SQLUtils.getExpressionWithoutOutsideParentheses(distinctExpression); } + + public AggregationDistinctProjectionSegment(final int startIndex, final int stopIndex, final AggregationType type, final String expression, final String distinctExpression, + final String separator) { + super(startIndex, stopIndex, type, expression, separator); + distinctInnerExpression = SQLUtils.getExpressionWithoutOutsideParentheses(distinctExpression); + } } diff --git a/parser/sql/statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/dml/item/AggregationProjectionSegment.java b/parser/sql/statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/dml/item/AggregationProjectionSegment.java index 0b3a8116845f9..3b4c511681167 100644 --- a/parser/sql/statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/dml/item/AggregationProjectionSegment.java +++ b/parser/sql/statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/dml/item/AggregationProjectionSegment.java @@ -43,8 +43,7 @@ public class AggregationProjectionSegment implements ProjectionSegment, AliasAva private final String expression; - @Setter - private String separator; + private final String separator; private final Collection parameters = new LinkedList<>(); @@ -56,6 +55,15 @@ public AggregationProjectionSegment(final int startIndex, final int stopIndex, f this.stopIndex = stopIndex; this.type = type; this.expression = expression; + this.separator = null; + } + + public AggregationProjectionSegment(final int startIndex, final int stopIndex, final AggregationType type, final String expression, final String separator) { + this.startIndex = startIndex; + this.stopIndex = stopIndex; + this.type = type; + this.expression = expression; + this.separator = separator; } @Override From 879b107700b8964c251501f5f62cad1a3a81748d Mon Sep 17 00:00:00 2001 From: yaofly Date: Wed, 27 Nov 2024 13:40:10 +0800 Subject: [PATCH 08/31] group_concat distinct compatible (#33797) --- .../DistinctGroupConcatAggregationUnit.java | 15 +++++++++------ .../visitor/statement/MySQLStatementVisitor.java | 4 ---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java index eb371da387ad5..71c4f240681bc 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java @@ -17,21 +17,21 @@ package org.apache.shardingsphere.sharding.merge.dql.groupby.aggregation; -import java.util.Arrays; import java.util.Collection; import java.util.HashSet; import java.util.List; +import java.util.stream.Collectors; public class DistinctGroupConcatAggregationUnit implements AggregationUnit { private static final String DEFAULT_SEPARATOR = ","; - private final Collection values = new HashSet<>(); + private final Collection> values = new HashSet<>(); - private final String separator; + private String separator; public DistinctGroupConcatAggregationUnit(final String separator) { - this.separator = null == separator ? DEFAULT_SEPARATOR : separator; + this.separator = separator; } @Override @@ -39,11 +39,14 @@ public void merge(final List> values) { if (null == values || null == values.get(0)) { return; } - this.values.addAll(Arrays.asList(values.get(0).toString().split(separator))); + this.values.add(values.get(0)); } @Override public Comparable getResult() { - return String.join(separator, values); + if (null == separator) { + separator = DEFAULT_SEPARATOR; + } + return values.stream().map(Object::toString).collect(Collectors.joining(separator)); } } diff --git a/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java b/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java index 8f2e7c36cb856..69bf39f65f2e9 100644 --- a/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java +++ b/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java @@ -990,10 +990,6 @@ protected Collection getExpressions(final List e } private String getDistinctExpression(final AggregationFunctionContext ctx) { - StringBuilder result = new StringBuilder(); - for (int i = 3; i < ctx.getChildCount() - 1; i++) { - result.append(ctx.getChild(i).getText()); - } return ctx.aggregationExpression().getText(); } From 8feb64476e9dbdb01d6ece01c0df4b91b0029aa0 Mon Sep 17 00:00:00 2001 From: yaofly Date: Wed, 27 Nov 2024 16:56:00 +0800 Subject: [PATCH 09/31] unit test fix for distinct group_concat (#33797) --- .../dql/groupby/aggregation/GroupConcatAggregationUnitTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnitTest.java b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnitTest.java index 0fc69c75dafea..24b6e08550011 100644 --- a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnitTest.java +++ b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnitTest.java @@ -45,7 +45,7 @@ void assertDistinctGroupConcatAggregation() { distinctGroupConcatAggregationUnit.merge(Collections.singletonList("")); distinctGroupConcatAggregationUnit.merge(Collections.singletonList("001")); distinctGroupConcatAggregationUnit.merge(Collections.singletonList("001")); - distinctGroupConcatAggregationUnit.merge(Collections.singletonList("001 003")); + distinctGroupConcatAggregationUnit.merge(Collections.singletonList("003")); assertThat(distinctGroupConcatAggregationUnit.getResult(), is(" 001 003")); } } From f0b2f9c49dc4fbd07385d0af2f89b224bea761dc Mon Sep 17 00:00:00 2001 From: yaofly Date: Mon, 2 Dec 2024 10:16:16 +0800 Subject: [PATCH 10/31] e2e test for group_concat (#33797) --- .../DistinctGroupConcatAggregationUnit.java | 4 ++-- .../cases/dql/e2e-dql-select-aggregate.xml | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java index 71c4f240681bc..4a886fd3a4aaa 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java @@ -18,7 +18,7 @@ package org.apache.shardingsphere.sharding.merge.dql.groupby.aggregation; import java.util.Collection; -import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.stream.Collectors; @@ -26,7 +26,7 @@ public class DistinctGroupConcatAggregationUnit implements AggregationUnit { private static final String DEFAULT_SEPARATOR = ","; - private final Collection> values = new HashSet<>(); + private final Collection> values = new LinkedHashSet<>(); private String separator; diff --git a/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml b/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml index f8c39535f1f10..561babe7592ed 100644 --- a/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml +++ b/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml @@ -144,4 +144,20 @@ + + + + + + + + + + + + + + + + From 4b028ec82036ca3c1e4b08a4c83eac0b0a32eef9 Mon Sep 17 00:00:00 2001 From: yaofly Date: Mon, 2 Dec 2024 14:37:33 +0800 Subject: [PATCH 11/31] e2e test for group_concat (#33797) --- .../src/main/antlr4/imports/mysql/BaseRule.g4 | 2 +- .../dql/dataset/db/select_group_concat.xml | 28 +++++++ .../db/select_group_concat_distinct.xml | 28 +++++++ .../dql/dataset/tbl/select_group_concat.xml | 28 +++++++ .../tbl/select_group_concat_distinct.xml | 28 +++++++ .../cases/dql/e2e-dql-select-aggregate.xml | 74 +++++++++---------- 6 files changed, 146 insertions(+), 42 deletions(-) create mode 100644 test/e2e/sql/src/test/resources/cases/dql/dataset/db/select_group_concat.xml create mode 100644 test/e2e/sql/src/test/resources/cases/dql/dataset/db/select_group_concat_distinct.xml create mode 100644 test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat.xml create mode 100644 test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat_distinct.xml diff --git a/parser/sql/dialect/mysql/src/main/antlr4/imports/mysql/BaseRule.g4 b/parser/sql/dialect/mysql/src/main/antlr4/imports/mysql/BaseRule.g4 index b9f00332dc84d..41771c61adee3 100644 --- a/parser/sql/dialect/mysql/src/main/antlr4/imports/mysql/BaseRule.g4 +++ b/parser/sql/dialect/mysql/src/main/antlr4/imports/mysql/BaseRule.g4 @@ -971,7 +971,7 @@ aggregationExpression ; aggregationFunction - : aggregationFunctionName LP_ distinct? aggregationExpression? collateClause? orderByClause? separatorName? RP_ overClause? + : aggregationFunctionName LP_ distinct? aggregationExpression? collateClause? separatorName? RP_ overClause? ; jsonFunction diff --git a/test/e2e/sql/src/test/resources/cases/dql/dataset/db/select_group_concat.xml b/test/e2e/sql/src/test/resources/cases/dql/dataset/db/select_group_concat.xml new file mode 100644 index 0000000000000..2a4c131253b77 --- /dev/null +++ b/test/e2e/sql/src/test/resources/cases/dql/dataset/db/select_group_concat.xml @@ -0,0 +1,28 @@ + + + + + + + + + + + + + diff --git a/test/e2e/sql/src/test/resources/cases/dql/dataset/db/select_group_concat_distinct.xml b/test/e2e/sql/src/test/resources/cases/dql/dataset/db/select_group_concat_distinct.xml new file mode 100644 index 0000000000000..8f2599d98ccbc --- /dev/null +++ b/test/e2e/sql/src/test/resources/cases/dql/dataset/db/select_group_concat_distinct.xml @@ -0,0 +1,28 @@ + + + + + + + + + + + + + diff --git a/test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat.xml b/test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat.xml new file mode 100644 index 0000000000000..2a4c131253b77 --- /dev/null +++ b/test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat.xml @@ -0,0 +1,28 @@ + + + + + + + + + + + + + diff --git a/test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat_distinct.xml b/test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat_distinct.xml new file mode 100644 index 0000000000000..8f2599d98ccbc --- /dev/null +++ b/test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat_distinct.xml @@ -0,0 +1,28 @@ + + + + + + + + + + + + + diff --git a/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml b/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml index 561babe7592ed..07aeb538fdf8d 100644 --- a/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml +++ b/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml @@ -20,123 +20,123 @@ - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + @@ -145,19 +145,11 @@ - - - - - - - - - - + + - - + + From 91278c41c14efcc501cc3f6e2db2c19f31cd5e3f Mon Sep 17 00:00:00 2001 From: yaofly Date: Mon, 2 Dec 2024 15:38:32 +0800 Subject: [PATCH 12/31] code format (#33797) --- .../src/test/resources/cases/dql/e2e-dql-select-aggregate.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml b/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml index 07aeb538fdf8d..5a75143b5eb51 100644 --- a/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml +++ b/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml @@ -145,11 +145,11 @@ - + - + From 1f9a97f0157ec94ed209f7881a8fa01d6343d420 Mon Sep 17 00:00:00 2001 From: yaofly Date: Mon, 2 Dec 2024 16:08:43 +0800 Subject: [PATCH 13/31] e2e test (#33797) --- .../dataset/{db => }/select_group_concat.xml | 7 +- .../{db => }/select_group_concat_distinct.xml | 7 +- .../dql/dataset/tbl/select_group_concat.xml | 28 ---- .../tbl/select_group_concat_distinct.xml | 28 ---- .../cases/dql/e2e-dql-select-aggregate.xml | 131 +----------------- 5 files changed, 4 insertions(+), 197 deletions(-) rename test/e2e/sql/src/test/resources/cases/dql/dataset/{db => }/select_group_concat.xml (81%) rename test/e2e/sql/src/test/resources/cases/dql/dataset/{db => }/select_group_concat_distinct.xml (80%) delete mode 100644 test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat.xml delete mode 100644 test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat_distinct.xml diff --git a/test/e2e/sql/src/test/resources/cases/dql/dataset/db/select_group_concat.xml b/test/e2e/sql/src/test/resources/cases/dql/dataset/select_group_concat.xml similarity index 81% rename from test/e2e/sql/src/test/resources/cases/dql/dataset/db/select_group_concat.xml rename to test/e2e/sql/src/test/resources/cases/dql/dataset/select_group_concat.xml index 2a4c131253b77..95e901eb4f182 100644 --- a/test/e2e/sql/src/test/resources/cases/dql/dataset/db/select_group_concat.xml +++ b/test/e2e/sql/src/test/resources/cases/dql/dataset/select_group_concat.xml @@ -17,12 +17,7 @@ - - - - - - + diff --git a/test/e2e/sql/src/test/resources/cases/dql/dataset/db/select_group_concat_distinct.xml b/test/e2e/sql/src/test/resources/cases/dql/dataset/select_group_concat_distinct.xml similarity index 80% rename from test/e2e/sql/src/test/resources/cases/dql/dataset/db/select_group_concat_distinct.xml rename to test/e2e/sql/src/test/resources/cases/dql/dataset/select_group_concat_distinct.xml index 8f2599d98ccbc..752f7b7e7c19f 100644 --- a/test/e2e/sql/src/test/resources/cases/dql/dataset/db/select_group_concat_distinct.xml +++ b/test/e2e/sql/src/test/resources/cases/dql/dataset/select_group_concat_distinct.xml @@ -17,12 +17,7 @@ - - - - - - + diff --git a/test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat.xml b/test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat.xml deleted file mode 100644 index 2a4c131253b77..0000000000000 --- a/test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat.xml +++ /dev/null @@ -1,28 +0,0 @@ - - - - - - - - - - - - - diff --git a/test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat_distinct.xml b/test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat_distinct.xml deleted file mode 100644 index 8f2599d98ccbc..0000000000000 --- a/test/e2e/sql/src/test/resources/cases/dql/dataset/tbl/select_group_concat_distinct.xml +++ /dev/null @@ -1,28 +0,0 @@ - - - - - - - - - - - - - diff --git a/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml b/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml index 5a75143b5eb51..9da81abfe7bc0 100644 --- a/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml +++ b/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml @@ -17,139 +17,12 @@ --> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + - + From c2411b800c772b96567be44425f41144b42dfeb8 Mon Sep 17 00:00:00 2001 From: yaofly Date: Mon, 2 Dec 2024 16:10:34 +0800 Subject: [PATCH 14/31] e2e test (#33797) --- .../cases/dql/e2e-dql-select-aggregate.xml | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml b/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml index 9da81abfe7bc0..5e52dbee25574 100644 --- a/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml +++ b/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml @@ -17,6 +17,133 @@ --> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From ded37459047e4776da975156234a1212345aa7a2 Mon Sep 17 00:00:00 2001 From: yaofly Date: Mon, 2 Dec 2024 17:07:46 +0800 Subject: [PATCH 15/31] e2e test (#33797) --- .../test/resources/cases/dql/e2e-dql-select-aggregate.xml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml b/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml index 5e52dbee25574..f4bd5b4809205 100644 --- a/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml +++ b/test/e2e/sql/src/test/resources/cases/dql/e2e-dql-select-aggregate.xml @@ -145,11 +145,11 @@ - - + + - - + + From c080d714de1868103e68ddc1b902245025f027dd Mon Sep 17 00:00:00 2001 From: yaofly Date: Mon, 2 Dec 2024 17:10:19 +0800 Subject: [PATCH 16/31] remove useless code(#33797) --- .../cases/dql/dataset/select_group_concat.xml | 23 ------------------- .../dataset/select_group_concat_distinct.xml | 23 ------------------- 2 files changed, 46 deletions(-) delete mode 100644 test/e2e/sql/src/test/resources/cases/dql/dataset/select_group_concat.xml delete mode 100644 test/e2e/sql/src/test/resources/cases/dql/dataset/select_group_concat_distinct.xml diff --git a/test/e2e/sql/src/test/resources/cases/dql/dataset/select_group_concat.xml b/test/e2e/sql/src/test/resources/cases/dql/dataset/select_group_concat.xml deleted file mode 100644 index 95e901eb4f182..0000000000000 --- a/test/e2e/sql/src/test/resources/cases/dql/dataset/select_group_concat.xml +++ /dev/null @@ -1,23 +0,0 @@ - - - - - - - - diff --git a/test/e2e/sql/src/test/resources/cases/dql/dataset/select_group_concat_distinct.xml b/test/e2e/sql/src/test/resources/cases/dql/dataset/select_group_concat_distinct.xml deleted file mode 100644 index 752f7b7e7c19f..0000000000000 --- a/test/e2e/sql/src/test/resources/cases/dql/dataset/select_group_concat_distinct.xml +++ /dev/null @@ -1,23 +0,0 @@ - - - - - - - - From c96dd07a25f6f5aace80ffc228c2e0f3d5425c5f Mon Sep 17 00:00:00 2001 From: yaofly Date: Wed, 4 Dec 2024 09:49:09 +0800 Subject: [PATCH 17/31] code optimization (#33797) --- .../aggregation/AggregationUnitFactory.java | 12 --------- .../DistinctGroupConcatAggregationUnit.java | 19 +++++++------- .../GroupConcatAggregationUnit.java | 22 +++++++--------- .../AggregationUnitFactoryTest.java | 26 ++++++++++++------- .../statement/MySQLStatementVisitor.java | 4 +-- .../resources/case/dml/select-aggregate.xml | 24 +++++++++++++++++ .../sql/supported/dml/select-aggregate.xml | 2 ++ 7 files changed, 62 insertions(+), 47 deletions(-) diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactory.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactory.java index 9e0a1e36720e9..02caeecf6c103 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactory.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactory.java @@ -28,18 +28,6 @@ @NoArgsConstructor(access = AccessLevel.PRIVATE) public final class AggregationUnitFactory { - /** - * Create aggregation unit instance. - * - * @param type aggregation function type - * @param isDistinct is distinct - * @return aggregation unit instance - * @throws UnsupportedSQLOperationException unsupported SQL operation exception - */ - public static AggregationUnit create(final AggregationType type, final boolean isDistinct) { - return create(type, isDistinct, null); - } - /** * Create aggregation unit instance. * diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java index 4a886fd3a4aaa..93a19c21d6999 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/DistinctGroupConcatAggregationUnit.java @@ -20,18 +20,20 @@ import java.util.Collection; import java.util.LinkedHashSet; import java.util.List; -import java.util.stream.Collectors; -public class DistinctGroupConcatAggregationUnit implements AggregationUnit { +/** + * Distinct group concat aggregation unit. + */ +public final class DistinctGroupConcatAggregationUnit implements AggregationUnit { private static final String DEFAULT_SEPARATOR = ","; - private final Collection> values = new LinkedHashSet<>(); + private final Collection values = new LinkedHashSet<>(); - private String separator; + private final String separator; public DistinctGroupConcatAggregationUnit(final String separator) { - this.separator = separator; + this.separator = null == separator ? DEFAULT_SEPARATOR : separator; } @Override @@ -39,14 +41,11 @@ public void merge(final List> values) { if (null == values || null == values.get(0)) { return; } - this.values.add(values.get(0)); + this.values.add(String.valueOf(values.get(0))); } @Override public Comparable getResult() { - if (null == separator) { - separator = DEFAULT_SEPARATOR; - } - return values.stream().map(Object::toString).collect(Collectors.joining(separator)); + return String.join(separator, values); } } diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java index c2fcccfc23b4f..c3e1796106d15 100644 --- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java +++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/GroupConcatAggregationUnit.java @@ -17,24 +17,23 @@ package org.apache.shardingsphere.sharding.merge.dql.groupby.aggregation; -import lombok.NoArgsConstructor; - import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.stream.Collectors; -@NoArgsConstructor -public class GroupConcatAggregationUnit implements AggregationUnit { +/** + * Group concat aggregation unit. + */ +public final class GroupConcatAggregationUnit implements AggregationUnit { private static final String DEFAULT_SEPARATOR = ","; - private final Collection> values = new ArrayList<>(); + private final Collection values = new ArrayList<>(); - private String separator; + private final String separator; public GroupConcatAggregationUnit(final String separator) { - this.separator = separator; + this.separator = null == separator ? DEFAULT_SEPARATOR : separator; } @Override @@ -42,14 +41,11 @@ public void merge(final List> values) { if (null == values || null == values.get(0)) { return; } - this.values.add(values.get(0)); + this.values.add(String.valueOf(values.get(0))); } @Override public Comparable getResult() { - if (null == separator) { - separator = DEFAULT_SEPARATOR; - } - return values.stream().map(Object::toString).collect(Collectors.joining(separator)); + return String.join(separator, values); } } diff --git a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactoryTest.java b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactoryTest.java index 3aa5efc70ff63..92bc71a5fa377 100644 --- a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactoryTest.java +++ b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/groupby/aggregation/AggregationUnitFactoryTest.java @@ -27,44 +27,50 @@ class AggregationUnitFactoryTest { @Test void assertCreateComparableAggregationUnit() { - assertThat(AggregationUnitFactory.create(AggregationType.MIN, false), instanceOf(ComparableAggregationUnit.class)); - assertThat(AggregationUnitFactory.create(AggregationType.MAX, false), instanceOf(ComparableAggregationUnit.class)); + assertThat(AggregationUnitFactory.create(AggregationType.MIN, false, null), instanceOf(ComparableAggregationUnit.class)); + assertThat(AggregationUnitFactory.create(AggregationType.MAX, false, null), instanceOf(ComparableAggregationUnit.class)); } @Test void assertCreateAccumulationAggregationUnit() { - assertThat(AggregationUnitFactory.create(AggregationType.SUM, false), instanceOf(AccumulationAggregationUnit.class)); - assertThat(AggregationUnitFactory.create(AggregationType.COUNT, false), instanceOf(AccumulationAggregationUnit.class)); + assertThat(AggregationUnitFactory.create(AggregationType.SUM, false, null), instanceOf(AccumulationAggregationUnit.class)); + assertThat(AggregationUnitFactory.create(AggregationType.COUNT, false, null), instanceOf(AccumulationAggregationUnit.class)); } @Test void assertCreateAverageAggregationUnit() { - assertThat(AggregationUnitFactory.create(AggregationType.AVG, false), instanceOf(AverageAggregationUnit.class)); + assertThat(AggregationUnitFactory.create(AggregationType.AVG, false, null), instanceOf(AverageAggregationUnit.class)); } @Test void assertCreateDistinctSumAggregationUnit() { - assertThat(AggregationUnitFactory.create(AggregationType.SUM, true), instanceOf(DistinctSumAggregationUnit.class)); + assertThat(AggregationUnitFactory.create(AggregationType.SUM, true, null), instanceOf(DistinctSumAggregationUnit.class)); } @Test void assertCreateDistinctCountAggregationUnit() { - assertThat(AggregationUnitFactory.create(AggregationType.COUNT, true), instanceOf(DistinctCountAggregationUnit.class)); + assertThat(AggregationUnitFactory.create(AggregationType.COUNT, true, null), instanceOf(DistinctCountAggregationUnit.class)); } @Test void assertCreateDistinctAverageAggregationUnit() { - assertThat(AggregationUnitFactory.create(AggregationType.AVG, true), instanceOf(DistinctAverageAggregationUnit.class)); + assertThat(AggregationUnitFactory.create(AggregationType.AVG, true, null), instanceOf(DistinctAverageAggregationUnit.class)); } @Test void assertCreateBitXorAggregationUnit() { - assertThat(AggregationUnitFactory.create(AggregationType.BIT_XOR, false), instanceOf(BitXorAggregationUnit.class)); + assertThat(AggregationUnitFactory.create(AggregationType.BIT_XOR, false, null), instanceOf(BitXorAggregationUnit.class)); } @Test void assertGroupConcatAggregationUnit() { - assertThat(AggregationUnitFactory.create(AggregationType.GROUP_CONCAT, true), instanceOf(DistinctGroupConcatAggregationUnit.class)); + assertThat(AggregationUnitFactory.create(AggregationType.GROUP_CONCAT, false, null), instanceOf(GroupConcatAggregationUnit.class)); + assertThat(AggregationUnitFactory.create(AggregationType.GROUP_CONCAT, false, " "), instanceOf(GroupConcatAggregationUnit.class)); + } + + @Test + void assertDistinctGroupConcatAggregationUnit() { + assertThat(AggregationUnitFactory.create(AggregationType.GROUP_CONCAT, true, null), instanceOf(DistinctGroupConcatAggregationUnit.class)); assertThat(AggregationUnitFactory.create(AggregationType.GROUP_CONCAT, true, " "), instanceOf(DistinctGroupConcatAggregationUnit.class)); } } diff --git a/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java b/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java index e981b246b4e4d..6ab433e59a16c 100644 --- a/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java +++ b/parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java @@ -25,7 +25,7 @@ import org.antlr.v4.runtime.tree.TerminalNode; import org.apache.shardingsphere.sql.parser.api.ASTNode; import org.apache.shardingsphere.sql.parser.autogen.MySQLStatementBaseVisitor; -import org.apache.shardingsphere.sql.parser.autogen.MySQLStatementParser; +import org.apache.shardingsphere.sql.parser.autogen.MySQLStatementParser.SeparatorNameContext; import org.apache.shardingsphere.sql.parser.autogen.MySQLStatementParser.AggregationFunctionContext; import org.apache.shardingsphere.sql.parser.autogen.MySQLStatementParser.AliasContext; import org.apache.shardingsphere.sql.parser.autogen.MySQLStatementParser.AssignmentContext; @@ -918,7 +918,7 @@ public final ASTNode visitAggregationFunction(final AggregationFunctionContext c } @Override - public ASTNode visitSeparatorName(final MySQLStatementParser.SeparatorNameContext ctx) { + public ASTNode visitSeparatorName(final SeparatorNameContext ctx) { return new StringLiteralValue(ctx.string_().getText()); } diff --git a/test/it/parser/src/main/resources/case/dml/select-aggregate.xml b/test/it/parser/src/main/resources/case/dml/select-aggregate.xml index 50d7b5bd00fb5..f49c0620dbc83 100644 --- a/test/it/parser/src/main/resources/case/dml/select-aggregate.xml +++ b/test/it/parser/src/main/resources/case/dml/select-aggregate.xml @@ -518,4 +518,28 @@ + + diff --git a/test/it/parser/src/main/resources/sql/supported/dml/select-aggregate.xml b/test/it/parser/src/main/resources/sql/supported/dml/select-aggregate.xml index 583ad04884737..a419612760697 100644 --- a/test/it/parser/src/main/resources/sql/supported/dml/select-aggregate.xml +++ b/test/it/parser/src/main/resources/sql/supported/dml/select-aggregate.xml @@ -38,4 +38,6 @@ + + From 576cc33a30ae7ffcf29b0f80e5194c4bdb9e3f25 Mon Sep 17 00:00:00 2001 From: yaofly Date: Wed, 4 Dec 2024 12:02:55 +0800 Subject: [PATCH 18/31] sql parse unit test (#33797) --- .../asserts/segment/projection/ProjectionAssert.java | 1 + .../aggregation/ExpectedAggregationProjection.java | 3 +++ .../src/main/resources/case/dml/select-aggregate.xml | 11 ++++++----- .../resources/case/dml/select-special-function.xml | 12 ++++++------ .../sql/supported/dml/select-special-function.xml | 2 +- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/test/it/parser/src/main/java/org/apache/shardingsphere/test/it/sql/parser/internal/asserts/segment/projection/ProjectionAssert.java b/test/it/parser/src/main/java/org/apache/shardingsphere/test/it/sql/parser/internal/asserts/segment/projection/ProjectionAssert.java index 7a8fbe2a079d5..9efc1acba3885 100644 --- a/test/it/parser/src/main/java/org/apache/shardingsphere/test/it/sql/parser/internal/asserts/segment/projection/ProjectionAssert.java +++ b/test/it/parser/src/main/java/org/apache/shardingsphere/test/it/sql/parser/internal/asserts/segment/projection/ProjectionAssert.java @@ -169,6 +169,7 @@ private static void assertAggregationProjection(final SQLCaseAssertContext asser assertThat(assertContext.getText("Aggregation projection type assertion error: "), actual.getType().name(), is(expected.getType())); assertThat(assertContext.getText("Aggregation projection inner expression assertion error: "), actual.getExpression(), is(expected.getExpression())); assertThat(assertContext.getText("Aggregation projection alias assertion error: "), actual.getAliasName().orElse(null), is(expected.getAlias())); + assertThat(assertContext.getText("Aggregation projection separator assertion error: "), actual.getSeparator(), is(expected.getSeparator())); if (actual instanceof AggregationDistinctProjectionSegment) { assertThat(assertContext.getText("Projection type assertion error: "), expected, instanceOf(ExpectedAggregationDistinctProjection.class)); assertThat(assertContext.getText("Aggregation projection distinct inner expression assertion error: "), diff --git a/test/it/parser/src/main/java/org/apache/shardingsphere/test/it/sql/parser/internal/cases/parser/jaxb/segment/impl/projection/impl/aggregation/ExpectedAggregationProjection.java b/test/it/parser/src/main/java/org/apache/shardingsphere/test/it/sql/parser/internal/cases/parser/jaxb/segment/impl/projection/impl/aggregation/ExpectedAggregationProjection.java index 2cf8da6f4229d..384048fa74cca 100644 --- a/test/it/parser/src/main/java/org/apache/shardingsphere/test/it/sql/parser/internal/cases/parser/jaxb/segment/impl/projection/impl/aggregation/ExpectedAggregationProjection.java +++ b/test/it/parser/src/main/java/org/apache/shardingsphere/test/it/sql/parser/internal/cases/parser/jaxb/segment/impl/projection/impl/aggregation/ExpectedAggregationProjection.java @@ -44,6 +44,9 @@ public class ExpectedAggregationProjection extends AbstractExpectedSQLSegment im @XmlAttribute private String alias; + @XmlAttribute + private String separator; + @XmlElement(name = "parameters") private final List parameters = new LinkedList<>(); } diff --git a/test/it/parser/src/main/resources/case/dml/select-aggregate.xml b/test/it/parser/src/main/resources/case/dml/select-aggregate.xml index f49c0620dbc83..08dd5ab86853d 100644 --- a/test/it/parser/src/main/resources/case/dml/select-aggregate.xml +++ b/test/it/parser/src/main/resources/case/dml/select-aggregate.xml @@ -520,26 +520,27 @@ diff --git a/test/it/parser/src/main/resources/case/dml/select-special-function.xml b/test/it/parser/src/main/resources/case/dml/select-special-function.xml index ac2e2441214b6..102a9540647e6 100644 --- a/test/it/parser/src/main/resources/case/dml/select-special-function.xml +++ b/test/it/parser/src/main/resources/case/dml/select-special-function.xml @@ -17,20 +17,20 @@ --> - - + - - + + - + - +