korlov42 commented on code in PR #2431:
URL: https://github.com/apache/ignite-3/pull/2431#discussion_r1293403745
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/CastResolutionTest.java:
##########
@@ -165,23 +183,23 @@ private static String makeUsableIntervalType(String
typeName) {
private enum CastMatrix {
Review Comment:
is this matrix for allowed casts or disallowed? Or this matrix is meant to
be exhaustive enumeration of all possible type pairs?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/IgniteCustomAssigmentsRules.java:
##########
@@ -97,6 +81,17 @@ private IgniteCustomAssigmentsRules(
rules.add(type, rule);
}
+ rule.addAll(YEAR_INTERVAL_TYPES);
+ rule.addAll(DAY_INTERVAL_TYPES);
Review Comment:
both this collections contains exhaustive set of interval types, while the
rule allows casting if interval 'contain only a single <primary datetime
field>'.
We need to cover these cases by tests.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/IgniteCustomAssigmentsRules.java:
##########
@@ -81,14 +73,6 @@ private IgniteCustomAssigmentsRules(
rule.addAll(APPROX_TYPES);
rule.addAll(CHAR_TYPES);
- // TINYINT is assignable from...
- // SMALLINT is assignable from...
- // INTEGER is assignable from...
- // BIGINT is assignable from...
- for (SqlTypeName type : EXACT_TYPES) {
- rules.add(type, rule);
- }
-
// FLOAT (up to 64 bit floating point) is assignable from...
// REAL (32 bit floating point) is assignable from...
// DOUBLE is assignable from...
Review Comment:
why do you set rules for DECIMAL here, if you are overriding it on the very
next step?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/IgniteCustomAssigmentsRules.java:
##########
@@ -178,16 +173,20 @@ private IgniteCustomAssigmentsRules(
Review Comment:
why do you have special treatment of TIME WITH LOCAL TIME ZONE?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/IgniteCustomAssigmentsRules.java:
##########
@@ -97,6 +81,17 @@ private IgniteCustomAssigmentsRules(
rules.add(type, rule);
}
+ rule.addAll(YEAR_INTERVAL_TYPES);
+ rule.addAll(DAY_INTERVAL_TYPES);
+
+ // TINYINT is assignable from...
+ // SMALLINT is assignable from...
+ // INTEGER is assignable from...
+ // BIGINT is assignable from...
+ for (SqlTypeName type : EXACT_TYPES) {
+ rules.add(type, rule);
+ }
+
// VARBINARY is assignable from...
rule.clear();
rule.addAll(BINARY_TYPES);
Review Comment:
conversion CHARACTER <-> BINARY is not allowed.
BTW, rules for BINARY and VARBINRY are exactly the same. Why not reuse the
rule set? The same for [VAR-]CHAR
--
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]
