Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Double values with 0 fraction written without fraction #107

Closed
francisdb opened this issue Sep 20, 2017 · 8 comments · Fixed by #831 or #839
Closed

Double values with 0 fraction written without fraction #107

francisdb opened this issue Sep 20, 2017 · 8 comments · Fixed by #831 or #839

Comments

@francisdb
Copy link

francisdb commented Sep 20, 2017

play-json 2.6.5

issue

Play json will serialize JsNumber(BigDecimal("1.0")) to 1 where this should be 1.0

reproducer

  val mapper = new ObjectMapper()
  val jacksonObject = mapper.createObjectNode()
  jacksonObject.put("test", 1d)
  println(mapper.writerWithDefaultPrettyPrinter.writeValueAsString(jacksonObject))
  
  val playObject2 = obj(
    "test" -> 1d
  )
  println(prettyPrint(playObject2))

output looks like this

// correct with jackson
{
  "test" : 1.0
}
// incorrect with play
{
  "test" : 1
}

The code explicitly strips trailing zeros, this is a loss of precision:
https://github.com/playframework/play-json/blob/master/play-json/jvm/src/main/scala/play/api/libs/json/jackson/JacksonJson.scala#L66

usecase

Writing to KairosDB will create a schema for Real values if the incoming datapoint contains a ., otherwise data is seen as Integer.

@gmethvin
Copy link
Member

gmethvin commented Sep 20, 2017

This sounds more like a feature request than a bug, and I don't know if your use case is the right approach. JSON itself does not distinguish between integers and real numbers. There is only one number type.

It is common for JSON implementations to normalize the representation of numbers. I can see the value of pretty-printing JSON for presentational purposes, and I think it'd be reasonable for play-json to make that configurable, but I don't think it's something you should rely on.

@francisdb
Copy link
Author

KairosDB (time series database) wants to see json values with .0 to make a difference between long and double columns. And as you can see above the underlying library play-json is using (jackson) supports this.

RFC 7159 does not mention anything about normalization. I have a hard time to believe this would be common unless in the language itself you can not make a difference (js).

I see no reason why play-json could not keep the precision and represent the numbers as printed by BigDecimal or with at least one fractional digit if the source of the number was a double. This will not break anything for existing users. The only reason that comes to mind would be a (marginal) reduction of bandwidth which the user could mitigate by replacing doubles without fraction by integers

scala> val x = BigDecimal(1d)
x: scala.math.BigDecimal = 1.0

scala> val x = BigDecimal(1)
x: scala.math.BigDecimal = 1

scala> val x = BigDecimal(1.00d)
x: scala.math.BigDecimal = 1.0

scala> val x = BigDecimal("1.00")
x: scala.math.BigDecimal = 1.00

@cchantep
Copy link
Member

Even if Jackson has a different default behaviour, the Play one is not invalid. Issue with changing it in Play for specific use case as the one with KairoDB is prone to break the compatibility.

I would rather suggest to try to customize Writes keeping the current default behaviour (feature request).

@francisdb
Copy link
Author

I agree that the current behaviour is not invalid as the spec has no specifics on the subject. Feel free to close this, we can always write our own stringify.

@gmethvin
Copy link
Member

KairosDB (time series database) wants to see json values with .0 to make a difference between long and double columns. And as you can see above the underlying library play-json is using (jackson) supports this.

My understanding has always been that a JSON number is a representation of a number, and was intended to be interpreted as only a number by the recipient of the JSON. For example Wikipedia says:

Numbers in JSON are agnostic with regard to their representation within programming languages. No differentiation is made between an integer and floating-point value: some implementations may treat 42, 42.0, and 4.2E+1 as the same number while others may not.

I don't know much about KairosDB specifically but it seems like a sloppy choice to choose JSON when the spec does not explicitly distinguish between different representations of numbers.

RFC 7159 does not mention anything about normalization. I have a hard time to believe this would be common unless in the language itself you can not make a difference (js).

You're right, it doesn't. That's why I said this could be a configurable option, e.g. to print the BigDecimal verbatim rather than trying to format it. I'd say printing the BigDecimal as-is would be fine as a default, but others seemed to disagree in the past. See playframework/playframework#3784 for example. Unfortunately it's kind of difficult to make everyone happy with a default. The current default is the result of a lot of tweaking.

I see no reason why play-json could not keep the precision and represent the numbers as printed by BigDecimal or with at least one fractional digit if the source of the number was a double. This will not break anything for existing users.

It won't break anything for users who don't already expect some particular rendering of numbers, but it would probably annoy users who expect the current rendering and rely on it in some respect.

Now that we've introduced a configuration mechanism for play-json I think we should explore how to resolve it that way. I would not be opposed to switching the default behavior to printing the string value of the BigDecimal as long as there's a way to configure it back.

@cchantep
Copy link
Member

Now that we've introduced a configuration mechanism for play-json I think we should explore how to resolve it that way. I would not be opposed to switching the default behavior to printing the string value of the BigDecimal as long as there's a way to configure it back.

Rather keep the default behaviour and also to configure it in other way.

@francisdb
Copy link
Author

francisdb commented Sep 26, 2017

One more remark, on the reading side play-json supports making the difference between 1.0 and 1 (and even 1.00)

val json = Json.parse("""{"test": 1.0}""")
println((json \ "test").as[BigDecimal])

val json2 = Json.parse("""{"test": 1}""")
println((json2 \ "test").as[BigDecimal])

output

1.0
1

@trbogart
Copy link

Another use case for this is with ElasticSearch's dynamic mapping feature. That will map 1.0 as a double field, but 1 as a long field.

Doing custom serialization in our code base isn't ideal, so I went ahead and created a PR for this. The new behavior is opt-in using a system property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants