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

Remove duplicate crates.version column #544

Closed
jyn514 opened this issue Jan 4, 2020 · 4 comments
Closed

Remove duplicate crates.version column #544

jyn514 opened this issue Jan 4, 2020 · 4 comments
Assignees
Labels
P-low Low priority issues

Comments

@jyn514
Copy link
Member

jyn514 commented Jan 4, 2020

Reported by @koenaad:

So, I was checking out the database of docs.rs, I am a bit confused about why there is a column versions in the crates table: https://github.com/rust-lang/docs.rs/blob/master/src/db/migrate.rs#L105
The table releases contains the releases per crate and also has a column version. So the same information can be retrieved by doing a join between crates and releases. Something like this:

select crates."name", releases."version", releases.release_time, releases.build_status
from releases
inner join crates on releases.crate_id = crates.id
where crates.name = 'fie'
order by releases.release_time desc;

It seems to kept in sync by add_package: https://github.com/rust-lang/docs.rs/blob/master/src/db/add_package.rs#L151.

There's no sense in storing duplicate data in postgres, so I'd like to see the crates.version column removed eventually.

@jyn514 jyn514 added the wishlist label Jan 4, 2020
@yvrhdn
Copy link

yvrhdn commented Jan 4, 2020

I can remove the use of crates.versions in crate_details.rs (https://github.com/rust-lang/docs.rs/blob/master/src/web/crate_details.rs#L114) when implementing the 2nd change of #516 (comment)

@jyn514
Copy link
Member Author

jyn514 commented Jan 4, 2020

I would rather separate feature PRs from refactoring PRs. Our test suite still isn't as good as I'd like and smaller PRs makes it easier to rollback commits. This is especially true for the SQL queries which are mostly not type checked, if you delete crates.version there will be no compile errors but the site may panic at runtime.

@jyn514 jyn514 self-assigned this Jan 4, 2020
@jyn514
Copy link
Member Author

jyn514 commented Jan 4, 2020

See #466 for an example of a PR that passed all tests but still crashed the site.

(This is why this issue is marked as wishlist)

@jyn514
Copy link
Member Author

jyn514 commented Jun 27, 2020

Fixed in #804

@jyn514 jyn514 closed this as completed Jun 27, 2020
@jyn514 jyn514 added P-low Low priority issues and removed wishlist labels Jun 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-low Low priority issues
Projects
None yet
Development

No branches or pull requests

2 participants