calcite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julian Hyde <jul...@hydromatic.net>
Subject Re: [jira] [Commented] (CALCITE-647) Avatica cursor type cast for number cause exception in AvaticaResultSet
Date Fri, 27 Mar 2015 07:25:15 GMT
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.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.
>

Mime
View raw message