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

add showSqlTypeWithSchema for types in a schema #589

Closed
wants to merge 5 commits into from

Conversation

stevemao
Copy link
Contributor

@stevemao stevemao commented Apr 8, 2024

  • similar to enumMapperWithSchema
  • enums needs an instance of IsSqlType if used with an array
  • enums can have a schema

- similar to enumMapperWithSchema
- enums needs an instance of IsSqlType if used with an array
- enums can have a schema
@tomjaguarpaw
Copy link
Owner

If you want a schema I don't understand why you can't just do showSqlType = "\"foo\".\"bar\"".

@stevemao
Copy link
Contributor Author

stevemao commented Apr 8, 2024

I can, but other APIs such as enumMapperWithSchema has a spot for schema. This is to make it consistent with these APIs.

@tomjaguarpaw
Copy link
Owner

I see. Then in that case I think showSqlTypeWithSchema wants just to be a function

showSqlTypeWithSchema :: (String, String) -> String
showSqlTypeWithSchema (schema, type_) =
  render (doubleQuotes (text schema) <> text "." <> doubleQuotes (text type_))

that you use like

instance IsSqlType Whatever where
  showSqlType = showSqlTypeWithSchema ("schema", "type_name")

Making it a class method means that all instances have to implement it, which is not what we want because not all type names have a schema.

@stevemao
Copy link
Contributor Author

stevemao commented Apr 9, 2024

Happy with a function to make it simple. But I have some questions

showSqlTypeWithSchema seems to be quite generic. Maybe we can call it sqlTypeWithSchema?

Making it a class method means that all instances have to implement it, which is not what we want because not all type names have a schema.

Not really? the instance can choose to implement either method {-# MINIMAL showSqlType | showSqlTypeWithSchema #-}

@tomjaguarpaw
Copy link
Owner

Making it a class method means that all instances have to implement it, which is not what we want because not all type names have a schema.

Not really? the instance can choose to implement either method {-# MINIMAL showSqlType | showSqlTypeWithSchema #-}

But what if an instance choose not to implement it but then a user calls it on that instance? Run time crash, presumably.

@stevemao
Copy link
Contributor Author

stevemao commented Apr 9, 2024

Ok, definitely not ideal even though it's an internal module

@tomjaguarpaw
Copy link
Owner

Thanks, I tweaked this and merged it in f012b27

@tomjaguarpaw
Copy link
Owner

Released as https://hackage.haskell.org/package/opaleye-0.10.3.0

@stevemao stevemao closed this Apr 14, 2024
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

Successfully merging this pull request may close these issues.

2 participants