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

Different encoding behavior for PersistDbSpecific with persistent-postgresql and persistent-mysql #1110

Open
sol opened this issue Aug 14, 2020 · 1 comment

Comments

@sol
Copy link
Contributor

sol commented Aug 14, 2020

What is the discrepancy?

persistent-mysql and persistent-postgresql encode PersistDbSpecific differently.

persistent-mysql uses Plain:

render (P (PersistDbSpecific s)) = MySQL.Plain $ BBS.fromByteString s

However, persistent-postgresql uses Escape (effectively, treating it as a string-like value):

toField (Unknown a) = PGTF.Escape a

When was this introduced?

The behavior for PostgreSQL changed with 9287943.

Why is this a problem?

With the current behavior it is not possible to encode something like point '(2.0,0)' for PostgreSQL.

(I would also be very surprised if the example given in the docs for PersistDbSpecific is still anywhere close working; but I haven't tried that specifically.)

How to address this?

I'm not sure how to best address this. Altering how PersistDbSpecific is encoded would be a breaking change that isn't detected by the type checker. One solution might be to remove PersistDbSpecific and add two new constructors:

  • PersistDbSpecificEscape, which uses Escape (current behavior of persistent-postgresql)
  • PersistDbSpecificPlain, which uses Plain (current behavior of persistent-mysql)
@parsonsmatt parsonsmatt added this to the 2.11 milestone Nov 2, 2020
@parsonsmatt
Copy link
Collaborator

#1123 covers this in a PR

In #1122, I outline a proposal for fixing the issue over a few release cycles.

tl;dr: Deprecate PersistDbSpecific in favor of PersistLiteral and PersistLiteralEscaped, basically your suggestion @sol :)

@parsonsmatt parsonsmatt removed this from the 2.11 milestone Nov 4, 2020
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