korlov42 commented on code in PR #6406:
URL: https://github.com/apache/ignite-3/pull/6406#discussion_r2269874402
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/partitionawareness/PartitionAwarenessMetadataExtractor.java:
##########
@@ -113,11 +124,31 @@ public static PartitionAwarenessMetadata
getMetadata(IgniteKeyValueModify kv) {
if (expr instanceof RexDynamicParam) {
RexDynamicParam dynamicParam = (RexDynamicParam) expr;
indexes[i] = dynamicParam.getIndex();
+ onlyLiterals = false;
+ } else if (expr instanceof RexLiteral) {
+ RexLiteral expr0 = (RexLiteral) expr;
+
+ // depends on supplied zoneId, it can`t be cached
+ if (expr0.getTypeName() ==
SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE) {
+ return null;
+ }
+
+ indexes[i] = hashPos--;
+ NativeType nativeType =
IgniteTypeFactory.relDataTypeToNative(expr0.getType());
+ Object val = getValueFromLiteral(nativeType, expr0);
+ hashFields.add(ColocationUtils.hash(val, nativeType));
} else {
return null;
}
}
+ // case for IgniteKeyValueModify with only literals in colocation
columns.
+ if (fullRow && onlyLiterals) {
+ return null;
Review Comment:
what is the point of discarding expensive computation here?
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/prepare/partitionawareness/PartitionAwarenessMetadataTest.java:
##########
@@ -158,33 +184,33 @@ public void shortKey(String query,
PartitionAwarenessMetadata expected) {
private static Stream<Arguments> shortKeyMetadata() {
return Stream.of(
// KV GET
- Arguments.of("SELECT * FROM t WHERE c3=? and c2=?",
dynamicParams(0)),
- Arguments.of("SELECT * FROM t WHERE c2=? and c3=?",
dynamicParams(1)),
- Arguments.of("SELECT * FROM t WHERE c1=? and c2=? and c3=?",
dynamicParams(2)),
- Arguments.of("SELECT * FROM t WHERE c3=? and c1=? and c2=?",
dynamicParams(0)),
- Arguments.of("SELECT * FROM t WHERE c3=? and c2=1",
dynamicParams(0)),
- Arguments.of("SELECT * FROM t WHERE c1=1 and c2=? and c3=?",
dynamicParams(1)),
+ Arguments.of("SELECT * FROM t WHERE c3=? and c2=?", meta(0)),
+ Arguments.of("SELECT * FROM t WHERE c2=? and c3=?", meta(1)),
+ Arguments.of("SELECT * FROM t WHERE c1=? and c2=? and c3=?",
meta(2)),
+ Arguments.of("SELECT * FROM t WHERE c3=? and c1=? and c2=?",
meta(0)),
+ Arguments.of("SELECT * FROM t WHERE c3=? and c2=1", meta(0)),
+ Arguments.of("SELECT * FROM t WHERE c1=1 and c2=? and c3=?",
meta(1)),
Arguments.of("SELECT * FROM t WHERE c3=3", null),
- Arguments.of("SELECT * FROM t WHERE c2=? and c3=3", null),
- Arguments.of("SELECT * FROM t WHERE c1=? and c2=? and c3=3",
null),
+ Arguments.of("SELECT * FROM t WHERE c2=? and c3=3", meta(new
int[]{-1}, new int[]{3})),
+ Arguments.of("SELECT * FROM t WHERE c1=? and c2=? and c3=3",
meta(new int[]{-1}, new int[]{3})),
// KV PUT
- Arguments.of("INSERT INTO t VALUES (?, ?, ?)",
dynamicParamsTrackingRequired(2)),
- Arguments.of("INSERT INTO t (c1, c2, c3) VALUES (?, ?, ?)",
dynamicParamsTrackingRequired(2)),
- Arguments.of("INSERT INTO t (c3, c1, c2) VALUES (?, ?, ?)",
dynamicParamsTrackingRequired(0)),
+ Arguments.of("INSERT INTO t VALUES (?, ?, ?)",
metaTrackingRequired(2)),
+ Arguments.of("INSERT INTO t (c1, c2, c3) VALUES (?, ?, ?)",
metaTrackingRequired(2)),
+ Arguments.of("INSERT INTO t (c3, c1, c2) VALUES (?, ?, ?)",
metaTrackingRequired(0)),
Arguments.of("INSERT INTO t (c1, c2, c3) VALUES (?, ?, 3)",
null),
Arguments.of("INSERT INTO t (c1, c3, c2) VALUES (?, 3, ?)",
null),
Arguments.of("INSERT INTO t (c3, c1, c2) VALUES (3, ?, ?)",
null),
// KV DELETE
- Arguments.of("SELECT * FROM t WHERE c3=? and c2=?",
dynamicParams(0)),
- Arguments.of("SELECT * FROM t WHERE c2=? and c3=?",
dynamicParams(1)),
- Arguments.of("SELECT * FROM t WHERE c3=? and c2=1",
dynamicParams(0)),
+ Arguments.of("SELECT * FROM t WHERE c3=? and c2=?", meta(0)),
Review Comment:
these 5 test cases are supposed to be `DELETE` statements, but I forgot to
change this copy-past in my PR I merged recently. May you fix this please?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/RexUtils.java:
##########
@@ -1191,6 +1202,34 @@ public Void visitLocalRef(RexLocalRef locRef) {
return val;
}
+ /** Return literal holding java object according to it`s type. */
+ public static @Nullable Object getValueFromLiteral(NativeType
physicalType, RexLiteral lit) {
Review Comment:
I'm wondering if we really need one more way to extract values from literal
expressions. We already have `literalValue`+`TypeUtils#fromInternal`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
