Re: [PR] IGNITE-25716 Change TIME dynamic parameter default precision to 9 [ignite-3]
zstan commented on code in PR #6274:
URL: https://github.com/apache/ignite-3/pull/6274#discussion_r2224473433
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctions.java:
##
@@ -362,8 +363,33 @@ public static BigDecimal toBigDecimal(Number value, int
precision, int scale) {
}
}
+/** Adjusts precision of {@link SqlTypeName#TIME} value. */
+public static @Nullable Integer toTimeExact(@Nullable Object object, int
precision) {
+if (object == null) {
+return null;
+}
+
+assert object instanceof Integer : object.getClass();
+
+return toTimeExact((int) object, precision);
Review Comment:
i see that we can avoid unboxing here, isn`t it ?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] IGNITE-25716 Change TIME dynamic parameter default precision to 9 [ignite-3]
zstan commented on code in PR #6274:
URL: https://github.com/apache/ignite-3/pull/6274#discussion_r2224474201
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctions.java:
##
@@ -362,8 +363,33 @@ public static BigDecimal toBigDecimal(Number value, int
precision, int scale) {
}
}
+/** Adjusts precision of {@link SqlTypeName#TIME} value. */
+public static @Nullable Integer toTimeExact(@Nullable Object object, int
precision) {
+if (object == null) {
+return null;
+}
+
+assert object instanceof Integer : object.getClass();
+
+return toTimeExact((int) object, precision);
+}
+
+/** Adjusts precision of {@link SqlTypeName#TIME} value. */
+public static int toTimeExact(int val, int precision) {
+assert precision >= 0 : "Invalid precision: " + precision;
+
+return IgniteSqlDateTimeUtils.adjustTimeMillis(val, precision);
+}
+
+/** Adjusts precision of {@link SqlTypeName#TIME} value. */
+public static int toTimeExact(long val, int precision) {
Review Comment:
do we have a tests for such a case ?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] IGNITE-25716 Change TIME dynamic parameter default precision to 9 [ignite-3]
zstan commented on code in PR #6274:
URL: https://github.com/apache/ignite-3/pull/6274#discussion_r2224427255
##
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItTemporalIndexTest.java:
##
@@ -158,6 +181,27 @@ public void testSearchGtLtWithPkIdxUsage(String table,
String predicate, Object
}
}
+/** Check range predicates with dynamic parameter. */
+@ParameterizedTest(name = "table = {0}, predicate = {1}")
+@MethodSource("geLeSearchDynParamArguments")
+public void testSearchGtLtWithPkIdxUsageDynamicParam(String table, String
predicate, Object parameter, Object result) {
+assertQuery(format("SELECT /*+ FORCE_INDEX({}),
DISABLE_RULE('TableScanToKeyValueGetRule') */ val FROM {} WHERE pk {} ORDER BY
val",
Review Comment:
involved predicates can not match TableScanToKeyValueGetRule usage, thus it
need to be removed if EQ predicates are not planned to use, otherwise it
confused. Or probably it more clear to extend SORTED_INDEXES with
"pkIndexName(table)" wdyt ?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] IGNITE-25716 Change TIME dynamic parameter default precision to 9 [ignite-3]
zstan commented on code in PR #6274:
URL: https://github.com/apache/ignite-3/pull/6274#discussion_r2223040471
##
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItTemporalIndexTest.java:
##
@@ -70,10 +70,10 @@ static void initTestData() {
.app("CREATE TABLE TIME1 (pk TIME, val TIME, PRIMARY KEY USING
SORTED (pk));").nl()
.app("CREATE TABLE TIME2 (pk TIME, val TIME, PRIMARY KEY USING
HASH (pk));").nl()
.app("CREATE TABLE TIMESTAMP1 (pk TIMESTAMP, val TIMESTAMP,
PRIMARY KEY USING SORTED (pk));").nl()
-.app("CREATE TABLE TIMESTAMP2 (pk TIMESTAMP, val TIMESTAMP,
PRIMARY KEY USING HASH (pk));").nl()
+.app("CREATE TABLE TIMESTAMP2 (pk TIMESTAMP(0), val TIMESTAMP,
PRIMARY KEY USING HASH (pk));").nl()
Review Comment:
in such a case the same tests need for TIME, TS_WITH_LOCAL_TZ ? and seems PR
and issue description changes
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] IGNITE-25716 Change TIME dynamic parameter default precision to 9 [ignite-3]
zstan commented on code in PR #6274: URL: https://github.com/apache/ignite-3/pull/6274#discussion_r2223034471 ## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/TypeUtils.java: ## @@ -598,6 +598,12 @@ public static boolean needCastInSearchBounds(IgniteTypeFactory typeFactory, RelD return false; } +// TIME, TIMESTAMP and TIMESTAMP_LTZ can use index, ignoring precision. +if (fromType.getSqlTypeName() == toType.getSqlTypeName() Review Comment: next cases are unclear for now : create table ... col TIME(3) and further query : select from .. where col = TIME '12:12:12.1234' is it need to be implicitly casted ? And on the contrary: create table ... col TIME(3) and further : select from .. where col = TIME '12:12:12.12'. Other words - temporal predicate with great or less precision than column is. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
