Add warning for float->string conversions.#212
Add warning for float->string conversions.#212ryanleh wants to merge 1 commit intomc2-project:masterfrom ryanleh:add-float-conversion-error-to-docs
Conversation
| - ``TimestampTime``, ``DateType`` | ||
| - ``ArrayType``, ``MapType`` | ||
|
|
||
| *Warning:* Conversions from ``FloatType`` to ``StringType`` may produce incorrect results in the least significant portion of the floating point value. See `this issue <https://github.com/mc2-project/opaque/issues/211>`_ for further details. |
There was a problem hiding this comment.
I don't think "incorrect" is the right word to use here; after reading the issue, it seems that they're both the same float representation. Maybe something like "may produce a correct result rounded to a different decimal place than the one shown by FloatType"? But I also kind of don't think this warning is really necessary, since that should be pretty obvious when comparing show and collect results for example.
There was a problem hiding this comment.
Agreed that "incorrect" is the wrong word. Maybe the rephrase could be:
"Converting a FloatType to StringType may display a result rounded to a different decimal place than the one shown by FloatType even though the internal representations are identical."
I don't know if it's good to assume a user would run both show and collect. Even if they did, I it's very confusing if there was a discrepancy between the two. Thoughts @wzheng ?
There was a problem hiding this comment.
Another point to consider is that show only has "inconsistent behavior" when a user explicitly compares the results to Spark SQL. Opaque's behavior is correct as a standalone SQL engine. So I'd add this point as well to specify what we're comparing to.
3 participants
Closes #211