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

Postgresql primary key is Int4, not Int8 #519

Closed
connrs opened this issue Dec 19, 2015 · 10 comments
Closed

Postgresql primary key is Int4, not Int8 #519

connrs opened this issue Dec 19, 2015 · 10 comments
Assignees

Comments

@connrs
Copy link

connrs commented Dec 19, 2015

Hi there,

I'm very new to the Yesod project, so I do apologise if I've missed something here. I've really enjoyed reading about this framework and I was experimenting with setting up the database (Postgres) tables in config/models; after setting up some models, I noticed that the primary key was Int4 but arbitrary columns set as Int would be created as Int8.

I took a look around the codebase and spotted that "SERIAL" is defined on line 491 of Postgresql.hs. The serial column type creates an Int4 column, whereas bigserial/serial8 creates an Int8 column. It's currently causing me problems when defining foreign keys as the column types aren't matching.

I took a look around the documentation and found nothing about this in the wiki or the Yesod Web Framework Book. I took a look around the existing issues and issue #455 looked vaguely similar, but I wanted to raise a new query just in case. Is there a configuration option that I've missed here or some documentation that I've missed?

Thanks,
connrs.

@snoyberg
Copy link
Member

I was unaware of SERIAL vs SERIAL8, thanks for pointing it out. Do you know if migrating a database from SERIAL to SERIAL8 would have negative ramifications? It's something we can consider changing. (Pinging @gregwebs.)

@connrs
Copy link
Author

connrs commented Dec 29, 2015

Looking at the Postgres docs, all they have to say is:

The type names bigserial and serial8 work the same way, except that they create a bigint column

8.1. Numeric Types

Looking at some of my pre-existing databases, the sequence generated for the serial and serial8 column both have a max integer value of 263-1. This seems great from a migration perspective, as it should be as simple as converting the column from int4 to int8.

snoyberg added a commit that referenced this issue Dec 29, 2015
@snoyberg snoyberg self-assigned this Dec 29, 2015
@snoyberg
Copy link
Member

I've just pushed a commit to the 519-postgres-serial8 branch that changes from SERIAL to SERIAL8. Would you be able to try that out and see if it addresses the issue for you?

@connrs
Copy link
Author

connrs commented Dec 30, 2015

Thanks for the speedy response here, I really appreciate it. I'll gladly test this branch out. My yesod experiments have been managed with stack instead of plain ol' cabal (yet another awesome haskell project). Is there a simple way for me to specify this git branch as a dependency in a stack.yaml or do you think it would be easier managed with cabal sandboxes?

@snoyberg
Copy link
Member

The Stack documentation explains how to specify a Git commit for a package
source:
http://docs.haskellstack.org/en/stable/yaml_configuration.html#packages

On Wed, Dec 30, 2015 at 3:06 PM, Paul Connolley notifications@github.com
wrote:

Thanks for the speedy response here, I really appreciate it. I'll gladly
test this branch out. My yesod experiments have been managed with stack
instead of plain ol' cabal (yet another awesome haskell project). Is there
a simple way for me to specify this git branch as a dependency in a
stack.yaml or do you think it would be easier managed with cabal sandboxes?


Reply to this email directly or view it on GitHub
#519 (comment)
.

@connrs
Copy link
Author

connrs commented Dec 31, 2015

Thanks for the pointer to the Stack documentation - it certainly did the trick.

From my perspective, this is working perfectly. I've pasted in some of the sample output:

Migrating: CREATe TABLE "email"("id" SERIAL8 PRIMARY KEY UNIQUE,"email" VARCHAR NOT NULL,"user_id" INT8 NULL,"verkey" VARCHAR NULL)
Migrating: CREATe TABLE "comment"("id" SERIAL8 PRIMARY KEY UNIQUE,"message" VARCHAR NOT NULL,"user_id" INT8 NULL)
ALTER TABLE "comment" ADD CONSTRAINT "comment_user_id_fkey" FOREIGN KEY("user_id") REFERENCES "user"("user_id"); []

Apologies for the brief reply here, I'm getting ready for the new year. Thank you very much for your help with tackling this issue and happy New Year!

@snoyberg
Copy link
Member

snoyberg commented Jan 4, 2016

The problem with making this change would be causing regression for current users with 4-byte keys. I'm not sure if others will have an issue with this change. A good way to move the ball forward on this would be to email the Yesod mailing list and look for feedback on incorporating this change in master.

@connrs
Copy link
Author

connrs commented Jan 6, 2016

No problem. I've sent a post to the mailing list, so I'll see what sort of feedback we get about it.

snoyberg added a commit that referenced this issue Jan 19, 2016
@spl
Copy link
Contributor

spl commented Feb 4, 2016

@connrs: I'm glad you reported this discrepancy between the foreign and primary key integer sizes. It's been confusing me for a while. I never looked into why it existed, so I'm happy somebody did.

@snoyberg: Thanks for the quick turnaround.

I did discover a problem with this due to the use of SQL functions that expected certain key types, which led me to this issue. Fortunately, it came up in continuous integration building. Even though this change might cause surprising problems for people (who also might end up here), I'm glad you made it.

@spl
Copy link
Contributor

spl commented Feb 4, 2016

It just occurred to me that maybe it would be a good idea to add a row to the table on https://github.com/yesodweb/persistent/wiki/Persistent-entity-syntax with the key types for each backend.

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

3 participants