Export Hasql.Private.Decoders.Value.Value to allow for other time libraries

[axman6]

Recently I tried to implement some Value parsers for the thyme time library, which uses an internal representation very similar to PostgreSQL's (Microseconds since the MJD epoch, with Postgres using microseconds since the Unix epoch). In the end I had to settle for assuming that the database I was connecting to was using integer date times because I couldn't get access to the ReaderT Bool inside Hasql.Private.Decoders.Value.Value.

Generally I've found that packages which have Internal (a.k.a, the Hasql.Private.* modules) modules all eventually end up exporting these internals because not exposing them makes the work of others trying to extend libraries much more difficult - exposing them puts the onus on the third party developer to keep up with the library, with the understanding that .Internal modules might change at any time, without warning and possibly breaking things between major releases as long as the official API is kept consistent.

[nikita-volkov]

I agree that "time" is quite far from the quality level required from a standard library and I acknowledge that the API should expose some lower level mechanisms that would allow to bind to other time libraries efficiently. However I disagree with the suggested approach, because I'm highly against the practice of exposing the internal modules. There are other ways of dealing with this kind of problems, which, unlike the "Internal" convention, are consistent. I'm talking about exposing the lower-level APIs in separate libraries. However this is outta scope of this thread.

I believe this issue should be solved by designing some intermediate data-structure, which represents the time values as composite types, representing the actual data structure being transmitted over the wire to Postgres. Unfortunately, the current implementation does not expose any such structure and just directly en(de)codes the binary data to/from the "time" values.

I'll tell you this: I'll keep this issue in mind for the next major Hasql release. For the time being I can recommend nothing better than to just implement your "thyme" integration by converting from/to the "time" values.

[mgsloan]

I just ran into this. It is inconvenient that users of the library cannot use custom exceptions, and providing access to internals would be a good workaround. It is hard to anticipate every use of a library.

In particular, I wanted to cleanly throw an exception in the case that an UPDATE does not return 1, but this seems to be impossible with hasql's API due to the hiding of internals. For now I am just using an error hack with a TODO

[nikita-volkov]

@mgsloan I would highly recommend against using exceptions for logic. What's stopping you from using Either? Also can you please create a separate thread for this discussion, because it seems unrelated to the subject of this thread

[mgsloan]

This is using exceptions for exceptions. I am commenting here because this issue mentions exporting internals. IMHO it is good to export all internals, because then users aren't unnecessarily restricted, yet they are still aware that they may be using less stable portions of the API. I've opened #94

[nikita-volkov]

I've opened #94

Thanks!

it is good to export all internals

Since this seems like a sensitive subject we can discuss the exposure of internal APIs in yet another isolated thread.

[axman6]

I've never actually seen a good reason to hide the internals of a library in Haskell, as long as it is made clear that a any code making use of the internals can break at any time and you won't support its use. Generally hiding internals make understanding a library for users*. There is a good reason why libraries like ByteString and Text have .Internal modules - there are many cases where being able to access the internals is necessary for users because of factors that aren't necessarily predictable (I've needed to access the internals of ByteString many times to make converting to other types such as Storable Vectors efficient many times).

For my particular use case I just ended up fmapping over a parser for int8 but this won't work if for some reason the PostgreSQL uses double for times, my code will break and there's not a lot I can do about it. custom looks like a possibility, but then I have to figure out what the format of the byte string actually is, which has already been done in the Hasql library and has been done in the postgresql-binary, but I as a user can't make use of any of this work.

*A great example of this is the type-of-html library which is built around two classes which you can't actually see in any documentation what instances there are, making the library very error prone to use.

[nikita-volkov]

I've never actually seen a good reason to hide the internals of a library in Haskell, as long as it is made clear that a any code making use of the internals can break at any time and you won't support its use. Generally hiding internals make understanding a library for users*. There is a good reason why libraries like ByteString and Text have .Internal modules - there are many cases where being able to access the internals is necessary for users because of factors that aren't necessarily predictable (I've needed to access the internals of ByteString many times to make converting to other types such as Storable Vectors efficient many times).

I've started an isolated thread of discussion for that: #95 (comment)

For my particular use case I just ended up fmapping over a parser for int8 but this won't work if for some reason the PostgreSQL uses double for times, my code will break and there's not a lot I can do about it. custom looks like a possibility, but then I have to figure out what the format of the byte string actually is, which has already been done in the Hasql library and has been done in the postgresql-binary, but I as a user can't make use of any of this work.

As I've said before, it is a good argument for having a more general decoder for dates, and I do agree that we should have it. I just don't have the time to implement it right now. PRs are welcome, but no "Internals" modules please.

[mgsloan]

How about an internals package like hasql-internal?

There is an interesting tradeoff here where if you export internals modules, users are more likely to organically develop such utilities. However, by making this convenient, the utilities might be less likely to end up in the upstream package. So, I suppose that internals modules might give users more power and make it more likely that they'll write their own utilities based on them, while simultaneously somewaht lowering the likelihood of upstream contribution.

[nikita-volkov]

Yep! I was planning to release such a package with the new generation of the package. If you wanna continue the discussion on that subject please switch to the "Why not use the "Internals" convention?" thread.

[domenkozar]

This prevents me from using https://hackage.haskell.org/package/ulid in hasql-th.

[nikita-volkov]

This prevents me from using https://hackage.haskell.org/package/ulid in hasql-th.

I would separate that into several unrelated issues:

1. Ability to add extra codecs outside of the "hasql" package. On the decoder end it is already fully adressed by Decoders.custom. On the encoder end it is partially addressed by Encoders.unknown, though it could use an extension that would allow the user to identify the Postgres type, e.g., via OIDs.

2. Support for more postgres data-types by "hasql". That is best addressed by directly extending the "hasql" and "postgresql-binary" packages via PRs.

3. Extensibility of "hasql-th". In the current design it only supports a limited set of types and the strategy for its extension is by extending the library itself via PRs. There's no mechanism for custom extensibility because that would likely complicate the design a lot.

As I see neither of the mentioned issues requires resorting to exposing the internal modules. Am I missing something?