You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Xavier Leong <le...@persistent.com> on 2015/03/27 05:01:46 UTC
RE: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for
number cause exception in AvaticaResultSet
Hi Julian,
Moving this to dev thread for wider audience.
The accessor is created using the table column meta from the signature, which I agree as if we are to reconstruct the data, the right place to look for the correct type is from the column meta.
The frame rows are serialized as Object, if we to make each result item to be with strongly typed, then it will be inefficient to retain the type info in the payload, but if we let the serializer to do the decision, some type are not translated correctly, eg ByteString use for BINARY type, at the remote side, it gets constructed as LinkedHashMap from the JSON {"bytes":"fwAAAQ=="} string.
So, what I see there's 3 way to approach this:
1) Is to have the payload to be strongly typed with typed-value representation
2) Is to reconstruct the data during deserializing based on the column meta
3) Do conversion in accessor to present the correct data based on column meta.
Option 1 will be expensive on the payload, option 2 and 3 is somewhat similar, 2 is to do it 1 time with penalty of longer blocking response, and 3 is to do it lazily when data is access and penalty of repeated conversion for every get.
Thoughts?
-----Original Message-----
From: Julian Hyde (JIRA) [mailto:jira@apache.org]
Sent: Friday, March 27, 2015 6:47 AM
To: Xavier Leong
Subject: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for number cause exception in AvaticaResultSet
[ https://issues.apache.org/jira/browse/CALCITE-647?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14382864#comment-14382864 ]
Julian Hyde commented on CALCITE-647:
-------------------------------------
I agree with your analysis of the cause, but I don't agree with your solution. ByteAccessor is an accessor for values that are known to be of type Byte, so it doesn't need to be fixed; you shouldn't be using it for JSON data. I think you should be using a different accessor, maybe like NumberAccessor, that doesn't make assumptions about the exact type of the values.
Note I added the "boolean json" field to LocalService recently. That was for a very similar purpose.
This fix is not complete without a test case. Without one, we will very easily regress.
I don't think there is a test suite in Avatica that serializes requests and responses to JSON and back. (It doesn't need to send them over HTTP, so it can run in a single thread in a single JVM.) That would have discovered this issue, and probably several similar issues. Maybe you could create a variant of RemoteDriverTest that does this.
Can you also please check whether this issue exists for parameters? I think if you have a long or double parameter and set it to 0, the value will arrive at the server as an int. The server needs to be able to handle that.
[~ndimiduk] Can you please review the tests in Avatica, plus CalciteRemoteDriverTest. If the tests don't have coverage for basic functionality (all data types, all request/response types, over all transports) please log jira cases. (Now we have JdbcMeta and the scott JDBC database, we could consider moving much of CalciteRemoteDriverTest into Avatica; CalciteRemoteDriverTest would be just a sub-class of that test that uses CalciteMeta rather than JdbcMeta.)
> Avatica cursor type cast for number cause exception in AvaticaResultSet
> -----------------------------------------------------------------------
>
> Key: CALCITE-647
> URL: https://issues.apache.org/jira/browse/CALCITE-647
> Project: Calcite
> Issue Type: Bug
> Affects Versions: 1.1.0-incubating
> Reporter: Xavier FH Leong
> Assignee: Julian Hyde
> Labels: avatica
> Attachments: CALCITE-647-cursor-numberTypeCast.patch
>
>
> After the result are deserialized from JSON on remote side, the object is not with it's original type, forcing casing of box type Long on Integer raise exception.
> For all box number, it will type cast to Number and extract using the Number method instead.
> 2015-03-26 15:49:48,154 [Thread-10] ERROR net.sourceforge.squirrel_sql.fw.sql.ResultSetReader - Error reading column data, column index = 3
> java.lang.ClassCastException: java.lang.Integer incompatible with java.lang.Long
> at org.apache.calcite.avatica.util.AbstractCursor$LongAccessor.getLong(AbstractCursor.java:483)
> at org.apache.calcite.avatica.AvaticaResultSet.getLong(AvaticaResultSet.java:252)
> at net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.DataTypeLong.readResultSet(DataTypeLong.java:365)
> at net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.CellComponentFactory.readResultSet(CellComponentFactory.java:488)
> at net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.doContentTabRead(ResultSetReader.java:613)
> at net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.readRow(ResultSetReader.java:184)
> at net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.createRow(ResultSetDataSet.java:237)
> at net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet._setResultSet(ResultSetDataSet.java:203)
> at net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.setSqlExecutionTabResultSet(ResultSetDataSet.java:126)
> at net.sourceforge.squirrel_sql.client.session.mainpanel.SQLExecutionHandler.sqlResultSetAvailable(SQLExecutionHandler.java:410)
> at net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processResultSet(SQLExecuterTask.java:542)
> at net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processQuery(SQLExecuterTask.java:407)
> at net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.run(SQLExecuterTask.java:205)
> at net.sourceforge.squirrel_sql.fw.util.TaskExecuter.run(TaskExecuter.java:82)
> at java.lang.Thread.run(Thread.java:853)
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
DISCLAIMER
==========
This e-mail may contain privileged and confidential information which is the property of Persistent Systems Ltd. It is intended only for the use of the individual or entity to which it is addressed. If you are not the intended recipient, you are not authorized to read, retain, copy, print, distribute or use this message. If you have received this communication in error, please notify the sender and delete all copies of this message. Persistent Systems Ltd. does not accept any liability for virus infected mails.
Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for number cause exception in AvaticaResultSet
Posted by Julian Hyde <ju...@hydromatic.net>.
My idea was to change the representation type to numeric but not change the external type. So, getColumnType should be returning INTEGER not NUMERIC. The representation type causes a NumericAccessor to be created.
To understand the difference between representation and external type, consider: timestamps are internally represented as longs (signed 64 bit integers).
Furthermore, the object that is returned from getObject should be consistent with the external type, and not depend on the representation type.
This is important functionality, so I won't accept a patch without a unit test.
Julian
> On Mar 31, 2015, at 3:52 AM, Xavier Leong <le...@persistent.com> wrote:
>
> Submitted patch at https://issues.apache.org/jira/browse/CALCITE-647 but still lack of unit test.
>
> Normally for Integer or Long, squirrel will show as 442528, but for Numeric, it will show 442,528 (Numerical syntax). It's still acceptable, but import export of data will post problem for scripting to take care of the extra character.
>
> Displaying the meta will show getColumnTypeName=INTEGER, getColumnClassName=java.lang.Integer, getColumnDisplaySize=10 but getColumnType=2 (Numeric)
>
> So most likely that data is formatted based on getColumnType. Even though correct getXXX is called. This is client specific, but the point is that the original ColumnType is modified due to lack of support in transport layer, which in my opinion should not expose to the client application.
>
> Best regards,
> -Xavier.
>
> -----Original Message-----
> From: Julian Hyde [mailto:julianhyde@gmail.com]
> Sent: Monday, March 30, 2015 10:22 PM
> To: dev@calcite.incubator.apache.org
> Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for number cause exception in AvaticaResultSet
>
> Is Squirrel causing getObject? If it were calling type-specific methods such as getInt or getDouble the formatting would be OK.
>
> I think getObject needs to convert to the specific type not keep the value in the representation type.
>
>> On Mar 30, 2015, at 05:54, Xavier Leong <le...@persistent.com> wrote:
>>
>> I'd made toResponse in LocalService using the json boolean too, but not in total agreement that the primitives type be change to Numeric, the layout in Squirrel is formating it as numbers base on number locale. I would vote for the column meta to be untouched and have separate cursor and accessor at the client end to do conversion to the right type.
>>
>> As for having cache mechanism, I think is over kill, as most client use case would be fire sql, get results and store in local data structure and close (statement and result). Seldom will the getter be called on same row and column multiple times. I think options 3 lazy conversion works well till there's usage of IDL suggest in another post.
>>
>> Best regards - Xavier
>>
>> Original Message
>> From: Julian Hyde
>> Sent: Saturday, 28 March 2015 02:20
>> To: dev@calcite.incubator.apache.org
>> Reply To: dev@calcite.incubator.apache.org
>> Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast
>> for number cause exception in AvaticaResultSet
>>
>>
>> Yes, the "json" field (currently always true) is a hack. Ideally the transport (not the services at either end of it) should have opportunity to say that it does not preserve types.
>>
>> What are you proposing to memoize? I would not memoize a result set. It uses significant memory on the client and might go stale.
>>
>> Julian
>>
>>
>>> On Mar 27, 2015, at 9:54 AM, Nick Dimiduk <nd...@gmail.com> wrote:
>>>
>>> This same issue bit me along the way as well. I don't particularly
>>> care for the if (json) block in LocalService. Option (3) above sounds
>>> like a good choice, despite the repetition. Maybe we can memoize the
>>> results to avoid re-computation? Let's say we abstract out the
>>> transport later (maybe for
>>> 2.0) and allow pluggable implementations -- thrift, protobuf, &c.
>>> What would the ideal solution be for that scenario? Both of those
>>> tools provide stronger schema semantics.
>>>
>>> On Fri, Mar 27, 2015 at 9:00 AM, Julian Hyde <ju...@gmail.com> wrote:
>>>
>>>>> So that the JSON will now look like "7f000001" instead of
>>>> {"bytes":"fwAAAQ=="}
>>>>
>>>> Nothing wrong with base64 encoding.
>>>>
>>>> On Fri, Mar 27, 2015 at 1:24 AM, Xavier Leong
>>>> <le...@persistent.com>
>>>> wrote:
>>>>> Thanks for the feedback. I'll look into creating a
>>>>> ListIteratorCursor to
>>>> use at the remote side, so that it will create the "generic" number
>>>> accessor instead of the ExactNumericAccessor, which I agree, should
>>>> match exact.
>>>>>
>>>>> I'll also be looking into fixing the ByteString serialized and
>>>> deserialized, it will use JSON getter to convert byte[] to String,
>>>> and BinaryAccessor will reconvert it back to byte[] from String.
>>>>>
>>>>> So that the JSON will now look like "7f000001" instead of
>>>> {"bytes":"fwAAAQ=="}
>>>>>
>>>>> -----Original Message-----
>>>>> From: Julian Hyde [mailto:julian@hydromatic.net]
>>>>> Sent: Friday, March 27, 2015 3:25 PM
>>>>> To: dev
>>>>> Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type
>>>>> cast
>>>> for number cause exception in AvaticaResultSet
>>>>>
>>>>> I agree with your analysis. 1, 2, 3 are all viable options. 1 makes
>>>>> the
>>>> payload much larger, 2 would require invasive changes to the JSON
>>>> parser, so 3 is my preferred option. I think NumberAccessor does what you need.
>>>>>
>>>>> For the related problem, sending parameter from client to server, I
>>>> think we need to use option 1. The reason is that we don't know what
>>>> the parameter type "is supposed to be". The client can set a
>>>> parameter to
>>>> (Long) 0, which would come across JSON as int 0. So we would need to
>>>> also send the type of each parameter value. Luckily, for parameters
>>>> payload size is not a concern.
>>>>>
>>>>> Julian
>>>>>
>>>>> On Thu, Mar 26, 2015 at 9:01 PM, Xavier Leong
>>>>> <le...@persistent.com>
>>>> wrote:
>>>>>> Hi Julian,
>>>>>>
>>>>>> Moving this to dev thread for wider audience.
>>>>>>
>>>>>> The accessor is created using the table column meta from the
>>>>>> signature,
>>>> which I agree as if we are to reconstruct the data, the right place
>>>> to look for the correct type is from the column meta.
>>>>>>
>>>>>> The frame rows are serialized as Object, if we to make each result
>>>>>> item
>>>> to be with strongly typed, then it will be inefficient to retain the
>>>> type info in the payload, but if we let the serializer to do the
>>>> decision, some type are not translated correctly, eg ByteString use
>>>> for BINARY type, at the remote side, it gets constructed as
>>>> LinkedHashMap from the JSON {"bytes":"fwAAAQ=="} string.
>>>>>>
>>>>>> So, what I see there's 3 way to approach this:
>>>>>>
>>>>>> 1) Is to have the payload to be strongly typed with typed-value
>>>>>> representation
>>>>>> 2) Is to reconstruct the data during deserializing based on the
>>>>>> column meta
>>>>>> 3) Do conversion in accessor to present the correct data based on
>>>> column meta.
>>>>>>
>>>>>> Option 1 will be expensive on the payload, option 2 and 3 is
>>>>>> somewhat
>>>> similar, 2 is to do it 1 time with penalty of longer blocking
>>>> response, and
>>>> 3 is to do it lazily when data is access and penalty of repeated
>>>> conversion for every get.
>>>>>>
>>>>>> Thoughts?
>>>>>>
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Julian Hyde (JIRA) [mailto:jira@apache.org]
>>>>>> Sent: Friday, March 27, 2015 6:47 AM
>>>>>> To: Xavier Leong
>>>>>> Subject: [jira] [Commented] (CALCITE-647) Avatica cursor type cast
>>>>>> for number cause exception in AvaticaResultSet
>>>>>>
>>>>>>
>>>>>> [
>>>>>> https://issues.apache.org/jira/browse/CALCITE-647?page=com.atlassi
>>>>>> an.j
>>>>>> ira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId
>>>>>> =143
>>>>>> 82864#comment-14382864 ]
>>>>>>
>>>>>> Julian Hyde commented on CALCITE-647:
>>>>>> -------------------------------------
>>>>>>
>>>>>> I agree with your analysis of the cause, but I don't agree with
>>>>>> your
>>>> solution. ByteAccessor is an accessor for values that are known to
>>>> be of type Byte, so it doesn't need to be fixed; you shouldn't be
>>>> using it for JSON data. I think you should be using a different
>>>> accessor, maybe like NumberAccessor, that doesn't make assumptions
>>>> about the exact type of the values.
>>>>>>
>>>>>> Note I added the "boolean json" field to LocalService recently.
>>>>>> That
>>>> was for a very similar purpose.
>>>>>>
>>>>>> This fix is not complete without a test case. Without one, we will
>>>>>> very
>>>> easily regress.
>>>>>>
>>>>>> I don't think there is a test suite in Avatica that serializes
>>>>>> requests
>>>> and responses to JSON and back. (It doesn't need to send them over
>>>> HTTP, so it can run in a single thread in a single JVM.) That would
>>>> have discovered this issue, and probably several similar issues.
>>>> Maybe you could create a variant of RemoteDriverTest that does this.
>>>>>>
>>>>>> Can you also please check whether this issue exists for
>>>>>> parameters? I
>>>> think if you have a long or double parameter and set it to 0, the
>>>> value will arrive at the server as an int. The server needs to be
>>>> able to handle that.
>>>>>>
>>>>>> [~ndimiduk] Can you please review the tests in Avatica, plus
>>>>>> CalciteRemoteDriverTest. If the tests don't have coverage for
>>>>>> basic functionality (all data types, all request/response types,
>>>>>> over all
>>>>>> transports) please log jira cases. (Now we have JdbcMeta and the
>>>>>> scott JDBC database, we could consider moving much of
>>>>>> CalciteRemoteDriverTest into Avatica; CalciteRemoteDriverTest
>>>>>> would be just a sub-class of that test that uses CalciteMeta
>>>>>> rather than
>>>>>> JdbcMeta.)
>>>>>>
>>>>>>> Avatica cursor type cast for number cause exception in
>>>>>>> AvaticaResultSet
>>>>>>> -----------------------------------------------------------------
>>>>>>> ----
>>>>>>> --
>>>>>>>
>>>>>>> Key: CALCITE-647
>>>>>>> URL: https://issues.apache.org/jira/browse/CALCITE-647
>>>>>>> Project: Calcite
>>>>>>> Issue Type: Bug
>>>>>>> Affects Versions: 1.1.0-incubating
>>>>>>> Reporter: Xavier FH Leong
>>>>>>> Assignee: Julian Hyde
>>>>>>> Labels: avatica
>>>>>>> Attachments: CALCITE-647-cursor-numberTypeCast.patch
>>>>>>>
>>>>>>>
>>>>>>> After the result are deserialized from JSON on remote side, the
>>>>>>> object
>>>> is not with it's original type, forcing casing of box type Long on
>>>> Integer raise exception.
>>>>>>> For all box number, it will type cast to Number and extract using
>>>>>>> the
>>>> Number method instead.
>>>>>>> 2015-03-26 15:49:48,154 [Thread-10] ERROR
>>>>>>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader - Error
>>>>>>> reading column data, column index = 3
>>>>>>> java.lang.ClassCastException: java.lang.Integer incompatible with
>>>> java.lang.Long
>>>>>>> at
>>>> org.apache.calcite.avatica.util.AbstractCursor$LongAccessor.getLong(
>>>> AbstractCursor.java:483)
>>>>>>> at
>>>> org.apache.calcite.avatica.AvaticaResultSet.getLong(AvaticaResultSet
>>>> .java:252)
>>>>>>> at
>>>> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.DataType
>>>> Long.readResultSet(DataTypeLong.java:365)
>>>>>>> at
>>>> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.CellComp
>>>> onentFactory.readResultSet(CellComponentFactory.java:488)
>>>>>>> at
>>>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.doContentTabRead
>>>> (ResultSetReader.java:613)
>>>>>>> at
>>>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.readRow(ResultSe
>>>> tReader.java:184)
>>>>>>> at
>>>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.creat
>>>> eRow(ResultSetDataSet.java:237)
>>>>>>> at
>>>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet._setR
>>>> esultSet(ResultSetDataSet.java:203)
>>>>>>> at
>>>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.setSq
>>>> lExecutionTabResultSet(ResultSetDataSet.java:126)
>>>>>>> at
>>>> net.sourceforge.squirrel_sql.client.session.mainpanel.SQLExecutionHa
>>>> ndler.sqlResultSetAvailable(SQLExecutionHandler.java:410)
>>>>>>> at
>>>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processR
>>>> esultSet(SQLExecuterTask.java:542)
>>>>>>> at
>>>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processQ
>>>> uery(SQLExecuterTask.java:407)
>>>>>>> at
>>>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.run(SQLE
>>>> xecuterTask.java:205)
>>>>>>> at
>>>> net.sourceforge.squirrel_sql.fw.util.TaskExecuter.run(TaskExecuter.j
>>>> ava:82)
>>>>>>> at java.lang.Thread.run(Thread.java:853)
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> This message was sent by Atlassian JIRA
>>>>>> (v6.3.4#6332)
>>>>>>
>>>>>> DISCLAIMER
>>>>>> ==========
>>>>>> This e-mail may contain privileged and confidential information
>>>>>> which
>>>> is the property of Persistent Systems Ltd. It is intended only for
>>>> the use of the individual or entity to which it is addressed. If you
>>>> are not the intended recipient, you are not authorized to read,
>>>> retain, copy, print, distribute or use this message. If you have
>>>> received this communication in error, please notify the sender and delete all copies of this message.
>>>> Persistent Systems Ltd. does not accept any liability for virus
>>>> infected mails.
>>>>>
>>>>> DISCLAIMER
>>>>> ==========
>>>>> This e-mail may contain privileged and confidential information
>>>>> which is
>>>> the property of Persistent Systems Ltd. It is intended only for the
>>>> use of the individual or entity to which it is addressed. If you are
>>>> not the intended recipient, you are not authorized to read, retain,
>>>> copy, print, distribute or use this message. If you have received
>>>> this communication in error, please notify the sender and delete all copies of this message.
>>>> Persistent Systems Ltd. does not accept any liability for virus
>>>> infected mails.
>>
>>
>> DISCLAIMER
>> ==========
>> This e-mail may contain privileged and confidential information which is the property of Persistent Systems Ltd. It is intended only for the use of the individual or entity to which it is addressed. If you are not the intended recipient, you are not authorized to read, retain, copy, print, distribute or use this message. If you have received this communication in error, please notify the sender and delete all copies of this message. Persistent Systems Ltd. does not accept any liability for virus infected mails.
>>
>
> DISCLAIMER
> ==========
> This e-mail may contain privileged and confidential information which is the property of Persistent Systems Ltd. It is intended only for the use of the individual or entity to which it is addressed. If you are not the intended recipient, you are not authorized to read, retain, copy, print, distribute or use this message. If you have received this communication in error, please notify the sender and delete all copies of this message. Persistent Systems Ltd. does not accept any liability for virus infected mails.
>
RE: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for
number cause exception in AvaticaResultSet
Posted by Xavier Leong <le...@persistent.com>.
Submitted patch at https://issues.apache.org/jira/browse/CALCITE-647 but still lack of unit test.
Normally for Integer or Long, squirrel will show as 442528, but for Numeric, it will show 442,528 (Numerical syntax). It's still acceptable, but import export of data will post problem for scripting to take care of the extra character.
Displaying the meta will show getColumnTypeName=INTEGER, getColumnClassName=java.lang.Integer, getColumnDisplaySize=10 but getColumnType=2 (Numeric)
So most likely that data is formatted based on getColumnType. Even though correct getXXX is called. This is client specific, but the point is that the original ColumnType is modified due to lack of support in transport layer, which in my opinion should not expose to the client application.
Best regards,
-Xavier.
-----Original Message-----
From: Julian Hyde [mailto:julianhyde@gmail.com]
Sent: Monday, March 30, 2015 10:22 PM
To: dev@calcite.incubator.apache.org
Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for number cause exception in AvaticaResultSet
Is Squirrel causing getObject? If it were calling type-specific methods such as getInt or getDouble the formatting would be OK.
I think getObject needs to convert to the specific type not keep the value in the representation type.
> On Mar 30, 2015, at 05:54, Xavier Leong <le...@persistent.com> wrote:
>
> I'd made toResponse in LocalService using the json boolean too, but not in total agreement that the primitives type be change to Numeric, the layout in Squirrel is formating it as numbers base on number locale. I would vote for the column meta to be untouched and have separate cursor and accessor at the client end to do conversion to the right type.
>
> As for having cache mechanism, I think is over kill, as most client use case would be fire sql, get results and store in local data structure and close (statement and result). Seldom will the getter be called on same row and column multiple times. I think options 3 lazy conversion works well till there's usage of IDL suggest in another post.
>
> Best regards - Xavier
>
> Original Message
> From: Julian Hyde
> Sent: Saturday, 28 March 2015 02:20
> To: dev@calcite.incubator.apache.org
> Reply To: dev@calcite.incubator.apache.org
> Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast
> for number cause exception in AvaticaResultSet
>
>
> Yes, the "json" field (currently always true) is a hack. Ideally the transport (not the services at either end of it) should have opportunity to say that it does not preserve types.
>
> What are you proposing to memoize? I would not memoize a result set. It uses significant memory on the client and might go stale.
>
> Julian
>
>
>> On Mar 27, 2015, at 9:54 AM, Nick Dimiduk <nd...@gmail.com> wrote:
>>
>> This same issue bit me along the way as well. I don't particularly
>> care for the if (json) block in LocalService. Option (3) above sounds
>> like a good choice, despite the repetition. Maybe we can memoize the
>> results to avoid re-computation? Let's say we abstract out the
>> transport later (maybe for
>> 2.0) and allow pluggable implementations -- thrift, protobuf, &c.
>> What would the ideal solution be for that scenario? Both of those
>> tools provide stronger schema semantics.
>>
>> On Fri, Mar 27, 2015 at 9:00 AM, Julian Hyde <ju...@gmail.com> wrote:
>>
>>>> So that the JSON will now look like "7f000001" instead of
>>> {"bytes":"fwAAAQ=="}
>>>
>>> Nothing wrong with base64 encoding.
>>>
>>> On Fri, Mar 27, 2015 at 1:24 AM, Xavier Leong
>>> <le...@persistent.com>
>>> wrote:
>>>> Thanks for the feedback. I'll look into creating a
>>>> ListIteratorCursor to
>>> use at the remote side, so that it will create the "generic" number
>>> accessor instead of the ExactNumericAccessor, which I agree, should
>>> match exact.
>>>>
>>>> I'll also be looking into fixing the ByteString serialized and
>>> deserialized, it will use JSON getter to convert byte[] to String,
>>> and BinaryAccessor will reconvert it back to byte[] from String.
>>>>
>>>> So that the JSON will now look like "7f000001" instead of
>>> {"bytes":"fwAAAQ=="}
>>>>
>>>> -----Original Message-----
>>>> From: Julian Hyde [mailto:julian@hydromatic.net]
>>>> Sent: Friday, March 27, 2015 3:25 PM
>>>> To: dev
>>>> Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type
>>>> cast
>>> for number cause exception in AvaticaResultSet
>>>>
>>>> I agree with your analysis. 1, 2, 3 are all viable options. 1 makes
>>>> the
>>> payload much larger, 2 would require invasive changes to the JSON
>>> parser, so 3 is my preferred option. I think NumberAccessor does what you need.
>>>>
>>>> For the related problem, sending parameter from client to server, I
>>> think we need to use option 1. The reason is that we don't know what
>>> the parameter type "is supposed to be". The client can set a
>>> parameter to
>>> (Long) 0, which would come across JSON as int 0. So we would need to
>>> also send the type of each parameter value. Luckily, for parameters
>>> payload size is not a concern.
>>>>
>>>> Julian
>>>>
>>>> On Thu, Mar 26, 2015 at 9:01 PM, Xavier Leong
>>>> <le...@persistent.com>
>>> wrote:
>>>>> Hi Julian,
>>>>>
>>>>> Moving this to dev thread for wider audience.
>>>>>
>>>>> The accessor is created using the table column meta from the
>>>>> signature,
>>> which I agree as if we are to reconstruct the data, the right place
>>> to look for the correct type is from the column meta.
>>>>>
>>>>> The frame rows are serialized as Object, if we to make each result
>>>>> item
>>> to be with strongly typed, then it will be inefficient to retain the
>>> type info in the payload, but if we let the serializer to do the
>>> decision, some type are not translated correctly, eg ByteString use
>>> for BINARY type, at the remote side, it gets constructed as
>>> LinkedHashMap from the JSON {"bytes":"fwAAAQ=="} string.
>>>>>
>>>>> So, what I see there's 3 way to approach this:
>>>>>
>>>>> 1) Is to have the payload to be strongly typed with typed-value
>>>>> representation
>>>>> 2) Is to reconstruct the data during deserializing based on the
>>>>> column meta
>>>>> 3) Do conversion in accessor to present the correct data based on
>>> column meta.
>>>>>
>>>>> Option 1 will be expensive on the payload, option 2 and 3 is
>>>>> somewhat
>>> similar, 2 is to do it 1 time with penalty of longer blocking
>>> response, and
>>> 3 is to do it lazily when data is access and penalty of repeated
>>> conversion for every get.
>>>>>
>>>>> Thoughts?
>>>>>
>>>>>
>>>>> -----Original Message-----
>>>>> From: Julian Hyde (JIRA) [mailto:jira@apache.org]
>>>>> Sent: Friday, March 27, 2015 6:47 AM
>>>>> To: Xavier Leong
>>>>> Subject: [jira] [Commented] (CALCITE-647) Avatica cursor type cast
>>>>> for number cause exception in AvaticaResultSet
>>>>>
>>>>>
>>>>> [
>>>>> https://issues.apache.org/jira/browse/CALCITE-647?page=com.atlassi
>>>>> an.j
>>>>> ira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId
>>>>> =143
>>>>> 82864#comment-14382864 ]
>>>>>
>>>>> Julian Hyde commented on CALCITE-647:
>>>>> -------------------------------------
>>>>>
>>>>> I agree with your analysis of the cause, but I don't agree with
>>>>> your
>>> solution. ByteAccessor is an accessor for values that are known to
>>> be of type Byte, so it doesn't need to be fixed; you shouldn't be
>>> using it for JSON data. I think you should be using a different
>>> accessor, maybe like NumberAccessor, that doesn't make assumptions
>>> about the exact type of the values.
>>>>>
>>>>> Note I added the "boolean json" field to LocalService recently.
>>>>> That
>>> was for a very similar purpose.
>>>>>
>>>>> This fix is not complete without a test case. Without one, we will
>>>>> very
>>> easily regress.
>>>>>
>>>>> I don't think there is a test suite in Avatica that serializes
>>>>> requests
>>> and responses to JSON and back. (It doesn't need to send them over
>>> HTTP, so it can run in a single thread in a single JVM.) That would
>>> have discovered this issue, and probably several similar issues.
>>> Maybe you could create a variant of RemoteDriverTest that does this.
>>>>>
>>>>> Can you also please check whether this issue exists for
>>>>> parameters? I
>>> think if you have a long or double parameter and set it to 0, the
>>> value will arrive at the server as an int. The server needs to be
>>> able to handle that.
>>>>>
>>>>> [~ndimiduk] Can you please review the tests in Avatica, plus
>>>>> CalciteRemoteDriverTest. If the tests don't have coverage for
>>>>> basic functionality (all data types, all request/response types,
>>>>> over all
>>>>> transports) please log jira cases. (Now we have JdbcMeta and the
>>>>> scott JDBC database, we could consider moving much of
>>>>> CalciteRemoteDriverTest into Avatica; CalciteRemoteDriverTest
>>>>> would be just a sub-class of that test that uses CalciteMeta
>>>>> rather than
>>>>> JdbcMeta.)
>>>>>
>>>>>> Avatica cursor type cast for number cause exception in
>>>>>> AvaticaResultSet
>>>>>> -----------------------------------------------------------------
>>>>>> ----
>>>>>> --
>>>>>>
>>>>>> Key: CALCITE-647
>>>>>> URL: https://issues.apache.org/jira/browse/CALCITE-647
>>>>>> Project: Calcite
>>>>>> Issue Type: Bug
>>>>>> Affects Versions: 1.1.0-incubating
>>>>>> Reporter: Xavier FH Leong
>>>>>> Assignee: Julian Hyde
>>>>>> Labels: avatica
>>>>>> Attachments: CALCITE-647-cursor-numberTypeCast.patch
>>>>>>
>>>>>>
>>>>>> After the result are deserialized from JSON on remote side, the
>>>>>> object
>>> is not with it's original type, forcing casing of box type Long on
>>> Integer raise exception.
>>>>>> For all box number, it will type cast to Number and extract using
>>>>>> the
>>> Number method instead.
>>>>>> 2015-03-26 15:49:48,154 [Thread-10] ERROR
>>>>>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader - Error
>>>>>> reading column data, column index = 3
>>>>>> java.lang.ClassCastException: java.lang.Integer incompatible with
>>> java.lang.Long
>>>>>> at
>>> org.apache.calcite.avatica.util.AbstractCursor$LongAccessor.getLong(
>>> AbstractCursor.java:483)
>>>>>> at
>>> org.apache.calcite.avatica.AvaticaResultSet.getLong(AvaticaResultSet
>>> .java:252)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.DataType
>>> Long.readResultSet(DataTypeLong.java:365)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.CellComp
>>> onentFactory.readResultSet(CellComponentFactory.java:488)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.doContentTabRead
>>> (ResultSetReader.java:613)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.readRow(ResultSe
>>> tReader.java:184)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.creat
>>> eRow(ResultSetDataSet.java:237)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet._setR
>>> esultSet(ResultSetDataSet.java:203)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.setSq
>>> lExecutionTabResultSet(ResultSetDataSet.java:126)
>>>>>> at
>>> net.sourceforge.squirrel_sql.client.session.mainpanel.SQLExecutionHa
>>> ndler.sqlResultSetAvailable(SQLExecutionHandler.java:410)
>>>>>> at
>>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processR
>>> esultSet(SQLExecuterTask.java:542)
>>>>>> at
>>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processQ
>>> uery(SQLExecuterTask.java:407)
>>>>>> at
>>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.run(SQLE
>>> xecuterTask.java:205)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.util.TaskExecuter.run(TaskExecuter.j
>>> ava:82)
>>>>>> at java.lang.Thread.run(Thread.java:853)
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> This message was sent by Atlassian JIRA
>>>>> (v6.3.4#6332)
>>>>>
>>>>> DISCLAIMER
>>>>> ==========
>>>>> This e-mail may contain privileged and confidential information
>>>>> which
>>> is the property of Persistent Systems Ltd. It is intended only for
>>> the use of the individual or entity to which it is addressed. If you
>>> are not the intended recipient, you are not authorized to read,
>>> retain, copy, print, distribute or use this message. If you have
>>> received this communication in error, please notify the sender and delete all copies of this message.
>>> Persistent Systems Ltd. does not accept any liability for virus
>>> infected mails.
>>>>
>>>> DISCLAIMER
>>>> ==========
>>>> This e-mail may contain privileged and confidential information
>>>> which is
>>> the property of Persistent Systems Ltd. It is intended only for the
>>> use of the individual or entity to which it is addressed. If you are
>>> not the intended recipient, you are not authorized to read, retain,
>>> copy, print, distribute or use this message. If you have received
>>> this communication in error, please notify the sender and delete all copies of this message.
>>> Persistent Systems Ltd. does not accept any liability for virus
>>> infected mails.
>
>
> DISCLAIMER
> ==========
> This e-mail may contain privileged and confidential information which is the property of Persistent Systems Ltd. It is intended only for the use of the individual or entity to which it is addressed. If you are not the intended recipient, you are not authorized to read, retain, copy, print, distribute or use this message. If you have received this communication in error, please notify the sender and delete all copies of this message. Persistent Systems Ltd. does not accept any liability for virus infected mails.
>
DISCLAIMER
==========
This e-mail may contain privileged and confidential information which is the property of Persistent Systems Ltd. It is intended only for the use of the individual or entity to which it is addressed. If you are not the intended recipient, you are not authorized to read, retain, copy, print, distribute or use this message. If you have received this communication in error, please notify the sender and delete all copies of this message. Persistent Systems Ltd. does not accept any liability for virus infected mails.
Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for number cause exception in AvaticaResultSet
Posted by Julian Hyde <ju...@gmail.com>.
I don't think we can declare victory on our support for even basic data types until we have done exhaustive testing.
In CalciteRemoteDriverTest.testParameterConvert, I test PreparedStatement.setObject(int, Object, int) with every combination of source & destination types that are allowed by the JDBC spec.
We also need to test PreparedStatement.setObject(int, Object) (i.e. converting to what Calcite thinks the parameter should be), all of the PreparedStatement.setXxx methods, and all of the ResultSet.getXxx methods.
I have logged https://issues.apache.org/jira/browse/CALCITE-656 for this. Any volunteers to implement it?
Julian
> On Mar 30, 2015, at 12:56 PM, Nick Dimiduk <nd...@gmail.com> wrote:
>
> I saw something similar going the other direction with JMeter for
> PreparedStatements -- it was using setObject always and expecting the
> underlying implementation do coerce. See CALCITE-630.
>
> On Mon, Mar 30, 2015 at 7:22 AM, Julian Hyde <ju...@gmail.com> wrote:
>
>> Is Squirrel causing getObject? If it were calling type-specific methods
>> such as getInt or getDouble the formatting would be OK.
>>
>> I think getObject needs to convert to the specific type not keep the value
>> in the representation type.
>>
>>> On Mar 30, 2015, at 05:54, Xavier Leong <le...@persistent.com> wrote:
>>>
>>> I'd made toResponse in LocalService using the json boolean too, but not
>> in total agreement that the primitives type be change to Numeric, the
>> layout in Squirrel is formating it as numbers base on number locale. I
>> would vote for the column meta to be untouched and have separate cursor and
>> accessor at the client end to do conversion to the right type.
>>>
>>> As for having cache mechanism, I think is over kill, as most client use
>> case would be fire sql, get results and store in local data structure and
>> close (statement and result). Seldom will the getter be called on same row
>> and column multiple times. I think options 3 lazy conversion works well
>> till there's usage of IDL suggest in another post.
>>>
>>> Best regards - Xavier
>>>
>>> Original Message
>>> From: Julian Hyde
>>> Sent: Saturday, 28 March 2015 02:20
>>> To: dev@calcite.incubator.apache.org
>>> Reply To: dev@calcite.incubator.apache.org
>>> Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast
>> for number cause exception in AvaticaResultSet
>>>
>>>
>>> Yes, the "json" field (currently always true) is a hack. Ideally the
>> transport (not the services at either end of it) should have opportunity to
>> say that it does not preserve types.
>>>
>>> What are you proposing to memoize? I would not memoize a result set. It
>> uses significant memory on the client and might go stale.
>>>
>>> Julian
>>>
>>>
>>>> On Mar 27, 2015, at 9:54 AM, Nick Dimiduk <nd...@gmail.com> wrote:
>>>>
>>>> This same issue bit me along the way as well. I don't particularly care
>> for
>>>> the if (json) block in LocalService. Option (3) above sounds like a good
>>>> choice, despite the repetition. Maybe we can memoize the results to
>> avoid
>>>> re-computation? Let's say we abstract out the transport later (maybe for
>>>> 2.0) and allow pluggable implementations -- thrift, protobuf, &c. What
>>>> would the ideal solution be for that scenario? Both of those tools
>> provide
>>>> stronger schema semantics.
>>>>
>>>> On Fri, Mar 27, 2015 at 9:00 AM, Julian Hyde <ju...@gmail.com>
>> wrote:
>>>>
>>>>>> So that the JSON will now look like "7f000001" instead of
>>>>> {"bytes":"fwAAAQ=="}
>>>>>
>>>>> Nothing wrong with base64 encoding.
>>>>>
>>>>> On Fri, Mar 27, 2015 at 1:24 AM, Xavier Leong <leongxfh@persistent.com
>>>
>>>>> wrote:
>>>>>> Thanks for the feedback. I'll look into creating a ListIteratorCursor
>> to
>>>>> use at the remote side, so that it will create the "generic" number
>>>>> accessor instead of the ExactNumericAccessor, which I agree, should
>> match
>>>>> exact.
>>>>>>
>>>>>> I'll also be looking into fixing the ByteString serialized and
>>>>> deserialized, it will use JSON getter to convert byte[] to String, and
>>>>> BinaryAccessor will reconvert it back to byte[] from String.
>>>>>>
>>>>>> So that the JSON will now look like "7f000001" instead of
>>>>> {"bytes":"fwAAAQ=="}
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Julian Hyde [mailto:julian@hydromatic.net]
>>>>>> Sent: Friday, March 27, 2015 3:25 PM
>>>>>> To: dev
>>>>>> Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast
>>>>> for number cause exception in AvaticaResultSet
>>>>>>
>>>>>> I agree with your analysis. 1, 2, 3 are all viable options. 1 makes
>> the
>>>>> payload much larger, 2 would require invasive changes to the JSON
>> parser,
>>>>> so 3 is my preferred option. I think NumberAccessor does what you need.
>>>>>>
>>>>>> For the related problem, sending parameter from client to server, I
>>>>> think we need to use option 1. The reason is that we don't know what
>> the
>>>>> parameter type "is supposed to be". The client can set a parameter to
>>>>> (Long) 0, which would come across JSON as int 0. So we would need to
>> also
>>>>> send the type of each parameter value. Luckily, for parameters payload
>> size
>>>>> is not a concern.
>>>>>>
>>>>>> Julian
>>>>>>
>>>>>> On Thu, Mar 26, 2015 at 9:01 PM, Xavier Leong <
>> leongxfh@persistent.com>
>>>>> wrote:
>>>>>>> Hi Julian,
>>>>>>>
>>>>>>> Moving this to dev thread for wider audience.
>>>>>>>
>>>>>>> The accessor is created using the table column meta from the
>> signature,
>>>>> which I agree as if we are to reconstruct the data, the right place to
>> look
>>>>> for the correct type is from the column meta.
>>>>>>>
>>>>>>> The frame rows are serialized as Object, if we to make each result
>> item
>>>>> to be with strongly typed, then it will be inefficient to retain the
>> type
>>>>> info in the payload, but if we let the serializer to do the decision,
>> some
>>>>> type are not translated correctly, eg ByteString use for BINARY type,
>> at
>>>>> the remote side, it gets constructed as LinkedHashMap from the JSON
>>>>> {"bytes":"fwAAAQ=="} string.
>>>>>>>
>>>>>>> So, what I see there's 3 way to approach this:
>>>>>>>
>>>>>>> 1) Is to have the payload to be strongly typed with typed-value
>>>>>>> representation
>>>>>>> 2) Is to reconstruct the data during deserializing based on the
>> column
>>>>>>> meta
>>>>>>> 3) Do conversion in accessor to present the correct data based on
>>>>> column meta.
>>>>>>>
>>>>>>> Option 1 will be expensive on the payload, option 2 and 3 is somewhat
>>>>> similar, 2 is to do it 1 time with penalty of longer blocking
>> response, and
>>>>> 3 is to do it lazily when data is access and penalty of repeated
>> conversion
>>>>> for every get.
>>>>>>>
>>>>>>> Thoughts?
>>>>>>>
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Julian Hyde (JIRA) [mailto:jira@apache.org]
>>>>>>> Sent: Friday, March 27, 2015 6:47 AM
>>>>>>> To: Xavier Leong
>>>>>>> Subject: [jira] [Commented] (CALCITE-647) Avatica cursor type cast
>> for
>>>>>>> number cause exception in AvaticaResultSet
>>>>>>>
>>>>>>>
>>>>>>> [
>>>>>>>
>> https://issues.apache.org/jira/browse/CALCITE-647?page=com.atlassian.j
>>>>>>>
>> ira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=143
>>>>>>> 82864#comment-14382864 ]
>>>>>>>
>>>>>>> Julian Hyde commented on CALCITE-647:
>>>>>>> -------------------------------------
>>>>>>>
>>>>>>> I agree with your analysis of the cause, but I don't agree with your
>>>>> solution. ByteAccessor is an accessor for values that are known to be
>> of
>>>>> type Byte, so it doesn't need to be fixed; you shouldn't be using it
>> for
>>>>> JSON data. I think you should be using a different accessor, maybe like
>>>>> NumberAccessor, that doesn't make assumptions about the exact type of
>> the
>>>>> values.
>>>>>>>
>>>>>>> Note I added the "boolean json" field to LocalService recently. That
>>>>> was for a very similar purpose.
>>>>>>>
>>>>>>> This fix is not complete without a test case. Without one, we will
>> very
>>>>> easily regress.
>>>>>>>
>>>>>>> I don't think there is a test suite in Avatica that serializes
>> requests
>>>>> and responses to JSON and back. (It doesn't need to send them over
>> HTTP, so
>>>>> it can run in a single thread in a single JVM.) That would have
>> discovered
>>>>> this issue, and probably several similar issues. Maybe you could
>> create a
>>>>> variant of RemoteDriverTest that does this.
>>>>>>>
>>>>>>> Can you also please check whether this issue exists for parameters? I
>>>>> think if you have a long or double parameter and set it to 0, the value
>>>>> will arrive at the server as an int. The server needs to be able to
>> handle
>>>>> that.
>>>>>>>
>>>>>>> [~ndimiduk] Can you please review the tests in Avatica, plus
>>>>>>> CalciteRemoteDriverTest. If the tests don't have coverage for basic
>>>>>>> functionality (all data types, all request/response types, over all
>>>>>>> transports) please log jira cases. (Now we have JdbcMeta and the
>>>>>>> scott JDBC database, we could consider moving much of
>>>>>>> CalciteRemoteDriverTest into Avatica; CalciteRemoteDriverTest would
>> be
>>>>>>> just a sub-class of that test that uses CalciteMeta rather than
>>>>>>> JdbcMeta.)
>>>>>>>
>>>>>>>> Avatica cursor type cast for number cause exception in
>>>>>>>> AvaticaResultSet
>>>>>>>>
>> ---------------------------------------------------------------------
>>>>>>>> --
>>>>>>>>
>>>>>>>> Key: CALCITE-647
>>>>>>>> URL:
>> https://issues.apache.org/jira/browse/CALCITE-647
>>>>>>>> Project: Calcite
>>>>>>>> Issue Type: Bug
>>>>>>>> Affects Versions: 1.1.0-incubating
>>>>>>>> Reporter: Xavier FH Leong
>>>>>>>> Assignee: Julian Hyde
>>>>>>>> Labels: avatica
>>>>>>>> Attachments: CALCITE-647-cursor-numberTypeCast.patch
>>>>>>>>
>>>>>>>>
>>>>>>>> After the result are deserialized from JSON on remote side, the
>> object
>>>>> is not with it's original type, forcing casing of box type Long on
>> Integer
>>>>> raise exception.
>>>>>>>> For all box number, it will type cast to Number and extract using
>> the
>>>>> Number method instead.
>>>>>>>> 2015-03-26 15:49:48,154 [Thread-10] ERROR
>>>>>>>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader - Error reading
>>>>>>>> column data, column index = 3
>>>>>>>> java.lang.ClassCastException: java.lang.Integer incompatible with
>>>>> java.lang.Long
>>>>>>>> at
>>>>>
>> org.apache.calcite.avatica.util.AbstractCursor$LongAccessor.getLong(AbstractCursor.java:483)
>>>>>>>> at
>>>>>
>> org.apache.calcite.avatica.AvaticaResultSet.getLong(AvaticaResultSet.java:252)
>>>>>>>> at
>>>>>
>> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.DataTypeLong.readResultSet(DataTypeLong.java:365)
>>>>>>>> at
>>>>>
>> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.CellComponentFactory.readResultSet(CellComponentFactory.java:488)
>>>>>>>> at
>>>>>
>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.doContentTabRead(ResultSetReader.java:613)
>>>>>>>> at
>>>>>
>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.readRow(ResultSetReader.java:184)
>>>>>>>> at
>>>>>
>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.createRow(ResultSetDataSet.java:237)
>>>>>>>> at
>>>>>
>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet._setResultSet(ResultSetDataSet.java:203)
>>>>>>>> at
>>>>>
>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.setSqlExecutionTabResultSet(ResultSetDataSet.java:126)
>>>>>>>> at
>>>>>
>> net.sourceforge.squirrel_sql.client.session.mainpanel.SQLExecutionHandler.sqlResultSetAvailable(SQLExecutionHandler.java:410)
>>>>>>>> at
>>>>>
>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processResultSet(SQLExecuterTask.java:542)
>>>>>>>> at
>>>>>
>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processQuery(SQLExecuterTask.java:407)
>>>>>>>> at
>>>>>
>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.run(SQLExecuterTask.java:205)
>>>>>>>> at
>>>>>
>> net.sourceforge.squirrel_sql.fw.util.TaskExecuter.run(TaskExecuter.java:82)
>>>>>>>> at java.lang.Thread.run(Thread.java:853)
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> This message was sent by Atlassian JIRA
>>>>>>> (v6.3.4#6332)
>>>>>>>
>>>>>>> DISCLAIMER
>>>>>>> ==========
>>>>>>> This e-mail may contain privileged and confidential information which
>>>>> is the property of Persistent Systems Ltd. It is intended only for the
>> use
>>>>> of the individual or entity to which it is addressed. If you are not
>> the
>>>>> intended recipient, you are not authorized to read, retain, copy,
>> print,
>>>>> distribute or use this message. If you have received this
>> communication in
>>>>> error, please notify the sender and delete all copies of this message.
>>>>> Persistent Systems Ltd. does not accept any liability for virus
>> infected
>>>>> mails.
>>>>>>
>>>>>> DISCLAIMER
>>>>>> ==========
>>>>>> This e-mail may contain privileged and confidential information which
>> is
>>>>> the property of Persistent Systems Ltd. It is intended only for the
>> use of
>>>>> the individual or entity to which it is addressed. If you are not the
>>>>> intended recipient, you are not authorized to read, retain, copy,
>> print,
>>>>> distribute or use this message. If you have received this
>> communication in
>>>>> error, please notify the sender and delete all copies of this message.
>>>>> Persistent Systems Ltd. does not accept any liability for virus
>> infected
>>>>> mails.
>>>
>>>
>>> DISCLAIMER
>>> ==========
>>> This e-mail may contain privileged and confidential information which is
>> the property of Persistent Systems Ltd. It is intended only for the use of
>> the individual or entity to which it is addressed. If you are not the
>> intended recipient, you are not authorized to read, retain, copy, print,
>> distribute or use this message. If you have received this communication in
>> error, please notify the sender and delete all copies of this message.
>> Persistent Systems Ltd. does not accept any liability for virus infected
>> mails.
>>>
>>
Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for
number cause exception in AvaticaResultSet
Posted by Nick Dimiduk <nd...@gmail.com>.
I saw something similar going the other direction with JMeter for
PreparedStatements -- it was using setObject always and expecting the
underlying implementation do coerce. See CALCITE-630.
On Mon, Mar 30, 2015 at 7:22 AM, Julian Hyde <ju...@gmail.com> wrote:
> Is Squirrel causing getObject? If it were calling type-specific methods
> such as getInt or getDouble the formatting would be OK.
>
> I think getObject needs to convert to the specific type not keep the value
> in the representation type.
>
> > On Mar 30, 2015, at 05:54, Xavier Leong <le...@persistent.com> wrote:
> >
> > I'd made toResponse in LocalService using the json boolean too, but not
> in total agreement that the primitives type be change to Numeric, the
> layout in Squirrel is formating it as numbers base on number locale. I
> would vote for the column meta to be untouched and have separate cursor and
> accessor at the client end to do conversion to the right type.
> >
> > As for having cache mechanism, I think is over kill, as most client use
> case would be fire sql, get results and store in local data structure and
> close (statement and result). Seldom will the getter be called on same row
> and column multiple times. I think options 3 lazy conversion works well
> till there's usage of IDL suggest in another post.
> >
> > Best regards - Xavier
> >
> > Original Message
> > From: Julian Hyde
> > Sent: Saturday, 28 March 2015 02:20
> > To: dev@calcite.incubator.apache.org
> > Reply To: dev@calcite.incubator.apache.org
> > Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast
> for number cause exception in AvaticaResultSet
> >
> >
> > Yes, the "json" field (currently always true) is a hack. Ideally the
> transport (not the services at either end of it) should have opportunity to
> say that it does not preserve types.
> >
> > What are you proposing to memoize? I would not memoize a result set. It
> uses significant memory on the client and might go stale.
> >
> > Julian
> >
> >
> >> On Mar 27, 2015, at 9:54 AM, Nick Dimiduk <nd...@gmail.com> wrote:
> >>
> >> This same issue bit me along the way as well. I don't particularly care
> for
> >> the if (json) block in LocalService. Option (3) above sounds like a good
> >> choice, despite the repetition. Maybe we can memoize the results to
> avoid
> >> re-computation? Let's say we abstract out the transport later (maybe for
> >> 2.0) and allow pluggable implementations -- thrift, protobuf, &c. What
> >> would the ideal solution be for that scenario? Both of those tools
> provide
> >> stronger schema semantics.
> >>
> >> On Fri, Mar 27, 2015 at 9:00 AM, Julian Hyde <ju...@gmail.com>
> wrote:
> >>
> >>>> So that the JSON will now look like "7f000001" instead of
> >>> {"bytes":"fwAAAQ=="}
> >>>
> >>> Nothing wrong with base64 encoding.
> >>>
> >>> On Fri, Mar 27, 2015 at 1:24 AM, Xavier Leong <leongxfh@persistent.com
> >
> >>> wrote:
> >>>> Thanks for the feedback. I'll look into creating a ListIteratorCursor
> to
> >>> use at the remote side, so that it will create the "generic" number
> >>> accessor instead of the ExactNumericAccessor, which I agree, should
> match
> >>> exact.
> >>>>
> >>>> I'll also be looking into fixing the ByteString serialized and
> >>> deserialized, it will use JSON getter to convert byte[] to String, and
> >>> BinaryAccessor will reconvert it back to byte[] from String.
> >>>>
> >>>> So that the JSON will now look like "7f000001" instead of
> >>> {"bytes":"fwAAAQ=="}
> >>>>
> >>>> -----Original Message-----
> >>>> From: Julian Hyde [mailto:julian@hydromatic.net]
> >>>> Sent: Friday, March 27, 2015 3:25 PM
> >>>> To: dev
> >>>> Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast
> >>> for number cause exception in AvaticaResultSet
> >>>>
> >>>> I agree with your analysis. 1, 2, 3 are all viable options. 1 makes
> the
> >>> payload much larger, 2 would require invasive changes to the JSON
> parser,
> >>> so 3 is my preferred option. I think NumberAccessor does what you need.
> >>>>
> >>>> For the related problem, sending parameter from client to server, I
> >>> think we need to use option 1. The reason is that we don't know what
> the
> >>> parameter type "is supposed to be". The client can set a parameter to
> >>> (Long) 0, which would come across JSON as int 0. So we would need to
> also
> >>> send the type of each parameter value. Luckily, for parameters payload
> size
> >>> is not a concern.
> >>>>
> >>>> Julian
> >>>>
> >>>> On Thu, Mar 26, 2015 at 9:01 PM, Xavier Leong <
> leongxfh@persistent.com>
> >>> wrote:
> >>>>> Hi Julian,
> >>>>>
> >>>>> Moving this to dev thread for wider audience.
> >>>>>
> >>>>> The accessor is created using the table column meta from the
> signature,
> >>> which I agree as if we are to reconstruct the data, the right place to
> look
> >>> for the correct type is from the column meta.
> >>>>>
> >>>>> The frame rows are serialized as Object, if we to make each result
> item
> >>> to be with strongly typed, then it will be inefficient to retain the
> type
> >>> info in the payload, but if we let the serializer to do the decision,
> some
> >>> type are not translated correctly, eg ByteString use for BINARY type,
> at
> >>> the remote side, it gets constructed as LinkedHashMap from the JSON
> >>> {"bytes":"fwAAAQ=="} string.
> >>>>>
> >>>>> So, what I see there's 3 way to approach this:
> >>>>>
> >>>>> 1) Is to have the payload to be strongly typed with typed-value
> >>>>> representation
> >>>>> 2) Is to reconstruct the data during deserializing based on the
> column
> >>>>> meta
> >>>>> 3) Do conversion in accessor to present the correct data based on
> >>> column meta.
> >>>>>
> >>>>> Option 1 will be expensive on the payload, option 2 and 3 is somewhat
> >>> similar, 2 is to do it 1 time with penalty of longer blocking
> response, and
> >>> 3 is to do it lazily when data is access and penalty of repeated
> conversion
> >>> for every get.
> >>>>>
> >>>>> Thoughts?
> >>>>>
> >>>>>
> >>>>> -----Original Message-----
> >>>>> From: Julian Hyde (JIRA) [mailto:jira@apache.org]
> >>>>> Sent: Friday, March 27, 2015 6:47 AM
> >>>>> To: Xavier Leong
> >>>>> Subject: [jira] [Commented] (CALCITE-647) Avatica cursor type cast
> for
> >>>>> number cause exception in AvaticaResultSet
> >>>>>
> >>>>>
> >>>>> [
> >>>>>
> https://issues.apache.org/jira/browse/CALCITE-647?page=com.atlassian.j
> >>>>>
> ira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=143
> >>>>> 82864#comment-14382864 ]
> >>>>>
> >>>>> Julian Hyde commented on CALCITE-647:
> >>>>> -------------------------------------
> >>>>>
> >>>>> I agree with your analysis of the cause, but I don't agree with your
> >>> solution. ByteAccessor is an accessor for values that are known to be
> of
> >>> type Byte, so it doesn't need to be fixed; you shouldn't be using it
> for
> >>> JSON data. I think you should be using a different accessor, maybe like
> >>> NumberAccessor, that doesn't make assumptions about the exact type of
> the
> >>> values.
> >>>>>
> >>>>> Note I added the "boolean json" field to LocalService recently. That
> >>> was for a very similar purpose.
> >>>>>
> >>>>> This fix is not complete without a test case. Without one, we will
> very
> >>> easily regress.
> >>>>>
> >>>>> I don't think there is a test suite in Avatica that serializes
> requests
> >>> and responses to JSON and back. (It doesn't need to send them over
> HTTP, so
> >>> it can run in a single thread in a single JVM.) That would have
> discovered
> >>> this issue, and probably several similar issues. Maybe you could
> create a
> >>> variant of RemoteDriverTest that does this.
> >>>>>
> >>>>> Can you also please check whether this issue exists for parameters? I
> >>> think if you have a long or double parameter and set it to 0, the value
> >>> will arrive at the server as an int. The server needs to be able to
> handle
> >>> that.
> >>>>>
> >>>>> [~ndimiduk] Can you please review the tests in Avatica, plus
> >>>>> CalciteRemoteDriverTest. If the tests don't have coverage for basic
> >>>>> functionality (all data types, all request/response types, over all
> >>>>> transports) please log jira cases. (Now we have JdbcMeta and the
> >>>>> scott JDBC database, we could consider moving much of
> >>>>> CalciteRemoteDriverTest into Avatica; CalciteRemoteDriverTest would
> be
> >>>>> just a sub-class of that test that uses CalciteMeta rather than
> >>>>> JdbcMeta.)
> >>>>>
> >>>>>> Avatica cursor type cast for number cause exception in
> >>>>>> AvaticaResultSet
> >>>>>>
> ---------------------------------------------------------------------
> >>>>>> --
> >>>>>>
> >>>>>> Key: CALCITE-647
> >>>>>> URL:
> https://issues.apache.org/jira/browse/CALCITE-647
> >>>>>> Project: Calcite
> >>>>>> Issue Type: Bug
> >>>>>> Affects Versions: 1.1.0-incubating
> >>>>>> Reporter: Xavier FH Leong
> >>>>>> Assignee: Julian Hyde
> >>>>>> Labels: avatica
> >>>>>> Attachments: CALCITE-647-cursor-numberTypeCast.patch
> >>>>>>
> >>>>>>
> >>>>>> After the result are deserialized from JSON on remote side, the
> object
> >>> is not with it's original type, forcing casing of box type Long on
> Integer
> >>> raise exception.
> >>>>>> For all box number, it will type cast to Number and extract using
> the
> >>> Number method instead.
> >>>>>> 2015-03-26 15:49:48,154 [Thread-10] ERROR
> >>>>>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader - Error reading
> >>>>>> column data, column index = 3
> >>>>>> java.lang.ClassCastException: java.lang.Integer incompatible with
> >>> java.lang.Long
> >>>>>> at
> >>>
> org.apache.calcite.avatica.util.AbstractCursor$LongAccessor.getLong(AbstractCursor.java:483)
> >>>>>> at
> >>>
> org.apache.calcite.avatica.AvaticaResultSet.getLong(AvaticaResultSet.java:252)
> >>>>>> at
> >>>
> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.DataTypeLong.readResultSet(DataTypeLong.java:365)
> >>>>>> at
> >>>
> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.CellComponentFactory.readResultSet(CellComponentFactory.java:488)
> >>>>>> at
> >>>
> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.doContentTabRead(ResultSetReader.java:613)
> >>>>>> at
> >>>
> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.readRow(ResultSetReader.java:184)
> >>>>>> at
> >>>
> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.createRow(ResultSetDataSet.java:237)
> >>>>>> at
> >>>
> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet._setResultSet(ResultSetDataSet.java:203)
> >>>>>> at
> >>>
> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.setSqlExecutionTabResultSet(ResultSetDataSet.java:126)
> >>>>>> at
> >>>
> net.sourceforge.squirrel_sql.client.session.mainpanel.SQLExecutionHandler.sqlResultSetAvailable(SQLExecutionHandler.java:410)
> >>>>>> at
> >>>
> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processResultSet(SQLExecuterTask.java:542)
> >>>>>> at
> >>>
> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processQuery(SQLExecuterTask.java:407)
> >>>>>> at
> >>>
> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.run(SQLExecuterTask.java:205)
> >>>>>> at
> >>>
> net.sourceforge.squirrel_sql.fw.util.TaskExecuter.run(TaskExecuter.java:82)
> >>>>>> at java.lang.Thread.run(Thread.java:853)
> >>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> This message was sent by Atlassian JIRA
> >>>>> (v6.3.4#6332)
> >>>>>
> >>>>> DISCLAIMER
> >>>>> ==========
> >>>>> This e-mail may contain privileged and confidential information which
> >>> is the property of Persistent Systems Ltd. It is intended only for the
> use
> >>> of the individual or entity to which it is addressed. If you are not
> the
> >>> intended recipient, you are not authorized to read, retain, copy,
> print,
> >>> distribute or use this message. If you have received this
> communication in
> >>> error, please notify the sender and delete all copies of this message.
> >>> Persistent Systems Ltd. does not accept any liability for virus
> infected
> >>> mails.
> >>>>
> >>>> DISCLAIMER
> >>>> ==========
> >>>> This e-mail may contain privileged and confidential information which
> is
> >>> the property of Persistent Systems Ltd. It is intended only for the
> use of
> >>> the individual or entity to which it is addressed. If you are not the
> >>> intended recipient, you are not authorized to read, retain, copy,
> print,
> >>> distribute or use this message. If you have received this
> communication in
> >>> error, please notify the sender and delete all copies of this message.
> >>> Persistent Systems Ltd. does not accept any liability for virus
> infected
> >>> mails.
> >
> >
> > DISCLAIMER
> > ==========
> > This e-mail may contain privileged and confidential information which is
> the property of Persistent Systems Ltd. It is intended only for the use of
> the individual or entity to which it is addressed. If you are not the
> intended recipient, you are not authorized to read, retain, copy, print,
> distribute or use this message. If you have received this communication in
> error, please notify the sender and delete all copies of this message.
> Persistent Systems Ltd. does not accept any liability for virus infected
> mails.
> >
>
Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for number cause exception in AvaticaResultSet
Posted by Julian Hyde <ju...@gmail.com>.
Is Squirrel causing getObject? If it were calling type-specific methods such as getInt or getDouble the formatting would be OK.
I think getObject needs to convert to the specific type not keep the value in the representation type.
> On Mar 30, 2015, at 05:54, Xavier Leong <le...@persistent.com> wrote:
>
> I'd made toResponse in LocalService using the json boolean too, but not in total agreement that the primitives type be change to Numeric, the layout in Squirrel is formating it as numbers base on number locale. I would vote for the column meta to be untouched and have separate cursor and accessor at the client end to do conversion to the right type.
>
> As for having cache mechanism, I think is over kill, as most client use case would be fire sql, get results and store in local data structure and close (statement and result). Seldom will the getter be called on same row and column multiple times. I think options 3 lazy conversion works well till there's usage of IDL suggest in another post.
>
> Best regards - Xavier
>
> Original Message
> From: Julian Hyde
> Sent: Saturday, 28 March 2015 02:20
> To: dev@calcite.incubator.apache.org
> Reply To: dev@calcite.incubator.apache.org
> Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for number cause exception in AvaticaResultSet
>
>
> Yes, the "json" field (currently always true) is a hack. Ideally the transport (not the services at either end of it) should have opportunity to say that it does not preserve types.
>
> What are you proposing to memoize? I would not memoize a result set. It uses significant memory on the client and might go stale.
>
> Julian
>
>
>> On Mar 27, 2015, at 9:54 AM, Nick Dimiduk <nd...@gmail.com> wrote:
>>
>> This same issue bit me along the way as well. I don't particularly care for
>> the if (json) block in LocalService. Option (3) above sounds like a good
>> choice, despite the repetition. Maybe we can memoize the results to avoid
>> re-computation? Let's say we abstract out the transport later (maybe for
>> 2.0) and allow pluggable implementations -- thrift, protobuf, &c. What
>> would the ideal solution be for that scenario? Both of those tools provide
>> stronger schema semantics.
>>
>> On Fri, Mar 27, 2015 at 9:00 AM, Julian Hyde <ju...@gmail.com> wrote:
>>
>>>> So that the JSON will now look like "7f000001" instead of
>>> {"bytes":"fwAAAQ=="}
>>>
>>> Nothing wrong with base64 encoding.
>>>
>>> On Fri, Mar 27, 2015 at 1:24 AM, Xavier Leong <le...@persistent.com>
>>> wrote:
>>>> Thanks for the feedback. I'll look into creating a ListIteratorCursor to
>>> use at the remote side, so that it will create the "generic" number
>>> accessor instead of the ExactNumericAccessor, which I agree, should match
>>> exact.
>>>>
>>>> I'll also be looking into fixing the ByteString serialized and
>>> deserialized, it will use JSON getter to convert byte[] to String, and
>>> BinaryAccessor will reconvert it back to byte[] from String.
>>>>
>>>> So that the JSON will now look like "7f000001" instead of
>>> {"bytes":"fwAAAQ=="}
>>>>
>>>> -----Original Message-----
>>>> From: Julian Hyde [mailto:julian@hydromatic.net]
>>>> Sent: Friday, March 27, 2015 3:25 PM
>>>> To: dev
>>>> Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast
>>> for number cause exception in AvaticaResultSet
>>>>
>>>> I agree with your analysis. 1, 2, 3 are all viable options. 1 makes the
>>> payload much larger, 2 would require invasive changes to the JSON parser,
>>> so 3 is my preferred option. I think NumberAccessor does what you need.
>>>>
>>>> For the related problem, sending parameter from client to server, I
>>> think we need to use option 1. The reason is that we don't know what the
>>> parameter type "is supposed to be". The client can set a parameter to
>>> (Long) 0, which would come across JSON as int 0. So we would need to also
>>> send the type of each parameter value. Luckily, for parameters payload size
>>> is not a concern.
>>>>
>>>> Julian
>>>>
>>>> On Thu, Mar 26, 2015 at 9:01 PM, Xavier Leong <le...@persistent.com>
>>> wrote:
>>>>> Hi Julian,
>>>>>
>>>>> Moving this to dev thread for wider audience.
>>>>>
>>>>> The accessor is created using the table column meta from the signature,
>>> which I agree as if we are to reconstruct the data, the right place to look
>>> for the correct type is from the column meta.
>>>>>
>>>>> The frame rows are serialized as Object, if we to make each result item
>>> to be with strongly typed, then it will be inefficient to retain the type
>>> info in the payload, but if we let the serializer to do the decision, some
>>> type are not translated correctly, eg ByteString use for BINARY type, at
>>> the remote side, it gets constructed as LinkedHashMap from the JSON
>>> {"bytes":"fwAAAQ=="} string.
>>>>>
>>>>> So, what I see there's 3 way to approach this:
>>>>>
>>>>> 1) Is to have the payload to be strongly typed with typed-value
>>>>> representation
>>>>> 2) Is to reconstruct the data during deserializing based on the column
>>>>> meta
>>>>> 3) Do conversion in accessor to present the correct data based on
>>> column meta.
>>>>>
>>>>> Option 1 will be expensive on the payload, option 2 and 3 is somewhat
>>> similar, 2 is to do it 1 time with penalty of longer blocking response, and
>>> 3 is to do it lazily when data is access and penalty of repeated conversion
>>> for every get.
>>>>>
>>>>> Thoughts?
>>>>>
>>>>>
>>>>> -----Original Message-----
>>>>> From: Julian Hyde (JIRA) [mailto:jira@apache.org]
>>>>> Sent: Friday, March 27, 2015 6:47 AM
>>>>> To: Xavier Leong
>>>>> Subject: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for
>>>>> number cause exception in AvaticaResultSet
>>>>>
>>>>>
>>>>> [
>>>>> https://issues.apache.org/jira/browse/CALCITE-647?page=com.atlassian.j
>>>>> ira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=143
>>>>> 82864#comment-14382864 ]
>>>>>
>>>>> Julian Hyde commented on CALCITE-647:
>>>>> -------------------------------------
>>>>>
>>>>> I agree with your analysis of the cause, but I don't agree with your
>>> solution. ByteAccessor is an accessor for values that are known to be of
>>> type Byte, so it doesn't need to be fixed; you shouldn't be using it for
>>> JSON data. I think you should be using a different accessor, maybe like
>>> NumberAccessor, that doesn't make assumptions about the exact type of the
>>> values.
>>>>>
>>>>> Note I added the "boolean json" field to LocalService recently. That
>>> was for a very similar purpose.
>>>>>
>>>>> This fix is not complete without a test case. Without one, we will very
>>> easily regress.
>>>>>
>>>>> I don't think there is a test suite in Avatica that serializes requests
>>> and responses to JSON and back. (It doesn't need to send them over HTTP, so
>>> it can run in a single thread in a single JVM.) That would have discovered
>>> this issue, and probably several similar issues. Maybe you could create a
>>> variant of RemoteDriverTest that does this.
>>>>>
>>>>> Can you also please check whether this issue exists for parameters? I
>>> think if you have a long or double parameter and set it to 0, the value
>>> will arrive at the server as an int. The server needs to be able to handle
>>> that.
>>>>>
>>>>> [~ndimiduk] Can you please review the tests in Avatica, plus
>>>>> CalciteRemoteDriverTest. If the tests don't have coverage for basic
>>>>> functionality (all data types, all request/response types, over all
>>>>> transports) please log jira cases. (Now we have JdbcMeta and the
>>>>> scott JDBC database, we could consider moving much of
>>>>> CalciteRemoteDriverTest into Avatica; CalciteRemoteDriverTest would be
>>>>> just a sub-class of that test that uses CalciteMeta rather than
>>>>> JdbcMeta.)
>>>>>
>>>>>> Avatica cursor type cast for number cause exception in
>>>>>> AvaticaResultSet
>>>>>> ---------------------------------------------------------------------
>>>>>> --
>>>>>>
>>>>>> Key: CALCITE-647
>>>>>> URL: https://issues.apache.org/jira/browse/CALCITE-647
>>>>>> Project: Calcite
>>>>>> Issue Type: Bug
>>>>>> Affects Versions: 1.1.0-incubating
>>>>>> Reporter: Xavier FH Leong
>>>>>> Assignee: Julian Hyde
>>>>>> Labels: avatica
>>>>>> Attachments: CALCITE-647-cursor-numberTypeCast.patch
>>>>>>
>>>>>>
>>>>>> After the result are deserialized from JSON on remote side, the object
>>> is not with it's original type, forcing casing of box type Long on Integer
>>> raise exception.
>>>>>> For all box number, it will type cast to Number and extract using the
>>> Number method instead.
>>>>>> 2015-03-26 15:49:48,154 [Thread-10] ERROR
>>>>>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader - Error reading
>>>>>> column data, column index = 3
>>>>>> java.lang.ClassCastException: java.lang.Integer incompatible with
>>> java.lang.Long
>>>>>> at
>>> org.apache.calcite.avatica.util.AbstractCursor$LongAccessor.getLong(AbstractCursor.java:483)
>>>>>> at
>>> org.apache.calcite.avatica.AvaticaResultSet.getLong(AvaticaResultSet.java:252)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.DataTypeLong.readResultSet(DataTypeLong.java:365)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.CellComponentFactory.readResultSet(CellComponentFactory.java:488)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.doContentTabRead(ResultSetReader.java:613)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.readRow(ResultSetReader.java:184)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.createRow(ResultSetDataSet.java:237)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet._setResultSet(ResultSetDataSet.java:203)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.setSqlExecutionTabResultSet(ResultSetDataSet.java:126)
>>>>>> at
>>> net.sourceforge.squirrel_sql.client.session.mainpanel.SQLExecutionHandler.sqlResultSetAvailable(SQLExecutionHandler.java:410)
>>>>>> at
>>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processResultSet(SQLExecuterTask.java:542)
>>>>>> at
>>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processQuery(SQLExecuterTask.java:407)
>>>>>> at
>>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.run(SQLExecuterTask.java:205)
>>>>>> at
>>> net.sourceforge.squirrel_sql.fw.util.TaskExecuter.run(TaskExecuter.java:82)
>>>>>> at java.lang.Thread.run(Thread.java:853)
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> This message was sent by Atlassian JIRA
>>>>> (v6.3.4#6332)
>>>>>
>>>>> DISCLAIMER
>>>>> ==========
>>>>> This e-mail may contain privileged and confidential information which
>>> is the property of Persistent Systems Ltd. It is intended only for the use
>>> of the individual or entity to which it is addressed. If you are not the
>>> intended recipient, you are not authorized to read, retain, copy, print,
>>> distribute or use this message. If you have received this communication in
>>> error, please notify the sender and delete all copies of this message.
>>> Persistent Systems Ltd. does not accept any liability for virus infected
>>> mails.
>>>>
>>>> DISCLAIMER
>>>> ==========
>>>> This e-mail may contain privileged and confidential information which is
>>> the property of Persistent Systems Ltd. It is intended only for the use of
>>> the individual or entity to which it is addressed. If you are not the
>>> intended recipient, you are not authorized to read, retain, copy, print,
>>> distribute or use this message. If you have received this communication in
>>> error, please notify the sender and delete all copies of this message.
>>> Persistent Systems Ltd. does not accept any liability for virus infected
>>> mails.
>
>
> DISCLAIMER
> ==========
> This e-mail may contain privileged and confidential information which is the property of Persistent Systems Ltd. It is intended only for the use of the individual or entity to which it is addressed. If you are not the intended recipient, you are not authorized to read, retain, copy, print, distribute or use this message. If you have received this communication in error, please notify the sender and delete all copies of this message. Persistent Systems Ltd. does not accept any liability for virus infected mails.
>
Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for
number cause exception in AvaticaResultSet
Posted by Xavier Leong <le...@persistent.com>.
I'd made toResponse in LocalService using the json boolean too, but not in total agreement that the primitives type be change to Numeric, the layout in Squirrel is formating it as numbers base on number locale. I would vote for the column meta to be untouched and have separate cursor and accessor at the client end to do conversion to the right type.
As for having cache mechanism, I think is over kill, as most client use case would be fire sql, get results and store in local data structure and close (statement and result). Seldom will the getter be called on same row and column multiple times. I think options 3 lazy conversion works well till there's usage of IDL suggest in another post.
Best regards - Xavier
Original Message
From: Julian Hyde
Sent: Saturday, 28 March 2015 02:20
To: dev@calcite.incubator.apache.org
Reply To: dev@calcite.incubator.apache.org
Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for number cause exception in AvaticaResultSet
Yes, the "json" field (currently always true) is a hack. Ideally the transport (not the services at either end of it) should have opportunity to say that it does not preserve types.
What are you proposing to memoize? I would not memoize a result set. It uses significant memory on the client and might go stale.
Julian
> On Mar 27, 2015, at 9:54 AM, Nick Dimiduk <nd...@gmail.com> wrote:
>
> This same issue bit me along the way as well. I don't particularly care for
> the if (json) block in LocalService. Option (3) above sounds like a good
> choice, despite the repetition. Maybe we can memoize the results to avoid
> re-computation? Let's say we abstract out the transport later (maybe for
> 2.0) and allow pluggable implementations -- thrift, protobuf, &c. What
> would the ideal solution be for that scenario? Both of those tools provide
> stronger schema semantics.
>
> On Fri, Mar 27, 2015 at 9:00 AM, Julian Hyde <ju...@gmail.com> wrote:
>
>>> So that the JSON will now look like "7f000001" instead of
>> {"bytes":"fwAAAQ=="}
>>
>> Nothing wrong with base64 encoding.
>>
>> On Fri, Mar 27, 2015 at 1:24 AM, Xavier Leong <le...@persistent.com>
>> wrote:
>>> Thanks for the feedback. I'll look into creating a ListIteratorCursor to
>> use at the remote side, so that it will create the "generic" number
>> accessor instead of the ExactNumericAccessor, which I agree, should match
>> exact.
>>>
>>> I'll also be looking into fixing the ByteString serialized and
>> deserialized, it will use JSON getter to convert byte[] to String, and
>> BinaryAccessor will reconvert it back to byte[] from String.
>>>
>>> So that the JSON will now look like "7f000001" instead of
>> {"bytes":"fwAAAQ=="}
>>>
>>> -----Original Message-----
>>> From: Julian Hyde [mailto:julian@hydromatic.net]
>>> Sent: Friday, March 27, 2015 3:25 PM
>>> To: dev
>>> Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast
>> for number cause exception in AvaticaResultSet
>>>
>>> I agree with your analysis. 1, 2, 3 are all viable options. 1 makes the
>> payload much larger, 2 would require invasive changes to the JSON parser,
>> so 3 is my preferred option. I think NumberAccessor does what you need.
>>>
>>> For the related problem, sending parameter from client to server, I
>> think we need to use option 1. The reason is that we don't know what the
>> parameter type "is supposed to be". The client can set a parameter to
>> (Long) 0, which would come across JSON as int 0. So we would need to also
>> send the type of each parameter value. Luckily, for parameters payload size
>> is not a concern.
>>>
>>> Julian
>>>
>>> On Thu, Mar 26, 2015 at 9:01 PM, Xavier Leong <le...@persistent.com>
>> wrote:
>>>> Hi Julian,
>>>>
>>>> Moving this to dev thread for wider audience.
>>>>
>>>> The accessor is created using the table column meta from the signature,
>> which I agree as if we are to reconstruct the data, the right place to look
>> for the correct type is from the column meta.
>>>>
>>>> The frame rows are serialized as Object, if we to make each result item
>> to be with strongly typed, then it will be inefficient to retain the type
>> info in the payload, but if we let the serializer to do the decision, some
>> type are not translated correctly, eg ByteString use for BINARY type, at
>> the remote side, it gets constructed as LinkedHashMap from the JSON
>> {"bytes":"fwAAAQ=="} string.
>>>>
>>>> So, what I see there's 3 way to approach this:
>>>>
>>>> 1) Is to have the payload to be strongly typed with typed-value
>>>> representation
>>>> 2) Is to reconstruct the data during deserializing based on the column
>>>> meta
>>>> 3) Do conversion in accessor to present the correct data based on
>> column meta.
>>>>
>>>> Option 1 will be expensive on the payload, option 2 and 3 is somewhat
>> similar, 2 is to do it 1 time with penalty of longer blocking response, and
>> 3 is to do it lazily when data is access and penalty of repeated conversion
>> for every get.
>>>>
>>>> Thoughts?
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: Julian Hyde (JIRA) [mailto:jira@apache.org]
>>>> Sent: Friday, March 27, 2015 6:47 AM
>>>> To: Xavier Leong
>>>> Subject: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for
>>>> number cause exception in AvaticaResultSet
>>>>
>>>>
>>>> [
>>>> https://issues.apache.org/jira/browse/CALCITE-647?page=com.atlassian.j
>>>> ira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=143
>>>> 82864#comment-14382864 ]
>>>>
>>>> Julian Hyde commented on CALCITE-647:
>>>> -------------------------------------
>>>>
>>>> I agree with your analysis of the cause, but I don't agree with your
>> solution. ByteAccessor is an accessor for values that are known to be of
>> type Byte, so it doesn't need to be fixed; you shouldn't be using it for
>> JSON data. I think you should be using a different accessor, maybe like
>> NumberAccessor, that doesn't make assumptions about the exact type of the
>> values.
>>>>
>>>> Note I added the "boolean json" field to LocalService recently. That
>> was for a very similar purpose.
>>>>
>>>> This fix is not complete without a test case. Without one, we will very
>> easily regress.
>>>>
>>>> I don't think there is a test suite in Avatica that serializes requests
>> and responses to JSON and back. (It doesn't need to send them over HTTP, so
>> it can run in a single thread in a single JVM.) That would have discovered
>> this issue, and probably several similar issues. Maybe you could create a
>> variant of RemoteDriverTest that does this.
>>>>
>>>> Can you also please check whether this issue exists for parameters? I
>> think if you have a long or double parameter and set it to 0, the value
>> will arrive at the server as an int. The server needs to be able to handle
>> that.
>>>>
>>>> [~ndimiduk] Can you please review the tests in Avatica, plus
>>>> CalciteRemoteDriverTest. If the tests don't have coverage for basic
>>>> functionality (all data types, all request/response types, over all
>>>> transports) please log jira cases. (Now we have JdbcMeta and the
>>>> scott JDBC database, we could consider moving much of
>>>> CalciteRemoteDriverTest into Avatica; CalciteRemoteDriverTest would be
>>>> just a sub-class of that test that uses CalciteMeta rather than
>>>> JdbcMeta.)
>>>>
>>>>> Avatica cursor type cast for number cause exception in
>>>>> AvaticaResultSet
>>>>> ---------------------------------------------------------------------
>>>>> --
>>>>>
>>>>> Key: CALCITE-647
>>>>> URL: https://issues.apache.org/jira/browse/CALCITE-647
>>>>> Project: Calcite
>>>>> Issue Type: Bug
>>>>> Affects Versions: 1.1.0-incubating
>>>>> Reporter: Xavier FH Leong
>>>>> Assignee: Julian Hyde
>>>>> Labels: avatica
>>>>> Attachments: CALCITE-647-cursor-numberTypeCast.patch
>>>>>
>>>>>
>>>>> After the result are deserialized from JSON on remote side, the object
>> is not with it's original type, forcing casing of box type Long on Integer
>> raise exception.
>>>>> For all box number, it will type cast to Number and extract using the
>> Number method instead.
>>>>> 2015-03-26 15:49:48,154 [Thread-10] ERROR
>>>>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader - Error reading
>>>>> column data, column index = 3
>>>>> java.lang.ClassCastException: java.lang.Integer incompatible with
>> java.lang.Long
>>>>> at
>> org.apache.calcite.avatica.util.AbstractCursor$LongAccessor.getLong(AbstractCursor.java:483)
>>>>> at
>> org.apache.calcite.avatica.AvaticaResultSet.getLong(AvaticaResultSet.java:252)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.DataTypeLong.readResultSet(DataTypeLong.java:365)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.CellComponentFactory.readResultSet(CellComponentFactory.java:488)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.doContentTabRead(ResultSetReader.java:613)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.readRow(ResultSetReader.java:184)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.createRow(ResultSetDataSet.java:237)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet._setResultSet(ResultSetDataSet.java:203)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.setSqlExecutionTabResultSet(ResultSetDataSet.java:126)
>>>>> at
>> net.sourceforge.squirrel_sql.client.session.mainpanel.SQLExecutionHandler.sqlResultSetAvailable(SQLExecutionHandler.java:410)
>>>>> at
>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processResultSet(SQLExecuterTask.java:542)
>>>>> at
>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processQuery(SQLExecuterTask.java:407)
>>>>> at
>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.run(SQLExecuterTask.java:205)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.util.TaskExecuter.run(TaskExecuter.java:82)
>>>>> at java.lang.Thread.run(Thread.java:853)
>>>>
>>>>
>>>>
>>>> --
>>>> This message was sent by Atlassian JIRA
>>>> (v6.3.4#6332)
>>>>
>>>> DISCLAIMER
>>>> ==========
>>>> This e-mail may contain privileged and confidential information which
>> is the property of Persistent Systems Ltd. It is intended only for the use
>> of the individual or entity to which it is addressed. If you are not the
>> intended recipient, you are not authorized to read, retain, copy, print,
>> distribute or use this message. If you have received this communication in
>> error, please notify the sender and delete all copies of this message.
>> Persistent Systems Ltd. does not accept any liability for virus infected
>> mails.
>>>>
>>>
>>> DISCLAIMER
>>> ==========
>>> This e-mail may contain privileged and confidential information which is
>> the property of Persistent Systems Ltd. It is intended only for the use of
>> the individual or entity to which it is addressed. If you are not the
>> intended recipient, you are not authorized to read, retain, copy, print,
>> distribute or use this message. If you have received this communication in
>> error, please notify the sender and delete all copies of this message.
>> Persistent Systems Ltd. does not accept any liability for virus infected
>> mails.
>>>
>>
DISCLAIMER
==========
This e-mail may contain privileged and confidential information which is the property of Persistent Systems Ltd. It is intended only for the use of the individual or entity to which it is addressed. If you are not the intended recipient, you are not authorized to read, retain, copy, print, distribute or use this message. If you have received this communication in error, please notify the sender and delete all copies of this message. Persistent Systems Ltd. does not accept any liability for virus infected mails.
Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for number cause exception in AvaticaResultSet
Posted by Julian Hyde <ju...@hydromatic.net>.
Yes, the "json" field (currently always true) is a hack. Ideally the transport (not the services at either end of it) should have opportunity to say that it does not preserve types.
What are you proposing to memoize? I would not memoize a result set. It uses significant memory on the client and might go stale.
Julian
> On Mar 27, 2015, at 9:54 AM, Nick Dimiduk <nd...@gmail.com> wrote:
>
> This same issue bit me along the way as well. I don't particularly care for
> the if (json) block in LocalService. Option (3) above sounds like a good
> choice, despite the repetition. Maybe we can memoize the results to avoid
> re-computation? Let's say we abstract out the transport later (maybe for
> 2.0) and allow pluggable implementations -- thrift, protobuf, &c. What
> would the ideal solution be for that scenario? Both of those tools provide
> stronger schema semantics.
>
> On Fri, Mar 27, 2015 at 9:00 AM, Julian Hyde <ju...@gmail.com> wrote:
>
>>> So that the JSON will now look like "7f000001" instead of
>> {"bytes":"fwAAAQ=="}
>>
>> Nothing wrong with base64 encoding.
>>
>> On Fri, Mar 27, 2015 at 1:24 AM, Xavier Leong <le...@persistent.com>
>> wrote:
>>> Thanks for the feedback. I'll look into creating a ListIteratorCursor to
>> use at the remote side, so that it will create the "generic" number
>> accessor instead of the ExactNumericAccessor, which I agree, should match
>> exact.
>>>
>>> I'll also be looking into fixing the ByteString serialized and
>> deserialized, it will use JSON getter to convert byte[] to String, and
>> BinaryAccessor will reconvert it back to byte[] from String.
>>>
>>> So that the JSON will now look like "7f000001" instead of
>> {"bytes":"fwAAAQ=="}
>>>
>>> -----Original Message-----
>>> From: Julian Hyde [mailto:julian@hydromatic.net]
>>> Sent: Friday, March 27, 2015 3:25 PM
>>> To: dev
>>> Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast
>> for number cause exception in AvaticaResultSet
>>>
>>> I agree with your analysis. 1, 2, 3 are all viable options. 1 makes the
>> payload much larger, 2 would require invasive changes to the JSON parser,
>> so 3 is my preferred option. I think NumberAccessor does what you need.
>>>
>>> For the related problem, sending parameter from client to server, I
>> think we need to use option 1. The reason is that we don't know what the
>> parameter type "is supposed to be". The client can set a parameter to
>> (Long) 0, which would come across JSON as int 0. So we would need to also
>> send the type of each parameter value. Luckily, for parameters payload size
>> is not a concern.
>>>
>>> Julian
>>>
>>> On Thu, Mar 26, 2015 at 9:01 PM, Xavier Leong <le...@persistent.com>
>> wrote:
>>>> Hi Julian,
>>>>
>>>> Moving this to dev thread for wider audience.
>>>>
>>>> The accessor is created using the table column meta from the signature,
>> which I agree as if we are to reconstruct the data, the right place to look
>> for the correct type is from the column meta.
>>>>
>>>> The frame rows are serialized as Object, if we to make each result item
>> to be with strongly typed, then it will be inefficient to retain the type
>> info in the payload, but if we let the serializer to do the decision, some
>> type are not translated correctly, eg ByteString use for BINARY type, at
>> the remote side, it gets constructed as LinkedHashMap from the JSON
>> {"bytes":"fwAAAQ=="} string.
>>>>
>>>> So, what I see there's 3 way to approach this:
>>>>
>>>> 1) Is to have the payload to be strongly typed with typed-value
>>>> representation
>>>> 2) Is to reconstruct the data during deserializing based on the column
>>>> meta
>>>> 3) Do conversion in accessor to present the correct data based on
>> column meta.
>>>>
>>>> Option 1 will be expensive on the payload, option 2 and 3 is somewhat
>> similar, 2 is to do it 1 time with penalty of longer blocking response, and
>> 3 is to do it lazily when data is access and penalty of repeated conversion
>> for every get.
>>>>
>>>> Thoughts?
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: Julian Hyde (JIRA) [mailto:jira@apache.org]
>>>> Sent: Friday, March 27, 2015 6:47 AM
>>>> To: Xavier Leong
>>>> Subject: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for
>>>> number cause exception in AvaticaResultSet
>>>>
>>>>
>>>> [
>>>> https://issues.apache.org/jira/browse/CALCITE-647?page=com.atlassian.j
>>>> ira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=143
>>>> 82864#comment-14382864 ]
>>>>
>>>> Julian Hyde commented on CALCITE-647:
>>>> -------------------------------------
>>>>
>>>> I agree with your analysis of the cause, but I don't agree with your
>> solution. ByteAccessor is an accessor for values that are known to be of
>> type Byte, so it doesn't need to be fixed; you shouldn't be using it for
>> JSON data. I think you should be using a different accessor, maybe like
>> NumberAccessor, that doesn't make assumptions about the exact type of the
>> values.
>>>>
>>>> Note I added the "boolean json" field to LocalService recently. That
>> was for a very similar purpose.
>>>>
>>>> This fix is not complete without a test case. Without one, we will very
>> easily regress.
>>>>
>>>> I don't think there is a test suite in Avatica that serializes requests
>> and responses to JSON and back. (It doesn't need to send them over HTTP, so
>> it can run in a single thread in a single JVM.) That would have discovered
>> this issue, and probably several similar issues. Maybe you could create a
>> variant of RemoteDriverTest that does this.
>>>>
>>>> Can you also please check whether this issue exists for parameters? I
>> think if you have a long or double parameter and set it to 0, the value
>> will arrive at the server as an int. The server needs to be able to handle
>> that.
>>>>
>>>> [~ndimiduk] Can you please review the tests in Avatica, plus
>>>> CalciteRemoteDriverTest. If the tests don't have coverage for basic
>>>> functionality (all data types, all request/response types, over all
>>>> transports) please log jira cases. (Now we have JdbcMeta and the
>>>> scott JDBC database, we could consider moving much of
>>>> CalciteRemoteDriverTest into Avatica; CalciteRemoteDriverTest would be
>>>> just a sub-class of that test that uses CalciteMeta rather than
>>>> JdbcMeta.)
>>>>
>>>>> Avatica cursor type cast for number cause exception in
>>>>> AvaticaResultSet
>>>>> ---------------------------------------------------------------------
>>>>> --
>>>>>
>>>>> Key: CALCITE-647
>>>>> URL: https://issues.apache.org/jira/browse/CALCITE-647
>>>>> Project: Calcite
>>>>> Issue Type: Bug
>>>>> Affects Versions: 1.1.0-incubating
>>>>> Reporter: Xavier FH Leong
>>>>> Assignee: Julian Hyde
>>>>> Labels: avatica
>>>>> Attachments: CALCITE-647-cursor-numberTypeCast.patch
>>>>>
>>>>>
>>>>> After the result are deserialized from JSON on remote side, the object
>> is not with it's original type, forcing casing of box type Long on Integer
>> raise exception.
>>>>> For all box number, it will type cast to Number and extract using the
>> Number method instead.
>>>>> 2015-03-26 15:49:48,154 [Thread-10] ERROR
>>>>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader - Error reading
>>>>> column data, column index = 3
>>>>> java.lang.ClassCastException: java.lang.Integer incompatible with
>> java.lang.Long
>>>>> at
>> org.apache.calcite.avatica.util.AbstractCursor$LongAccessor.getLong(AbstractCursor.java:483)
>>>>> at
>> org.apache.calcite.avatica.AvaticaResultSet.getLong(AvaticaResultSet.java:252)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.DataTypeLong.readResultSet(DataTypeLong.java:365)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.CellComponentFactory.readResultSet(CellComponentFactory.java:488)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.doContentTabRead(ResultSetReader.java:613)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.readRow(ResultSetReader.java:184)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.createRow(ResultSetDataSet.java:237)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet._setResultSet(ResultSetDataSet.java:203)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.setSqlExecutionTabResultSet(ResultSetDataSet.java:126)
>>>>> at
>> net.sourceforge.squirrel_sql.client.session.mainpanel.SQLExecutionHandler.sqlResultSetAvailable(SQLExecutionHandler.java:410)
>>>>> at
>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processResultSet(SQLExecuterTask.java:542)
>>>>> at
>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processQuery(SQLExecuterTask.java:407)
>>>>> at
>> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.run(SQLExecuterTask.java:205)
>>>>> at
>> net.sourceforge.squirrel_sql.fw.util.TaskExecuter.run(TaskExecuter.java:82)
>>>>> at java.lang.Thread.run(Thread.java:853)
>>>>
>>>>
>>>>
>>>> --
>>>> This message was sent by Atlassian JIRA
>>>> (v6.3.4#6332)
>>>>
>>>> DISCLAIMER
>>>> ==========
>>>> This e-mail may contain privileged and confidential information which
>> is the property of Persistent Systems Ltd. It is intended only for the use
>> of the individual or entity to which it is addressed. If you are not the
>> intended recipient, you are not authorized to read, retain, copy, print,
>> distribute or use this message. If you have received this communication in
>> error, please notify the sender and delete all copies of this message.
>> Persistent Systems Ltd. does not accept any liability for virus infected
>> mails.
>>>>
>>>
>>> DISCLAIMER
>>> ==========
>>> This e-mail may contain privileged and confidential information which is
>> the property of Persistent Systems Ltd. It is intended only for the use of
>> the individual or entity to which it is addressed. If you are not the
>> intended recipient, you are not authorized to read, retain, copy, print,
>> distribute or use this message. If you have received this communication in
>> error, please notify the sender and delete all copies of this message.
>> Persistent Systems Ltd. does not accept any liability for virus infected
>> mails.
>>>
>>
Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for
number cause exception in AvaticaResultSet
Posted by Nick Dimiduk <nd...@gmail.com>.
This same issue bit me along the way as well. I don't particularly care for
the if (json) block in LocalService. Option (3) above sounds like a good
choice, despite the repetition. Maybe we can memoize the results to avoid
re-computation? Let's say we abstract out the transport later (maybe for
2.0) and allow pluggable implementations -- thrift, protobuf, &c. What
would the ideal solution be for that scenario? Both of those tools provide
stronger schema semantics.
On Fri, Mar 27, 2015 at 9:00 AM, Julian Hyde <ju...@gmail.com> wrote:
> > So that the JSON will now look like "7f000001" instead of
> {"bytes":"fwAAAQ=="}
>
> Nothing wrong with base64 encoding.
>
> On Fri, Mar 27, 2015 at 1:24 AM, Xavier Leong <le...@persistent.com>
> wrote:
> > Thanks for the feedback. I'll look into creating a ListIteratorCursor to
> use at the remote side, so that it will create the "generic" number
> accessor instead of the ExactNumericAccessor, which I agree, should match
> exact.
> >
> > I'll also be looking into fixing the ByteString serialized and
> deserialized, it will use JSON getter to convert byte[] to String, and
> BinaryAccessor will reconvert it back to byte[] from String.
> >
> > So that the JSON will now look like "7f000001" instead of
> {"bytes":"fwAAAQ=="}
> >
> > -----Original Message-----
> > From: Julian Hyde [mailto:julian@hydromatic.net]
> > Sent: Friday, March 27, 2015 3:25 PM
> > To: dev
> > Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast
> for number cause exception in AvaticaResultSet
> >
> > I agree with your analysis. 1, 2, 3 are all viable options. 1 makes the
> payload much larger, 2 would require invasive changes to the JSON parser,
> so 3 is my preferred option. I think NumberAccessor does what you need.
> >
> > For the related problem, sending parameter from client to server, I
> think we need to use option 1. The reason is that we don't know what the
> parameter type "is supposed to be". The client can set a parameter to
> (Long) 0, which would come across JSON as int 0. So we would need to also
> send the type of each parameter value. Luckily, for parameters payload size
> is not a concern.
> >
> > Julian
> >
> > On Thu, Mar 26, 2015 at 9:01 PM, Xavier Leong <le...@persistent.com>
> wrote:
> >> Hi Julian,
> >>
> >> Moving this to dev thread for wider audience.
> >>
> >> The accessor is created using the table column meta from the signature,
> which I agree as if we are to reconstruct the data, the right place to look
> for the correct type is from the column meta.
> >>
> >> The frame rows are serialized as Object, if we to make each result item
> to be with strongly typed, then it will be inefficient to retain the type
> info in the payload, but if we let the serializer to do the decision, some
> type are not translated correctly, eg ByteString use for BINARY type, at
> the remote side, it gets constructed as LinkedHashMap from the JSON
> {"bytes":"fwAAAQ=="} string.
> >>
> >> So, what I see there's 3 way to approach this:
> >>
> >> 1) Is to have the payload to be strongly typed with typed-value
> >> representation
> >> 2) Is to reconstruct the data during deserializing based on the column
> >> meta
> >> 3) Do conversion in accessor to present the correct data based on
> column meta.
> >>
> >> Option 1 will be expensive on the payload, option 2 and 3 is somewhat
> similar, 2 is to do it 1 time with penalty of longer blocking response, and
> 3 is to do it lazily when data is access and penalty of repeated conversion
> for every get.
> >>
> >> Thoughts?
> >>
> >>
> >> -----Original Message-----
> >> From: Julian Hyde (JIRA) [mailto:jira@apache.org]
> >> Sent: Friday, March 27, 2015 6:47 AM
> >> To: Xavier Leong
> >> Subject: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for
> >> number cause exception in AvaticaResultSet
> >>
> >>
> >> [
> >> https://issues.apache.org/jira/browse/CALCITE-647?page=com.atlassian.j
> >> ira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=143
> >> 82864#comment-14382864 ]
> >>
> >> Julian Hyde commented on CALCITE-647:
> >> -------------------------------------
> >>
> >> I agree with your analysis of the cause, but I don't agree with your
> solution. ByteAccessor is an accessor for values that are known to be of
> type Byte, so it doesn't need to be fixed; you shouldn't be using it for
> JSON data. I think you should be using a different accessor, maybe like
> NumberAccessor, that doesn't make assumptions about the exact type of the
> values.
> >>
> >> Note I added the "boolean json" field to LocalService recently. That
> was for a very similar purpose.
> >>
> >> This fix is not complete without a test case. Without one, we will very
> easily regress.
> >>
> >> I don't think there is a test suite in Avatica that serializes requests
> and responses to JSON and back. (It doesn't need to send them over HTTP, so
> it can run in a single thread in a single JVM.) That would have discovered
> this issue, and probably several similar issues. Maybe you could create a
> variant of RemoteDriverTest that does this.
> >>
> >> Can you also please check whether this issue exists for parameters? I
> think if you have a long or double parameter and set it to 0, the value
> will arrive at the server as an int. The server needs to be able to handle
> that.
> >>
> >> [~ndimiduk] Can you please review the tests in Avatica, plus
> >> CalciteRemoteDriverTest. If the tests don't have coverage for basic
> >> functionality (all data types, all request/response types, over all
> >> transports) please log jira cases. (Now we have JdbcMeta and the
> >> scott JDBC database, we could consider moving much of
> >> CalciteRemoteDriverTest into Avatica; CalciteRemoteDriverTest would be
> >> just a sub-class of that test that uses CalciteMeta rather than
> >> JdbcMeta.)
> >>
> >>> Avatica cursor type cast for number cause exception in
> >>> AvaticaResultSet
> >>> ---------------------------------------------------------------------
> >>> --
> >>>
> >>> Key: CALCITE-647
> >>> URL: https://issues.apache.org/jira/browse/CALCITE-647
> >>> Project: Calcite
> >>> Issue Type: Bug
> >>> Affects Versions: 1.1.0-incubating
> >>> Reporter: Xavier FH Leong
> >>> Assignee: Julian Hyde
> >>> Labels: avatica
> >>> Attachments: CALCITE-647-cursor-numberTypeCast.patch
> >>>
> >>>
> >>> After the result are deserialized from JSON on remote side, the object
> is not with it's original type, forcing casing of box type Long on Integer
> raise exception.
> >>> For all box number, it will type cast to Number and extract using the
> Number method instead.
> >>> 2015-03-26 15:49:48,154 [Thread-10] ERROR
> >>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader - Error reading
> >>> column data, column index = 3
> >>> java.lang.ClassCastException: java.lang.Integer incompatible with
> java.lang.Long
> >>> at
> org.apache.calcite.avatica.util.AbstractCursor$LongAccessor.getLong(AbstractCursor.java:483)
> >>> at
> org.apache.calcite.avatica.AvaticaResultSet.getLong(AvaticaResultSet.java:252)
> >>> at
> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.DataTypeLong.readResultSet(DataTypeLong.java:365)
> >>> at
> net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.CellComponentFactory.readResultSet(CellComponentFactory.java:488)
> >>> at
> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.doContentTabRead(ResultSetReader.java:613)
> >>> at
> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.readRow(ResultSetReader.java:184)
> >>> at
> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.createRow(ResultSetDataSet.java:237)
> >>> at
> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet._setResultSet(ResultSetDataSet.java:203)
> >>> at
> net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.setSqlExecutionTabResultSet(ResultSetDataSet.java:126)
> >>> at
> net.sourceforge.squirrel_sql.client.session.mainpanel.SQLExecutionHandler.sqlResultSetAvailable(SQLExecutionHandler.java:410)
> >>> at
> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processResultSet(SQLExecuterTask.java:542)
> >>> at
> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processQuery(SQLExecuterTask.java:407)
> >>> at
> net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.run(SQLExecuterTask.java:205)
> >>> at
> net.sourceforge.squirrel_sql.fw.util.TaskExecuter.run(TaskExecuter.java:82)
> >>> at java.lang.Thread.run(Thread.java:853)
> >>
> >>
> >>
> >> --
> >> This message was sent by Atlassian JIRA
> >> (v6.3.4#6332)
> >>
> >> DISCLAIMER
> >> ==========
> >> This e-mail may contain privileged and confidential information which
> is the property of Persistent Systems Ltd. It is intended only for the use
> of the individual or entity to which it is addressed. If you are not the
> intended recipient, you are not authorized to read, retain, copy, print,
> distribute or use this message. If you have received this communication in
> error, please notify the sender and delete all copies of this message.
> Persistent Systems Ltd. does not accept any liability for virus infected
> mails.
> >>
> >
> > DISCLAIMER
> > ==========
> > This e-mail may contain privileged and confidential information which is
> the property of Persistent Systems Ltd. It is intended only for the use of
> the individual or entity to which it is addressed. If you are not the
> intended recipient, you are not authorized to read, retain, copy, print,
> distribute or use this message. If you have received this communication in
> error, please notify the sender and delete all copies of this message.
> Persistent Systems Ltd. does not accept any liability for virus infected
> mails.
> >
>
Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for
number cause exception in AvaticaResultSet
Posted by Julian Hyde <ju...@gmail.com>.
> So that the JSON will now look like "7f000001" instead of {"bytes":"fwAAAQ=="}
Nothing wrong with base64 encoding.
On Fri, Mar 27, 2015 at 1:24 AM, Xavier Leong <le...@persistent.com> wrote:
> Thanks for the feedback. I'll look into creating a ListIteratorCursor to use at the remote side, so that it will create the "generic" number accessor instead of the ExactNumericAccessor, which I agree, should match exact.
>
> I'll also be looking into fixing the ByteString serialized and deserialized, it will use JSON getter to convert byte[] to String, and BinaryAccessor will reconvert it back to byte[] from String.
>
> So that the JSON will now look like "7f000001" instead of {"bytes":"fwAAAQ=="}
>
> -----Original Message-----
> From: Julian Hyde [mailto:julian@hydromatic.net]
> Sent: Friday, March 27, 2015 3:25 PM
> To: dev
> Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for number cause exception in AvaticaResultSet
>
> I agree with your analysis. 1, 2, 3 are all viable options. 1 makes the payload much larger, 2 would require invasive changes to the JSON parser, so 3 is my preferred option. I think NumberAccessor does what you need.
>
> For the related problem, sending parameter from client to server, I think we need to use option 1. The reason is that we don't know what the parameter type "is supposed to be". The client can set a parameter to (Long) 0, which would come across JSON as int 0. So we would need to also send the type of each parameter value. Luckily, for parameters payload size is not a concern.
>
> Julian
>
> On Thu, Mar 26, 2015 at 9:01 PM, Xavier Leong <le...@persistent.com> wrote:
>> Hi Julian,
>>
>> Moving this to dev thread for wider audience.
>>
>> The accessor is created using the table column meta from the signature, which I agree as if we are to reconstruct the data, the right place to look for the correct type is from the column meta.
>>
>> The frame rows are serialized as Object, if we to make each result item to be with strongly typed, then it will be inefficient to retain the type info in the payload, but if we let the serializer to do the decision, some type are not translated correctly, eg ByteString use for BINARY type, at the remote side, it gets constructed as LinkedHashMap from the JSON {"bytes":"fwAAAQ=="} string.
>>
>> So, what I see there's 3 way to approach this:
>>
>> 1) Is to have the payload to be strongly typed with typed-value
>> representation
>> 2) Is to reconstruct the data during deserializing based on the column
>> meta
>> 3) Do conversion in accessor to present the correct data based on column meta.
>>
>> Option 1 will be expensive on the payload, option 2 and 3 is somewhat similar, 2 is to do it 1 time with penalty of longer blocking response, and 3 is to do it lazily when data is access and penalty of repeated conversion for every get.
>>
>> Thoughts?
>>
>>
>> -----Original Message-----
>> From: Julian Hyde (JIRA) [mailto:jira@apache.org]
>> Sent: Friday, March 27, 2015 6:47 AM
>> To: Xavier Leong
>> Subject: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for
>> number cause exception in AvaticaResultSet
>>
>>
>> [
>> https://issues.apache.org/jira/browse/CALCITE-647?page=com.atlassian.j
>> ira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=143
>> 82864#comment-14382864 ]
>>
>> Julian Hyde commented on CALCITE-647:
>> -------------------------------------
>>
>> I agree with your analysis of the cause, but I don't agree with your solution. ByteAccessor is an accessor for values that are known to be of type Byte, so it doesn't need to be fixed; you shouldn't be using it for JSON data. I think you should be using a different accessor, maybe like NumberAccessor, that doesn't make assumptions about the exact type of the values.
>>
>> Note I added the "boolean json" field to LocalService recently. That was for a very similar purpose.
>>
>> This fix is not complete without a test case. Without one, we will very easily regress.
>>
>> I don't think there is a test suite in Avatica that serializes requests and responses to JSON and back. (It doesn't need to send them over HTTP, so it can run in a single thread in a single JVM.) That would have discovered this issue, and probably several similar issues. Maybe you could create a variant of RemoteDriverTest that does this.
>>
>> Can you also please check whether this issue exists for parameters? I think if you have a long or double parameter and set it to 0, the value will arrive at the server as an int. The server needs to be able to handle that.
>>
>> [~ndimiduk] Can you please review the tests in Avatica, plus
>> CalciteRemoteDriverTest. If the tests don't have coverage for basic
>> functionality (all data types, all request/response types, over all
>> transports) please log jira cases. (Now we have JdbcMeta and the
>> scott JDBC database, we could consider moving much of
>> CalciteRemoteDriverTest into Avatica; CalciteRemoteDriverTest would be
>> just a sub-class of that test that uses CalciteMeta rather than
>> JdbcMeta.)
>>
>>> Avatica cursor type cast for number cause exception in
>>> AvaticaResultSet
>>> ---------------------------------------------------------------------
>>> --
>>>
>>> Key: CALCITE-647
>>> URL: https://issues.apache.org/jira/browse/CALCITE-647
>>> Project: Calcite
>>> Issue Type: Bug
>>> Affects Versions: 1.1.0-incubating
>>> Reporter: Xavier FH Leong
>>> Assignee: Julian Hyde
>>> Labels: avatica
>>> Attachments: CALCITE-647-cursor-numberTypeCast.patch
>>>
>>>
>>> After the result are deserialized from JSON on remote side, the object is not with it's original type, forcing casing of box type Long on Integer raise exception.
>>> For all box number, it will type cast to Number and extract using the Number method instead.
>>> 2015-03-26 15:49:48,154 [Thread-10] ERROR
>>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader - Error reading
>>> column data, column index = 3
>>> java.lang.ClassCastException: java.lang.Integer incompatible with java.lang.Long
>>> at org.apache.calcite.avatica.util.AbstractCursor$LongAccessor.getLong(AbstractCursor.java:483)
>>> at org.apache.calcite.avatica.AvaticaResultSet.getLong(AvaticaResultSet.java:252)
>>> at net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.DataTypeLong.readResultSet(DataTypeLong.java:365)
>>> at net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.CellComponentFactory.readResultSet(CellComponentFactory.java:488)
>>> at net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.doContentTabRead(ResultSetReader.java:613)
>>> at net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.readRow(ResultSetReader.java:184)
>>> at net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.createRow(ResultSetDataSet.java:237)
>>> at net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet._setResultSet(ResultSetDataSet.java:203)
>>> at net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.setSqlExecutionTabResultSet(ResultSetDataSet.java:126)
>>> at net.sourceforge.squirrel_sql.client.session.mainpanel.SQLExecutionHandler.sqlResultSetAvailable(SQLExecutionHandler.java:410)
>>> at net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processResultSet(SQLExecuterTask.java:542)
>>> at net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processQuery(SQLExecuterTask.java:407)
>>> at net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.run(SQLExecuterTask.java:205)
>>> at net.sourceforge.squirrel_sql.fw.util.TaskExecuter.run(TaskExecuter.java:82)
>>> at java.lang.Thread.run(Thread.java:853)
>>
>>
>>
>> --
>> This message was sent by Atlassian JIRA
>> (v6.3.4#6332)
>>
>> DISCLAIMER
>> ==========
>> This e-mail may contain privileged and confidential information which is the property of Persistent Systems Ltd. It is intended only for the use of the individual or entity to which it is addressed. If you are not the intended recipient, you are not authorized to read, retain, copy, print, distribute or use this message. If you have received this communication in error, please notify the sender and delete all copies of this message. Persistent Systems Ltd. does not accept any liability for virus infected mails.
>>
>
> DISCLAIMER
> ==========
> This e-mail may contain privileged and confidential information which is the property of Persistent Systems Ltd. It is intended only for the use of the individual or entity to which it is addressed. If you are not the intended recipient, you are not authorized to read, retain, copy, print, distribute or use this message. If you have received this communication in error, please notify the sender and delete all copies of this message. Persistent Systems Ltd. does not accept any liability for virus infected mails.
>
RE: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for
number cause exception in AvaticaResultSet
Posted by Xavier Leong <le...@persistent.com>.
Thanks for the feedback. I'll look into creating a ListIteratorCursor to use at the remote side, so that it will create the "generic" number accessor instead of the ExactNumericAccessor, which I agree, should match exact.
I'll also be looking into fixing the ByteString serialized and deserialized, it will use JSON getter to convert byte[] to String, and BinaryAccessor will reconvert it back to byte[] from String.
So that the JSON will now look like "7f000001" instead of {"bytes":"fwAAAQ=="}
-----Original Message-----
From: Julian Hyde [mailto:julian@hydromatic.net]
Sent: Friday, March 27, 2015 3:25 PM
To: dev
Subject: Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for number cause exception in AvaticaResultSet
I agree with your analysis. 1, 2, 3 are all viable options. 1 makes the payload much larger, 2 would require invasive changes to the JSON parser, so 3 is my preferred option. I think NumberAccessor does what you need.
For the related problem, sending parameter from client to server, I think we need to use option 1. The reason is that we don't know what the parameter type "is supposed to be". The client can set a parameter to (Long) 0, which would come across JSON as int 0. So we would need to also send the type of each parameter value. Luckily, for parameters payload size is not a concern.
Julian
On Thu, Mar 26, 2015 at 9:01 PM, Xavier Leong <le...@persistent.com> wrote:
> Hi Julian,
>
> Moving this to dev thread for wider audience.
>
> The accessor is created using the table column meta from the signature, which I agree as if we are to reconstruct the data, the right place to look for the correct type is from the column meta.
>
> The frame rows are serialized as Object, if we to make each result item to be with strongly typed, then it will be inefficient to retain the type info in the payload, but if we let the serializer to do the decision, some type are not translated correctly, eg ByteString use for BINARY type, at the remote side, it gets constructed as LinkedHashMap from the JSON {"bytes":"fwAAAQ=="} string.
>
> So, what I see there's 3 way to approach this:
>
> 1) Is to have the payload to be strongly typed with typed-value
> representation
> 2) Is to reconstruct the data during deserializing based on the column
> meta
> 3) Do conversion in accessor to present the correct data based on column meta.
>
> Option 1 will be expensive on the payload, option 2 and 3 is somewhat similar, 2 is to do it 1 time with penalty of longer blocking response, and 3 is to do it lazily when data is access and penalty of repeated conversion for every get.
>
> Thoughts?
>
>
> -----Original Message-----
> From: Julian Hyde (JIRA) [mailto:jira@apache.org]
> Sent: Friday, March 27, 2015 6:47 AM
> To: Xavier Leong
> Subject: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for
> number cause exception in AvaticaResultSet
>
>
> [
> https://issues.apache.org/jira/browse/CALCITE-647?page=com.atlassian.j
> ira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=143
> 82864#comment-14382864 ]
>
> Julian Hyde commented on CALCITE-647:
> -------------------------------------
>
> I agree with your analysis of the cause, but I don't agree with your solution. ByteAccessor is an accessor for values that are known to be of type Byte, so it doesn't need to be fixed; you shouldn't be using it for JSON data. I think you should be using a different accessor, maybe like NumberAccessor, that doesn't make assumptions about the exact type of the values.
>
> Note I added the "boolean json" field to LocalService recently. That was for a very similar purpose.
>
> This fix is not complete without a test case. Without one, we will very easily regress.
>
> I don't think there is a test suite in Avatica that serializes requests and responses to JSON and back. (It doesn't need to send them over HTTP, so it can run in a single thread in a single JVM.) That would have discovered this issue, and probably several similar issues. Maybe you could create a variant of RemoteDriverTest that does this.
>
> Can you also please check whether this issue exists for parameters? I think if you have a long or double parameter and set it to 0, the value will arrive at the server as an int. The server needs to be able to handle that.
>
> [~ndimiduk] Can you please review the tests in Avatica, plus
> CalciteRemoteDriverTest. If the tests don't have coverage for basic
> functionality (all data types, all request/response types, over all
> transports) please log jira cases. (Now we have JdbcMeta and the
> scott JDBC database, we could consider moving much of
> CalciteRemoteDriverTest into Avatica; CalciteRemoteDriverTest would be
> just a sub-class of that test that uses CalciteMeta rather than
> JdbcMeta.)
>
>> Avatica cursor type cast for number cause exception in
>> AvaticaResultSet
>> ---------------------------------------------------------------------
>> --
>>
>> Key: CALCITE-647
>> URL: https://issues.apache.org/jira/browse/CALCITE-647
>> Project: Calcite
>> Issue Type: Bug
>> Affects Versions: 1.1.0-incubating
>> Reporter: Xavier FH Leong
>> Assignee: Julian Hyde
>> Labels: avatica
>> Attachments: CALCITE-647-cursor-numberTypeCast.patch
>>
>>
>> After the result are deserialized from JSON on remote side, the object is not with it's original type, forcing casing of box type Long on Integer raise exception.
>> For all box number, it will type cast to Number and extract using the Number method instead.
>> 2015-03-26 15:49:48,154 [Thread-10] ERROR
>> net.sourceforge.squirrel_sql.fw.sql.ResultSetReader - Error reading
>> column data, column index = 3
>> java.lang.ClassCastException: java.lang.Integer incompatible with java.lang.Long
>> at org.apache.calcite.avatica.util.AbstractCursor$LongAccessor.getLong(AbstractCursor.java:483)
>> at org.apache.calcite.avatica.AvaticaResultSet.getLong(AvaticaResultSet.java:252)
>> at net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.DataTypeLong.readResultSet(DataTypeLong.java:365)
>> at net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.CellComponentFactory.readResultSet(CellComponentFactory.java:488)
>> at net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.doContentTabRead(ResultSetReader.java:613)
>> at net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.readRow(ResultSetReader.java:184)
>> at net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.createRow(ResultSetDataSet.java:237)
>> at net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet._setResultSet(ResultSetDataSet.java:203)
>> at net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.setSqlExecutionTabResultSet(ResultSetDataSet.java:126)
>> at net.sourceforge.squirrel_sql.client.session.mainpanel.SQLExecutionHandler.sqlResultSetAvailable(SQLExecutionHandler.java:410)
>> at net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processResultSet(SQLExecuterTask.java:542)
>> at net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processQuery(SQLExecuterTask.java:407)
>> at net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.run(SQLExecuterTask.java:205)
>> at net.sourceforge.squirrel_sql.fw.util.TaskExecuter.run(TaskExecuter.java:82)
>> at java.lang.Thread.run(Thread.java:853)
>
>
>
> --
> This message was sent by Atlassian JIRA
> (v6.3.4#6332)
>
> DISCLAIMER
> ==========
> This e-mail may contain privileged and confidential information which is the property of Persistent Systems Ltd. It is intended only for the use of the individual or entity to which it is addressed. If you are not the intended recipient, you are not authorized to read, retain, copy, print, distribute or use this message. If you have received this communication in error, please notify the sender and delete all copies of this message. Persistent Systems Ltd. does not accept any liability for virus infected mails.
>
DISCLAIMER
==========
This e-mail may contain privileged and confidential information which is the property of Persistent Systems Ltd. It is intended only for the use of the individual or entity to which it is addressed. If you are not the intended recipient, you are not authorized to read, retain, copy, print, distribute or use this message. If you have received this communication in error, please notify the sender and delete all copies of this message. Persistent Systems Ltd. does not accept any liability for virus infected mails.
Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for
number cause exception in AvaticaResultSet
Posted by Julian Hyde <ju...@hydromatic.net>.
I agree with your analysis. 1, 2, 3 are all viable options. 1 makes
the payload much larger, 2 would require invasive changes to the JSON
parser, so 3 is my preferred option. I think NumberAccessor does what
you need.
For the related problem, sending parameter from client to server, I
think we need to use option 1. The reason is that we don't know what
the parameter type "is supposed to be". The client can set a parameter
to (Long) 0, which would come across JSON as int 0. So we would need
to also send the type of each parameter value. Luckily, for parameters
payload size is not a concern.
Julian
On Thu, Mar 26, 2015 at 9:01 PM, Xavier Leong <le...@persistent.com> wrote:
> Hi Julian,
>
> Moving this to dev thread for wider audience.
>
> The accessor is created using the table column meta from the signature, which I agree as if we are to reconstruct the data, the right place to look for the correct type is from the column meta.
>
> The frame rows are serialized as Object, if we to make each result item to be with strongly typed, then it will be inefficient to retain the type info in the payload, but if we let the serializer to do the decision, some type are not translated correctly, eg ByteString use for BINARY type, at the remote side, it gets constructed as LinkedHashMap from the JSON {"bytes":"fwAAAQ=="} string.
>
> So, what I see there's 3 way to approach this:
>
> 1) Is to have the payload to be strongly typed with typed-value representation
> 2) Is to reconstruct the data during deserializing based on the column meta
> 3) Do conversion in accessor to present the correct data based on column meta.
>
> Option 1 will be expensive on the payload, option 2 and 3 is somewhat similar, 2 is to do it 1 time with penalty of longer blocking response, and 3 is to do it lazily when data is access and penalty of repeated conversion for every get.
>
> Thoughts?
>
>
> -----Original Message-----
> From: Julian Hyde (JIRA) [mailto:jira@apache.org]
> Sent: Friday, March 27, 2015 6:47 AM
> To: Xavier Leong
> Subject: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for number cause exception in AvaticaResultSet
>
>
> [ https://issues.apache.org/jira/browse/CALCITE-647?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14382864#comment-14382864 ]
>
> Julian Hyde commented on CALCITE-647:
> -------------------------------------
>
> I agree with your analysis of the cause, but I don't agree with your solution. ByteAccessor is an accessor for values that are known to be of type Byte, so it doesn't need to be fixed; you shouldn't be using it for JSON data. I think you should be using a different accessor, maybe like NumberAccessor, that doesn't make assumptions about the exact type of the values.
>
> Note I added the "boolean json" field to LocalService recently. That was for a very similar purpose.
>
> This fix is not complete without a test case. Without one, we will very easily regress.
>
> I don't think there is a test suite in Avatica that serializes requests and responses to JSON and back. (It doesn't need to send them over HTTP, so it can run in a single thread in a single JVM.) That would have discovered this issue, and probably several similar issues. Maybe you could create a variant of RemoteDriverTest that does this.
>
> Can you also please check whether this issue exists for parameters? I think if you have a long or double parameter and set it to 0, the value will arrive at the server as an int. The server needs to be able to handle that.
>
> [~ndimiduk] Can you please review the tests in Avatica, plus CalciteRemoteDriverTest. If the tests don't have coverage for basic functionality (all data types, all request/response types, over all transports) please log jira cases. (Now we have JdbcMeta and the scott JDBC database, we could consider moving much of CalciteRemoteDriverTest into Avatica; CalciteRemoteDriverTest would be just a sub-class of that test that uses CalciteMeta rather than JdbcMeta.)
>
>> Avatica cursor type cast for number cause exception in AvaticaResultSet
>> -----------------------------------------------------------------------
>>
>> Key: CALCITE-647
>> URL: https://issues.apache.org/jira/browse/CALCITE-647
>> Project: Calcite
>> Issue Type: Bug
>> Affects Versions: 1.1.0-incubating
>> Reporter: Xavier FH Leong
>> Assignee: Julian Hyde
>> Labels: avatica
>> Attachments: CALCITE-647-cursor-numberTypeCast.patch
>>
>>
>> After the result are deserialized from JSON on remote side, the object is not with it's original type, forcing casing of box type Long on Integer raise exception.
>> For all box number, it will type cast to Number and extract using the Number method instead.
>> 2015-03-26 15:49:48,154 [Thread-10] ERROR net.sourceforge.squirrel_sql.fw.sql.ResultSetReader - Error reading column data, column index = 3
>> java.lang.ClassCastException: java.lang.Integer incompatible with java.lang.Long
>> at org.apache.calcite.avatica.util.AbstractCursor$LongAccessor.getLong(AbstractCursor.java:483)
>> at org.apache.calcite.avatica.AvaticaResultSet.getLong(AvaticaResultSet.java:252)
>> at net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.DataTypeLong.readResultSet(DataTypeLong.java:365)
>> at net.sourceforge.squirrel_sql.fw.datasetviewer.cellcomponent.CellComponentFactory.readResultSet(CellComponentFactory.java:488)
>> at net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.doContentTabRead(ResultSetReader.java:613)
>> at net.sourceforge.squirrel_sql.fw.sql.ResultSetReader.readRow(ResultSetReader.java:184)
>> at net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.createRow(ResultSetDataSet.java:237)
>> at net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet._setResultSet(ResultSetDataSet.java:203)
>> at net.sourceforge.squirrel_sql.fw.datasetviewer.ResultSetDataSet.setSqlExecutionTabResultSet(ResultSetDataSet.java:126)
>> at net.sourceforge.squirrel_sql.client.session.mainpanel.SQLExecutionHandler.sqlResultSetAvailable(SQLExecutionHandler.java:410)
>> at net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processResultSet(SQLExecuterTask.java:542)
>> at net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.processQuery(SQLExecuterTask.java:407)
>> at net.sourceforge.squirrel_sql.client.session.SQLExecuterTask.run(SQLExecuterTask.java:205)
>> at net.sourceforge.squirrel_sql.fw.util.TaskExecuter.run(TaskExecuter.java:82)
>> at java.lang.Thread.run(Thread.java:853)
>
>
>
> --
> This message was sent by Atlassian JIRA
> (v6.3.4#6332)
>
> DISCLAIMER
> ==========
> This e-mail may contain privileged and confidential information which is the property of Persistent Systems Ltd. It is intended only for the use of the individual or entity to which it is addressed. If you are not the intended recipient, you are not authorized to read, retain, copy, print, distribute or use this message. If you have received this communication in error, please notify the sender and delete all copies of this message. Persistent Systems Ltd. does not accept any liability for virus infected mails.
>