You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "hailin0 (via GitHub)" <gi...@apache.org> on 2023/03/01 12:16:30 UTC
[GitHub] [calcite] hailin0 opened a new pull request, #2855: [CALCITE-5214] Implicit type conversion fails in union sql
hailin0 opened a new pull request, #2855:
URL: https://github.com/apache/calcite/pull/2855
https://issues.apache.org/jira/browse/CALCITE-5214
The following test:
```java
//org.apache.calcite.test.SqlToRelConverterTest
@Test void testUnionNullableImplicitTypeCoercion() {
final String sql = "select id from (values(1), (null)) t (id) union select 'varchar-id' as id";
sql(sql).ok();
}
```
Fails with the following exception:
```java
Exception in thread "main" java.lang.AssertionError: Conversion to relational algebra failed to preserve datatypes:
validated type:
RecordType(VARCHAR NOT NULL ID) NOT NULL
converted type:
RecordType(VARCHAR EXPR$0) NOT NULL
rel:
LogicalUnion(all=[false])
LogicalProject(EXPR$0=[CAST($0):VARCHAR])
LogicalValues(tuples=[[{ 1 }, { null }]])
LogicalValues(tuples=[[{ 'varchar-id' }]]) at org.apache.calcite.sql2rel.SqlToRelConverter.checkConvertedType(SqlToRelConverter.java:486)
at org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:601)
at org.apache.calcite.prepare.PlannerImpl.rel(PlannerImpl.java:259)
```
--
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] hailin0 closed pull request #2855: [CALCITE-5214] Implicit type conversion fails in union sql
Posted by "hailin0 (via GitHub)" <gi...@apache.org>.
hailin0 closed pull request #2855: [CALCITE-5214] Implicit type conversion fails in union sql
URL: https://github.com/apache/calcite/pull/2855
--
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] hailin0 commented on pull request #2855: [CALCITE-5214] Implicit type conversion fails in union sql
Posted by GitBox <gi...@apache.org>.
hailin0 commented on PR #2855:
URL: https://github.com/apache/calcite/pull/2855#issuecomment-1186681960
@NobiGo
@chunweilei
Please help to review
--
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] hailin0 commented on a diff in pull request #2855: [CALCITE-5214] Implicit type conversion fails in union sql
Posted by GitBox <gi...@apache.org>.
hailin0 commented on code in PR #2855:
URL: https://github.com/apache/calcite/pull/2855#discussion_r922942312
##########
core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java:
##########
@@ -436,11 +436,13 @@ private RelDataType getTightestCommonTypeOrThrow(
// 2. VARCHAR has 65536 as default precision.
// 3. Following MS-SQL: BINARY or BOOLEAN can be casted to VARCHAR.
if (SqlTypeUtil.isAtomic(type1) && SqlTypeUtil.isCharacter(type2)) {
- resultType = factory.createSqlType(SqlTypeName.VARCHAR);
+ resultType = factory.createTypeWithNullability(factory.createSqlType(SqlTypeName.VARCHAR),
+ type1.isNullable() || type2.isNullable());
}
if (SqlTypeUtil.isCharacter(type1) && SqlTypeUtil.isAtomic(type2)) {
- resultType = factory.createSqlType(SqlTypeName.VARCHAR);
+ resultType = factory.createTypeWithNullability(factory.createSqlType(SqlTypeName.VARCHAR),
+ type1.isNullable() || type2.isNullable());
}
Review Comment:
https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java#L543-L548
Other type conversions already handle nullable settings in the getTightestCommonType 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: 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 #2855: [CALCITE-5214] Implicit type conversion fails in union sql
Posted by GitBox <gi...@apache.org>.
chunweilei commented on code in PR #2855:
URL: https://github.com/apache/calcite/pull/2855#discussion_r922932458
##########
core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java:
##########
@@ -436,11 +436,13 @@ private RelDataType getTightestCommonTypeOrThrow(
// 2. VARCHAR has 65536 as default precision.
// 3. Following MS-SQL: BINARY or BOOLEAN can be casted to VARCHAR.
if (SqlTypeUtil.isAtomic(type1) && SqlTypeUtil.isCharacter(type2)) {
- resultType = factory.createSqlType(SqlTypeName.VARCHAR);
+ resultType = factory.createTypeWithNullability(factory.createSqlType(SqlTypeName.VARCHAR),
+ type1.isNullable() || type2.isNullable());
}
if (SqlTypeUtil.isCharacter(type1) && SqlTypeUtil.isAtomic(type2)) {
- resultType = factory.createSqlType(SqlTypeName.VARCHAR);
+ resultType = factory.createTypeWithNullability(factory.createSqlType(SqlTypeName.VARCHAR),
+ type1.isNullable() || type2.isNullable());
}
Review Comment:
Do other kinds of conversions have the same issue?
--
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] hailin0 commented on pull request #2855: [CALCITE-5214] Implicit type conversion fails in union sql
Posted by "hailin0 (via GitHub)" <gi...@apache.org>.
hailin0 commented on PR #2855:
URL: https://github.com/apache/calcite/pull/2855#issuecomment-1460288694
what should i do
```xml
FAILURE 0.0sec, org.apache.calcite.test.SqlToRelConverterTest > executionError
java.lang.IllegalArgumentException: Actual and reference files differ. If you are adding new tests, replace the reference file with the current actual file, after checking its content.
diff /home/runner/work/calcite/calcite/core/build/resources/test/org/apache/calcite/test/SqlToRelConverterTest_actual.xml /home/runner/work/calcite/calcite/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
7854,7855c7854
< <![CDATA[select id from (values(1), (null)) t (id)
< union select 'varchar-id' as id]]>
---
> <![CDATA[select id from (values(1), (null)) t (id) union select 'varchar-id' as id]]>
at org.apache.calcite.test.DiffRepository.checkActualAndReferenceFiles(DiffRepository.java:261)
at org.apache.calcite.test.SqlToRelConverterTest.checkActualAndReferenceFiles(SqlToRelConverterTest.java:90)
```
--
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] hailin0 commented on pull request #2855: [CALCITE-5214] Implicit type conversion fails in union sql
Posted by "hailin0 (via GitHub)" <gi...@apache.org>.
hailin0 commented on PR #2855:
URL: https://github.com/apache/calcite/pull/2855#issuecomment-1460218492
> @hailin0 The checkstyle has changed recently, could you resolve these failures first and let the CI pass?
please recheck
--
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] libenchao commented on pull request #2855: [CALCITE-5214] Implicit type conversion fails in union sql
Posted by "libenchao (via GitHub)" <gi...@apache.org>.
libenchao commented on PR #2855:
URL: https://github.com/apache/calcite/pull/2855#issuecomment-1451305631
@hailin0 The checkstyle has changed recently, could you resolve these failures first and let the CI pass?
--
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] hailin0 commented on pull request #2855: [CALCITE-5214] Implicit type conversion fails in union sql
Posted by "hailin0 (via GitHub)" <gi...@apache.org>.
hailin0 commented on PR #2855:
URL: https://github.com/apache/calcite/pull/2855#issuecomment-1450056551
@NobiGo
@chunweilei
Please help to review
--
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