You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2020/03/27 12:17:27 UTC
[GitHub] [calcite] wenhuitang opened a new pull request #1881:
[CALCITE-3216] ClassCastException when running aggregate function and
window function over Union
wenhuitang opened a new pull request #1881: [CALCITE-3216] ClassCastException when running aggregate function and window function over Union
URL: https://github.com/apache/calcite/pull/1881
This pull request for the issue https://issues.apache.org/jira/browse/CALCITE-3216
According to SqlTypeAssignmentRule which determine whether a type is assignable from another type, this pull request mainly realized a ExtendedCastingEnumerator which enable the cast between most of the Java numeric 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] DonnyZone edited a comment on issue #1881: [CALCITE-3216]
ClassCastException when running aggregate function and window function over
Union
Posted by GitBox <gi...@apache.org>.
DonnyZone edited a comment on issue #1881: [CALCITE-3216] ClassCastException when running aggregate function and window function over Union
URL: https://github.com/apache/calcite/pull/1881#issuecomment-607136784
Could you make a conclusion on the topics below?
1) Legal type pairs that is assignable in validation but illegal in runtime.
2) Apart from Union, other SetOps (e.g., Intersect) have the same problem?
3) According to the context and discussion in [CALCITE-3565](https://issues.apache.org/jira/browse/CALCITE-3565) and [CALCITE-3414](https://issues.apache.org/jira/browse/CALCITE-3414), can we use `EnumUtils.convert`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] DonnyZone commented on issue #1881: [CALCITE-3216]
ClassCastException when running aggregate function and window function over
Union
Posted by GitBox <gi...@apache.org>.
DonnyZone commented on issue #1881: [CALCITE-3216] ClassCastException when running aggregate function and window function over Union
URL: https://github.com/apache/calcite/pull/1881#issuecomment-607136784
Could you make a conclusion on the topics below?
1) Legal type pairs that is assignable in validation but illegal in runtime.
2) Apart from Union, other SetOp (e.g., Intersect) has the same problem?
3) According to the context and discussion in [CALCITE-3565](https://issues.apache.org/jira/browse/CALCITE-3565) and [CALCITE-3414](https://issues.apache.org/jira/browse/CALCITE-3414), can we use `EnumUtils.convert`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] wenhuitang commented on issue #1881: [CALCITE-3216]
ClassCastException when running aggregate function and window function over
Union
Posted by GitBox <gi...@apache.org>.
wenhuitang commented on issue #1881: [CALCITE-3216] ClassCastException when running aggregate function and window function over Union
URL: https://github.com/apache/calcite/pull/1881#issuecomment-610904642
> 1. Legal type pairs that is assignable in validation but illegal in runtime.
The detail information like below:
Target SqlTypeName | JavaTypeName | Source SqlTypeName | JavaTypeName
-- | -- | -- | --
SMALLINT | Short.class , short.class | TINYINT | Byte.class , byte.class
INTEGER | Integer.class , int.class | SMALLINT | Short.class , short.class
INTEGER | Integer.class , int.class | TINYINT | Byte.class , byte.class
BIGINT | Long.class , long.class | INTEGER | Integer.class , int.class
BIGINT | Long.class , long.class | SMALLINT | Short.class , short.class
BIGINT | Long.class , long.class | TINYINT | Byte.class , byte.class
FLOAT | Double.class, double.class | TINYINT | Byte.class , byte.class
FLOAT | Double.class, double.class | INTEGER | Integer.class , int.class
FLOAT | Double.class, double.class | SMALLINT | Short.class , short.class
FLOAT | Double.class, double.class | DECIMAL | BigDecimal.class
FLOAT | Double.class, double.class | BIGINT | Long.class , long.class
REAL | Float.class , float.class | FLOAT | Double.class, double.class
REAL | Float.class , float.class | DECIMAL | BigDecimal.class
REAL | Float.class , float.class | BIGINT | Long.class , long.class
REAL | Float.class , float.class | SMALLINT | Short.class , short.class
REAL | Float.class , float.class | TINYINT | Byte.class , byte.class
REAL | Float.class , float.class | INTEGER | Integer.class , int.class
DOUBLE | Double.class , double.class | REAL | Float.class , float.class
DOUBLE | Double.class , double.class | FLOAT | Double.class, double.class
DOUBLE | Double.class , double.class | DECIMAL | BigDecimal.class
DOUBLE | Double.class , double.class | BIGINT | Long.class , long.class
DOUBLE | Double.class , double.class | SMALLINT | Short.class , short.class
DOUBLE | Double.class , double.class | TINYINT | Byte.class , byte.class
DOUBLE | Double.class , double.class | INTEGER | Integer.class , int.class
DECIMAL | BigDecimal.class | DOUBLE | Double.class , double.class
DECIMAL | BigDecimal.class | REAL | Float.class , float.class
DECIMAL | BigDecimal.class | BIGINT | Long.class , long.class
DECIMAL | BigDecimal.class | SMALLINT | Short.class , short.class
DECIMAL | BigDecimal.class | TINYINT | Byte.class , byte.class
DECIMAL | BigDecimal.class | INTEGER | Integer.class , int.class
DATE | Integer.class , int.class | TIMESTAMP | Long.class , long.class
TIME | Integer.class , int.class | TIMESTAMP | Long.class , long.class
As for DATE and TIME, it is related to issue [CALCITE-3894](https://issues.apache.org/jira/browse/CALCITE-3894). It should be processed individually.
> 2. Apart from Union, other SetOps (e.g., Intersect) have the same problem?
As for Intersect, sql like that "select count(*) as C from (\n select \"id\" from (VALUES(CAST(2 AS TINYINT))) \"foo\"(\"id\")\n intersect\n select \"id\" from (VALUES(2)) \"foo\"(\"id\"))" would not throw exception, but the result of it is 0. If we take a cast for its input, the result is 1.
As for Except, same as Union, it would throw ClassCastException.
I will improve this PR later.
> 3. According to the context and discussion in [CALCITE-3565](https://issues.apache.org/jira/browse/CALCITE-3565) and [CALCITE-3414](https://issues.apache.org/jira/browse/CALCITE-3414), can we use `EnumUtils.convert`?
I have tried use `EnumUtils.convert` to take a cast for Union's child expression. CompileException was thrown during compiling generated code. It seems `EnumUtils.convert` can not handle with Enumerator.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] DonnyZone commented on a change in pull request #1881:
[CALCITE-3216] ClassCastException when running aggregate function and
window function over Union
Posted by GitBox <gi...@apache.org>.
DonnyZone commented on a change in pull request #1881: [CALCITE-3216] ClassCastException when running aggregate function and window function over Union
URL: https://github.com/apache/calcite/pull/1881#discussion_r407343904
##########
File path: core/src/test/java/org/apache/calcite/test/JdbcTest.java
##########
@@ -3504,6 +3504,238 @@ public void checkOrderBy(final boolean desc,
"empid=110; name=Theodore");
}
+ /** Test case for
Review comment:
This test is too long. Please put the test into `ReflectiveSchemaTest` and simplify it. You can refer to `ReflectiveSchemaTest.java#testOp`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] DonnyZone commented on issue #1881: [CALCITE-3216]
ClassCastException when running aggregate function and window function over
Union
Posted by GitBox <gi...@apache.org>.
DonnyZone commented on issue #1881: [CALCITE-3216] ClassCastException when running aggregate function and window function over Union
URL: https://github.com/apache/calcite/pull/1881#issuecomment-612776540
> I have tried use EnumUtils.convert to take a cast for Union's child expression. CompileException was thrown during compiling generated code. It seems EnumUtils.convert can not handle with Enumerator.
`EnumUtils.convert` is the unified entrance for type cast/convert. When something causes compileException, we'd better fix 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] DonnyZone commented on a change in pull request #1881:
[CALCITE-3216] ClassCastException when running aggregate function and
window function over Union
Posted by GitBox <gi...@apache.org>.
DonnyZone commented on a change in pull request #1881: [CALCITE-3216] ClassCastException when running aggregate function and window function over Union
URL: https://github.com/apache/calcite/pull/1881#discussion_r407344927
##########
File path: linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##########
@@ -3650,6 +3668,68 @@ public void close() {
}
}
+ /** Extended casting Enumerator that casts each value.
+ * This Enumerator is the extension of CastingEnumerator.
+ * As for sql type, most of the numeric types are compatible, and are assignable from other
+ * numeric types. But after these sql types have been translated to Java types, the original
+ * compatible types cannot cast directly. This Enumerator can help these compatible types cast.
+ *
+ * @param <T> element type */
+ static class ExtendedCastingEnumerator<T> implements Enumerator<T> {
+ private final Enumerator<?> enumerator;
+ private final Class<T> clazz;
+
+ ExtendedCastingEnumerator(Enumerator<?> enumerator, Class<T> clazz) {
+ this.enumerator = enumerator;
+ this.clazz = clazz;
+ }
+
+ public T current() {
+ return extendedCast(enumerator.current());
+ }
+
+ public boolean moveNext() {
+ return enumerator.moveNext();
+ }
+
+ public void reset() {
+ enumerator.reset();
+ }
+
+ public void close() {
+ enumerator.close();
+ }
+
+ public T extendedCast(Object obj) {
Review comment:
According to the cast logic, I think we can achieve the same goal in `EnumUtils.convert`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services