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

Foreign key constraint name is weird #1124

Open
sir4ur0n opened this issue Sep 15, 2020 · 2 comments · Fixed by #1244
Open

Foreign key constraint name is weird #1124

sir4ur0n opened this issue Sep 15, 2020 · 2 comments · Fixed by #1244

Comments

@sir4ur0n
Copy link

Hello,

I am unsure if this is intentional - in which case this is a request for improvement - or not - in which case this is a bug report.

The foreign key constraint name is weird: it seems that the (mandatory) constraint name passed in the quasi quoter is appended with no separator (e.g. _) to the FK table name.

FYI I use:

  • Stack with resolver lts-16.5, so Persistent 2.10.5.2 and Persistent Template 2.8.2.3
  • PostgreSQL 11.8

To reproduce:

share
  [mkPersist sqlSettings, mkMigrate "migrateAll"]
  [persistLowerCase|
  FooBar
      foo String
      bar Bool
      Primary foo bar
      deriving Show
  FooBarBaz
      foo String
      bar Bool
      baz Int
      Primary foo bar baz
      Foreign FooBar fk_something foo bar
      deriving Show
|]

The foreign key constraint is named foo_bar_bazfk_something, i.e. it appends the table name foo_bar_baz to the provided QQ name fk_something with no separator.

While I don't mind being forced to prefix with the table name foo_bar_baz, I don't understand why there isn't at least a separator _ between the table name and the given suffix (so it would be foo_bar_baz_fk_something). And it's illegal (doesn't compile) to write Foreign FooBar _fk_something foo bar.

What do you think?

Cheers!

@parsonsmatt
Copy link
Collaborator

Great catch! I did not know this either.

To fix this, we'll want to introduce a field on the relevant Settings datatype that accepts a "foreign key first character" - eg mkPersistSettingsTableSeparator :: Maybe Text - with a default value of Nothing. Users can opt in to Just "_" or similar. This would be a breaking change, but could land in 2.11 or 2.12.

@parsonsmatt
Copy link
Collaborator

It should also be pretty easy to modify the parser to accept a leading underscore, which would not be a breaking change

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

Successfully merging a pull request may close this issue.

2 participants