> 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 <[email protected]> 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:[email protected]]
> 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 <[email protected]> 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:[email protected]]
>> 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.
>

Reply via email to