Skip to content

Conversation

@WolfDan
Copy link
Contributor

@WolfDan WolfDan commented Apr 12, 2021

Contributor checklist

  • Bug fixes include regression tests
  • Features include unit/acceptance tests

On my previous PR I modified the integer primary keys to generate a bigserial id, while the test was successful I didn't take into account relationships which should be bigint as well, so this PR aims to solve that

My worry here is the lack of tests regarding this, I'm not really familiar with the way you test things here @zachdaniel so a bit of help/guidance would be appreciated ^^/

@WolfDan WolfDan changed the title Relationships referecnes should follow bigint ids schema as well Relationships references should follow bigint ids schema as well Apr 12, 2021
@zachdaniel
Copy link
Contributor

zachdaniel commented Apr 12, 2021

Interesting, so I don't think this would be quite enough. We would only want to make the integer a bigint if the other end would also be a bigint. But there are two points worth mentioning:

1.) we should just default :integer to :bigint as well, that is a better default.
2.) we should actually just omit the type in references(...) because the type needs to be the same, and that is the default behavior.

@zachdaniel
Copy link
Contributor

I'll go ahead and make those changes.

@zachdaniel zachdaniel closed this Apr 12, 2021
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