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. > > >
