You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/10/27 19:37:04 UTC
[GitHub] [ignite] Vladsz83 opened a new pull request #9535: IGNITE-15596 : Calcite. java.lang.Integer cannot be cast to java.lang.Long with JOIN and EXISTS.
Vladsz83 opened a new pull request #9535:
URL: https://github.com/apache/ignite/pull/9535
Introduced own type coercion type: IgniteTypeCoercion
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] Vladsz83 commented on a change in pull request #9535: IGNITE-15596 : Calcite. java.lang.Integer cannot be cast to java.lang.Long with JOIN and EXISTS.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #9535:
URL: https://github.com/apache/ignite/pull/9535#discussion_r739079271
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/DataTypesTest.java
##########
@@ -43,6 +43,59 @@
qryEngine = Commons.lookupComponent(grid.context(), QueryEngine.class);
}
+ /**
+ * Tests numeric types mapping on Java types.
+ */
+ @Test
+ public void testNumericTypes() {
Review comment:
> also there is some tests in sqlrunner suite, seems we need uncomment them ?
Fixed
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] alex-plekhanov commented on a change in pull request #9535: IGNITE-15596 : Calcite. java.lang.Integer cannot be cast to java.lang.Long with JOIN and EXISTS.
Posted by GitBox <gi...@apache.org>.
alex-plekhanov commented on a change in pull request #9535:
URL: https://github.com/apache/ignite/pull/9535#discussion_r739172155
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/DataTypesTest.java
##########
@@ -88,7 +152,16 @@ public void testUnicodeStrings() {
}
/** */
- public List<List<?>> executeSql(String sql, Object... params) {
+ private List<List<?>> executeSql(String sql, Object... params) {
return qryEngine.query(null, "PUBLIC", sql, params).get(0).getAll();
}
+
+ /** Checks type and value of single record result. */
+ static void assertSingleEquals(List<List<?>> result, Object val) {
Review comment:
All `assertXXX` methods usually use the first parameter as expected result and the second parameter as the actual result.
Also, perhaps it's better to extend AbstractIntegrationTest and use standard `assertQuery(sql).returns(expected result).check();` instead of this method.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] Vladsz83 commented on a change in pull request #9535: IGNITE-15596 : Calcite. java.lang.Integer cannot be cast to java.lang.Long with JOIN and EXISTS.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #9535:
URL: https://github.com/apache/ignite/pull/9535#discussion_r739075991
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/DataTypesTest.java
##########
@@ -43,6 +43,59 @@
qryEngine = Commons.lookupComponent(grid.context(), QueryEngine.class);
}
+ /**
+ * Tests numeric types mapping on Java types.
+ */
+ @Test
+ public void testNumericTypes() {
Review comment:
> seems we need additional test cover that all numeric types have expected type from table definition?
Like `DataTypesTest#testNumericRanges()` ? it checks ranges and types.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] Vladsz83 commented on a change in pull request #9535: IGNITE-15596 : Calcite. java.lang.Integer cannot be cast to java.lang.Long with JOIN and EXISTS.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #9535:
URL: https://github.com/apache/ignite/pull/9535#discussion_r739229659
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/DataTypesTest.java
##########
@@ -88,7 +152,16 @@ public void testUnicodeStrings() {
}
/** */
- public List<List<?>> executeSql(String sql, Object... params) {
+ private List<List<?>> executeSql(String sql, Object... params) {
return qryEngine.query(null, "PUBLIC", sql, params).get(0).getAll();
}
+
+ /** Checks type and value of single record result. */
+ static void assertSingleEquals(List<List<?>> result, Object val) {
Review comment:
Fixed
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] zstan commented on a change in pull request #9535: IGNITE-15596 : Calcite. java.lang.Integer cannot be cast to java.lang.Long with JOIN and EXISTS.
Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #9535:
URL: https://github.com/apache/ignite/pull/9535#discussion_r739061475
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/DataTypesTest.java
##########
@@ -43,6 +43,59 @@
qryEngine = Commons.lookupComponent(grid.context(), QueryEngine.class);
}
+ /**
+ * Tests numeric types mapping on Java types.
+ */
+ @Test
+ public void testNumericTypes() {
Review comment:
seems we need additional test cover that all numeric types have expected type from table definition?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] Vladsz83 closed pull request #9535: IGNITE-15596 : Calcite. java.lang.Integer cannot be cast to java.lang.Long with JOIN and EXISTS.
Posted by GitBox <gi...@apache.org>.
Vladsz83 closed pull request #9535:
URL: https://github.com/apache/ignite/pull/9535
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] Vladsz83 commented on a change in pull request #9535: IGNITE-15596 : Calcite. java.lang.Integer cannot be cast to java.lang.Long with JOIN and EXISTS.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #9535:
URL: https://github.com/apache/ignite/pull/9535#discussion_r738190459
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgnitePlanner.java
##########
@@ -131,7 +131,7 @@
programs = frameworkCfg.getPrograms();
parserCfg = frameworkCfg.getParserConfig();
sqlToRelConverterCfg = frameworkCfg.getSqlToRelConverterConfig();
- validatorCfg = frameworkCfg.getSqlValidatorConfig();
+ validatorCfg = frameworkCfg.getSqlValidatorConfig().withTypeCoercionFactory(IgniteTypeCoercion::new);
Review comment:
Fixed
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] alex-plekhanov commented on a change in pull request #9535: IGNITE-15596 : Calcite. java.lang.Integer cannot be cast to java.lang.Long with JOIN and EXISTS.
Posted by GitBox <gi...@apache.org>.
alex-plekhanov commented on a change in pull request #9535:
URL: https://github.com/apache/ignite/pull/9535#discussion_r738172416
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgnitePlanner.java
##########
@@ -131,7 +131,7 @@
programs = frameworkCfg.getPrograms();
parserCfg = frameworkCfg.getParserConfig();
sqlToRelConverterCfg = frameworkCfg.getSqlToRelConverterConfig();
- validatorCfg = frameworkCfg.getSqlValidatorConfig();
+ validatorCfg = frameworkCfg.getSqlValidatorConfig().withTypeCoercionFactory(IgniteTypeCoercion::new);
Review comment:
Should be configured in `CalciteQueryProcessor.FRAMEWORK_CONFIG`
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] zstan commented on a change in pull request #9535: IGNITE-15596 : Calcite. java.lang.Integer cannot be cast to java.lang.Long with JOIN and EXISTS.
Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #9535:
URL: https://github.com/apache/ignite/pull/9535#discussion_r739061888
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/DataTypesTest.java
##########
@@ -43,6 +43,59 @@
qryEngine = Commons.lookupComponent(grid.context(), QueryEngine.class);
}
+ /**
+ * Tests numeric types mapping on Java types.
+ */
+ @Test
+ public void testNumericTypes() {
Review comment:
also there is some tests in sqlrunner suite, seems we need uncomment them ?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] Vladsz83 commented on a change in pull request #9535: IGNITE-15596 : Calcite. java.lang.Integer cannot be cast to java.lang.Long with JOIN and EXISTS.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #9535:
URL: https://github.com/apache/ignite/pull/9535#discussion_r739079271
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/DataTypesTest.java
##########
@@ -43,6 +43,59 @@
qryEngine = Commons.lookupComponent(grid.context(), QueryEngine.class);
}
+ /**
+ * Tests numeric types mapping on Java types.
+ */
+ @Test
+ public void testNumericTypes() {
Review comment:
> also there is some tests in sqlrunner suite, seems we need uncomment them ?
Didn't find. Is it in Ignite repository, not GG?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org