Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 31 additions & 4 deletions src/repr/src/strconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,13 +236,19 @@ where

/// Parses an OID from `s`.
pub fn parse_oid(s: &str) -> Result<u32, ParseError> {
// For historical reasons in PostgreSQL, OIDs are parsed as `i32`s and then
// reinterpreted as `u32`s.
// For historical reasons, PostgreSQL accepts OID inputs whose value fits in
// the range of either `u32` or `i32`. The full `u32` range is accepted
// directly, while values given with a minus sign are parsed as `i32` and
// reinterpreted as `u32` (e.g. `-1` becomes `4294967295`). Anything outside
// both ranges is rejected.
//
// Do not use this as a model for behavior in other contexts. OIDs should
// not in general be thought of as freely convertible from `i32`s.
let oid: i32 = s
.trim()
let trimmed = s.trim();
if let Ok(oid) = trimmed.parse::<u32>() {
return Ok(oid);
}
let oid: i32 = trimmed
.parse()
.map_err(|e| ParseError::invalid_input_syntax("oid", s).with_details(e))?;
Ok(u32::reinterpret_cast(oid))
Expand Down Expand Up @@ -2198,4 +2204,25 @@ mod tests {
let s = format!("{}{}", "x".repeat(62), "א");
assert_eq!("x".repeat(62), parse_pg_legacy_name(&s));
}

#[mz_ore::test]
fn test_parse_oid() {
// The full u32 range is accepted, matching PostgreSQL.
assert_eq!(parse_oid("0").unwrap(), 0);
assert_eq!(parse_oid("2147483647").unwrap(), 2147483647);
assert_eq!(parse_oid("2147483648").unwrap(), 2147483648);
assert_eq!(parse_oid("4294967295").unwrap(), 4294967295);

// Negative values in the i32 range are reinterpreted as u32.
assert_eq!(parse_oid("-1").unwrap(), 4294967295);
assert_eq!(parse_oid("-2147483648").unwrap(), 2147483648);

// Surrounding whitespace is ignored.
assert_eq!(parse_oid(" 42 ").unwrap(), 42);

// Values outside both the u32 and i32 ranges are rejected.
assert!(parse_oid("4294967296").is_err());
assert!(parse_oid("-2147483649").is_err());
assert!(parse_oid("nope").is_err());
}
}
12 changes: 11 additions & 1 deletion src/storage-types/src/sources/casts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@
//! to error variants, error messages, or output types are **breaking changes**
//! for storage and require a migration.
//!
//! NOTE: `CastStringToOid` is a deliberate exception. `strconv::parse_oid` was
//! widened to accept the full `u32` range, so text in `2147483648..=4294967295`
//! now casts to a value instead of erroring. A source that ingested such a value
//! as an error under an older release and then retracts it after upgrading will
//! leave that error unretracted. This was accepted without a migration because
//! ingesting `oid` columns is rare, and an errored source is recreated (which
//! resnapshots with the corrected cast) rather than kept around across upgrades.
//!
//! The eval implementations delegate to `mz_repr::strconv::parse_*` functions,
//! which in turn depend on these external crates:
//!
Expand Down Expand Up @@ -1128,11 +1136,13 @@ mod tests {
#[mz_ore::test]
fn parity_oid() {
use mz_expr::func::CastStringToOid;
// Cover the full u32 range that both casts accept, including values
// above i32::MAX and the negative reinterpretation.
assert_parity(
"Oid",
CastFunc::CastStringToOid,
UnaryFunc::CastStringToOid(CastStringToOid),
&["42", "0", "bad", ""],
&["42", "0", "2147483648", "4294967295", "-1", "bad", ""],
);
}

Expand Down
29 changes: 29 additions & 0 deletions test/sqllogictest/oid.slt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,35 @@ SELECT 4294967296::oid
query error "-1" OID out of range
SELECT (-1)::int8::oid

# Regression: the text -> oid parser must accept the full u32 range, not just
# the i32 range. PostgreSQL accepts the union of the u32 and i32 ranges (the
# latter reinterpreted), so 2147483648..4294967295 must parse.
query T
SELECT '4294967295'::oid
----
4294967295

query T
SELECT '2147483648'::oid
----
2147483648

query T
SELECT '-1'::oid
----
4294967295

query T
SELECT '-2147483648'::oid
----
2147483648

query error invalid input syntax for type oid
SELECT '4294967296'::oid

query error invalid input syntax for type oid
SELECT '-2147483649'::oid

query B
SELECT 1::oid = 2::oid
----
Expand Down
Loading