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

[ci skip] Add a section to the Active Record Migration Guide on creating tables with composite primary keys #49186

Merged
merged 1 commit into from Sep 8, 2023

Conversation

noahgibbs
Copy link
Contributor

No description provided.

@rails-bot rails-bot bot added the docs label Sep 7, 2023
Copy link
Contributor

@nvasilevski nvasilevski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -623,6 +623,26 @@ NOTE: Active Record only supports single column foreign keys. `execute` and
`structure.sql` are required to use composite foreign keys. See
[Schema Dumping and You](#schema-dumping-and-you).

### Composite Primary Keys

Sometimes a single column's value isn't enough to uniquely identify every row of a table, but a combination of two or more columns *does* uniquely identify it. This can be the case when using a legacy database schema without a single `id` column as a primary key, or when altering schemas for sharding or multitenancy.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New docs should wrap to 80 characters. I haven't been pushing for that on older docs that are updated. but any new ones should follow that convention.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just re-wrapped it.

```

INFO: Tables with composite primary keys require passing array values rather
than integer IDs to many methods. See also the [Active Record Querying][]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The link isn't rendering properly for me right now. I think we need to specify the URL below, ie.

[Active Record Querying]: active_record_querying.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied the syntax from elsewhere in the guide. But that might be too optimistic. The Active Record Associations guide is linked that way, though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that one works because it's defined below 😄

[Active Record Associations]: association_basics.html

(You can tophat the guides by doing cd guides; rake guides:generate:html ONLY=active_record_migrations, and then previewing the html file it generates!)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D'oh! Okay, I'll fix that quick.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fixed now.

@eileencodes
Copy link
Member

Can you squash your commits into one? We prefer to merge single commits into Rails.

@noahgibbs
Copy link
Contributor Author

Squashed.

@eileencodes eileencodes merged commit 8bea21e into rails:main Sep 8, 2023
3 checks passed
@eileencodes eileencodes deleted the migration_guide_cpk_create branch September 8, 2023 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants