Skip to content

Missing reverse dependencies #602

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.

Sign up for GitHub

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

Jump to bottom
Closed
dtolnay opened this issue Mar 8, 2017 · 5 comments
Closed

Missing reverse dependencies #602

dtolnay opened this issue Mar 8, 2017 · 5 comments

Comments

@dtolnay
Copy link
Member

dtolnay commented Mar 8, 2017

I refuse to believe that there are two reverse dependencies on serde.

selection_023
@dtolnay
Copy link
Member Author

dtolnay commented Mar 8, 2017 

$ curl -sSH 'Content-Type: application/json' \
  https://crates.io/api/v1/crates/serde/reverse_dependencies | jq
{
  "dependencies": [
    {
      "crate_id": "hyper",
      "default_features": true,
      "downloads": 885685,
      "features": "",
      "id": 126915,
      "kind": "normal",
      "optional": true,
      "req": "^0.7",
      "target": null,
      "version_id": 36977
    },
    {
      "crate_id": "serde_test",
      "default_features": true,
      "downloads": 22675,
      "features": "",
      "id": 174909,
      "kind": "normal",
      "optional": false,
      "req": "^0.9",
      "target": null,
      "version_id": 47111
    }
  ],
  "meta": {
    "total": 2
  }
}

@LeopoldArkham
Copy link
Contributor

Seems to affect all crates. @sgrif Could this be causing it?

@sgrif
Copy link
Contributor

sgrif commented Mar 8, 2017

👀

@sgrif
Copy link
Contributor

sgrif commented Mar 8, 2017

Ah I see the bug. Working on a fix.

sgrif added a commit to sgrif/crates.io that referenced this issue Mar 8, 2017
@sgrif
Fix reverse_dependencies
6eef3e7 
So in rust-lang#592, I accidentally changed the meaning of the
`reverse_dependencies` query from "select 10 crates where the max
version of that crate depends on this one" to "select 10 crates where
that crate had a version with the same number as this one's max version,
which depended on this crate" which is nonsense and wrong.

There's actually no way that we can truly replicate the old behavior
without breaking pagination or loading all the versions of every crate
which has ever depended on another into memory and doing the limiting
locally... which would defeat the purpose of pagination.

The only real alternative here is to revert rust-lang#592 and go back to updating
the `max_version` column. I still think this approach is the right one,
even with the added complexity to this column, as updating `max_version`
will always be bug-prone unless we can do it in SQL itself. It's too bad
we can't enable arbitrary extensions on heroku PG, as we could actually
create a true semver type that links to the rust crate if we could.

The main difference in behavior between this and the `max_version`
column is that if a crate had *only* prerelease versions, and only some
of those prerelease versions depended on another crate, it's effectively
random whether it would appear in this list or not. It's a very niche
edge case and one that I'm not terribly concerned about.

I'd need to play around with indexes to see if this query could be
optimized further, but the performance should be reasonable, as the
window function will only require a single table scan.

Fixes rust-lang#602.
sgrif added a commit to sgrif/crates.io that referenced this issue Mar 8, 2017
@sgrif
Fix reverse_dependencies
698bb41 
So in rust-lang#592, I accidentally changed the meaning of the
`reverse_dependencies` query from "select 10 crates where the max
version of that crate depends on this one" to "select 10 crates where
that crate had a version with the same number as this one's max version,
which depended on this crate" which is nonsense and wrong.

There's actually no way that we can truly replicate the old behavior
without breaking pagination or loading all the versions of every crate
which has ever depended on another into memory and doing the limiting
locally... which would defeat the purpose of pagination.

The only real alternative here is to revert rust-lang#592 and go back to updating
the `max_version` column. I still think this approach is the right one,
even with the added complexity to this column, as updating `max_version`
will always be bug-prone unless we can do it in SQL itself. It's too bad
we can't enable arbitrary extensions on heroku PG, as we could actually
create a true semver type that links to the rust crate if we could.

The main difference in behavior between this and the `max_version`
column is that if a crate had *only* prerelease versions, and only some
of those prerelease versions depended on another crate, it's effectively
random whether it would appear in this list or not. It's a very niche
edge case and one that I'm not terribly concerned about.

I'd need to play around with indexes to see if this query could be
optimized further, but the performance should be reasonable, as the
window function will only require a single table scan.

Fixes rust-lang#602.
sgrif added a commit to sgrif/crates.io that referenced this issue Mar 8, 2017
@sgrif
Fix reverse_dependencies
a2d93a1 
So in rust-lang#592, I accidentally changed the meaning of the
`reverse_dependencies` query from "select 10 crates where the max
version of that crate depends on this one" to "select 10 crates where
that crate had a version with the same number as this one's max version,
which depended on this crate" which is nonsense and wrong.

There's actually no way that we can truly replicate the old behavior
without breaking pagination or loading all the versions of every crate
which has ever depended on another into memory and doing the limiting
locally... which would defeat the purpose of pagination.

The only real alternative here is to revert rust-lang#592 and go back to updating
the `max_version` column. I still think this approach is the right one,
even with the added complexity to this column, as updating `max_version`
will always be bug-prone unless we can do it in SQL itself. It's too bad
we can't enable arbitrary extensions on heroku PG, as we could actually
create a true semver type that links to the rust crate if we could.

The main difference in behavior between this and the `max_version`
column is that if a crate had *only* prerelease versions, and only some
of those prerelease versions depended on another crate, it's effectively
random whether it would appear in this list or not. It's a very niche
edge case and one that I'm not terribly concerned about.

I'd need to play around with indexes to see if this query could be
optimized further, but the performance should be reasonable, as the
window function will only require a single table scan.

Fixes rust-lang#602.
sgrif added a commit to sgrif/crates.io that referenced this issue Mar 8, 2017
@sgrif
Fix reverse_dependencies
d397345 
So in rust-lang#592, I accidentally changed the meaning of the
`reverse_dependencies` query from "select 10 crates where the max
version of that crate depends on this one" to "select 10 crates where
that crate had a version with the same number as this one's max version,
which depended on this crate" which is nonsense and wrong.

There's actually no way that we can truly replicate the old behavior
without breaking pagination or loading all the versions of every crate
which has ever depended on another into memory and doing the limiting
locally... which would defeat the purpose of pagination.

The only real alternative here is to revert rust-lang#592 and go back to updating
the `max_version` column. I still think this approach is the right one,
even with the added complexity to this column, as updating `max_version`
will always be bug-prone unless we can do it in SQL itself. It's too bad
we can't enable arbitrary extensions on heroku PG, as we could actually
create a true semver type that links to the rust crate if we could.

The main difference in behavior between this and the `max_version`
column is that if a crate had *only* prerelease versions, and only some
of those prerelease versions depended on another crate, it's effectively
random whether it would appear in this list or not. It's a very niche
edge case and one that I'm not terribly concerned about.

I'd need to play around with indexes to see if this query could be
optimized further, but the performance should be reasonable, as the
window function will only require a single table scan.

Fixes rust-lang#602.
@sgrif sgrif mentioned this issue Mar 8, 2017
Merged
@sgrif
Copy link
Contributor

sgrif commented Mar 8, 2017

What? 2 is basically the same as 700, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@sgrif @dtolnay @LeopoldArkham