Parenthesize negative numbers in ToField instances
#145
Conversation
The use of a lazy text builder dates back to 87135c1, when scientific-0.2.0.2 only provided that option. Now we require scientific >= 0.3.7.0, which also provides the `ByteString` builder.
This is necessary because in PostgreSQL, `-` is a unary operator that has lower precedence than the `::` operator, and that can cause problems at the edge of allowed ranges. For example, `-32768::int2` is parsed as `-(32768::int2)`, which throws an "out of range" error, even though `(-32768)::int2` is accepted. Closes haskellari#143.
|
Ah, I saw there were no tests of I've added a test now, let me know if you want something more thorough. |
|
Gentle nudge, could this get reviewed please? |
| testParenNegatives TestEnv{..} = do | ||
| [Only (x :: Int)] <- query conn "SELECT ?::int2" (Only (-32768 :: Int)) | ||
| x @?= -32768 | ||
| [Only (x :: Int)] <- query conn "SELECT ?::int2" (Only (-32768.4 :: Double)) |
There was a problem hiding this comment.
why not SELECT (?) :: int2?
I'm not yet convinced this must be in the library
There was a problem hiding this comment.
That works for this one. It wouldn't work for this:
query conn "SELECT * FROM ? tbl"
(Only $ Values ["int2", "int2"]
[(-32768 :: Integer, -32768.4 :: Float)]
)In general, I expect users of this library will be using ToField instances as though they produce atoms. The documentation doesn't quite say so explicitly, but that's the strong vibe I get; e.g. it doesn't typically write (?), which I'd expect if "toField might not produce an atom" was standard. The ToField (Values a) instance assumes that, too.
Closes #143.
This also switches the
Scientificinstance to use aByteStringbuilder instead of aText.Lazybuilder. Lazy text dates back to 87135c1, when scientific-0.2.0.2 only provided that option. Now we require scientific >= 0.3.7.0, which also provides theByteStringbuilder. The alternative was to reimplementparenNegativesfor this instance, which also wouldn't be too bad.For types with signed zeros,
-0isn't parenthesized. I wasn't sure how to detect it ("check whether the rendered value starts with-" should work but breaking out ofBuilderfeels bad for performance?), and I'm not aware of any situation where-(0::type)would be different than(-0)::type. I could easily imagine I'm missing something though.