Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Database.Esqueleto.PostgreSQL.values does not perform necessary type coercion at runtime #343

Open
rjmholt opened this issue Nov 10, 2022 · 1 comment

Comments

@rjmholt
Copy link

rjmholt commented Nov 10, 2022

If I define a simple enum type in PostgreSQL like this:

CREATE TYPE behavior AS ENUM ('Good', 'Bad');

And then give it the appropriate matching Haskell implementation (plus all the other stuff which is less material):

data Behavior = Good | Bad
  deriving stock (Show, Eq)

instance PersistField Behavior where
  toPersistValue = PersistText . pack . show
  fromPersistValue = undefined -- Not important here

instance PersistFieldSql Behavior where
  sqlType _ = SqlOther "behavior"

I can do something like join on values of that enum (like in this silly example):

from $
  table @Kids
    `innerJoin` behaviorDinner
      `on` (\(kid :& (bvr, _)) -> kid ^. KidBehavior ==. bvr)
where
  behaviorDinner :: From (Behavior, Dinner)
  behaviorDinner = values $ (Good, Pizza) :| [(Bad, Poison)]

When I run this though, I get an error message something like this (paraphrased from the real error):

uncaught exception: SqlError
       SqlError {sqlState = "42883", sqlExecStatus = FatalError, sqlErrorMsg = "operator does not exist: behavior = text", sqlErrorDetail = "", sqlErrorHint = "No operator matches the given name and argument types. You might need to add explicit type casts."}

Looking at the SQL Esqueleto generates for this, it looks something like this:

INNER JOIN (VALUES (?,?),(?,?)) AS "vq"("v","v2") ON "kids"."behavior" = "vq"."v"

So it seems like we're missing a type coercion (which sqlType makes available) to correctly distill the values. From the PostgreSQL documentation:

When VALUES is used in INSERT, the values are all automatically coerced to the data type of the corresponding destination column. When it's used in other contexts, it might be necessary to specify the correct data type. If the entries are all quoted literal constants, coercing the first is sufficient to determine the assumed type for all

In the implementation itself, a quick read indicates that we might want to add that here:

in (TLB.fromLazyText $ "(" <> TL.intercalate "," valsSql <> ")", params)

Not knowing much about Esqueleto, I'd guess we'd need to add some type constraints to extract the type underlying a.

Also I'd guess it would be more efficient to only add the coercion to the first row, but that would require breaking up the current mapping (where we currently have NE.toList, we'd want to actually process the head and tail differently).

@parsonsmatt
Copy link
Collaborator

We'd need to add a PersistFieldSql constraint into the SomeValue constructor but that should be fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants