search

lpsmith / postgresql-simple

Mid-level client library for accessing PostgreSQL from Haskell
Other
206 stars 71 forks source link

RangeBound does not faithfully model Postgres range bounds #267

Open fieldstrength opened 5 years ago

fieldstrength commented 5 years ago

RangeBound has too many constructors.

In general Postgres ranges do not have the concept of PosInfinity or NegInfinity. So it would be preferable for these to be replaced with a singe constructor which could be called Unbound. This would have the appealing property of disallowing obviously nonsensical values like. (+infinity,-infinity)

From the relevant portion of the documentation

some element types have a notion of “infinity”, but that is just another value so far as the range type mechanisms are concerned. For example, in timestamp ranges, [today,] means the same thing as [today,). But [today,infinity] means something different from [today,infinity) — the latter excludes the special timestamp value infinity.

lpsmith commented 5 years ago

Well, PGRange PosInfinity NegInfinity is a perfectly sensible value, and will be rendered to postgresql as 'empty'. In fact, that's actually the definition of empty in postgresql-simple. The range module itself will never produce either +infinity or -infinity in a postgresql value, but rather translate that to an appropriate syntax.

You might want to look at the actual implementation of this module.

fieldstrength commented 5 years ago

That does not seem like "sensible value" to me. It has no reasonable interpretation as a range. Postgres does the reasonable thing and rejects something like that, even for range types that do happen to have infinities.

Ive glanced in the module. My interest is mainly as a user of the library and I don't have much knowledge of the internals. I don't claim the library generates invalid SQL, but I claim the data type that's exposed is the wrong one. I think its important that types accurately reflect what they do, and in particular here the type that leads one to wrong conclusions about the Postgres feature it represents, as it did for me.

Is there some benefit to choosing this representation that does not correspond to the values that Postgres actually supports? I have a hard time imagining one.

I appreciate the library, and don't mean to imply an imposition on your time. I would consider writing a patch sometime if it would be accepted. If not I'm interested in the reasoning.