This is an automated email from the ASF dual-hosted git repository.
ppa pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 9ff0e672f4b IGNITE-25767 Sql. Fixed KeyValueGetPlan for out of range
key (#6193)
9ff0e672f4b is described below
commit 9ff0e672f4b17fea38a5b372d71a8da0a11d82b4
Author: Pavel Pereslegin <[email protected]>
AuthorDate: Mon Jul 14 17:43:18 2025 +0300
IGNITE-25767 Sql. Fixed KeyValueGetPlan for out of range key (#6193)
---
.../sql/engine/ItSqlUsesKeyValueGetTest.java | 21 +++++++++
.../ignite/internal/sql/engine/util/RexUtils.java | 51 ++++++++++++++++------
.../planner/PrimaryKeyLookupPlannerTest.java | 41 +++++++++++++++++
3 files changed, 99 insertions(+), 14 deletions(-)
diff --git
a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSqlUsesKeyValueGetTest.java
b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSqlUsesKeyValueGetTest.java
index dd8f4b7f7ff..3795c88cd70 100644
---
a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSqlUsesKeyValueGetTest.java
+++
b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSqlUsesKeyValueGetTest.java
@@ -21,6 +21,8 @@ import static
org.apache.ignite.internal.sql.engine.util.QueryChecker.containsSu
import java.util.concurrent.ThreadLocalRandom;
import org.apache.ignite.internal.sql.BaseSqlIntegrationTest;
+import org.apache.ignite.internal.tx.InternalTransaction;
+import org.apache.ignite.tx.Transaction;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.RepeatedTest;
import org.junit.jupiter.api.Test;
@@ -111,6 +113,25 @@ public class ItSqlUsesKeyValueGetTest extends
BaseSqlIntegrationTest {
.check();
}
+ @Test
+ void lookupOnOutOfRangeKey() {
+ Transaction tx = CLUSTER.aliveNode().transactions().begin();
+
+ try {
+ sql(tx, "INSERT INTO simple_key VALUES (2147483647, 0),
(-2147483648, 0);");
+
+ assertQuery((InternalTransaction) tx, "SELECT val FROM simple_key
WHERE id = 2147483648")
+ .returnNothing()
+ .check();
+
+ assertQuery((InternalTransaction) tx, "SELECT val FROM simple_key
WHERE id = -2147483649")
+ .returnNothing()
+ .check();
+ } finally {
+ tx.rollback();
+ }
+ }
+
private static int randomKey() {
int key = ThreadLocalRandom.current().nextInt(TABLE_SIZE) + 1;
diff --git
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/RexUtils.java
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/RexUtils.java
index aa6e7ab99bc..f851c32a4e4 100644
---
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/RexUtils.java
+++
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/RexUtils.java
@@ -500,8 +500,8 @@ public class RexUtils {
fldIdx = fld.getIntKey();
}
- RexNode casted = addCast(cluster, pred.operands.get(1),
types.get(fldIdx));
- bounds.set(fldIdx, new ExactBounds(pred, casted));
+ SaturatedRexNode casted = addCast(cluster,
pred.operands.get(1), types.get(fldIdx));
+ bounds.set(fldIdx, new ExactBounds(pred, casted.value));
}
}
@@ -542,9 +542,13 @@ public class RexUtils {
for (RexCall pred : collFldPreds) {
RexNode val = null;
RexNode ref = pred.getOperands().get(0);
+ boolean saturatedLookup = false;
if (isBinaryComparison(pred)) {
- val = addCast(cluster, pred.operands.get(1), fldType);
+ SaturatedRexNode saturatedNode = addCast(cluster,
pred.getOperands().get(1), fldType);
+
+ val = saturatedNode.value;
+ saturatedLookup = saturatedNode.saturated;
}
SqlOperator op = pred.getOperator();
@@ -552,7 +556,9 @@ public class RexUtils {
if (op.kind == EQUALS || op.kind == IS_NOT_DISTINCT_FROM) {
assert val != null;
- return new ExactBounds(pred, val);
+ RexNode pred0 = saturatedLookup ? builder.makeCall(op, ref,
val) : pred;
+
+ return new ExactBounds(pred0, val);
} else if (op.kind == IS_NULL) {
return new ExactBounds(pred, nullValue);
} else if (op.kind == OR) {
@@ -1146,20 +1152,20 @@ public class RexUtils {
}
}
- private static RexNode addCast(RelOptCluster cluster, RexNode condition,
RelDataType type) {
+ private static SaturatedRexNode addCast(RelOptCluster cluster, RexNode
condition, RelDataType type) {
RexNode node = removeCast(condition);
assert idxOpSupports(node) : "Unsupported RexNode in index condition:
" + node;
RexBuilder builder = cluster.getRexBuilder();
- RexNode saturated = toSaturatedValue(builder, node, type);
+ SaturatedRexNode saturatedLiteral = toSaturatedValue(builder, node,
type);
- if (saturated != null) {
- return saturated;
+ if (saturatedLiteral != null) {
+ return saturatedLiteral;
} else if (TypeUtils.needCastInSearchBounds(Commons.typeFactory(),
node.getType(), type)) {
- return builder.makeCast(type, node);
+ return new SaturatedRexNode(builder.makeCast(type, node), false);
} else {
- return node;
+ return new SaturatedRexNode(node, false);
}
}
@@ -1173,7 +1179,7 @@ public class RexUtils {
* <p>Otherwise returns {@code null}.
*/
@Nullable
- private static RexLiteral toSaturatedValue(RexBuilder builder, RexNode
node, RelDataType type) {
+ private static SaturatedRexNode toSaturatedValue(RexBuilder builder,
RexNode node, RelDataType type) {
if (!SqlTypeUtil.isNumeric(node.getType()) ||
!SqlTypeUtil.isNumeric(type) || !(node instanceof RexLiteral)) {
return null;
}
@@ -1204,24 +1210,29 @@ public class RexUtils {
}
BigDecimal newVal;
+ boolean saturated = false;
if (lower.compareTo(val) > 0) {
newVal = lower;
+ saturated = true;
} else if (val.compareTo(upper) > 0) {
newVal = upper;
+ saturated = true;
} else if (!SqlTypeUtil.equalSansNullability(node.getType(), type)) {
newVal = val;
} else {
// If literal types and required type, match ignoring nullability,
// then return a literal as is.
- return lit;
+ return new SaturatedRexNode(lit, false);
}
if (exact) {
- return builder.makeExactLiteral(newVal, type);
+ lit = builder.makeExactLiteral(newVal, type);
} else {
- return builder.makeApproxLiteral(newVal, type);
+ lit = builder.makeApproxLiteral(newVal, type);
}
+
+ return new SaturatedRexNode(lit, saturated);
}
/**
@@ -1367,4 +1378,16 @@ public class RexUtils {
return wasChanged ? newSearchBounds : searchBounds;
}
+
+ private static class SaturatedRexNode {
+ final RexNode value;
+
+ /** Flag indicating whether the numeric value has been saturated or
the original value is used. */
+ final boolean saturated;
+
+ private SaturatedRexNode(RexNode value, boolean saturated) {
+ this.value = value;
+ this.saturated = saturated;
+ }
+ }
}
diff --git
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/PrimaryKeyLookupPlannerTest.java
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/PrimaryKeyLookupPlannerTest.java
index 31813280a92..1f423f7b03f 100644
---
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/PrimaryKeyLookupPlannerTest.java
+++
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/PrimaryKeyLookupPlannerTest.java
@@ -192,6 +192,47 @@ public class PrimaryKeyLookupPlannerTest extends
AbstractPlannerTest {
assertEmptyCondition((KeyValueGetPlan) plan);
}
+ @Test
+ void optimizedGetWithOutOfRangeKey() {
+ node.initSchema("CREATE TABLE test (id TINYINT PRIMARY KEY, val INT)");
+
+ // Out of range: TINYINT_MAX + 1.
+ {
+ QueryPlan plan = node.prepare("SELECT * FROM test WHERE id = 128");
+
+ assertThat(plan, instanceOf(KeyValueGetPlan.class));
+ assertKeyExpressions((KeyValueGetPlan) plan, "127:TINYINT");
+ assertCondition((KeyValueGetPlan) plan, "=(CAST($t0):INTEGER NOT
NULL, 128)");
+ }
+
+ // Out of range: TINYINT_MIN - 1.
+ {
+ QueryPlan plan = node.prepare("SELECT * FROM test WHERE id =
-129");
+
+ assertThat(plan, instanceOf(KeyValueGetPlan.class));
+ assertKeyExpressions((KeyValueGetPlan) plan, "-128:TINYINT");
+ assertCondition((KeyValueGetPlan) plan, "=(CAST($t0):INTEGER NOT
NULL, -129)");
+ }
+
+ // TINYINT_MAX - no predicate expected.
+ {
+ QueryPlan plan = node.prepare("SELECT * FROM test WHERE id = 127");
+
+ assertThat(plan, instanceOf(KeyValueGetPlan.class));
+ assertKeyExpressions((KeyValueGetPlan) plan, "127:TINYINT");
+ assertEmptyCondition((KeyValueGetPlan) plan);
+ }
+
+ // TINYINT_MIN - no predicate expected.
+ {
+ QueryPlan plan = node.prepare("SELECT * FROM test WHERE id =
-128");
+
+ assertThat(plan, instanceOf(KeyValueGetPlan.class));
+ assertKeyExpressions((KeyValueGetPlan) plan, "-128:TINYINT");
+ assertEmptyCondition((KeyValueGetPlan) plan);
+ }
+ }
+
private static void assertKeyExpressions(KeyValueGetPlan plan, String...
expectedExpressions) {
List<String> keyExpressions = (plan.getRel()).keyExpressions().stream()
.map(RexNode::toString)
