[jira] [Commented] (CALCITE-3197) Convert data of Timestamp/Time/Date as original form when enumerating from ArrayTable
[
https://issues.apache.org/jira/browse/CALCITE-3197?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16890125#comment-16890125
]
jin xing commented on CALCITE-3197:
---
THX for your shepherd ~ [~julianhyde]
Now I can understand the point :)
> Convert data of Timestamp/Time/Date as original form when enumerating from
> ArrayTable
> -
>
> Key: CALCITE-3197
> URL: https://issues.apache.org/jira/browse/CALCITE-3197
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: jin xing
>Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> In current implementation ColumnLoader, data of
> {{Rep.JAVA_SQL_TIMESTAMP/Rep.JAVA_SQL_TIME/Rep.JAVA_SQL_DATE}} are converted
> as numeric during loading.
> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/adapter/clone/ColumnLoader.java#L234
> But current code seems forgot to revert the data back to original form when
> enumerating.
> As a result, below test is failing now
> {code:java}
> // MaterializationTest.java
> @Test public void testTimestampType() {
> String sql = "select \"eventid\", \"ts\"\n"
> + "from \"events\"\n"
> + "where \"eventid\" > 5";
> checkMaterialize(sql, sql);
> }{code}
> For type of {{Rep.JAVA_SQL_TIMESTAMP/Rep.JAVA_SQL_TIME/Rep.JAVA_SQL_DATE}},
> cursor acesses by {{TimestampAccessor/TimeAccessor/DateAccessor}}, which
> expect column value as {{Timestamp/Time/Date}}.
> It make sense to 'unwrap' the data as original form when enumerating from
> {{ArrayTable}}.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (CALCITE-3197) Convert data of Timestamp/Time/Date as original form when enumerating from ArrayTable
[
https://issues.apache.org/jira/browse/CALCITE-3197?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16889299#comment-16889299
]
Julian Hyde commented on CALCITE-3197:
--
Actually, cancel that.
When in enumerable convention, the correct format for SQL TIMESTAMP values is
as a long or Long. If you want them as java.sql.Timestamp, convert to JDBC
convention (i.e. using JdbcToEnumerableConverter).
It is not possible to convert a long (SQL TIMESTAMP) to a java.sql.Timestamp.
There is simply not enough information. You have to know what timezone the
TIMESTAMP is in, which means you have to call an API (such as
{{ResultSet.getTimestamp(int, Calendar)}}) that provides a timezone.
> Convert data of Timestamp/Time/Date as original form when enumerating from
> ArrayTable
> -
>
> Key: CALCITE-3197
> URL: https://issues.apache.org/jira/browse/CALCITE-3197
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: jin xing
>Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> In current implementation ColumnLoader, data of
> {{Rep.JAVA_SQL_TIMESTAMP/Rep.JAVA_SQL_TIME/Rep.JAVA_SQL_DATE}} are converted
> as numeric during loading.
> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/adapter/clone/ColumnLoader.java#L234
> But current code seems forgot to revert the data back to original form when
> enumerating.
> As a result, below test is failing now
> {code:java}
> // MaterializationTest.java
> @Test public void testTimestampType() {
> String sql = "select \"eventid\", \"ts\"\n"
> + "from \"events\"\n"
> + "where \"eventid\" > 5";
> checkMaterialize(sql, sql);
> }{code}
> For type of {{Rep.JAVA_SQL_TIMESTAMP/Rep.JAVA_SQL_TIME/Rep.JAVA_SQL_DATE}},
> cursor acesses by {{TimestampAccessor/TimeAccessor/DateAccessor}}, which
> expect column value as {{Timestamp/Time/Date}}.
> It make sense to 'unwrap' the data as original form when enumerating from
> {{ArrayTable}}.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (CALCITE-3197) Convert data of Timestamp/Time/Date as original form when enumerating from ArrayTable
[
https://issues.apache.org/jira/browse/CALCITE-3197?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16889298#comment-16889298
]
Julian Hyde commented on CALCITE-3197:
--
Column should not have an unwrapper field. It already has a Representation,
which is an interface. Can you not just create an implementation of
Representation that unwraps appropriately. I don't think you need to add any
new methods to Representation.
> Convert data of Timestamp/Time/Date as original form when enumerating from
> ArrayTable
> -
>
> Key: CALCITE-3197
> URL: https://issues.apache.org/jira/browse/CALCITE-3197
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: jin xing
>Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> In current implementation ColumnLoader, data of
> {{Rep.JAVA_SQL_TIMESTAMP/Rep.JAVA_SQL_TIME/Rep.JAVA_SQL_DATE}} are converted
> as numeric during loading.
> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/adapter/clone/ColumnLoader.java#L234
> But current code seems forgot to revert the data back to original form when
> enumerating.
> As a result, below test is failing now
> {code:java}
> // MaterializationTest.java
> @Test public void testTimestampType() {
> String sql = "select \"eventid\", \"ts\"\n"
> + "from \"events\"\n"
> + "where \"eventid\" > 5";
> checkMaterialize(sql, sql);
> }{code}
> For type of {{Rep.JAVA_SQL_TIMESTAMP/Rep.JAVA_SQL_TIME/Rep.JAVA_SQL_DATE}},
> cursor acesses by {{TimestampAccessor/TimeAccessor/DateAccessor}}, which
> expect column value as {{Timestamp/Time/Date}}.
> It make sense to 'unwrap' the data as original form when enumerating from
> {{ArrayTable}}.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (CALCITE-3197) Convert data of Timestamp/Time/Date as original form when enumerating from ArrayTable
[
https://issues.apache.org/jira/browse/CALCITE-3197?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=1610#comment-1610
]
jin xing commented on CALCITE-3197:
---
[~julianhyde]
As you shepherded -- it's bad idea to let representation know what it "really
should be", In current change I set the {{unwrapper}} into
{{ArrayTable.Column}} and unwrap when enumerate records. But checking the
patch, I think it's still bigger than your expectation.
Please check the patch again when you have time. If the fix is far from your
idea, it's great if you can point better direction
> Convert data of Timestamp/Time/Date as original form when enumerating from
> ArrayTable
> -
>
> Key: CALCITE-3197
> URL: https://issues.apache.org/jira/browse/CALCITE-3197
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: jin xing
>Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> In current implementation ColumnLoader, data of
> {{Rep.JAVA_SQL_TIMESTAMP/Rep.JAVA_SQL_TIME/Rep.JAVA_SQL_DATE}} are converted
> as numeric during loading.
> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/adapter/clone/ColumnLoader.java#L234
> But current code seems forgot to revert the data back to original form when
> enumerating.
> As a result, below test is failing now
> {code:java}
> // MaterializationTest.java
> @Test public void testTimestampType() {
> String sql = "select \"eventid\", \"ts\"\n"
> + "from \"events\"\n"
> + "where \"eventid\" > 5";
> checkMaterialize(sql, sql);
> }{code}
> For type of {{Rep.JAVA_SQL_TIMESTAMP/Rep.JAVA_SQL_TIME/Rep.JAVA_SQL_DATE}},
> cursor acesses by {{TimestampAccessor/TimeAccessor/DateAccessor}}, which
> expect column value as {{Timestamp/Time/Date}}.
> It make sense to 'unwrap' the data as original form when enumerating from
> {{ArrayTable}}.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (CALCITE-3197) Convert data of Timestamp/Time/Date as original form when enumerating from ArrayTable
[
https://issues.apache.org/jira/browse/CALCITE-3197?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16885816#comment-16885816
]
jin xing commented on CALCITE-3197:
---
[~julianhyde]
THX a lot for comment.
“The idea that a representation knows what it really should be seems very
wrong” -- This really helps me to get clear about the design.
I will refine my change by your comment.
> Convert data of Timestamp/Time/Date as original form when enumerating from
> ArrayTable
> -
>
> Key: CALCITE-3197
> URL: https://issues.apache.org/jira/browse/CALCITE-3197
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: jin xing
>Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> In current implementation ColumnLoader, data of
> {{Rep.JAVA_SQL_TIMESTAMP/Rep.JAVA_SQL_TIME/Rep.JAVA_SQL_DATE}} are converted
> as numeric during loading.
> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/adapter/clone/ColumnLoader.java#L234
> But current code seems forgot to revert the data back to original form when
> enumerating.
> As a result, below test is failing now
> {code:java}
> // MaterializationTest.java
> @Test public void testTimestampType() {
> String sql = "select \"eventid\", \"ts\"\n"
> + "from \"events\"\n"
> + "where \"eventid\" > 5";
> checkMaterialize(sql, sql);
> }{code}
> For type of {{Rep.JAVA_SQL_TIMESTAMP/Rep.JAVA_SQL_TIME/Rep.JAVA_SQL_DATE}},
> cursor acesses by {{TimestampAccessor/TimeAccessor/DateAccessor}}, which
> expect column value as {{Timestamp/Time/Date}}.
> It make sense to 'unwrap' the data as original form when enumerating from
> {{ArrayTable}}.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (CALCITE-3197) Convert data of Timestamp/Time/Date as original form when enumerating from ArrayTable
[
https://issues.apache.org/jira/browse/CALCITE-3197?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16885520#comment-16885520
]
Julian Hyde commented on CALCITE-3197:
--
This PR makes the code a lot less elegant. The idea that a representation knows
what it "really should be" seems very wrong.
My gut tells me that there is a one or two line fix for this problem. This
isn't it.
> Convert data of Timestamp/Time/Date as original form when enumerating from
> ArrayTable
> -
>
> Key: CALCITE-3197
> URL: https://issues.apache.org/jira/browse/CALCITE-3197
> Project: Calcite
> Issue Type: Bug
> Components: core
>Reporter: jin xing
>Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> In current implementation ColumnLoader, data of
> {{Rep.JAVA_SQL_TIMESTAMP/Rep.JAVA_SQL_TIME/Rep.JAVA_SQL_DATE}} are converted
> as numeric during loading.
> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/adapter/clone/ColumnLoader.java#L234
> But current code seems forgot to revert the data back to original form when
> enumerating.
> As a result, below test is failing now
> {code:java}
> // MaterializationTest.java
> @Test public void testTimestampType() {
> String sql = "select \"eventid\", \"ts\"\n"
> + "from \"events\"\n"
> + "where \"eventid\" > 5";
> checkMaterialize(sql, sql);
> }{code}
> For type of {{Rep.JAVA_SQL_TIMESTAMP/Rep.JAVA_SQL_TIME/Rep.JAVA_SQL_DATE}},
> cursor acesses by {{TimestampAccessor/TimeAccessor/DateAccessor}}, which
> expect column value as {{Timestamp/Time/Date}}.
> It make sense to 'unwrap' the data as original form when enumerating from
> {{ArrayTable}}.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
