Skip to content

Commit ceb8c82

Browse files
committed
Auto merge of #1846 - integer32llc:integer-sizes-lolsob, r=jtgeibel
Support u64 version parts in SQL function semver_no_prerelease Fixes #1839. So the reverse dependencies API request for crc32fast started failing because the susu crate depends on crc32fast and has [published versions](https://crates.io/crates/susu/versions) like 0.1.20190509190436. In the logs for the request that were responding with 500, I saw a message that said `value "20190409163015" is out of range for type integer` 😰 So I went digging, and after isolating which query was the problem and then which part of the query was the problem, I figured out it was [this call to `to_semver_no_prerelease`](https://github.com/rust-lang/crates.io/blob/12c64bc716a4a7d1fd25f2a658f80cbdcbdcbf08/src/models/krate_reverse_dependencies.sql#L15). That function was defined in [this migration](https://github.com/rust-lang/crates.io/blob/a27c704faa2982ddd75a3dc564da85c0217b950e/migrations/20170308140537_create_to_semver_no_prerelease/up.sql), which defines a type `semver_triple` as PostgreSQL type `int4`, which corresponds to Rust type `i32`. And 20,190,409,163,015 > 2,147,483,647 😰 The SemVer spec [doesn't actually specify](https://semver.org/#does-semver-have-a-size-limit-on-the-version-string) the size of integers that should be used for the version parts. The `semver` crate, that we use in the Rust structures, [uses `u64`](https://github.com/steveklabnik/semver/blob/15189a1d97911abc7e22fb2e7959df12d5d6f462/src/version.rs#L115-L121). Cargo uses the `semver` crate as well, and `cargo build` doesn't work if you give the current crate a version number with one part set to the value of `std::u64::MAX` + 1. [PostgreSQL's integer types](https://www.postgresql.org/docs/9.1/datatype-numeric.html) include an `int8` type, but it's a signed integer, so that wouldn't support values between `i64::MAX` and `u64::MAX` that we need to support. So we have to use the `numeric` type. [Here's a travis build showing the test failing](https://travis-ci.com/rust-lang/crates.io/jobs/237375256#L833)
2 parents 7ce236f + 5b42015 commit ceb8c82

File tree

3 files changed

+64
-0
lines changed

3 files changed

+64
-0
lines changed
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
DROP FUNCTION to_semver_no_prerelease(text);
2+
DROP TYPE semver_triple;
3+
4+
-- Restores the type and function to what they were created as in
5+
-- migrations/20170308140537_create_to_semver_no_prerelease/up.sql
6+
7+
CREATE TYPE semver_triple AS (
8+
major int4,
9+
minor int4,
10+
teeny int4
11+
);
12+
13+
CREATE FUNCTION to_semver_no_prerelease(text) RETURNS semver_triple IMMUTABLE AS $$
14+
SELECT (
15+
split_part($1, '.', 1)::int4,
16+
split_part($1, '.', 2)::int4,
17+
split_part(split_part($1, '+', 1), '.', 3)::int4
18+
)::semver_triple
19+
WHERE strpos($1, '-') = 0
20+
$$ LANGUAGE SQL
21+
;
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
DROP FUNCTION to_semver_no_prerelease(text);
2+
DROP TYPE semver_triple;
3+
4+
CREATE TYPE semver_triple AS (
5+
major numeric,
6+
minor numeric,
7+
teeny numeric
8+
);
9+
10+
CREATE FUNCTION to_semver_no_prerelease(text) RETURNS semver_triple IMMUTABLE AS $$
11+
SELECT (
12+
split_part($1, '.', 1)::numeric,
13+
split_part($1, '.', 2)::numeric,
14+
split_part(split_part($1, '+', 1), '.', 3)::numeric
15+
)::semver_triple
16+
WHERE strpos($1, '-') = 0
17+
$$ LANGUAGE SQL
18+
;

src/tests/krate.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1903,6 +1903,31 @@ fn reverse_dependencies_includes_published_by_user_when_present() {
19031903
);
19041904
}
19051905

1906+
#[test]
1907+
fn reverse_dependencies_query_supports_u64_version_number_parts() {
1908+
let (app, anon, user) = TestApp::init().with_user();
1909+
let user = user.as_model();
1910+
1911+
let large_but_valid_version_number = format!("1.0.{}", std::u64::MAX);
1912+
1913+
app.db(|conn| {
1914+
let c1 = CrateBuilder::new("c1", user.id).expect_build(conn);
1915+
// The crate that depends on c1...
1916+
CrateBuilder::new("c2", user.id)
1917+
// ...has a patch version at the limits of what the semver crate supports
1918+
.version(VersionBuilder::new(&large_but_valid_version_number).dependency(&c1, None))
1919+
.expect_build(conn);
1920+
});
1921+
1922+
let deps = anon.reverse_dependencies("c1");
1923+
assert_eq!(deps.dependencies.len(), 1);
1924+
assert_eq!(deps.meta.total, 1);
1925+
assert_eq!(deps.dependencies[0].crate_id, "c1");
1926+
assert_eq!(deps.versions.len(), 1);
1927+
assert_eq!(deps.versions[0].krate, "c2");
1928+
assert_eq!(deps.versions[0].num, large_but_valid_version_number);
1929+
}
1930+
19061931
#[test]
19071932
fn author_license_and_description_required() {
19081933
let (_, _, _, token) = TestApp::init().with_token();

0 commit comments

Comments
 (0)