This is an automated email from the ASF dual-hosted git repository.
terrymanu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git
The following commit(s) were added to refs/heads/master by this push:
new 671eb2ce611 Fix duplicate rows in cross-shard SELECT DISTINCT when
NULLs sort first (#40002)
671eb2ce611 is described below
commit 671eb2ce611d71fa9af1cd861cae7016d4534a8a
Author: Hariom Lokhande
<[email protected]>
AuthorDate: Sat Oct 10 21:40:23 2026 +0530
Fix duplicate rows in cross-shard SELECT DISTINCT when NULLs sort first
(#40002)
* Fix duplicate rows in cross-shard SELECT DISTINCT when NULLs sort first
* Add DISTINCT merge regression tests for NULLS FIRST and NULLS LAST
dialects
* Apply wrapping, collection and helper order rules to DISTINCT merge test
* Re-run CI after the CheckStyle runner failed to download Maven
* Resolve dialect NULL order for DISTINCT group items in result merger
The GROUP BY items added for DISTINCT kept the unresolved dialect default
(LOW for MySQL), so the memory merge sorted NULL last when the DISTINCT
projections derive no ORDER BY items.
Co-authored-by: HhH666-ux <[email protected]>
* Re-run CI after the E2E runner failed to download build artifacts
---------
Co-authored-by: HhH666-ux <[email protected]>
---
RELEASE-NOTES.md | 1 +
features/sharding/core/pom.xml | 12 ++++
.../merge/dql/ShardingDQLResultMerger.java | 7 +-
.../merge/dql/ShardingDQLResultMergerTest.java | 74 ++++++++++++++++++++++
.../orderby/engine/OrderByContextEngine.java | 7 +-
.../orderby/engine/OrderByContextEngineTest.java | 2 +
6 files changed, 96 insertions(+), 7 deletions(-)
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
index 975bd92f717..53c514807a9 100644
--- a/RELEASE-NOTES.md
+++ b/RELEASE-NOTES.md
@@ -119,6 +119,7 @@
1. Shadow: Apply default shadow algorithm to shadow tables when swapping YAML
rule configuration -
[#39749](https://github.com/apache/shardingsphere/pull/39749)
1. Shadow: Fix INSERT SELECT statement being routed to shadow data source -
[#39751](https://github.com/apache/shardingsphere/pull/39751)
1. Sharding: Fix logic table lookup for MySQL SHOW CREATE TABLE and SHOW INDEX
- [#39856](https://github.com/apache/shardingsphere/pull/39856)
+1. Sharding: Fix duplicate rows in cross-shard SELECT DISTINCT when NULLs sort
first by default -
[#40001](https://github.com/apache/shardingsphere/issues/40001)
### Enhancements
diff --git a/features/sharding/core/pom.xml b/features/sharding/core/pom.xml
index f8a55ebe011..4245824fd64 100644
--- a/features/sharding/core/pom.xml
+++ b/features/sharding/core/pom.xml
@@ -127,6 +127,18 @@
<version>${project.version}</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.shardingsphere</groupId>
+
<artifactId>shardingsphere-parser-sql-engine-clickhouse</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.shardingsphere</groupId>
+ <artifactId>shardingsphere-parser-sql-engine-presto</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
<dependency>
<groupId>org.apache.shardingsphere</groupId>
<artifactId>shardingsphere-test-it-yaml</artifactId>
diff --git
a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/ShardingDQLResultMerger.java
b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/ShardingDQLResultMerger.java
index 39cfd6bca76..7437d4c6e0f 100644
---
a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/ShardingDQLResultMerger.java
+++
b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/merge/dql/ShardingDQLResultMerger.java
@@ -19,7 +19,7 @@ package org.apache.shardingsphere.sharding.merge.dql;
import com.cedarsoftware.util.CaseInsensitiveMap;
import lombok.RequiredArgsConstructor;
-import
org.apache.shardingsphere.database.connector.core.metadata.database.metadata.DialectDatabaseMetaData;
+import
org.apache.shardingsphere.database.connector.core.metadata.database.enums.NullsOrderType;
import
org.apache.shardingsphere.database.connector.core.spi.DatabaseTypedSPILoader;
import org.apache.shardingsphere.database.connector.core.type.DatabaseType;
import
org.apache.shardingsphere.database.connector.core.type.DatabaseTypeRegistry;
@@ -106,9 +106,10 @@ public final class ShardingDQLResultMerger implements
ResultMerger {
}
private void setGroupByForDistinctRow(final SelectStatementContext
selectStatementContext) {
- DialectDatabaseMetaData dialectDatabaseMetaData = new
DatabaseTypeRegistry(selectStatementContext.getSqlStatement().getDatabaseType()).getDialectDatabaseMetaData();
+ NullsOrderType nullsOrderType = new
DatabaseTypeRegistry(selectStatementContext.getSqlStatement().getDatabaseType()).getDialectDatabaseMetaData().getDefaultNullsOrderType()
+ .getResolvedOrderType(OrderDirection.ASC.name());
for (int index = 1; index <=
selectStatementContext.getProjectionsContext().getExpandProjections().size();
index++) {
- OrderByItem orderByItem = new OrderByItem(new
IndexOrderByItemSegment(-1, -1, index, OrderDirection.ASC,
dialectDatabaseMetaData.getDefaultNullsOrderType()));
+ OrderByItem orderByItem = new OrderByItem(new
IndexOrderByItemSegment(-1, -1, index, OrderDirection.ASC, nullsOrderType));
orderByItem.setIndex(index);
selectStatementContext.getGroupByContext().getItems().add(orderByItem);
}
diff --git
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/ShardingDQLResultMergerTest.java
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/ShardingDQLResultMergerTest.java
index 2353806c75d..bcd547a2d8e 100644
---
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/ShardingDQLResultMergerTest.java
+++
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/merge/dql/ShardingDQLResultMergerTest.java
@@ -19,6 +19,7 @@ package org.apache.shardingsphere.sharding.merge.dql;
import
org.apache.shardingsphere.database.connector.core.metadata.database.enums.NullsOrderType;
import org.apache.shardingsphere.database.connector.core.type.DatabaseType;
+import
org.apache.shardingsphere.database.connector.core.type.DatabaseTypeRegistry;
import
org.apache.shardingsphere.infra.binder.context.statement.type.dml.SelectStatementContext;
import org.apache.shardingsphere.infra.config.props.ConfigurationProperties;
import
org.apache.shardingsphere.infra.executor.sql.execute.result.query.QueryResult;
@@ -46,6 +47,8 @@ import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.simp
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.subquery.SubquerySegment;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.item.AggregationProjectionSegment;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.item.ColumnProjectionSegment;
+import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.item.ExpressionProjectionSegment;
+import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.item.ProjectionSegment;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.item.ProjectionsSegment;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.item.ShorthandProjectionSegment;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.order.GroupBySegment;
@@ -62,15 +65,22 @@ import
org.apache.shardingsphere.sql.parser.statement.core.segment.generic.table
import
org.apache.shardingsphere.sql.parser.statement.core.statement.type.dml.SelectStatement;
import
org.apache.shardingsphere.sql.parser.statement.core.value.identifier.IdentifierValue;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.MockedConstruction;
import java.sql.SQLException;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
+import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Optional;
import java.util.Properties;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Stream;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
@@ -316,6 +326,70 @@ class ShardingDQLResultMergerTest {
assertThat(resultMerger.merge(createQueryResults(),
selectStatementContext, createDatabase(), mock(ConnectionContext.class)),
isA(GroupByStreamMergedResult.class));
}
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("assertMergeDistinctRowWithNullValuesArguments")
+ void assertMergeDistinctRowWithNullValues(final String name, final String
databaseTypeName, final Collection<Object> firstShardValues, final
Collection<Object> secondShardValues,
+ final List<Object>
expectedValues) throws SQLException {
+ ColumnProjectionSegment projection = new ColumnProjectionSegment(new
ColumnSegment(0, 0, new IdentifierValue("col1")));
+ assertThat(getMergedDistinctRowValues(databaseTypeName, projection,
firstShardValues, secondShardValues), is(expectedValues));
+ }
+
+ private List<Object> getMergedDistinctRowValues(final String
databaseTypeName, final ProjectionSegment projection, final Collection<Object>
firstShardValues,
+ final Collection<Object>
secondShardValues) throws SQLException {
+ DatabaseType databaseType =
TypedSPILoader.getService(DatabaseType.class, databaseTypeName);
+ ProjectionsSegment projectionsSegment = new ProjectionsSegment(0, 0);
+ projectionsSegment.setDistinctRow(true);
+ projectionsSegment.getProjections().add(projection);
+ SelectStatement selectStatement =
withProjections(buildSelectStatement(databaseType), projectionsSegment);
+ ShardingSphereDatabase database = mock(ShardingSphereDatabase.class,
RETURNS_DEEP_STUBS);
+ SelectStatementContext selectStatementContext = new
SelectStatementContext(selectStatement, createShardingSphereMetaData(database),
"foo_db", Collections.emptyList());
+ MergedResult mergedResult = new
ShardingDQLResultMerger(databaseType).merge(Arrays.asList(createSingleColumnQueryResult(firstShardValues),
createSingleColumnQueryResult(secondShardValues)),
+ selectStatementContext, createDialectDatabase(databaseType),
mock(ConnectionContext.class));
+ List<Object> result = new LinkedList<>();
+ while (mergedResult.next()) {
+ result.add(mergedResult.getValue(1, Object.class));
+ }
+ return result;
+ }
+
+ private QueryResult createSingleColumnQueryResult(final Collection<Object>
values) throws SQLException {
+ QueryResult result = mock(QueryResult.class, RETURNS_DEEP_STUBS);
+ when(result.getMetaData().getColumnCount()).thenReturn(1);
+ when(result.getMetaData().getColumnLabel(1)).thenReturn("col1");
+ Iterator<Object> iterator = values.iterator();
+ AtomicReference<Object> currentValue = new AtomicReference<>();
+ when(result.next()).thenAnswer(invocation -> {
+ if (!iterator.hasNext()) {
+ return false;
+ }
+ currentValue.set(iterator.next());
+ return true;
+ });
+ when(result.getValue(1, Object.class)).thenAnswer(invocation ->
currentValue.get());
+ return result;
+ }
+
+ private ShardingSphereDatabase createDialectDatabase(final DatabaseType
databaseType) {
+ String schemaName = new
DatabaseTypeRegistry(databaseType).getDialectDatabaseMetaData().getSchemaOption().getDefaultSchema().orElse("foo_db");
+ ShardingSphereSchema schema = new ShardingSphereSchema(schemaName,
databaseType, Collections.singleton(createTable()), Collections.emptyList());
+ return new ShardingSphereDatabase("foo_db", databaseType,
mock(ResourceMetaData.class), mock(RuleMetaData.class),
Collections.singleton(schema), new ConfigurationProperties(new Properties()));
+ }
+
+ private static Stream<Arguments>
assertMergeDistinctRowWithNullValuesArguments() {
+ return Stream.of(
+ Arguments.of("MySQL sorts NULL values first", "MySQL",
Arrays.asList(null, 5), Arrays.asList(null, 10), Arrays.asList(null, 5, 10)),
+ Arguments.of("ClickHouse sorts NULL values last",
"ClickHouse", Arrays.asList(5, null), Arrays.asList(10, null), Arrays.asList(5,
10, null)),
+ Arguments.of("Presto sorts NULL values last", "Presto",
Arrays.asList(5, null), Arrays.asList(10, null), Arrays.asList(5, 10, null)));
+ }
+
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("assertMergeDistinctRowWithNullValuesArguments")
+ void assertMergeDistinctExpressionWithNullValues(final String name, final
String databaseTypeName, final Collection<Object> firstShardValues, final
Collection<Object> secondShardValues,
+ final List<Object>
expectedValues) throws SQLException {
+ ExpressionProjectionSegment projection = new
ExpressionProjectionSegment(0, 0, "col1 + 1");
+ assertThat(getMergedDistinctRowValues(databaseTypeName, projection,
firstShardValues, secondShardValues), is(expectedValues));
+ }
+
@Test
void assertBuildGroupByStreamMergedResultWithMySQLLimit() throws
SQLException {
SelectStatement selectStatement =
buildSelectStatement(mysqlDatabaseType);
diff --git
a/infra/binder/core/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/orderby/engine/OrderByContextEngine.java
b/infra/binder/core/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/orderby/engine/OrderByContextEngine.java
index 1711fa1e110..b2f1621c58a 100644
---
a/infra/binder/core/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/orderby/engine/OrderByContextEngine.java
+++
b/infra/binder/core/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/orderby/engine/OrderByContextEngine.java
@@ -18,7 +18,7 @@
package
org.apache.shardingsphere.infra.binder.context.segment.select.orderby.engine;
import lombok.RequiredArgsConstructor;
-import
org.apache.shardingsphere.database.connector.core.metadata.database.metadata.DialectDatabaseMetaData;
+import
org.apache.shardingsphere.database.connector.core.metadata.database.enums.NullsOrderType;
import org.apache.shardingsphere.database.connector.core.type.DatabaseType;
import
org.apache.shardingsphere.database.connector.core.type.DatabaseTypeRegistry;
import
org.apache.shardingsphere.infra.binder.context.segment.select.groupby.GroupByContext;
@@ -70,12 +70,11 @@ public final class OrderByContextEngine {
if (groupByContext.getItems().isEmpty() &&
selectStatement.getProjections().isDistinctRow()) {
int index = 0;
List<OrderByItem> orderByItems = new LinkedList<>();
- DialectDatabaseMetaData dialectDatabaseMetaData = new
DatabaseTypeRegistry(databaseType).getDialectDatabaseMetaData();
+ NullsOrderType nullsOrderType = new
DatabaseTypeRegistry(databaseType).getDialectDatabaseMetaData().getDefaultNullsOrderType().getResolvedOrderType(OrderDirection.ASC.name());
for (ProjectionSegment projectionSegment :
selectStatement.getProjections().getProjections()) {
if (projectionSegment instanceof ColumnProjectionSegment) {
ColumnProjectionSegment columnProjectionSegment =
(ColumnProjectionSegment) projectionSegment;
- ColumnOrderByItemSegment columnOrderByItemSegment =
- new
ColumnOrderByItemSegment(columnProjectionSegment.getColumn(),
OrderDirection.ASC, dialectDatabaseMetaData.getDefaultNullsOrderType());
+ ColumnOrderByItemSegment columnOrderByItemSegment = new
ColumnOrderByItemSegment(columnProjectionSegment.getColumn(),
OrderDirection.ASC, nullsOrderType);
OrderByItem item = new
OrderByItem(columnOrderByItemSegment);
item.setIndex(index++);
orderByItems.add(item);
diff --git
a/infra/binder/core/src/test/java/org/apache/shardingsphere/infra/binder/context/segment/select/orderby/engine/OrderByContextEngineTest.java
b/infra/binder/core/src/test/java/org/apache/shardingsphere/infra/binder/context/segment/select/orderby/engine/OrderByContextEngineTest.java
index 9f1cf89bac7..044985e0745 100644
---
a/infra/binder/core/src/test/java/org/apache/shardingsphere/infra/binder/context/segment/select/orderby/engine/OrderByContextEngineTest.java
+++
b/infra/binder/core/src/test/java/org/apache/shardingsphere/infra/binder/context/segment/select/orderby/engine/OrderByContextEngineTest.java
@@ -95,6 +95,8 @@ class OrderByContextEngineTest {
List<OrderByItem> items = (List<OrderByItem>)
actualOrderByContext.getItems();
assertThat(((ColumnOrderByItemSegment)
items.get(0).getSegment()).getColumn(),
is(columnProjectionSegment1.getColumn()));
assertThat(((ColumnOrderByItemSegment)
items.get(1).getSegment()).getColumn(),
is(columnProjectionSegment2.getColumn()));
+ assertThat(items.get(0).getSegment().getNullsOrderType(databaseType),
is(NullsOrderType.FIRST));
+ assertThat(items.get(1).getSegment().getNullsOrderType(databaseType),
is(NullsOrderType.FIRST));
assertTrue(actualOrderByContext.isGenerated());
}
}
