You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "olivrlee (via GitHub)" <gi...@apache.org> on 2023/04/19 22:32:16 UTC
[GitHub] [calcite] olivrlee opened a new pull request, #3168: CALCITE-5662/ Allow CAST(BOOLEAN as INTEGER)
olivrlee opened a new pull request, #3168:
URL: https://github.com/apache/calcite/pull/3168
Previously, `CAST(TRUE as INTEGER)` was throwing a `NumberFormatException`.
This PR fixes the cast simplification for boolean literals.
Note: No additional tests were added because the test `testSimplifyCastLiteral` https://github.com/apache/calcite/blob/main/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java#L2300 loops through the `BOOLEAN` type and checks that it now can be casted into `INTEGER`, `BIGINT`, `SMALLINT`, `TINYINT`
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] asfgit closed pull request #3168: CALCITE-5662/ Allow CAST(BOOLEAN as INTEGER)
Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit closed pull request #3168: CALCITE-5662/ Allow CAST(BOOLEAN as INTEGER)
URL: https://github.com/apache/calcite/pull/3168
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] chunweilei commented on pull request #3168: CALCITE-5662/ Allow CAST(BOOLEAN as INTEGER)
Posted by "chunweilei (via GitHub)" <gi...@apache.org>.
chunweilei commented on PR #3168:
URL: https://github.com/apache/calcite/pull/3168#issuecomment-1521093194
Could you please correct the format of commit message?
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] chunweilei commented on a diff in pull request #3168: CALCITE-5662/ Allow CAST(BOOLEAN as INTEGER)
Posted by "chunweilei (via GitHub)" <gi...@apache.org>.
chunweilei commented on code in PR #3168:
URL: https://github.com/apache/calcite/pull/3168#discussion_r1175966928
##########
core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java:
##########
@@ -955,6 +961,52 @@ public static boolean canCastFrom(
return rules.canApplyFrom(toTypeName, fromTypeName);
}
+ /**
+ * Compares two types and returns true if fromType can be cast to toType or if the connection
+ * conformance allows exceptions.
+ *
+ * @param toType target of assignment
+ * @param fromType source of assignment
+ * @param coerce if true, the SQL rules for CAST are used; if false, the
+ * rules are similar to Java; e.g. you can't assign short x =
+ * (int) y, and you can't assign int x = (String) z.
+ * @param conformance value for connection conformance
+ * @return true if cast is legal or connection conformance allows exceptions
+ */
+ public static boolean canCastFrom(
+ RelDataType toType,
+ RelDataType fromType,
+ boolean coerce,
+ SqlConformance conformance) {
+ if (castingBooleanToNumber(toType, fromType)) {
+ return conformance.allowLenientBooleanCastTypes();
+ } else {
+ return canCastFrom(toType, fromType, coerce);
+ }
+ }
+
+ /**
+ * Returns whether the attempted cast is from a BOOLEAN to a NUMERIC where
+ * NUMERIC is one of [TINYINT, SMALLINT, INTEGER, BIGINT].
+ * @param toType Target of assignment
+ * @param fromType Source of assignment
+ * @return true iff fromType is boolean and toType is one of [TINYINT, SMALLINT, INTEGER, BIGINT]
+ */
+ public static boolean castingBooleanToNumber(
+ RelDataType toType,
+ RelDataType fromType) {
+ // These following types support casting from booleans in some dialects such as BigQuery.
+ List<String> numericTypes =
+ ImmutableList.of(TINYINT,
+ SMALLINT,
+ INTEGER,
+ BIGINT)
+ .stream().map(type -> type.toString()).collect(Collectors.toList());
+
Review Comment:
You can use `SqlTypeName.INT_TYPES` instead.
##########
core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java:
##########
@@ -955,6 +961,52 @@ public static boolean canCastFrom(
return rules.canApplyFrom(toTypeName, fromTypeName);
}
+ /**
+ * Compares two types and returns true if fromType can be cast to toType or if the connection
+ * conformance allows exceptions.
+ *
+ * @param toType target of assignment
+ * @param fromType source of assignment
+ * @param coerce if true, the SQL rules for CAST are used; if false, the
+ * rules are similar to Java; e.g. you can't assign short x =
+ * (int) y, and you can't assign int x = (String) z.
+ * @param conformance value for connection conformance
+ * @return true if cast is legal or connection conformance allows exceptions
+ */
+ public static boolean canCastFrom(
+ RelDataType toType,
+ RelDataType fromType,
+ boolean coerce,
+ SqlConformance conformance) {
+ if (castingBooleanToNumber(toType, fromType)) {
+ return conformance.allowLenientBooleanCastTypes();
+ } else {
+ return canCastFrom(toType, fromType, coerce);
+ }
+ }
+
+ /**
+ * Returns whether the attempted cast is from a BOOLEAN to a NUMERIC where
+ * NUMERIC is one of [TINYINT, SMALLINT, INTEGER, BIGINT].
+ * @param toType Target of assignment
+ * @param fromType Source of assignment
+ * @return true iff fromType is boolean and toType is one of [TINYINT, SMALLINT, INTEGER, BIGINT]
+ */
+ public static boolean castingBooleanToNumber(
+ RelDataType toType,
+ RelDataType fromType) {
+ // These following types support casting from booleans in some dialects such as BigQuery.
+ List<String> numericTypes =
+ ImmutableList.of(TINYINT,
+ SMALLINT,
+ INTEGER,
+ BIGINT)
+ .stream().map(type -> type.toString()).collect(Collectors.toList());
+
+ return numericTypes.contains(toType.getSqlTypeName().getName()) && fromType.getSqlTypeName().getName()
+ .equals(SqlType.BOOLEAN.name());
+ }
Review Comment:
Use `SqlTypeName` instead of `String` to compare.
##########
core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java:
##########
@@ -955,6 +961,52 @@ public static boolean canCastFrom(
return rules.canApplyFrom(toTypeName, fromTypeName);
}
+ /**
+ * Compares two types and returns true if fromType can be cast to toType or if the connection
+ * conformance allows exceptions.
+ *
+ * @param toType target of assignment
+ * @param fromType source of assignment
+ * @param coerce if true, the SQL rules for CAST are used; if false, the
+ * rules are similar to Java; e.g. you can't assign short x =
+ * (int) y, and you can't assign int x = (String) z.
+ * @param conformance value for connection conformance
+ * @return true if cast is legal or connection conformance allows exceptions
+ */
+ public static boolean canCastFrom(
+ RelDataType toType,
+ RelDataType fromType,
+ boolean coerce,
+ SqlConformance conformance) {
+ if (castingBooleanToNumber(toType, fromType)) {
+ return conformance.allowLenientBooleanCastTypes();
+ } else {
+ return canCastFrom(toType, fromType, coerce);
+ }
+ }
+
+ /**
+ * Returns whether the attempted cast is from a BOOLEAN to a NUMERIC where
+ * NUMERIC is one of [TINYINT, SMALLINT, INTEGER, BIGINT].
+ * @param toType Target of assignment
+ * @param fromType Source of assignment
+ * @return true iff fromType is boolean and toType is one of [TINYINT, SMALLINT, INTEGER, BIGINT]
+ */
+ public static boolean castingBooleanToNumber(
+ RelDataType toType,
+ RelDataType fromType) {
Review Comment:
The name is not clear. `IsCastBooleanToInt` may be better.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] olivrlee commented on a diff in pull request #3168: CALCITE-5662/ Allow CAST(BOOLEAN as INTEGER)
Posted by "olivrlee (via GitHub)" <gi...@apache.org>.
olivrlee commented on code in PR #3168:
URL: https://github.com/apache/calcite/pull/3168#discussion_r1178198833
##########
core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java:
##########
@@ -955,6 +961,52 @@ public static boolean canCastFrom(
return rules.canApplyFrom(toTypeName, fromTypeName);
}
+ /**
+ * Compares two types and returns true if fromType can be cast to toType or if the connection
+ * conformance allows exceptions.
+ *
+ * @param toType target of assignment
+ * @param fromType source of assignment
+ * @param coerce if true, the SQL rules for CAST are used; if false, the
+ * rules are similar to Java; e.g. you can't assign short x =
+ * (int) y, and you can't assign int x = (String) z.
+ * @param conformance value for connection conformance
+ * @return true if cast is legal or connection conformance allows exceptions
+ */
+ public static boolean canCastFrom(
+ RelDataType toType,
+ RelDataType fromType,
+ boolean coerce,
+ SqlConformance conformance) {
+ if (castingBooleanToNumber(toType, fromType)) {
+ return conformance.allowLenientBooleanCastTypes();
+ } else {
+ return canCastFrom(toType, fromType, coerce);
+ }
+ }
+
+ /**
+ * Returns whether the attempted cast is from a BOOLEAN to a NUMERIC where
+ * NUMERIC is one of [TINYINT, SMALLINT, INTEGER, BIGINT].
+ * @param toType Target of assignment
+ * @param fromType Source of assignment
+ * @return true iff fromType is boolean and toType is one of [TINYINT, SMALLINT, INTEGER, BIGINT]
+ */
+ public static boolean castingBooleanToNumber(
+ RelDataType toType,
+ RelDataType fromType) {
+ // These following types support casting from booleans in some dialects such as BigQuery.
+ List<String> numericTypes =
+ ImmutableList.of(TINYINT,
+ SMALLINT,
+ INTEGER,
+ BIGINT)
+ .stream().map(type -> type.toString()).collect(Collectors.toList());
+
Review Comment:
Thanks for the review! Updated.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] julianhyde commented on a diff in pull request #3168: CALCITE-5662/ Allow CAST(BOOLEAN as INTEGER)
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3168:
URL: https://github.com/apache/calcite/pull/3168#discussion_r1188027589
##########
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java:
##########
@@ -208,6 +209,8 @@ RelDataType createTypeWithCharsetAndCollation(
*/
@Nullable RelDataType leastRestrictive(List<RelDataType> types);
+ @Nullable RelDataType leastRestrictive(List<RelDataType> types, SqlTypeMappingRule mappingRule);
Review Comment:
javadoc!
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3168: CALCITE-5662/ Allow CAST(BOOLEAN as INTEGER)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3168:
URL: https://github.com/apache/calcite/pull/3168#issuecomment-1548799989
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3168)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3168&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3168&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3168&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3168&metric=new_coverage&view=list) [92.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3168&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3168&metric=new_duplicated_lines_density&view=list) [5.6% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3168&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] julianhyde commented on a diff in pull request #3168: CALCITE-5662/ Allow CAST(BOOLEAN as INTEGER)
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3168:
URL: https://github.com/apache/calcite/pull/3168#discussion_r1188030256
##########
core/src/test/java/org/apache/calcite/sql/type/SqlTypeUtilTest.java:
##########
@@ -253,7 +257,17 @@ private void compareTypesIgnoringNullability(
}
}
+ @Test void testCanCastDifferentConformance() {
Review Comment:
What is this test testing? Add a comment. Don't force future maintainers to read the source code of the test.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3168: CALCITE-5662/ Allow CAST(BOOLEAN as INTEGER)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3168:
URL: https://github.com/apache/calcite/pull/3168#issuecomment-1553448559
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3168)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3168&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3168&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3168&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=CODE_SMELL) [7 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3168&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3168&metric=new_coverage&view=list) [92.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3168&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3168&metric=new_duplicated_lines_density&view=list) [5.6% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3168&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] olivrlee commented on a diff in pull request #3168: CALCITE-5662/ Allow CAST(BOOLEAN as INTEGER)
Posted by "olivrlee (via GitHub)" <gi...@apache.org>.
olivrlee commented on code in PR #3168:
URL: https://github.com/apache/calcite/pull/3168#discussion_r1194445781
##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -10501,7 +10508,8 @@ private void checkCustomColumnResolving(String table) {
final String sql4 = "insert into EMP_MODIFIABLEVIEW2(\"extra\" INTEGER)"
+ " (empno, ename, job, \"extra\")\n"
+ "values (1, 'Arthur', 'clown', true)";
- s.withSql(sql4).ok();
+ s.withConformance(SqlConformanceEnum.BIG_QUERY).withTypeCoercion(true).withSql(sql4).ok();
Review Comment:
I've verified for that for other conformance enums these should also be included:
- MySql 5
It should not be included for :
- Oracle 10
- Oracle 12
- Strict 2003
I wasn't able to determine it for Presto
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] olivrlee commented on pull request #3168: CALCITE-5662/ Allow CAST(BOOLEAN as INTEGER)
Posted by "olivrlee (via GitHub)" <gi...@apache.org>.
olivrlee commented on PR #3168:
URL: https://github.com/apache/calcite/pull/3168#issuecomment-1522613693
@julianhyde, commit `5d36106677b705a522dbde324c854e08e284300c` is pretty nasty, let me know if you have other suggestions.
The goal was so that we have access to `conformance` in SqlTypeUtil.canCastFrom() here: https://github.com/apache/calcite/pull/3168/commits/5d36106677b705a522dbde324c854e08e284300c#diff-2966c59464d13efb7f54aba526317f063365ea01d653683e1257978fe79820bdR193
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] julianhyde commented on a diff in pull request #3168: CALCITE-5662/ Allow CAST(BOOLEAN as INTEGER)
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3168:
URL: https://github.com/apache/calcite/pull/3168#discussion_r1188031401
##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -10501,7 +10508,8 @@ private void checkCustomColumnResolving(String table) {
final String sql4 = "insert into EMP_MODIFIABLEVIEW2(\"extra\" INTEGER)"
+ " (empno, ename, job, \"extra\")\n"
+ "values (1, 'Arthur', 'clown', true)";
- s.withSql(sql4).ok();
+ s.withConformance(SqlConformanceEnum.BIG_QUERY).withTypeCoercion(true).withSql(sql4).ok();
Review Comment:
you've checked that it's ok on BigQuery... but if so, why only BigQuery? should it fail on other conformances?
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org