hasql-th icon indicating copy to clipboard operation
hasql-th copied to clipboard

Types without exported constructors

Open torgeirsh opened this issue 4 years ago • 10 comments

With hasql it's possible to make newtype wrappers for e.g. different integer based ids, keep the constructors private, and just export decoders and encoders. With hasql-th it seems like constructors are required, which means that while we gain safety for the SQL statements, we lose some safety for the parameters during dimap. Is there a way around that? Would a typeclass be able to solve that? Or could encoders and decoders somehow be optional? Something else entirely?

torgeirsh avatar Feb 18 '21 08:02 torgeirsh

You're trying to implement a form of an anti-pattern. Here's a detailed explanation.

nikita-volkov avatar Feb 18 '21 10:02 nikita-volkov

What I'm trying is to avoid "id blindness", where mixing up unrelated ids is accepted by the compiler, because they're all Int64. Making that mistake will also cause runtime errors, or in the worst case, wrong data to be affected. I understand the concerns in your explanation, and agree that there's a risk when codecs for newtype wrappers are defined separately from the SQL for which they're used, but I'd argue it's the lesser of two evils. At any rate, I'm not strongly attached to that particular solution (or workaround), as long as "id blindness" is avoided. Do you have other suggestions?

torgeirsh avatar Feb 18 '21 11:02 torgeirsh

You can always dimap over statements. Just expose the constructors in a package-internal module and reexport the types without constructors in the public module.

nikita-volkov avatar Feb 18 '21 11:02 nikita-volkov

I'm not sure I follow. Does that assume dimap will performed in the internal module? What if dimap is performed in an external module, where the constructors are not exported?

torgeirsh avatar Feb 18 '21 11:02 torgeirsh

In .cabal:

  exposed-modules:
    ModelWithoutConstructors
  other-modules:
    ModelWithConstructors
    Statements

Both Statements and ModelWithoutConstructors import ModelWithConstructors.

nikita-volkov avatar Feb 18 '21 13:02 nikita-volkov

Ah, I see. But they're still in the same project. What if they are in different projects? I'm also not sure that it solves the "id blindness" problem. If you use the constructors inside dimap, can't you easily mix up the integers?

torgeirsh avatar Feb 18 '21 14:02 torgeirsh

Why would you keep the types representing entities in a database separately from the database integration code?

No typechecker can protect you from making mistakes. You can however use typechecker to enforce your users to correctly use your API. That's what I thought you wanted to achieve.

nikita-volkov avatar Feb 19 '21 03:02 nikita-volkov

When the types are shared by multiple applications, it makes sense to keep them in a library instead of copying and pasting them around. One example is a library that takes care of authentication and provides a type for user ids, which is then used by different applications to connect to databases that contain said user id. By exporting Hasql codecs from the library, application writers are prevented from mixing up user ids with e.g. organisation ids in the Haskell code. While the typechecker isn't a guarantee for correctness, this scenario seems safer to me than juggling different integers in dimap.

torgeirsh avatar Feb 19 '21 08:02 torgeirsh

I have to stress on this again, a codec only makes sense in the context of a query. One query may produce an ID as int4, another may have the same ID packed inside of jsonb or text or int8. It is only the query that determines what it is.

By distributing codecs with your model you're introducing an implicit dependency on all the queries in all the databases that the codec will be used in. By doing this you're coupling it all with all the sprawling naughty consequences when changes are introduced. You're also mixing the responsibilities of your projects. E.g., your database logic gets distributed across the model package and the actual DB layer package instead of being isolated to just the DB layer.

As a suggestion, if you want to keep having a central model package reused across other packages, I recommend to just expose the constructors.

nikita-volkov avatar Feb 19 '21 11:02 nikita-volkov

The dependency between the queries and the codec is intentional in the case of user id, since the shared authentication code must always get the same user id type. I don't consider authentication to be database logic, so I believe the responsibilities remain separated. I thank you for the advice, but I don't see how exposing constructors is strictly better than a typeclass or something else that can perform the conversion automatically. At any rate it sounds like this isn't on the table for hasql-th.

torgeirsh avatar Feb 19 '21 13:02 torgeirsh