From 2ecf4ddbe8b0c0a3726093480ae07188d720f36d Mon Sep 17 00:00:00 2001 From: Dennis Felsing Date: Thu, 2 Jul 2026 11:54:49 +0000 Subject: [PATCH] repr/strconv: accept full u32 range when parsing oid from text `parse_oid` parsed OIDs as `i32` and reinterpreted them as `u32`, which rejected valid OIDs in `2147483648..=4294967295`. For example `'4294967295'::oid` errored even though `4294967295::oid` succeeded, and `COPY` into such a column failed with "invalid input syntax for type oid". PostgreSQL accepts the union of the `u32` and `i32` ranges (negatives are reinterpreted), so parse the full `u32` range directly and fall back to `i32` for negative inputs. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/repr/src/strconv.rs | 35 +++++++++++++++++++++++--- src/storage-types/src/sources/casts.rs | 12 ++++++++- test/sqllogictest/oid.slt | 29 +++++++++++++++++++++ 3 files changed, 71 insertions(+), 5 deletions(-) diff --git a/src/repr/src/strconv.rs b/src/repr/src/strconv.rs index d9ca650845907..88438be2be41c 100644 --- a/src/repr/src/strconv.rs +++ b/src/repr/src/strconv.rs @@ -236,13 +236,19 @@ where /// Parses an OID from `s`. pub fn parse_oid(s: &str) -> Result { - // 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::() { + 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)) @@ -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()); + } } diff --git a/src/storage-types/src/sources/casts.rs b/src/storage-types/src/sources/casts.rs index f0cb93a0dc2dc..9d86a903f4cdd 100644 --- a/src/storage-types/src/sources/casts.rs +++ b/src/storage-types/src/sources/casts.rs @@ -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: //! @@ -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", ""], ); } diff --git a/test/sqllogictest/oid.slt b/test/sqllogictest/oid.slt index 6e34708a2b546..560be9339d735 100644 --- a/test/sqllogictest/oid.slt +++ b/test/sqllogictest/oid.slt @@ -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 ----