From 67669d034b60bde15d47d64608bde0e31fb60325 Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Sun, 20 Sep 2026 07:03:25 +0000 Subject: [PATCH 1/5] fix: handle `take` bounds near the limits of i64 Combining nested `take` ranges added the bounds without checking for overflow, and `LIMIT` rendered any value at or above 2^32 with sqlparser's `long` flag, appending an `L` that no dialect parses. --- prqlc/prqlc/src/sql/gen_expr.rs | 114 ++++++++++++++++++++++++++++++-- 1 file changed, 109 insertions(+), 5 deletions(-) diff --git a/prqlc/prqlc/src/sql/gen_expr.rs b/prqlc/prqlc/src/sql/gen_expr.rs index e5eea8510998..257f273e9bb1 100644 --- a/prqlc/prqlc/src/sql/gen_expr.rs +++ b/prqlc/prqlc/src/sql/gen_expr.rs @@ -770,11 +770,34 @@ pub(super) fn translate_sstring( pub(super) fn range_of_ranges(ranges: Vec>) -> Result> { let mut current = Range::default(); for range in ranges { - let mut range = try_range_into_int(range)?; + // Kept before the conversion below, which consumes the bounds, so an + // overflow can be reported against the `take` that caused it. + let span = range + .start + .as_ref() + .or(range.end.as_ref()) + .and_then(|bound| bound.span); + let range = try_range_into_int(range)?; // b = b + a.start -1 (take care of 1-based index!) - range.start = range.start.or_map(current.start, |a, b| a + b - 1); - range.end = range.end.map(|b| current.start.unwrap_or(1) + b - 1); + // + // Both bounds are checked: a `take` nested inside another one shifts + // its bounds by the outer range's start, and two bounds near + // `i64::MAX` sum past it. Overflowing here panics in a debug build and, + // because `[profile.release]` leaves `overflow-checks` off, silently + // wraps to a nonsense `LIMIT`/`OFFSET` in a release one — so it is + // reported as a compile error instead. + let mut range = Range { + start: match (range.start, current.start) { + (Some(a), Some(b)) => Some(shift_bound(a, b, span)?), + (a, None) => a, + (None, b) => b, + }, + end: range + .end + .map(|b| shift_bound(current.start.unwrap_or(1), b, span)) + .transpose()?, + }; // b.end = min(a.end, b.end) range.end = current.end.or_map(range.end, i64::min); @@ -792,6 +815,23 @@ pub(super) fn range_of_ranges(ranges: Vec>) -> Result Ok(current) } +/// Shifts a 1-based range bound by the start of the range it is nested in, +/// i.e. `a + b - 1`. +/// +/// Subtracting before adding keeps the intermediate in range: lowering has +/// already rejected bounds below 1, so `a - 1` cannot underflow, and the sum +/// then overflows only when `a + b - 1` genuinely exceeds `i64::MAX`. Adding +/// first would reject `take ..9223372036854775807`, whose result is +/// representable. +fn shift_bound(a: i64, b: i64, span: Option) -> Result { + a.checked_sub(1) + .and_then(|a| a.checked_add(b)) + .ok_or_else(|| { + Error::new_simple("`take` bounds are too large to combine with the enclosing `take`") + .with_span(span) + }) +} + fn unpack_as_int_literal(bound: rq::Expr) -> Result { Some(bound.kind) .and_then(|x| x.into_literal().ok()) @@ -807,7 +847,11 @@ fn try_range_into_int(range: Range) -> Result> { } pub(super) fn expr_of_i64(number: i64) -> sql_ast::Expr { - sql_ast::Expr::Value(Value::Number(number.to_string(), number.leading_zeros() < 32).into()) + // The second field is sqlparser's `long` flag, which renders an `L` suffix + // — not a width hint. Every other number this module emits passes `false`, + // and `fetch_of_i64` renders the same value through `translate_literal`, + // so a dialect using FETCH already got it right where LIMIT did not. + sql_ast::Expr::Value(Value::Number(number.to_string(), false).into()) } pub(super) fn fetch_of_i64(take: i64, ctx: &mut Context) -> Fetch { @@ -1242,7 +1286,7 @@ impl From for ExprOrSource { #[cfg(test)] mod test { - use insta::assert_yaml_snapshot; + use insta::{assert_snapshot, assert_yaml_snapshot}; use super::*; @@ -1321,4 +1365,64 @@ mod test { Ok(()) } + + /// The end bound is shifted by the enclosing range's start, so it + /// overflows on its own inputs — covered separately from the start bound. + #[test] + fn test_range_of_ranges_overflow_end() { + let query = "from a | take 2.. | take ..9223372036854775807"; + assert_snapshot!(crate::tests::compile(query).unwrap_err(), @" + Error: + ╭─[ :1:28 ] + │ + 1 │ from a | take 2.. | take ..9223372036854775807 + │ ─────────┬───────── + │ ╰─────────── `take` bounds are too large to combine with the enclosing `take` + ───╯ + "); + } + + /// A `LIMIT` above `u32::MAX` must render as a plain integer; sqlparser's + /// `long` flag would append an `L` that no dialect parses. + #[test] + fn test_large_limit_has_no_long_suffix() { + let query = "from a | take 5000000000"; + assert_snapshot!(crate::tests::compile(query).unwrap(), @" + SELECT + * + FROM + a + LIMIT + 5000000000 + "); + } + + /// An end bound at `i64::MAX` in the outermost `take` is representable — + /// only the intermediate of a naive `a + b - 1` would overflow. + #[test] + fn test_range_of_ranges_max_end_is_not_an_overflow() { + let query = "from a | take ..9223372036854775807"; + assert_snapshot!(crate::tests::compile(query).unwrap(), @" + SELECT + * + FROM + a + LIMIT + 9223372036854775807 + "); + } + + #[test] + fn test_range_of_ranges_overflow() { + let query = "from a | take 9223372036854775807.. | take 2.."; + assert_snapshot!(crate::tests::compile(query).unwrap_err(), @" + Error: + ╭─[ :1:44 ] + │ + 1 │ from a | take 9223372036854775807.. | take 2.. + │ ┬ + │ ╰── `take` bounds are too large to combine with the enclosing `take` + ───╯ + "); + } } From 3099408fbe4eb469b13e7b856eff8ea6fed9a435 Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Sun, 20 Sep 2026 07:04:11 +0000 Subject: [PATCH 2/5] docs: add changelog entry for #6347 --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 646ac6f811d3..179f18517de1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,14 @@ **Fixes**: +- `take` bounds at the extremes of `i64` no longer produce a panic or invalid + SQL. Combining nested ranges (`take 9223372036854775807.. | take 2..`) + overflowed while re-basing the inner bounds, panicking in a debug build and + silently emitting a wrapped `OFFSET` in a release one; it now reports a + compile error. Separately, a `LIMIT` at or above 2^32 was rendered with + sqlparser's `long` flag, so `take 5000000000` compiled to + `LIMIT 5000000000 L`, which no dialect parses. (@prql-bot, #6347) + - `prqlc experimental doc --format=html` now escapes HTML in the page it generates. A doc comment containing `<`, `>` or `&` — as ordinary prose such as `a < b` does — was interpolated verbatim, so a browser parsed it as markup From 1b68c776271ad5aa71aabc2f067f005c278ab810 Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Sun, 20 Sep 2026 07:16:02 +0000 Subject: [PATCH 3/5] fix: clamp an overflowing take end bound, and span the offending bound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An end bound that overflows while being shifted onto the enclosing range is intersected with that range's end on the next line, and the enclosing end is necessarily the smaller of the two once the shift has run past i64::MAX — so `take 2..10 | take ..9223372036854775807` has the same answer as `take 2..10 | take ..100`. Fall back to the enclosing end in that case, and error only when the enclosing range is unbounded. Give each bound its own span, so a `take` whose end overflowed no longer underlines its start bound. --- prqlc/prqlc/src/sql/gen_expr.rs | 72 +++++++++++++++++++++++++-------- 1 file changed, 55 insertions(+), 17 deletions(-) diff --git a/prqlc/prqlc/src/sql/gen_expr.rs b/prqlc/prqlc/src/sql/gen_expr.rs index 257f273e9bb1..d07c92d3ba90 100644 --- a/prqlc/prqlc/src/sql/gen_expr.rs +++ b/prqlc/prqlc/src/sql/gen_expr.rs @@ -771,12 +771,9 @@ pub(super) fn range_of_ranges(ranges: Vec>) -> Result let mut current = Range::default(); for range in ranges { // Kept before the conversion below, which consumes the bounds, so an - // overflow can be reported against the `take` that caused it. - let span = range - .start - .as_ref() - .or(range.end.as_ref()) - .and_then(|bound| bound.span); + // overflow can be reported against the bound that caused it. + let start_span = range.start.as_ref().and_then(|bound| bound.span); + let end_span = range.end.as_ref().and_then(|bound| bound.span); let range = try_range_into_int(range)?; // b = b + a.start -1 (take care of 1-based index!) @@ -785,19 +782,28 @@ pub(super) fn range_of_ranges(ranges: Vec>) -> Result // its bounds by the outer range's start, and two bounds near // `i64::MAX` sum past it. Overflowing here panics in a debug build and, // because `[profile.release]` leaves `overflow-checks` off, silently - // wraps to a nonsense `LIMIT`/`OFFSET` in a release one — so it is - // reported as a compile error instead. - let mut range = Range { - start: match (range.start, current.start) { - (Some(a), Some(b)) => Some(shift_bound(a, b, span)?), - (a, None) => a, - (None, b) => b, + // wraps to a nonsense `LIMIT`/`OFFSET` in a release one — so an + // overflow that the intersection below cannot discard is reported as a + // compile error instead. + let start = match (range.start, current.start) { + (Some(a), Some(b)) => Some(shift_bound(a, b, start_span)?), + (a, None) => a, + (None, b) => b, + }; + let end = match range.end { + Some(b) => match shift_bound(current.start.unwrap_or(1), b, end_span) { + Ok(end) => Some(end), + // The intersection below clamps the end to the enclosing one, + // which is necessarily the smaller of the two once the shifted + // bound has run past `i64::MAX`. So an overflow the + // intersection would discard is not an error; only an + // unbounded enclosing range leaves it with no representable + // answer. + Err(err) => Some(current.end.ok_or(err)?), }, - end: range - .end - .map(|b| shift_bound(current.start.unwrap_or(1), b, span)) - .transpose()?, + None => None, }; + let mut range = Range { start, end }; // b.end = min(a.end, b.end) range.end = current.end.or_map(range.end, i64::min); @@ -1382,6 +1388,38 @@ mod test { "); } + /// An end bound that overflows while being shifted is still bounded by the + /// enclosing range's end, which the intersection picks — so this is an + /// ordinary query, not an overflow. + #[test] + fn test_range_of_ranges_overflowing_end_is_clamped_by_the_enclosing_end() { + let query = "from a | take 2..10 | take ..9223372036854775807"; + assert_snapshot!(crate::tests::compile(query).unwrap(), @" + SELECT + * + FROM + a + LIMIT + 9 OFFSET 1 + "); + } + + /// The error points at the bound that overflowed, not at whichever bound + /// of the same `take` happens to come first. + #[test] + fn test_range_of_ranges_overflow_points_at_the_offending_bound() { + let query = "from a | take 2.. | take 3..9223372036854775807"; + assert_snapshot!(crate::tests::compile(query).unwrap_err(), @" + Error: + ╭─[ :1:29 ] + │ + 1 │ from a | take 2.. | take 3..9223372036854775807 + │ ─────────┬───────── + │ ╰─────────── `take` bounds are too large to combine with the enclosing `take` + ───╯ + "); + } + /// A `LIMIT` above `u32::MAX` must render as a plain integer; sqlparser's /// `long` flag would append an `L` that no dialect parses. #[test] From da05065d66fb0f6ff2cd16ac7cb3eadca2aabd81 Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Sun, 20 Sep 2026 07:31:15 +0000 Subject: [PATCH 4/5] fix: treat an overflowing take start as an empty range when the enclosing end is bounded --- prqlc/prqlc/src/sql/gen_expr.rs | 44 +++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/prqlc/prqlc/src/sql/gen_expr.rs b/prqlc/prqlc/src/sql/gen_expr.rs index d07c92d3ba90..a5e68fe139de 100644 --- a/prqlc/prqlc/src/sql/gen_expr.rs +++ b/prqlc/prqlc/src/sql/gen_expr.rs @@ -783,10 +783,19 @@ pub(super) fn range_of_ranges(ranges: Vec>) -> Result // `i64::MAX` sum past it. Overflowing here panics in a debug build and, // because `[profile.release]` leaves `overflow-checks` off, silently // wraps to a nonsense `LIMIT`/`OFFSET` in a release one — so an - // overflow that the intersection below cannot discard is reported as a - // compile error instead. + // overflow that the intersection and emptiness check below cannot + // discard is reported as a compile error instead. let start = match (range.start, current.start) { - (Some(a), Some(b)) => Some(shift_bound(a, b, start_span)?), + (Some(a), Some(b)) => match shift_bound(a, b, start_span) { + Ok(start) => Some(start), + // A start past `i64::MAX` is past every representable end, so + // a bounded enclosing range selects nothing — the same result + // the emptiness check below reaches for `take 2..3 | take 5..`. + // Only an unbounded enclosing end leaves no representable + // answer. + Err(err) if current.end.is_none() => return Err(err), + Err(_) => return Ok(empty_range()), + }, (a, None) => a, (None, b) => b, }; @@ -812,15 +821,20 @@ pub(super) fn range_of_ranges(ranges: Vec>) -> Result if let Some((s, e)) = current.start.zip(current.end) { if e < s { - return Ok(Range { - start: None, - end: Some(0), - }); + return Ok(empty_range()); } } Ok(current) } +/// The range that selects no rows. +fn empty_range() -> Range { + Range { + start: None, + end: Some(0), + } +} + /// Shifts a 1-based range bound by the start of the range it is nested in, /// i.e. `a + b - 1`. /// @@ -1450,6 +1464,22 @@ mod test { "); } + /// A start bound that overflows while being shifted has run past every + /// representable end, so a bounded enclosing range selects nothing — the + /// same result `take 2..3 | take 5..` reaches without overflowing. + #[test] + fn test_range_of_ranges_overflowing_start_is_empty_when_enclosing_end_is_bounded() { + let query = "from a | take 9223372036854775807..9223372036854775807 | take 2.."; + assert_snapshot!(crate::tests::compile(query).unwrap(), @" + SELECT + * + FROM + a + LIMIT + 0 + "); + } + #[test] fn test_range_of_ranges_overflow() { let query = "from a | take 9223372036854775807.. | take 2.."; From fd42c086bbbd1bb2b1cc82f42ac3023424992ced Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Sun, 20 Sep 2026 07:47:30 +0000 Subject: [PATCH 5/5] refine: name shift_bound's parameters and pass the bound first at both call sites The doc justifies subtracting from the first parameter with lowering having rejected bounds below 1, but the end call site passed the enclosing start there instead. The sum is commutative so nothing changes at runtime; this makes the call sites match the argument the doc gives. --- prqlc/prqlc/src/sql/gen_expr.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/prqlc/prqlc/src/sql/gen_expr.rs b/prqlc/prqlc/src/sql/gen_expr.rs index a5e68fe139de..23cb575b550f 100644 --- a/prqlc/prqlc/src/sql/gen_expr.rs +++ b/prqlc/prqlc/src/sql/gen_expr.rs @@ -800,7 +800,7 @@ pub(super) fn range_of_ranges(ranges: Vec>) -> Result (None, b) => b, }; let end = match range.end { - Some(b) => match shift_bound(current.start.unwrap_or(1), b, end_span) { + Some(b) => match shift_bound(b, current.start.unwrap_or(1), end_span) { Ok(end) => Some(end), // The intersection below clamps the end to the enclosing one, // which is necessarily the smaller of the two once the shifted @@ -836,16 +836,17 @@ fn empty_range() -> Range { } /// Shifts a 1-based range bound by the start of the range it is nested in, -/// i.e. `a + b - 1`. +/// i.e. `bound + enclosing_start - 1`. /// /// Subtracting before adding keeps the intermediate in range: lowering has -/// already rejected bounds below 1, so `a - 1` cannot underflow, and the sum -/// then overflows only when `a + b - 1` genuinely exceeds `i64::MAX`. Adding -/// first would reject `take ..9223372036854775807`, whose result is +/// already rejected bounds below 1, so `bound - 1` cannot underflow, and the +/// sum then overflows only when the result genuinely exceeds `i64::MAX`. +/// Adding first would reject `take ..9223372036854775807`, whose result is /// representable. -fn shift_bound(a: i64, b: i64, span: Option) -> Result { - a.checked_sub(1) - .and_then(|a| a.checked_add(b)) +fn shift_bound(bound: i64, enclosing_start: i64, span: Option) -> Result { + bound + .checked_sub(1) + .and_then(|shifted| shifted.checked_add(enclosing_start)) .ok_or_else(|| { Error::new_simple("`take` bounds are too large to combine with the enclosing `take`") .with_span(span)