Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-795

Loss of precision when sending a decimal number via the remote JSON service

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.4.0-incubating
    • Component/s: None
    • Labels:
      None
    • Flags:
      Patch

      Description

      When sending for example

      {"type":"NUMBER","value":"333.333"}

      to the "fetch" request of the remote JSON service, the exact value will not be kept and after one UPSERT/SELECT roundtrip in Phoenix I will get 333.332 back.

      Interestingly enough, this worked in Calcite 1.2 before the TypedValue class was introduced, but I think that was working just by accident.

      The attached patch changes the Jackson object mapper to decode any JSON float to BigDecimal, unless the code explicitly asks for Float/Double. I think that shouldn't break anything and it solves this problem.

        Activity

        Hide
        julianhyde Julian Hyde added a comment -

        Did you forget to attach a patch?

        The behavior you want sounds correct, but we should not specify Avatica's behavior in terms of Java bindings, rather in terms of the SQL type system. JSON's handling of numeric values is complicated, but the goal is that the value gets from client to server and back to client and every bit of the value is preserved. That probably means transmitting exact numbers as exact and FP numbers as FP.

        Show
        julianhyde Julian Hyde added a comment - Did you forget to attach a patch? The behavior you want sounds correct, but we should not specify Avatica's behavior in terms of Java bindings, rather in terms of the SQL type system. JSON's handling of numeric values is complicated, but the goal is that the value gets from client to server and back to client and every bit of the value is preserved. That probably means transmitting exact numbers as exact and FP numbers as FP.
        Hide
        lukaslalinsky Lukas Lalinsky added a comment -

        Sorry, opened a pull request a forgot to link it here. https://github.com/apache/incubator-calcite/pull/105

        Regarding the Java/SQL data types, that's a bit complicated. As I was browsing the code, I have noticed that there are many data type definitions in Avatica. For example, in the remote server, clients get one set of type names in the "columns" signature and another in the "parameters" signature. I think it would be best to use the SQL type names everywhere, which would mean using `NUMERIC` instead of `NUMBER` and `NUMERIC` implies a decimal number with exact precision.

        Show
        lukaslalinsky Lukas Lalinsky added a comment - Sorry, opened a pull request a forgot to link it here. https://github.com/apache/incubator-calcite/pull/105 Regarding the Java/SQL data types, that's a bit complicated. As I was browsing the code, I have noticed that there are many data type definitions in Avatica. For example, in the remote server, clients get one set of type names in the "columns" signature and another in the "parameters" signature. I think it would be best to use the SQL type names everywhere, which would mean using `NUMERIC` instead of `NUMBER` and `NUMERIC` implies a decimal number with exact precision.
        Hide
        julianhyde Julian Hyde added a comment -

        Agreed, stay as close as possible to the SQL type system. I get confused about NUMBER vs NUMERIC, so you may be right.

        Show
        julianhyde Julian Hyde added a comment - Agreed, stay as close as possible to the SQL type system. I get confused about NUMBER vs NUMERIC, so you may be right.
        Hide
        julianhyde Julian Hyde added a comment -
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/efecdade . Thanks for the patch, Lukas!
        Hide
        jnadeau Jacques Nadeau added a comment -

        Resolved in release 1.4.0-incubating (2015-08-23)

        Show
        jnadeau Jacques Nadeau added a comment - Resolved in release 1.4.0-incubating (2015-08-23)

          People

          • Assignee:
            julianhyde Julian Hyde
            Reporter:
            lukaslalinsky Lukas Lalinsky
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development