From 269b0c50166b7efb3ddc78ee8e571ed59b8e1aa9 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Sun, 10 Jul 2022 20:44:54 -0600 Subject: [PATCH] fix regression in null tracking by improving tracking of cursor empty/full state --- sqlx-core/src/sqlite/connection/explain.rs | 115 ++++++++++++++++----- 1 file changed, 87 insertions(+), 28 deletions(-) diff --git a/sqlx-core/src/sqlite/connection/explain.rs b/sqlx-core/src/sqlite/connection/explain.rs index 101ca56a2b..5fe53bae2b 100644 --- a/sqlx-core/src/sqlite/connection/explain.rs +++ b/sqlx-core/src/sqlite/connection/explain.rs @@ -67,6 +67,7 @@ const OP_SEEK_LT: &str = "SeekLT"; const OP_SEEK_ROW_ID: &str = "SeekRowId"; const OP_SEEK_SCAN: &str = "SeekScan"; const OP_SEQUENCE_TEST: &str = "SequenceTest"; +const OP_SORT: &str = "Sort"; const OP_SORTER_NEXT: &str = "SorterNext"; const OP_SORTER_SORT: &str = "SorterSort"; const OP_V_FILTER: &str = "VFilter"; @@ -180,29 +181,36 @@ impl RegDataType { #[derive(Debug, Clone, Eq, PartialEq)] enum CursorDataType { - Normal(HashMap), + Normal { + cols: HashMap, + is_empty: Option, + }, Pseudo(i64), } impl CursorDataType { - fn from_sparse_record(record: &HashMap) -> Self { - Self::Normal( - record + fn from_sparse_record(record: &HashMap, is_empty: Option) -> Self { + Self::Normal { + cols: record .iter() .map(|(&colnum, &datatype)| (colnum, datatype)) .collect(), - ) + is_empty, + } } - fn from_dense_record(record: &Vec) -> Self { - Self::Normal((0..).zip(record.iter().copied()).collect()) + fn from_dense_record(record: &Vec, is_empty: Option) -> Self { + Self::Normal { + cols: (0..).zip(record.iter().copied()).collect(), + is_empty, + } } fn map_to_dense_record(&self, registers: &HashMap) -> Vec { match self { - Self::Normal(record) => { - let mut rowdata = vec![ColumnType::default(); record.len()]; - for (idx, col) in record.iter() { + Self::Normal { cols, .. } => { + let mut rowdata = vec![ColumnType::default(); cols.len()]; + for (idx, col) in cols.iter() { rowdata[*idx as usize] = col.clone(); } rowdata @@ -219,13 +227,20 @@ impl CursorDataType { registers: &HashMap, ) -> HashMap { match self { - Self::Normal(c) => c.clone(), + Self::Normal { cols, .. } => cols.clone(), Self::Pseudo(i) => match registers.get(i) { Some(RegDataType::Record(r)) => (0..).zip(r.iter().copied()).collect(), _ => HashMap::new(), }, } } + + fn is_empty(&self) -> Option { + match self { + Self::Normal { is_empty, .. } => *is_empty, + Self::Pseudo(_) => Some(false), //pseudo cursors have exactly one row + } + } } #[allow(clippy::wildcard_in_or_patterns)] @@ -379,11 +394,11 @@ pub(super) fn explain( | OP_GE | OP_GO_SUB | OP_GT | OP_IDX_GE | OP_IDX_GT | OP_IDX_LE | OP_IDX_LT | OP_IF | OP_IF_NO_HOPE | OP_IF_NOT | OP_IF_NOT_OPEN | OP_IF_NOT_ZERO | OP_IF_NULL_ROW | OP_IF_POS | OP_IF_SMALLER | OP_INCR_VACUUM | OP_IS_NULL - | OP_IS_NULL_OR_TYPE | OP_LE | OP_LAST | OP_LT | OP_MUST_BE_INT | OP_NE - | OP_NEXT | OP_NO_CONFLICT | OP_NOT_EXISTS | OP_NOT_NULL | OP_ONCE | OP_PREV - | OP_PROGRAM | OP_ROW_SET_READ | OP_ROW_SET_TEST | OP_SEEK_GE | OP_SEEK_GT - | OP_SEEK_LE | OP_SEEK_LT | OP_SEEK_ROW_ID | OP_SEEK_SCAN | OP_SEQUENCE_TEST - | OP_SORTER_NEXT | OP_SORTER_SORT | OP_V_FILTER | OP_V_NEXT | OP_REWIND => { + | OP_IS_NULL_OR_TYPE | OP_LE | OP_LT | OP_MUST_BE_INT | OP_NE | OP_NEXT + | OP_NO_CONFLICT | OP_NOT_EXISTS | OP_NOT_NULL | OP_ONCE | OP_PREV | OP_PROGRAM + | OP_ROW_SET_READ | OP_ROW_SET_TEST | OP_SEEK_GE | OP_SEEK_GT | OP_SEEK_LE + | OP_SEEK_LT | OP_SEEK_ROW_ID | OP_SEEK_SCAN | OP_SEQUENCE_TEST + | OP_SORTER_NEXT | OP_V_FILTER | OP_V_NEXT => { // goto or next instruction (depending on actual values) state.visited[state.program_i] = true; @@ -395,6 +410,35 @@ pub(super) fn explain( continue; } + OP_REWIND | OP_LAST | OP_SORT | OP_SORTER_SORT => { + // goto if cursor p1 is empty, else next instruction + state.visited[state.program_i] = true; + + if let Some(cursor) = state.p.get(&p1) { + if matches!(cursor.is_empty(), None | Some(true)) { + //only take this branch if the cursor is empty + + let mut branch_state = state.clone(); + branch_state.program_i = p2 as usize; + + if let Some(CursorDataType::Normal { is_empty, .. }) = + branch_state.p.get_mut(&p1) + { + *is_empty = Some(true); + } + states.push(branch_state); + } + + if matches!(cursor.is_empty(), None | Some(false)) { + //only take this branch if the cursor is non-empty + state.program_i += 1; + continue; + } + } + + break; + } + OP_INIT_COROUTINE => { // goto or next instruction (depending on actual values) state.visited[state.program_i] = true; @@ -530,9 +574,12 @@ pub(super) fn explain( OP_INSERT | OP_IDX_INSERT => { if let Some(RegDataType::Record(record)) = state.r.get(&p2) { - if let Some(CursorDataType::Normal(row)) = state.p.get_mut(&p1) { + if let Some(CursorDataType::Normal { cols, is_empty }) = + state.p.get_mut(&p1) + { // Insert the record into wherever pointer p1 is - *row = (0..).zip(record.iter().copied()).collect(); + *cols = (0..).zip(record.iter().copied()).collect(); + *is_empty = Some(false); } } //Noop if the register p2 isn't a record, or if pointer p1 does not exist @@ -548,16 +595,24 @@ pub(super) fn explain( if let Some(columns) = root_block_cols.get(&p2) { state .p - .insert(p1, CursorDataType::from_sparse_record(columns)); + .insert(p1, CursorDataType::from_sparse_record(columns, None)); } else { - state - .p - .insert(p1, CursorDataType::Normal(HashMap::with_capacity(6))); + state.p.insert( + p1, + CursorDataType::Normal { + cols: HashMap::with_capacity(6), + is_empty: None, + }, + ); } } else { - state - .p - .insert(p1, CursorDataType::Normal(HashMap::with_capacity(6))); + state.p.insert( + p1, + CursorDataType::Normal { + cols: HashMap::with_capacity(6), + is_empty: None, + }, + ); } } @@ -565,7 +620,10 @@ pub(super) fn explain( //Create a new pointer which is referenced by p1 state.p.insert( p1, - CursorDataType::from_dense_record(&vec![ColumnType::null(); p2 as usize]), + CursorDataType::from_dense_record( + &vec![ColumnType::null(); p2 as usize], + Some(true), + ), ); } @@ -594,8 +652,9 @@ pub(super) fn explain( OP_NULL_ROW => { // all columns in cursor X are potentially nullable - if let Some(CursorDataType::Normal(ref mut cursor)) = state.p.get_mut(&p1) { - for ref mut col in cursor.values_mut() { + if let Some(CursorDataType::Normal { ref mut cols, .. }) = state.p.get_mut(&p1) + { + for ref mut col in cols.values_mut() { col.nullable = Some(true); } }