search

tomjaguarpaw / haskell-opaleye

Other
599 stars 115 forks source link

Generalize `in_` #602

Open tysonzero opened 1 week ago

tysonzero commented 1 week ago

The current type is:

in_ :: (Functor f, Foldable f) => f (Field a) -> Field a -> Field SqlBool

But it seems like a nicer type would be:

in_ :: (Default EqPP fields fields, Foldable f) => f fields -> fields -> Field SqlBool

We can drop the Functor as the mapping is/can-be done after the toList/foldr type stuff, and we take advantage of the Default EqPP machinery that .=== uses since IN seems to work just fine in PostgreSQL on products/tuples.

This should close #404, #406, #419 and the issue is mentioned in open issues #431 and #599.

tomjaguarpaw commented 1 week ago

This seems like a good idea. Is it a breaking change? If so it would prefer to introduce it under a new name first, but I'm struggling to think of any code that could be broken by this.

ocharles commented 1 week ago

Anything that wraps in_ and has a type signature (myIn = in_) would break, because the type has changed. It's hypothetical, but probably a better path than trying to find a threshold of "realistic" breaks

ocharles commented 1 week ago

Oh wait, maybe I'm wrong because Field a has that instance? Might be worth checking that one,

tysonzero commented 1 week ago

Yes Field a does have that instance. Should be a strict generalization, so only breakage I can think of would be if it leads to type ambiguity or if someone's using TypeApplications on it.