Skip to content

Commit d7ba94d

Browse files
authored
repeat_row: Make subquery guard aware of negative diffs (#36185)
This is the 4. PR in the `repeat_row` productionization work stream. This resolves #32978 (comment). It was not important originally, but now that we are productionizing `repeat_row`, the original error message would become confusing: it would say that "more than one record produced in subquery", when in fact there was a negative number of rows. I didn't add a test, because I wasn't able to find a query where this error deterministically happens. If I just naively put a `repeat_row(-1)` inside a subquery, then sometimes this error happens, but sometimes a `Distinct` gives the negative accumulation error message.
1 parent ad614d5 commit d7ba94d

4 files changed

Lines changed: 20 additions & 3 deletions

File tree

src/expr/src/relation/func.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use mz_lowertest::MzReflect;
2222
use mz_ore::cast::CastFrom;
2323

2424
use mz_ore::str::separated;
25-
use mz_ore::{soft_assert_eq_no_log, soft_assert_or_log};
25+
use mz_ore::{soft_assert_eq_no_log, soft_assert_or_log, soft_panic_or_log};
2626
use mz_repr::adt::array::ArrayDimension;
2727
use mz_repr::adt::date::Date;
2828
use mz_repr::adt::interval::Interval;
@@ -3560,10 +3560,19 @@ impl TableFunc {
35603560
// We error if the count is not one,
35613561
// and produce no rows if equal to one.
35623562
let count = datums[0].unwrap_int64();
3563-
if count != 1 {
3563+
if count == 1 {
3564+
Ok(Box::new([].into_iter()))
3565+
} else if count > 1 {
35643566
Err(EvalError::MultipleRowsFromSubquery)
3567+
} else if count < 0 {
3568+
Err(EvalError::NegativeRowsFromSubquery)
35653569
} else {
3566-
Ok(Box::new([].into_iter()))
3570+
// This shouldn't happen because this is not an SQL `count` but an MIR `count`,
3571+
// which produces no output on 0 input rows.
3572+
soft_panic_or_log!("subquery counting unexpectedly produced 0");
3573+
Err(EvalError::Internal(
3574+
"subquery counting unexpectedly produced 0".into(),
3575+
))
35673576
}
35683577
}
35693578
TableFunc::Repeat => Ok(Box::new(repeat(datums[0]).into_iter())),

src/expr/src/scalar.proto

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,5 +164,6 @@ message ProtoEvalError {
164164
google.protobuf.Empty key_cannot_be_null = 80;
165165
string invalid_catalog_json = 81;
166166
string redact_error = 82;
167+
google.protobuf.Empty negative_rows_from_subquery = 83;
167168
}
168169
}

src/expr/src/scalar.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2683,6 +2683,7 @@ pub enum EvalError {
26832683
OutOfDomain(DomainLimit, DomainLimit, Box<str>),
26842684
ComplexOutOfRange(Box<str>),
26852685
MultipleRowsFromSubquery,
2686+
NegativeRowsFromSubquery,
26862687
Undefined(Box<str>),
26872688
LikePatternTooLong,
26882689
LikeEscapeTooLong,
@@ -2868,6 +2869,9 @@ impl fmt::Display for EvalError {
28682869
EvalError::MultipleRowsFromSubquery => {
28692870
write!(f, "more than one record produced in subquery")
28702871
}
2872+
EvalError::NegativeRowsFromSubquery => {
2873+
write!(f, "negative number of rows produced in subquery")
2874+
}
28712875
EvalError::Undefined(s) => {
28722876
write!(f, "{} is undefined", s)
28732877
}
@@ -3148,6 +3152,7 @@ impl RustType<ProtoEvalError> for EvalError {
31483152
}),
31493153
EvalError::ComplexOutOfRange(v) => ComplexOutOfRange(v.into_proto()),
31503154
EvalError::MultipleRowsFromSubquery => MultipleRowsFromSubquery(()),
3155+
EvalError::NegativeRowsFromSubquery => NegativeRowsFromSubquery(()),
31513156
EvalError::Undefined(v) => Undefined(v.into_proto()),
31523157
EvalError::LikePatternTooLong => LikePatternTooLong(()),
31533158
EvalError::LikeEscapeTooLong => LikeEscapeTooLong(()),
@@ -3282,6 +3287,7 @@ impl RustType<ProtoEvalError> for EvalError {
32823287
)),
32833288
ComplexOutOfRange(v) => Ok(EvalError::ComplexOutOfRange(v.into())),
32843289
MultipleRowsFromSubquery(()) => Ok(EvalError::MultipleRowsFromSubquery),
3290+
NegativeRowsFromSubquery(()) => Ok(EvalError::NegativeRowsFromSubquery),
32853291
Undefined(v) => Ok(EvalError::Undefined(v.into())),
32863292
LikePatternTooLong(()) => Ok(EvalError::LikePatternTooLong),
32873293
LikeEscapeTooLong(()) => Ok(EvalError::LikeEscapeTooLong),

src/storage-types/src/errors.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,7 @@ mod columnation {
652652
| e @ EvalError::KeyCannotBeNull
653653
| e @ EvalError::UnterminatedLikeEscapeSequence
654654
| e @ EvalError::MultipleRowsFromSubquery
655+
| e @ EvalError::NegativeRowsFromSubquery
655656
| e @ EvalError::LikePatternTooLong
656657
| e @ EvalError::LikeEscapeTooLong
657658
| e @ EvalError::MultidimensionalArrayRemovalNotSupported

0 commit comments

Comments
 (0)