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

Primary key on dependent side of relationship is BIGINT even if declared as Int32 #421

Closed
ghost opened this issue Sep 2, 2020 · 2 comments

Comments

@ghost
Copy link

ghost commented Sep 2, 2020

Minimal test case:

Granite::Connections << Granite::Adapter::Pg.new(name: "db", url: "postgres://test@localhost/test")
require "granite/adapter/pg"

class Comment < Granite::Base
  connection db
  column id : Int32, primary: true
  column user_id : Int32
  belongs_to :user, primary_key: "user_id"
end

Comment.migrator.drop_and_create

The user_id column is created as BIGINT even though it's explicitly declared as Int32. With no belongs_to line, user_id is created as INT. (It's also not being given its NOT NULL either way.)

@Blacksmoke16
Copy link
Contributor

Blacksmoke16 commented Sep 2, 2020

I don't think there is anything to be done here. The current migrator implementation is more so used internally for testing. It would be worth testing this against #413; which is intended to be a more user facing migration feature.

The reasoning why it gets created as BIGINT is belongs_to macro basically redefines the user_id column as the foreign key, which defaults to Int64. If anything you should probably do belongs_to :user, foreign_key: user_id : Int32.

@ghost
Copy link
Author

ghost commented Sep 3, 2020

I see. Thanks for suggesting the alternate notation. foreign_key: user_id : Int32 created it as INT but still no NOT NULL.

I had looked at Micrate earlier but didn't go with it because it doesn't solve the only thing I was really looking for in it, which is a way around the need to define the schema in two places (in Crystal for ORM use and in SQL for setting up the DB). When I found out about Granite::Base.migrator, I got hopeful that it would provide that. I'm concerned about the DSL in #413 replacing this, since that DSL doesn't look like it offers this.

This issue was closed.
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

1 participant