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 dd9d59940f9 Refine BINARY operator handling in sharding compare
condition value generator (#35623) (#39243)
dd9d59940f9 is described below
commit dd9d59940f9c34a0f980abb0b5a0a25992ec50c0
Author: fudian <[email protected]>
AuthorDate: Tue Aug 18 15:25:05 2026 +0800
Refine BINARY operator handling in sharding compare condition value
generator (#35623) (#39243)
* Refine BINARY operator handling in sharding compare condition value
generator
MySQL's BINARY operator wraps a column or value as a
UnaryOperationExpression.
The compare condition value generator did not unwrap BINARY before checking
whether a side is a ColumnSegment, so sharding conditions on BINARY-prefixed
columns (e.g. WHERE BINARY id = 100) were silently dropped.
Add unwrapBinaryOperator to strip the BINARY unary wrapper so the underlying
column/value is used for sharding condition generation.
Refs #35623
* Enable BINARY rewrite integration assertions
Signed-off-by: 付典 <[email protected]>
* Restrict BINARY unwrap to equality predicates in compare condition value
generator
MySQL evaluates BINARY range comparisons byte-wise, while range routing
prunes
partitions on converted endpoints, so unwrapping BINARY is
semantics-preserving
only for equality predicates. Range predicates with BINARY operands now keep
the pre-existing broadcast routing.
- Gate unwrapBinaryOperator behind the equality operator
- Add generator tests asserting empty conditions for BINARY with >, >=, <,
<=
on both operand sides
- Add where-clause engine test asserting no sharding condition for a string
sharding key with BINARY and a range operator
- Add route engine test asserting full-route broadcast without sharding
conditions under a VOLUME_RANGE algorithm
Signed-off-by: 付典 <[email protected]>
---------
Signed-off-by: 付典 <[email protected]>
---
.../ConditionValueCompareOperatorGenerator.java | 18 ++++++-
.../WhereClauseShardingConditionEngineTest.java | 14 +++++
...ConditionValueCompareOperatorGeneratorTest.java | 61 ++++++++++++++++++++++
.../fixture/ShardingRouteEngineFixtureBuilder.java | 16 ++++++
.../standard/ShardingStandardRouteEngineTest.java | 15 ++++++
.../scenario/sharding/case/dml/select.xml | 5 +-
6 files changed, 125 insertions(+), 4 deletions(-)
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 d793d90a88b..aedd7f2f223 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
@@ -28,6 +28,7 @@ import
org.apache.shardingsphere.sharding.route.engine.condition.value.ShardingC
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.column.ColumnSegment;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.BinaryOperationExpression;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.ExpressionSegment;
+import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.UnaryOperationExpression;
import org.apache.shardingsphere.timeservice.core.rule.TimestampServiceRule;
import java.util.ArrayList;
@@ -64,7 +65,15 @@ public final class ConditionValueCompareOperatorGenerator
implements ConditionVa
if (!isSupportedOperator(operator)) {
return Optional.empty();
}
- ExpressionSegment valueExpression = predicate.getLeft() instanceof
ColumnSegment ? predicate.getRight() : predicate.getLeft();
+ ExpressionSegment left = predicate.getLeft();
+ ExpressionSegment right = predicate.getRight();
+ // MySQL evaluates BINARY range comparisons byte-wise, and range
routing prunes partitions on converted endpoints,
+ // so unwrapping BINARY is semantics-preserving only for equality
predicates; range predicates keep broadcast routing.
+ if (EQUAL.equals(operator)) {
+ left = unwrapBinaryOperator(left);
+ right = unwrapBinaryOperator(right);
+ }
+ ExpressionSegment valueExpression = left instanceof ColumnSegment ?
right : left;
ConditionValue conditionValue = new ConditionValue(valueExpression,
params);
if (conditionValue.isNull()) {
return generate(null, column, operator,
conditionValue.getParameterMarkerIndex().orElse(-1));
@@ -106,4 +115,11 @@ public final class ConditionValueCompareOperatorGenerator
implements ConditionVa
private boolean isSupportedOperator(final String operator) {
return OPERATORS.contains(operator);
}
+
+ private ExpressionSegment unwrapBinaryOperator(final ExpressionSegment
segment) {
+ return segment instanceof UnaryOperationExpression
+ && "BINARY".equalsIgnoreCase(((UnaryOperationExpression)
segment).getOperator())
+ ? ((UnaryOperationExpression) segment).getExpression()
+ : segment;
+ }
}
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 cebbd8a3fab..e5b76068a17 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
@@ -28,9 +28,11 @@ import
org.apache.shardingsphere.sharding.route.engine.condition.value.RangeShar
import org.apache.shardingsphere.sharding.rule.ShardingRule;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.column.ColumnSegment;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.BetweenExpression;
+import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.BinaryOperationExpression;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.ExpressionSegment;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.InExpression;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.ListExpression;
+import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.UnaryOperationExpression;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.simple.LiteralExpressionSegment;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.predicate.WhereSegment;
import
org.apache.shardingsphere.sql.parser.statement.core.value.identifier.IdentifierValue;
@@ -48,6 +50,7 @@ import java.util.Optional;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.isA;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -113,4 +116,15 @@ class WhereClauseShardingConditionEngineTest {
assertThat(actual.get(0).getStartIndex(), is(0));
assertThat(actual.get(0).getValues().get(0),
isA(ListShardingConditionValue.class));
}
+
+ @Test
+ void assertCreateEmptyShardingConditionsForBinaryOperatorRangeStatement() {
+ ColumnSegment left = new ColumnSegment(0, 0, new
IdentifierValue("foo_sharding_col"));
+ ExpressionSegment binaryColumn = new UnaryOperationExpression(0, 0,
left, "BINARY", "BINARY foo_sharding_col");
+ BinaryOperationExpression expression = new
BinaryOperationExpression(0, 0, binaryColumn, new LiteralExpressionSegment(0,
0, "100"), ">", null);
+ when(whereSegment.getExpr()).thenReturn(expression);
+ when(rule.findShardingColumn("foo_sharding_col",
"")).thenReturn(Optional.of("foo_sharding_col"));
+ List<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 a5f4bc23446..be876558175 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
@@ -24,6 +24,7 @@ import
org.apache.shardingsphere.sharding.route.engine.condition.value.RangeShar
import
org.apache.shardingsphere.sharding.route.engine.condition.value.ShardingConditionValue;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.column.ColumnSegment;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.BinaryOperationExpression;
+import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.UnaryOperationExpression;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.complex.CommonExpressionSegment;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.simple.LiteralExpressionSegment;
import
org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.simple.ParameterMarkerExpressionSegment;
@@ -59,6 +60,66 @@ class ConditionValueCompareOperatorGeneratorTest {
assertTrue(shardingConditionValue.get().getParameterMarkerIndexes().isEmpty());
}
+ @SuppressWarnings("unchecked")
+ @Test
+ void assertGenerateConditionValueWithBinaryOperatorPrefix() {
+ int value = 100;
+ BinaryOperationExpression predicate = new BinaryOperationExpression(0,
0,
+ new UnaryOperationExpression(0, 0, mock(ColumnSegment.class),
"BINARY", "BINARY id"),
+ new LiteralExpressionSegment(0, 0, value), "=", null);
+ Optional<ShardingConditionValue> shardingConditionValue =
generator.generate(predicate, column, new LinkedList<>(),
mock(TimestampServiceRule.class));
+ assertTrue(shardingConditionValue.isPresent());
+ assertTrue(((ListShardingConditionValue<Integer>)
shardingConditionValue.get()).getValues().contains(value));
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ void assertGenerateConditionValueWithBinaryOperatorValue() {
+ int value = 100;
+ BinaryOperationExpression predicate = new BinaryOperationExpression(0,
0,
+ mock(ColumnSegment.class),
+ new UnaryOperationExpression(0, 0, new
LiteralExpressionSegment(0, 0, value), "BINARY", "BINARY 100"), "=", null);
+ Optional<ShardingConditionValue> shardingConditionValue =
generator.generate(predicate, column, new LinkedList<>(),
mock(TimestampServiceRule.class));
+ assertTrue(shardingConditionValue.isPresent());
+ assertTrue(((ListShardingConditionValue<Integer>)
shardingConditionValue.get()).getValues().contains(value));
+ }
+
+ @Test
+ void
assertGenerateEmptyConditionValueWithBinaryOperatorPrefixAndGreaterThanOperator()
{
+ BinaryOperationExpression predicate = new BinaryOperationExpression(0,
0,
+ new UnaryOperationExpression(0, 0, mock(ColumnSegment.class),
"BINARY", "BINARY id"), new LiteralExpressionSegment(0, 0, "100"), ">", null);
+ assertFalse(generator.generate(predicate, column, new LinkedList<>(),
mock(TimestampServiceRule.class)).isPresent());
+ }
+
+ @Test
+ void
assertGenerateEmptyConditionValueWithBinaryOperatorPrefixAndGreaterThanOrEqualOperator()
{
+ BinaryOperationExpression predicate = new BinaryOperationExpression(0,
0,
+ new UnaryOperationExpression(0, 0, mock(ColumnSegment.class),
"BINARY", "BINARY id"), new LiteralExpressionSegment(0, 0, "100"), ">=", null);
+ assertFalse(generator.generate(predicate, column, new LinkedList<>(),
mock(TimestampServiceRule.class)).isPresent());
+ }
+
+ @Test
+ void
assertGenerateEmptyConditionValueWithBinaryOperatorPrefixAndLessThanOperator() {
+ BinaryOperationExpression predicate = new BinaryOperationExpression(0,
0,
+ new UnaryOperationExpression(0, 0, mock(ColumnSegment.class),
"BINARY", "BINARY id"), new LiteralExpressionSegment(0, 0, "100"), "<", null);
+ assertFalse(generator.generate(predicate, column, new LinkedList<>(),
mock(TimestampServiceRule.class)).isPresent());
+ }
+
+ @Test
+ void
assertGenerateEmptyConditionValueWithBinaryOperatorPrefixAndLessThanOrEqualOperator()
{
+ BinaryOperationExpression predicate = new BinaryOperationExpression(0,
0,
+ new UnaryOperationExpression(0, 0, mock(ColumnSegment.class),
"BINARY", "BINARY id"), new LiteralExpressionSegment(0, 0, "100"), "<=", null);
+ assertFalse(generator.generate(predicate, column, new LinkedList<>(),
mock(TimestampServiceRule.class)).isPresent());
+ }
+
+ @Test
+ void
assertGenerateEmptyConditionValueWithBinaryOperatorValueAndGreaterThanOperator()
{
+ BinaryOperationExpression predicate = new BinaryOperationExpression(0,
0,
+ mock(ColumnSegment.class),
+ new UnaryOperationExpression(0, 0, new
LiteralExpressionSegment(0, 0, "100"), "BINARY", "BINARY '100'"), ">", null);
+ assertFalse(generator.generate(predicate, column, new LinkedList<>(),
mock(TimestampServiceRule.class)).isPresent());
+ }
+
@SuppressWarnings("unchecked")
@Test
void assertGenerateNullConditionValue() {
diff --git
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/fixture/ShardingRouteEngineFixtureBuilder.java
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/fixture/ShardingRouteEngineFixtureBuilder.java
index 9c60a54801e..a1c1a0cdb2c 100644
---
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/fixture/ShardingRouteEngineFixtureBuilder.java
+++
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/fixture/ShardingRouteEngineFixtureBuilder.java
@@ -31,6 +31,7 @@ import
org.apache.shardingsphere.infra.spi.type.typed.TypedSPILoader;
import org.apache.shardingsphere.infra.util.props.PropertiesBuilder;
import org.apache.shardingsphere.infra.util.props.PropertiesBuilder.Property;
import org.apache.shardingsphere.sharding.api.config.ShardingRuleConfiguration;
+import
org.apache.shardingsphere.sharding.api.config.rule.ShardingAutoTableRuleConfiguration;
import
org.apache.shardingsphere.sharding.api.config.rule.ShardingTableReferenceRuleConfiguration;
import
org.apache.shardingsphere.sharding.api.config.rule.ShardingTableRuleConfiguration;
import
org.apache.shardingsphere.sharding.api.config.strategy.sharding.HintShardingStrategyConfiguration;
@@ -82,6 +83,21 @@ public final class ShardingRouteEngineFixtureBuilder {
return new ShardingRule(shardingRuleConfig, createDataSources(),
mock(ComputeNodeInstanceContext.class), Collections.emptyList());
}
+ /**
+ * Create volume range sharding rule.
+ *
+ * @return created sharding rule
+ */
+ public static ShardingRule createVolumeRangeShardingRule() {
+ ShardingRuleConfiguration shardingRuleConfig = new
ShardingRuleConfiguration();
+ ShardingAutoTableRuleConfiguration autoTableRuleConfig = new
ShardingAutoTableRuleConfiguration("t_order", "ds_0");
+ autoTableRuleConfig.setShardingStrategy(new
StandardShardingStrategyConfiguration("order_no", "t_order_volume_range"));
+ shardingRuleConfig.getAutoTables().add(autoTableRuleConfig);
+ shardingRuleConfig.getShardingAlgorithms().put("t_order_volume_range",
new AlgorithmConfiguration("VOLUME_RANGE",
+ PropertiesBuilder.build(new Property("range-lower", "10"), new
Property("range-upper", "1000"), new Property("sharding-volume", "330"))));
+ return new ShardingRule(shardingRuleConfig, createDataSources(),
mock(ComputeNodeInstanceContext.class), Collections.emptyList());
+ }
+
/**
* Create error sharding rule.
*
diff --git
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/type/standard/ShardingStandardRouteEngineTest.java
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/type/standard/ShardingStandardRouteEngineTest.java
index 218d9ae172a..392e8f86793 100644
---
a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/type/standard/ShardingStandardRouteEngineTest.java
+++
b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/type/standard/ShardingStandardRouteEngineTest.java
@@ -75,6 +75,21 @@ class ShardingStandardRouteEngineTest {
assertThat(routeUnits.get(3).getTableMappers().iterator().next().getLogicName(),
is("t_order"));
}
+ @Test
+ void assertRouteToAllTablesByNonConditionsUnderRangeAlgorithm() {
+ ShardingStandardRouteEngine routeEngine =
createShardingStandardRouteEngine("t_order",
+ new ShardingConditions(Collections.emptyList(),
mock(SQLStatementContext.class), mock(ShardingRule.class)),
mock(SQLStatementContext.class), new HintValueContext());
+ RouteContext routeContext =
routeEngine.route(ShardingRouteEngineFixtureBuilder.createVolumeRangeShardingRule());
+ List<RouteUnit> routeUnits = new
ArrayList<>(routeContext.getRouteUnits());
+ assertThat(routeContext.getRouteUnits().size(), is(5));
+ for (int i = 0; i < 5; i++) {
+
assertThat(routeUnits.get(i).getDataSourceMapper().getActualName(), is("ds_0"));
+ assertThat(routeUnits.get(i).getTableMappers().size(), is(1));
+
assertThat(routeUnits.get(i).getTableMappers().iterator().next().getActualName(),
is("t_order_" + i));
+
assertThat(routeUnits.get(i).getTableMappers().iterator().next().getLogicName(),
is("t_order"));
+ }
+ }
+
@Test
void assertRouteByShardingConditions() {
ShardingStandardRouteEngine routeEngine =
createShardingStandardRouteEngine("t_order",
diff --git
a/test/it/rewriter/src/test/resources/scenario/sharding/case/dml/select.xml
b/test/it/rewriter/src/test/resources/scenario/sharding/case/dml/select.xml
index d142bd52d2b..ef93363bc94 100644
--- a/test/it/rewriter/src/test/resources/scenario/sharding/case/dml/select.xml
+++ b/test/it/rewriter/src/test/resources/scenario/sharding/case/dml/select.xml
@@ -529,8 +529,7 @@
<output sql="SELECT account_id , CASE WHEN account_id > 0 AND
account_id <= 10 THEN '(0,10]' WHEN account_id > 10 THEN '(10,+∞)' ELSE ''
END AS GROUP_BY_DERIVED_0 FROM t_account_1 GROUP BY CASE WHEN account_id > 0
AND account_id <= 10 THEN '(0,10]' WHEN account_id > 10 THEN '(10,+∞)' ELSE
'' END ORDER BY CASE WHEN account_id > 0 AND account_id <= 10 THEN '(0,10]'
WHEN account_id > 10 THEN '(10,+∞)' ELSE '' END" parameters="100" />
</rewrite-assertion>
- <!-- FIXME -->
- <!--<rewrite-assertion
id="select_with_sharding_value_and_binary_column_for_parameters"
db-types="MySQL">
+ <rewrite-assertion
id="select_with_sharding_value_and_binary_column_for_parameters"
db-types="MySQL">
<input sql="SELECT * FROM t_account WHERE BINARY account_id = ?"
parameters="100" />
<output sql="SELECT * FROM t_account_0 WHERE BINARY account_id = ?"
parameters="100" />
</rewrite-assertion>
@@ -548,7 +547,7 @@
<rewrite-assertion
id="select_with_sharding_value_and_binary_value_for_literals" db-types="MySQL">
<input sql="SELECT * FROM t_account WHERE account_id = BINARY 100" />
<output sql="SELECT * FROM t_account_0 WHERE account_id = BINARY 100"
/>
- </rewrite-assertion>-->
+ </rewrite-assertion>
<rewrite-assertion id="select_with_schema_name_in_shorthand_projection"
db-types="MySQL">
<input sql="SELECT sharding_db.t_account.* FROM t_account WHERE
account_id = ?" parameters="100" />
