Skip to content

Commit 8c938b2

Browse files
PhixsuraPhixsura
committed
Align PostgreSQL numeric-to-integer cast rounding with the documented round-away-from-zero semantics and reject typmod-bearing cast targets so 2.5::int4 routes to 3 -2.5::int4 routes to -3 and varchar(1) numeric(3,1) char(2) targets fall through to broadcast, with WHERE production-path tests for compare IN BETWEEN and unsupported typmod
1 parent e2e3c56 commit 8c938b2

5 files changed

Lines changed: 152 additions & 16 deletions

File tree

‎features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/PostgreSQLCastEvaluator.java‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,14 @@
3838
* value to integer where rounding is undefined) return {@link Optional#empty()} so the caller leaves the cast in place
3939
* and routes broadcast.</p>
4040
*
41-
* <p>Numeric-to-integer rounding follows PostgreSQL's documented {@code ROUND_HALF_EVEN} behavior, e.g. {@code 1.5::int4}
42-
* is {@code 2} and {@code 2.5::int4} is also {@code 2}. Integer overflow is detected via
41+
* <p>Cast targets that carry a type modifier such as {@code varchar(1)}, {@code char(2)} or {@code numeric(3,1)} return
42+
* {@link Optional#empty()} because applying the modifier correctly requires character-length / precision-scale semantics
43+
* the routing path does not model, and routing on the unmodified value would diverge from PostgreSQL's documented
44+
* truncation and precision rules.</p>
45+
*
46+
* <p>Numeric-to-integer rounding follows PostgreSQL's documented behavior for the {@code numeric} type, which rounds
47+
* ties away from zero (Java {@link RoundingMode#HALF_UP}). For example {@code 1.5::int4} is {@code 2},
48+
* {@code 2.5::int4} is {@code 3} and {@code -2.5::int4} is {@code -3}. Integer overflow is detected via
4349
* {@link BigDecimal#intValueExact()} / {@link BigDecimal#longValueExact()} / {@link BigDecimal#shortValueExact()} and
4450
* surfaces as {@link Optional#empty()}.</p>
4551
*/
@@ -87,9 +93,7 @@ public static Optional<Comparable<?>> evaluate(final Object rawValue, final Stri
8793
}
8894

8995
private static String normalize(final String castTargetType) {
90-
String result = castTargetType.toUpperCase(Locale.ROOT).trim();
91-
int parenIndex = result.indexOf('(');
92-
return parenIndex > 0 ? result.substring(0, parenIndex).trim() : result;
96+
return castTargetType.toUpperCase(Locale.ROOT).trim();
9397
}
9498

9599
private static Category categoryOf(final String normalized) {
@@ -133,17 +137,17 @@ private static Category categoryOf(final String normalized) {
133137

134138
private static Optional<Comparable<?>> castToShort(final Object value) {
135139
BigDecimal bigDecimal = toBigDecimal(value);
136-
return null == bigDecimal ? Optional.empty() : Optional.of(bigDecimal.setScale(0, RoundingMode.HALF_EVEN).shortValueExact());
140+
return null == bigDecimal ? Optional.empty() : Optional.of(bigDecimal.setScale(0, RoundingMode.HALF_UP).shortValueExact());
137141
}
138142

139143
private static Optional<Comparable<?>> castToInteger(final Object value) {
140144
BigDecimal bigDecimal = toBigDecimal(value);
141-
return null == bigDecimal ? Optional.empty() : Optional.of(bigDecimal.setScale(0, RoundingMode.HALF_EVEN).intValueExact());
145+
return null == bigDecimal ? Optional.empty() : Optional.of(bigDecimal.setScale(0, RoundingMode.HALF_UP).intValueExact());
142146
}
143147

144148
private static Optional<Comparable<?>> castToLong(final Object value) {
145149
BigDecimal bigDecimal = toBigDecimal(value);
146-
return null == bigDecimal ? Optional.empty() : Optional.of(bigDecimal.setScale(0, RoundingMode.HALF_EVEN).longValueExact());
150+
return null == bigDecimal ? Optional.empty() : Optional.of(bigDecimal.setScale(0, RoundingMode.HALF_UP).longValueExact());
147151
}
148152

149153
private static Optional<Comparable<?>> castToBigDecimal(final Object value) {

‎features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/InsertClauseShardingConditionEngineTest.java‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ void assertCreateShardingConditionsWithTypeCastLiteralStringToIntRoutesByCastedI
301301
}
302302

303303
@Test
304-
void assertCreateShardingConditionsWithTypeCastBigDecimalToIntRoundsHalfEven() {
304+
void assertCreateShardingConditionsWithTypeCastBigDecimalPositiveOneHalfRoundsAwayFromZero() {
305305
TypeCastExpression typeCast = new TypeCastExpression(0, 10, "?::int4", new ParameterMarkerExpressionSegment(0, 0, 0), "int4");
306306
InsertValueContext insertValueContext = new InsertValueContext(Collections.singleton(typeCast), Collections.singletonList(new BigDecimal("1.5")), 0);
307307
when(insertStatementContext.getInsertValueContexts()).thenReturn(Collections.singletonList(insertValueContext));
@@ -311,6 +311,39 @@ void assertCreateShardingConditionsWithTypeCastBigDecimalToIntRoundsHalfEven() {
311311
assertThat(actual.getValues(), is(Collections.singletonList(2)));
312312
}
313313

314+
@Test
315+
void assertCreateShardingConditionsWithTypeCastBigDecimalPositiveTwoHalfRoundsAwayFromZero() {
316+
TypeCastExpression typeCast = new TypeCastExpression(0, 10, "?::int4", new ParameterMarkerExpressionSegment(0, 0, 0), "int4");
317+
InsertValueContext insertValueContext = new InsertValueContext(Collections.singleton(typeCast), Collections.singletonList(new BigDecimal("2.5")), 0);
318+
when(insertStatementContext.getInsertValueContexts()).thenReturn(Collections.singletonList(insertValueContext));
319+
when(rule.findShardingColumn("foo_col_1", "foo_tbl")).thenReturn(Optional.of("foo_col_1"));
320+
List<ShardingCondition> shardingConditions = shardingConditionEngine.createShardingConditions(insertStatementContext, Collections.singletonList(new BigDecimal("2.5")));
321+
ListShardingConditionValue<?> actual = (ListShardingConditionValue<?>) shardingConditions.get(0).getValues().get(0);
322+
assertThat(actual.getValues(), is(Collections.singletonList(3)));
323+
}
324+
325+
@Test
326+
void assertCreateShardingConditionsWithTypeCastBigDecimalNegativeTwoHalfRoundsAwayFromZero() {
327+
TypeCastExpression typeCast = new TypeCastExpression(0, 10, "?::int4", new ParameterMarkerExpressionSegment(0, 0, 0), "int4");
328+
InsertValueContext insertValueContext = new InsertValueContext(Collections.singleton(typeCast), Collections.singletonList(new BigDecimal("-2.5")), 0);
329+
when(insertStatementContext.getInsertValueContexts()).thenReturn(Collections.singletonList(insertValueContext));
330+
when(rule.findShardingColumn("foo_col_1", "foo_tbl")).thenReturn(Optional.of("foo_col_1"));
331+
List<ShardingCondition> shardingConditions = shardingConditionEngine.createShardingConditions(insertStatementContext, Collections.singletonList(new BigDecimal("-2.5")));
332+
ListShardingConditionValue<?> actual = (ListShardingConditionValue<?>) shardingConditions.get(0).getValues().get(0);
333+
assertThat(actual.getValues(), is(Collections.singletonList(-3)));
334+
}
335+
336+
@Test
337+
void assertCreateShardingConditionsWithTypeCastTypmodTargetDoesNotRoute() {
338+
TypeCastExpression typeCast = new TypeCastExpression(0, 10, "?::varchar(1)", new ParameterMarkerExpressionSegment(0, 0, 0), "varchar(1)");
339+
InsertValueContext insertValueContext = new InsertValueContext(Collections.singleton(typeCast), Collections.singletonList("ab"), 0);
340+
when(insertStatementContext.getInsertValueContexts()).thenReturn(Collections.singletonList(insertValueContext));
341+
when(rule.findShardingColumn("foo_col_1", "foo_tbl")).thenReturn(Optional.of("foo_col_1"));
342+
List<ShardingCondition> shardingConditions = shardingConditionEngine.createShardingConditions(insertStatementContext, Collections.singletonList("ab"));
343+
assertThat(shardingConditions.size(), is(1));
344+
assertTrue(shardingConditions.get(0).getValues().isEmpty());
345+
}
346+
314347
@Test
315348
void assertCreateShardingConditionsWithTypeCastOverflowDoesNotRoute() {
316349
TypeCastExpression typeCast = new TypeCastExpression(0, 10, "?::int4", new ParameterMarkerExpressionSegment(0, 0, 0), "int4");

‎features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/PostgreSQLCastEvaluatorTest.java‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,20 +91,25 @@ void assertStringUnparseableToInt4ReturnsEmpty() {
9191
}
9292

9393
@Test
94-
void assertBigDecimalHalfRoundsHalfEvenUpForOne() {
94+
void assertBigDecimalPositiveOneHalfRoundsAwayFromZero() {
9595
assertThat(PostgreSQLCastEvaluator.evaluate(new BigDecimal("1.5"), "int4").orElseThrow(AssertionError::new), is(2));
9696
}
9797

9898
@Test
99-
void assertBigDecimalHalfRoundsHalfEvenDownForTwo() {
100-
assertThat(PostgreSQLCastEvaluator.evaluate(new BigDecimal("2.5"), "int4").orElseThrow(AssertionError::new), is(2));
99+
void assertBigDecimalPositiveTwoHalfRoundsAwayFromZero() {
100+
assertThat(PostgreSQLCastEvaluator.evaluate(new BigDecimal("2.5"), "int4").orElseThrow(AssertionError::new), is(3));
101101
}
102102

103103
@Test
104-
void assertBigDecimalNegativeHalfRoundsHalfEven() {
104+
void assertBigDecimalNegativeOneHalfRoundsAwayFromZero() {
105105
assertThat(PostgreSQLCastEvaluator.evaluate(new BigDecimal("-1.5"), "int4").orElseThrow(AssertionError::new), is(-2));
106106
}
107107

108+
@Test
109+
void assertBigDecimalNegativeTwoHalfRoundsAwayFromZero() {
110+
assertThat(PostgreSQLCastEvaluator.evaluate(new BigDecimal("-2.5"), "int4").orElseThrow(AssertionError::new), is(-3));
111+
}
112+
108113
@Test
109114
void assertBigDecimalToNumeric() {
110115
Optional<Comparable<?>> actual = PostgreSQLCastEvaluator.evaluate(new BigDecimal("1.50"), "numeric");
@@ -145,8 +150,23 @@ void assertBooleanToText() {
145150
}
146151

147152
@Test
148-
void assertStringToVarcharWithSize() {
149-
assertThat(PostgreSQLCastEvaluator.evaluate("foo", "varchar(10)").orElseThrow(AssertionError::new), is("foo"));
153+
void assertStringToVarcharWithTypmodReturnsEmpty() {
154+
assertFalse(PostgreSQLCastEvaluator.evaluate("foo", "varchar(10)").isPresent());
155+
}
156+
157+
@Test
158+
void assertOverLengthStringToVarcharWithTypmodReturnsEmpty() {
159+
assertFalse(PostgreSQLCastEvaluator.evaluate("ab", "varchar(1)").isPresent());
160+
}
161+
162+
@Test
163+
void assertStringToCharWithTypmodReturnsEmpty() {
164+
assertFalse(PostgreSQLCastEvaluator.evaluate("ab", "char(2)").isPresent());
165+
}
166+
167+
@Test
168+
void assertBigDecimalToNumericWithPrecisionScaleReturnsEmpty() {
169+
assertFalse(PostgreSQLCastEvaluator.evaluate(new BigDecimal("1.55"), "numeric(3,1)").isPresent());
150170
}
151171

152172
@Test

‎features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/engine/WhereClauseShardingConditionEngineTest.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,13 @@
2828
import org.apache.shardingsphere.sharding.rule.ShardingRule;
2929
import org.apache.shardingsphere.sql.parser.statement.core.segment.dml.column.ColumnSegment;
3030
import org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.BetweenExpression;
31+
import org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.BinaryOperationExpression;
3132
import org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.ExpressionSegment;
3233
import org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.InExpression;
3334
import org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.ListExpression;
35+
import org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.TypeCastExpression;
3436
import org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.simple.LiteralExpressionSegment;
37+
import org.apache.shardingsphere.sql.parser.statement.core.segment.dml.expr.simple.ParameterMarkerExpressionSegment;
3538
import org.apache.shardingsphere.sql.parser.statement.core.segment.dml.predicate.WhereSegment;
3639
import org.apache.shardingsphere.sql.parser.statement.core.value.identifier.IdentifierValue;
3740
import org.apache.shardingsphere.timeservice.core.rule.TimestampServiceRule;
@@ -41,13 +44,16 @@
4144
import org.mockito.Mock;
4245
import org.mockito.junit.jupiter.MockitoExtension;
4346

47+
import java.math.BigDecimal;
48+
import java.util.Arrays;
4449
import java.util.Collections;
4550
import java.util.List;
4651
import java.util.Optional;
4752

4853
import static org.hamcrest.Matchers.is;
4954
import static org.hamcrest.MatcherAssert.assertThat;
5055
import static org.hamcrest.Matchers.isA;
56+
import static org.junit.jupiter.api.Assertions.assertTrue;
5157
import static org.mockito.Mockito.mock;
5258
import static org.mockito.Mockito.when;
5359

@@ -113,4 +119,54 @@ void assertCreateShardingConditionsForSelectInStatement() {
113119
assertThat(actual.get(0).getStartIndex(), is(0));
114120
assertThat(actual.get(0).getValues().get(0), isA(ListShardingConditionValue.class));
115121
}
122+
123+
@Test
124+
void assertCreateShardingConditionsForSelectCompareWithCastedParameter() {
125+
ColumnSegment left = new ColumnSegment(0, 0, new IdentifierValue("foo_sharding_col"));
126+
TypeCastExpression cast = new TypeCastExpression(0, 0, "?::int4", new ParameterMarkerExpressionSegment(0, 0, 0), "int4");
127+
BinaryOperationExpression predicate = new BinaryOperationExpression(0, 0, left, cast, "=", "foo_sharding_col = ?::int4");
128+
when(whereSegment.getExpr()).thenReturn(predicate);
129+
when(rule.findShardingColumn("foo_sharding_col", "")).thenReturn(Optional.of("foo_sharding_col"));
130+
List<ShardingCondition> actual = shardingConditionEngine.createShardingConditions(sqlStatementContext, Collections.singletonList("42"));
131+
ListShardingConditionValue<?> value = (ListShardingConditionValue<?>) actual.get(0).getValues().get(0);
132+
assertThat(value.getValues(), is(Collections.singletonList(42)));
133+
}
134+
135+
@Test
136+
void assertCreateShardingConditionsForSelectInWithCastedParameter() {
137+
ColumnSegment left = new ColumnSegment(0, 0, new IdentifierValue("foo_sharding_col"));
138+
ListExpression right = new ListExpression(0, 0);
139+
right.getItems().add(new TypeCastExpression(0, 0, "?::int4", new ParameterMarkerExpressionSegment(0, 0, 0), "int4"));
140+
InExpression inExpression = new InExpression(0, 0, left, right, false);
141+
when(whereSegment.getExpr()).thenReturn(inExpression);
142+
when(rule.findShardingColumn("foo_sharding_col", "")).thenReturn(Optional.of("foo_sharding_col"));
143+
List<ShardingCondition> actual = shardingConditionEngine.createShardingConditions(sqlStatementContext, Collections.singletonList("42"));
144+
ListShardingConditionValue<?> value = (ListShardingConditionValue<?>) actual.get(0).getValues().get(0);
145+
assertThat(value.getValues(), is(Collections.singletonList(42)));
146+
}
147+
148+
@Test
149+
void assertCreateShardingConditionsForSelectBetweenWithCastedParameters() {
150+
ColumnSegment left = new ColumnSegment(0, 0, new IdentifierValue("foo_sharding_col"));
151+
ExpressionSegment betweenCast = new TypeCastExpression(0, 0, "?::int4", new ParameterMarkerExpressionSegment(0, 0, 0), "int4");
152+
ExpressionSegment andCast = new TypeCastExpression(0, 0, "?::int4", new ParameterMarkerExpressionSegment(0, 0, 1), "int4");
153+
BetweenExpression betweenExpression = new BetweenExpression(0, 0, left, betweenCast, andCast, false);
154+
when(whereSegment.getExpr()).thenReturn(betweenExpression);
155+
when(rule.findShardingColumn("foo_sharding_col", "")).thenReturn(Optional.of("foo_sharding_col"));
156+
List<ShardingCondition> actual = shardingConditionEngine.createShardingConditions(sqlStatementContext, Arrays.asList("1", "100"));
157+
RangeShardingConditionValue<?> value = (RangeShardingConditionValue<?>) actual.get(0).getValues().get(0);
158+
assertThat(value.getValueRange().lowerEndpoint(), is(1));
159+
assertThat(value.getValueRange().upperEndpoint(), is(100));
160+
}
161+
162+
@Test
163+
void assertCreateShardingConditionsForSelectCompareWithUnsupportedTypmodCastDoesNotRoute() {
164+
ColumnSegment left = new ColumnSegment(0, 0, new IdentifierValue("foo_sharding_col"));
165+
TypeCastExpression cast = new TypeCastExpression(0, 0, "?::numeric(3,1)", new ParameterMarkerExpressionSegment(0, 0, 0), "numeric(3,1)");
166+
BinaryOperationExpression predicate = new BinaryOperationExpression(0, 0, left, cast, "=", "foo_sharding_col = ?::numeric(3,1)");
167+
when(whereSegment.getExpr()).thenReturn(predicate);
168+
when(rule.findShardingColumn("foo_sharding_col", "")).thenReturn(Optional.of("foo_sharding_col"));
169+
List<ShardingCondition> actual = shardingConditionEngine.createShardingConditions(sqlStatementContext, Collections.singletonList(new BigDecimal("1.55")));
170+
assertTrue(actual.isEmpty());
171+
}
116172
}

‎features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/ConditionValueTest.java‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,36 @@ void assertGetValueFromTypeCastOverStringParameterMarker() {
9898
}
9999

100100
@Test
101-
void assertGetValueFromTypeCastOverBigDecimalRoundsHalfEven() {
101+
void assertGetValueFromTypeCastOverBigDecimalPositiveOneHalfRoundsAwayFromZero() {
102102
TypeCastExpression typeCast = new TypeCastExpression(0, 10, "?::int4", new ParameterMarkerExpressionSegment(0, 0, 0), "int4");
103103
ConditionValue conditionValue = new ConditionValue(typeCast, Collections.singletonList(new BigDecimal("1.5")));
104104
assertTrue(conditionValue.getValue().isPresent());
105105
assertThat(conditionValue.getValue().get(), is(2));
106106
}
107107

108+
@Test
109+
void assertGetValueFromTypeCastOverBigDecimalPositiveTwoHalfRoundsAwayFromZero() {
110+
TypeCastExpression typeCast = new TypeCastExpression(0, 10, "?::int4", new ParameterMarkerExpressionSegment(0, 0, 0), "int4");
111+
ConditionValue conditionValue = new ConditionValue(typeCast, Collections.singletonList(new BigDecimal("2.5")));
112+
assertTrue(conditionValue.getValue().isPresent());
113+
assertThat(conditionValue.getValue().get(), is(3));
114+
}
115+
116+
@Test
117+
void assertGetValueFromTypeCastOverBigDecimalNegativeTwoHalfRoundsAwayFromZero() {
118+
TypeCastExpression typeCast = new TypeCastExpression(0, 10, "?::int4", new ParameterMarkerExpressionSegment(0, 0, 0), "int4");
119+
ConditionValue conditionValue = new ConditionValue(typeCast, Collections.singletonList(new BigDecimal("-2.5")));
120+
assertTrue(conditionValue.getValue().isPresent());
121+
assertThat(conditionValue.getValue().get(), is(-3));
122+
}
123+
124+
@Test
125+
void assertGetValueFromTypeCastWithTypmodReturnsEmpty() {
126+
TypeCastExpression typeCast = new TypeCastExpression(0, 10, "?::varchar(1)", new ParameterMarkerExpressionSegment(0, 0, 0), "varchar(1)");
127+
ConditionValue conditionValue = new ConditionValue(typeCast, Collections.singletonList("ab"));
128+
assertFalse(conditionValue.getValue().isPresent());
129+
}
130+
108131
@Test
109132
void assertGetValueFromTypeCastOverflowReturnsEmpty() {
110133
TypeCastExpression typeCast = new TypeCastExpression(0, 10, "?::int4", new ParameterMarkerExpressionSegment(0, 0, 0), "int4");

0 commit comments

Comments
 (0)