rel8 icon indicating copy to clipboard operation
rel8 copied to clipboard

Schema type checker

Open abigailalice opened this issue 6 months ago • 9 comments

This should close #274 and #186, and potentially #291. The semantics are primarily based on @KaneTW's gist in #274, except with two improvements:

  1. It handles array types
  2. It returns an error if two columns in a TableSchema will have identical names, instead of passing silently

It also displays information in a slightly different way, trying to show as many errors as it can rather than bailing on the first. It also adds a number of test cases to ensure it behaves correctly for the primitive types, higher kinded types, and a few tests to ensure it fails in cases where it should fail. I'm mostly putting this forward to see what other people think, as I'm not sure if it should have other changes before being accepted. Some of my (admittedly bikesheddy) thoughts on the current state:

  1. I wasn't able to re-export it from Rel8 without creating an import cycle, so it's only exported via its own module being public. For some reason the module wouldn't compile unless it imported Rel8, getting a type error that Columns wasn't injective otherwise. I'm assuming the issue is that something not being in scope is effecting type checking, but I've never actually seen an error like that, so really aren't sure. I'm also not really sure if Rel8.Table.Verify is the best name for the module, I just picked Table pretty arbitrarily.
  2. I reused tests/Rel8/Generic/Rel8able/Test.hs quite a bit to add some things necessary for my own test cases, but I'm not really sure if that's an appropriate place for them. I could move them if that's preferred.
  3. The current work won't show KaneTW's original work, since I just copy-pasted it. Not sure if there's a proper way to base work off a gist, or if it's even an issue, but I thought it should still be said
  4. I kind of want to have a version of the type checking function which also reads the data from the table after checking the schema, as having a valid schema isn't enough to ensure existing data can be read by rel8, which can be important when doing migrations. However, this isn't possible just using the Statement type. I could just add a version using Session which does this, but for people using hasql-transaction they would still need to rewrite that function manually. This isn't a terribly difficult thing for users of the library to do themselves, since anyone using it is already going to need to convert from Statement to Transaction, and can just read from the table after type checking, but it just feels like an oversite to not have this. For the moment the issue is explained in the documentation for the function.

It should also be said that #59 is kind of exploring similar ideas, and @KaneTW also was exploring some other ideas for interfaces in other changes to the original gist.

abigailalice avatar Aug 02 '24 01:08 abigailalice