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