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 39db8f78e08 Fix incorrect range routing for right-side sharding
columns (#39534)
39db8f78e08 is described below
commit 39db8f78e08f15e40e40b1dade1e6e242c4bec88
Author: Gimin Kim <[email protected]>
AuthorDate: Fri Oct 16 15:20:06 2026 +0900
Fix incorrect range routing for right-side sharding columns (#39534)
* Fix comparison routing with right-side sharding columns
* Add release note for right-side comparison routing fix
* Preserve broadcast routing for right-side binary columns
* Avoid range checks for equality routing
* Preserve broadcast routing for nested expressions
* Address review feedback on comparison routing
* Fix comparison generator blank-line formatting
---------
Co-authored-by: Gimin Kim <[email protected]>
---
RELEASE-NOTES.md | 1 +
.../ConditionValueCompareOperatorGenerator.java | 39 +++++++++++++++++++---
.../WhereClauseShardingConditionEngineTest.java | 23 +++++++++++++
...ConditionValueCompareOperatorGeneratorTest.java | 22 ++++++++++++
4 files changed, 80 insertions(+), 5 deletions(-)
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
index 0f60230e23a..7530cf7f48b 100644
--- a/RELEASE-NOTES.md
+++ b/RELEASE-NOTES.md
@@ -101,6 +101,7 @@
1. Sharding: Compute the Snowflake key generator epoch in UTC instead of the
JVM default timezone -
[#38932](https://github.com/apache/shardingsphere/pull/38932)
1. Sharding: Fix order-dependent data source intersection in Cartesian routing
- [#39407](https://github.com/apache/shardingsphere/pull/39407)
1. Sharding: Fix incorrect AVG(DISTINCT) merge result across shards -
[#39429](https://github.com/apache/shardingsphere/pull/39429)
+1. Sharding: Fix incorrect range routing for right-side sharding columns -
[#39534](https://github.com/apache/shardingsphere/pull/39534)
1. Sharding: Fix MySQL SHOW INDEX stripping table suffix from index names of
non-sharding tables -
[#39853](https://github.com/apache/shardingsphere/pull/39853)
1. Sharding: Fix NOT BETWEEN on sharding column being routed as BETWEEN -
[#39855](https://github.com/apache/shardingsphere/pull/39855)
1. Sharding: Use configured sharding column name when checking UPDATE
assignments - [#39864](https://github.com/apache/shardingsphere/pull/39864)
diff --git
a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueCompareOperatorGenerator.java
b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueCompareOperatorGenerator.java
index aedd7f2f223..87686becf8f 100644
---
a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueCompareOperatorGenerator.java
+++
b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueCompareOperatorGenerator.java
@@ -72,8 +72,17 @@ public final class ConditionValueCompareOperatorGenerator
implements ConditionVa
if (EQUAL.equals(operator)) {
left = unwrapBinaryOperator(left);
right = unwrapBinaryOperator(right);
+ } else if (isRangeOperator(operator) && (isBinaryOperator(left) ||
isBinaryOperator(right))) {
+ return Optional.empty();
+ }
+ boolean isLeftColumn = left instanceof ColumnSegment;
+ if (!isLeftColumn) {
+ if (!(right instanceof ColumnSegment)) {
+ return Optional.empty();
+ }
+ operator = reverseOperator(operator);
}
- ExpressionSegment valueExpression = left instanceof ColumnSegment ?
right : left;
+ ExpressionSegment valueExpression = isLeftColumn ? right : left;
ConditionValue conditionValue = new ConditionValue(valueExpression,
params);
if (conditionValue.isNull()) {
return generate(null, column, operator,
conditionValue.getParameterMarkerIndex().orElse(-1));
@@ -117,9 +126,29 @@ public final class ConditionValueCompareOperatorGenerator
implements ConditionVa
}
private ExpressionSegment unwrapBinaryOperator(final ExpressionSegment
segment) {
- return segment instanceof UnaryOperationExpression
- && "BINARY".equalsIgnoreCase(((UnaryOperationExpression)
segment).getOperator())
- ? ((UnaryOperationExpression) segment).getExpression()
- : segment;
+ return isBinaryOperator(segment) ? ((UnaryOperationExpression)
segment).getExpression() : segment;
+ }
+
+ private boolean isBinaryOperator(final ExpressionSegment segment) {
+ return segment instanceof UnaryOperationExpression &&
"BINARY".equalsIgnoreCase(((UnaryOperationExpression) segment).getOperator());
+ }
+
+ private boolean isRangeOperator(final String operator) {
+ return GREATER_THAN.equals(operator) || LESS_THAN.equals(operator) ||
AT_MOST.equals(operator) || AT_LEAST.equals(operator);
+ }
+
+ private String reverseOperator(final String operator) {
+ switch (operator) {
+ case GREATER_THAN:
+ return LESS_THAN;
+ case LESS_THAN:
+ return GREATER_THAN;
+ case AT_MOST:
+ return AT_LEAST;
+ case AT_LEAST:
+ return AT_MOST;
+ default:
+ return operator;
+ }
}
}
diff --git
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngineTest.java
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngineTest.java
index 06d4ea43a11..f3bf02152a8 100644
---
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngineTest.java
+++
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngineTest.java
@@ -51,6 +51,7 @@ import org.mockito.junit.jupiter.MockitoExtension;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
@@ -191,4 +192,26 @@ class WhereClauseShardingConditionEngineTest {
List<ShardingCondition> actual =
shardingConditionEngine.createShardingConditions(sqlStatementContext,
Collections.emptyList());
assertTrue(actual.isEmpty());
}
+
+ @Test
+ void
assertCreateEmptyShardingConditionsForRightBinaryOperatorRangeStatement() {
+ ColumnSegment right = new ColumnSegment(0, 0, new
IdentifierValue("foo_sharding_col"));
+ ExpressionSegment binaryColumn = new UnaryOperationExpression(0, 0,
right, "BINARY", "BINARY foo_sharding_col");
+ BinaryOperationExpression expression = new
BinaryOperationExpression(0, 0, new LiteralExpressionSegment(0, 0, "100"),
binaryColumn, "<", null);
+ when(whereSegment.getExpr()).thenReturn(expression);
+ when(rule.findShardingColumn("foo_sharding_col",
"")).thenReturn(Optional.of("foo_sharding_col"));
+ Collection<ShardingCondition> actual =
shardingConditionEngine.createShardingConditions(sqlStatementContext,
Collections.emptyList());
+ assertTrue(actual.isEmpty());
+ }
+
+ @Test
+ void
assertCreateEmptyShardingConditionsForRightNestedColumnRangeStatement() {
+ ColumnSegment column = new ColumnSegment(0, 0, new
IdentifierValue("foo_sharding_col"));
+ ExpressionSegment right = new BinaryOperationExpression(0, 0, column,
new LiteralExpressionSegment(0, 0, 1), "+", null);
+ BinaryOperationExpression expression = new
BinaryOperationExpression(0, 0, new LiteralExpressionSegment(0, 0, 100), right,
"<", null);
+ when(whereSegment.getExpr()).thenReturn(expression);
+ when(rule.findShardingColumn("foo_sharding_col",
"")).thenReturn(Optional.of("foo_sharding_col"));
+ Collection<ShardingCondition> actual =
shardingConditionEngine.createShardingConditions(sqlStatementContext,
Collections.emptyList());
+ assertTrue(actual.isEmpty());
+ }
}
diff --git
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueCompareOperatorGeneratorTest.java
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueCompareOperatorGeneratorTest.java
index a9e206e7c72..4610f9d9e9f 100644
---
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueCompareOperatorGeneratorTest.java
+++
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueCompareOperatorGeneratorTest.java
@@ -31,10 +31,14 @@ import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.simp
import
org.apache.shardingsphere.sql.parser.statement.core.value.identifier.IdentifierValue;
import org.apache.shardingsphere.timeservice.core.rule.TimestampServiceRule;
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 java.util.Collections;
import java.util.LinkedList;
import java.util.Optional;
+import java.util.stream.Stream;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
@@ -147,6 +151,16 @@ class ConditionValueCompareOperatorGeneratorTest {
assertFalse(shardingConditionValue.isPresent());
}
+ @SuppressWarnings("unchecked")
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("rightColumnComparisonArguments")
+ void assertGenerateConditionValueWithRightColumn(final String name, final
String operator, final Range<Integer> expectedRange) {
+ BinaryOperationExpression predicate = new BinaryOperationExpression(0,
0, new LiteralExpressionSegment(0, 0, 1), mock(ColumnSegment.class), operator,
null);
+ Optional<ShardingConditionValue> actual =
generator.generate(predicate, column, new LinkedList<>(),
mock(TimestampServiceRule.class));
+ assertTrue(actual.isPresent());
+ assertThat(((RangeShardingConditionValue<Integer>)
actual.get()).getValueRange(), is(expectedRange));
+ }
+
@SuppressWarnings("unchecked")
@Test
void assertGenerateConditionValueWithGreaterThanOperator() {
@@ -223,4 +237,12 @@ class ConditionValueCompareOperatorGeneratorTest {
Optional<ShardingConditionValue> actual =
generator.generate(predicate, column, new LinkedList<>(),
mock(TimestampServiceRule.class));
assertFalse(actual.isPresent());
}
+
+ private static Stream<Arguments> rightColumnComparisonArguments() {
+ return Stream.of(
+ Arguments.of("less than", "<", Range.greaterThan(1)),
+ Arguments.of("at most", "<=", Range.atLeast(1)),
+ Arguments.of("greater than", ">", Range.lessThan(1)),
+ Arguments.of("at least", ">=", Range.atMost(1)));
+ }
}
