You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by pa...@apache.org on 2021/10/20 10:52:04 UTC

[shardingsphere] branch master updated: add parameter check when execute sql without parameter (#13175)

This is an automated email from the ASF dual-hosted git repository.

panjuan 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 8dbdc9f  add parameter check when execute sql without parameter (#13175)
8dbdc9f is described below

commit 8dbdc9ff4f14bdd7f03f28af14dee3bb09f019c7
Author: Zhengqiang Duan <du...@apache.org>
AuthorDate: Wed Oct 20 18:51:26 2021 +0800

    add parameter check when execute sql without parameter (#13175)
---
 .../engine/condition/generator/ConditionValue.java |  4 ++-
 ...ConditionValueBetweenOperatorGeneratorTest.java | 31 ++++++++++++++++++++++
 ...ConditionValueCompareOperatorGeneratorTest.java | 29 ++++++++++++++++++++
 .../ConditionValueInOperatorGeneratorTest.java     | 30 +++++++++++++++++++++
 4 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/ConditionValue.java b/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/ConditionValue.java
index 39855e9..268bba5 100644
--- a/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/ConditionValue.java
+++ b/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/ConditionValue.java
@@ -47,7 +47,9 @@ public final class ConditionValue {
     }
     
     private Comparable<?> getValue(final ParameterMarkerExpressionSegment expressionSegment, final List<Object> parameters) {
-        Object result = parameters.get(expressionSegment.getParameterMarkerIndex());
+        int parameterMarkerIndex = expressionSegment.getParameterMarkerIndex();
+        Preconditions.checkArgument(parameterMarkerIndex < parameters.size(), "Parameter marker expression must have corresponding parameter value.");
+        Object result = parameters.get(parameterMarkerIndex);
         Preconditions.checkArgument(result instanceof Comparable, "Sharding value must implements Comparable.");
         return (Comparable<?>) result;
     }
diff --git a/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueBetweenOperatorGeneratorTest.java b/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueBetweenOperatorGeneratorTest.java
index e27e297..f2712a8 100644
--- a/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueBetweenOperatorGeneratorTest.java
+++ b/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueBetweenOperatorGeneratorTest.java
@@ -17,23 +17,29 @@
 
 package org.apache.shardingsphere.sharding.route.engine.condition.generator.impl;
 
+import com.google.common.collect.Range;
 import org.apache.shardingsphere.infra.datetime.DatetimeService;
 import org.apache.shardingsphere.sharding.route.engine.condition.Column;
 import org.apache.shardingsphere.sharding.route.engine.condition.value.RangeShardingConditionValue;
 import org.apache.shardingsphere.sharding.route.engine.condition.value.ShardingConditionValue;
 import org.apache.shardingsphere.spi.ShardingSphereServiceLoader;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.column.ColumnSegment;
 import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.BetweenExpression;
 import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.ExpressionSegment;
 import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.complex.CommonExpressionSegment;
 import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.simple.LiteralExpressionSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.simple.ParameterMarkerExpressionSegment;
 import org.apache.shardingsphere.sql.parser.sql.common.util.SafeNumberOperationUtil;
+import org.apache.shardingsphere.sql.parser.sql.common.value.identifier.IdentifierValue;
 import org.junit.Test;
 
+import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Date;
 import java.util.LinkedList;
 import java.util.Optional;
 
+import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
@@ -122,4 +128,29 @@ public final class ConditionValueBetweenOperatorGeneratorTest {
         assertThat(rangeShardingConditionValue.getTableName(), is(column.getTableName()));
         assertTrue(rangeShardingConditionValue.getValueRange().upperEndpoint().before(after));
     }
+    
+    @SuppressWarnings("unchecked")
+    @Test
+    public void assertGenerateConditionValueWithParameter() {
+        ColumnSegment left = new ColumnSegment(0, 0, new IdentifierValue("id"));
+        ParameterMarkerExpressionSegment between = new ParameterMarkerExpressionSegment(0, 0, 0);
+        ParameterMarkerExpressionSegment and = new ParameterMarkerExpressionSegment(0, 0, 1);
+        BetweenExpression predicate = new BetweenExpression(0, 0, left, between, and, false);
+        Optional<ShardingConditionValue> actual = generator.generate(predicate, column, Arrays.asList(1, 2));
+        assertTrue(actual.isPresent());
+        assertThat(actual.get(), instanceOf(RangeShardingConditionValue.class));
+        RangeShardingConditionValue<Integer> conditionValue = (RangeShardingConditionValue<Integer>) actual.get();
+        assertThat(conditionValue.getTableName(), is("tbl"));
+        assertThat(conditionValue.getColumnName(), is("id"));
+        assertThat(conditionValue.getValueRange(), is(Range.closed(1, 2)));
+    }
+    
+    @Test(expected = IllegalArgumentException.class)
+    public void assertGenerateConditionValueWithoutParameter() {
+        ColumnSegment left = new ColumnSegment(0, 0, new IdentifierValue("id"));
+        ParameterMarkerExpressionSegment between = new ParameterMarkerExpressionSegment(0, 0, 0);
+        ParameterMarkerExpressionSegment and = new ParameterMarkerExpressionSegment(0, 0, 1);
+        BetweenExpression predicate = new BetweenExpression(0, 0, left, between, and, false);
+        generator.generate(predicate, column, new LinkedList<>());
+    }
 }
diff --git a/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueCompareOperatorGeneratorTest.java b/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueCompareOperatorGeneratorTest.java
index a96fd64..4944c61 100644
--- a/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueCompareOperatorGeneratorTest.java
+++ b/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueCompareOperatorGeneratorTest.java
@@ -26,12 +26,18 @@ import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.column.Column
 import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.BinaryOperationExpression;
 import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.complex.CommonExpressionSegment;
 import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.simple.LiteralExpressionSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.simple.ParameterMarkerExpressionSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.value.identifier.IdentifierValue;
 import org.junit.Test;
 
+import java.util.Collections;
 import java.util.LinkedList;
 import java.util.Optional;
 
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
 
@@ -107,4 +113,27 @@ public final class ConditionValueCompareOperatorGeneratorTest {
         assertTrue(shardingConditionValue.isPresent());
         assertFalse(((ListShardingConditionValue<Integer>) shardingConditionValue.get()).getValues().isEmpty());
     }
+    
+    @SuppressWarnings("unchecked")
+    @Test
+    public void assertGenerateConditionValueWithParameter() {
+        ColumnSegment left = new ColumnSegment(0, 0, new IdentifierValue("id"));
+        ParameterMarkerExpressionSegment right = new ParameterMarkerExpressionSegment(0, 0, 0);
+        BinaryOperationExpression predicate = new BinaryOperationExpression(0, 0, left, right, "=", "id = ?");
+        Optional<ShardingConditionValue> actual = generator.generate(predicate, column, Collections.singletonList(1));
+        assertTrue(actual.isPresent());
+        assertThat(actual.get(), instanceOf(ListShardingConditionValue.class));
+        ListShardingConditionValue<Integer> conditionValue = (ListShardingConditionValue<Integer>) actual.get();
+        assertThat(conditionValue.getTableName(), is("tbl"));
+        assertThat(conditionValue.getColumnName(), is("id"));
+        assertThat(conditionValue.getValues(), is(Collections.singletonList(1)));
+    }
+    
+    @Test(expected = IllegalArgumentException.class)
+    public void assertGenerateConditionValueWithoutParameter() {
+        ColumnSegment left = new ColumnSegment(0, 0, new IdentifierValue("order_id"));
+        ParameterMarkerExpressionSegment right = new ParameterMarkerExpressionSegment(0, 0, 0);
+        BinaryOperationExpression predicate = new BinaryOperationExpression(0, 0, left, right, "=", "order_id = ?");
+        generator.generate(predicate, column, new LinkedList<>());
+    }
 }
diff --git a/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueInOperatorGeneratorTest.java b/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueInOperatorGeneratorTest.java
index 893c528..62cb29c 100644
--- a/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueInOperatorGeneratorTest.java
+++ b/shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/test/java/org/apache/shardingsphere/sharding/route/engine/condition/generator/impl/ConditionValueInOperatorGeneratorTest.java
@@ -22,17 +22,22 @@ import org.apache.shardingsphere.sharding.route.engine.condition.Column;
 import org.apache.shardingsphere.sharding.route.engine.condition.value.ListShardingConditionValue;
 import org.apache.shardingsphere.sharding.route.engine.condition.value.ShardingConditionValue;
 import org.apache.shardingsphere.spi.ShardingSphereServiceLoader;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.column.ColumnSegment;
 import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.InExpression;
 import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.ListExpression;
 import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.complex.CommonExpressionSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.simple.ParameterMarkerExpressionSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.value.identifier.IdentifierValue;
 import org.junit.Before;
 import org.junit.Test;
 
+import java.util.Collections;
 import java.util.Date;
 import java.util.LinkedList;
 import java.util.Optional;
 
 import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
@@ -56,4 +61,29 @@ public final class ConditionValueInOperatorGeneratorTest {
         assertTrue(shardingConditionValue.isPresent());
         assertThat(((ListShardingConditionValue) shardingConditionValue.get()).getValues().iterator().next(), instanceOf(Date.class));
     }
+    
+    @SuppressWarnings("unchecked")
+    @Test
+    public void assertGenerateConditionValueWithParameter() {
+        ColumnSegment left = new ColumnSegment(0, 0, new IdentifierValue("id"));
+        ListExpression right = new ListExpression(0, 0);
+        right.getItems().add(new ParameterMarkerExpressionSegment(0, 0, 0));
+        InExpression predicate = new InExpression(0, 0, left, right, false);
+        Optional<ShardingConditionValue> actual = generator.generate(predicate, column, Collections.singletonList(1));
+        assertTrue(actual.isPresent());
+        assertThat(actual.get(), instanceOf(ListShardingConditionValue.class));
+        ListShardingConditionValue<Integer> conditionValue = (ListShardingConditionValue<Integer>) actual.get();
+        assertThat(conditionValue.getTableName(), is("tbl"));
+        assertThat(conditionValue.getColumnName(), is("id"));
+        assertThat(conditionValue.getValues(), is(Collections.singletonList(1)));
+    }
+    
+    @Test(expected = IllegalArgumentException.class)
+    public void assertGenerateConditionValueWithoutParameter() {
+        ColumnSegment left = new ColumnSegment(0, 0, new IdentifierValue("order_id"));
+        ListExpression right = new ListExpression(0, 0);
+        right.getItems().add(new ParameterMarkerExpressionSegment(0, 0, 0));
+        InExpression predicate = new InExpression(0, 0, left, right, false);
+        generator.generate(predicate, column, new LinkedList<>());
+    }
 }