From 73943e6aae274d6e3ef3319faf7fd26df8e40093 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Sat, 26 Nov 2022 11:49:08 -0600 Subject: [PATCH 01/26] add failing test for nested orderby --- tests/sqlite/describe.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/sqlite/describe.rs b/tests/sqlite/describe.rs index ffd088badc..224cb6fa52 100644 --- a/tests/sqlite/describe.rs +++ b/tests/sqlite/describe.rs @@ -469,3 +469,17 @@ async fn it_describes_union() -> anyhow::Result<()> { Ok(()) } + +//documents a failure originally found through property testing +#[sqlx_macros::test] +async fn it_describes_nested_ordered() -> anyhow::Result<()> { + let mut conn = new::().await?; + let info = conn + .describe("SELECT true FROM (SELECT true) a ORDER BY true") + .await?; + + assert_eq!(info.column(0).type_info().name(), "INTEGER"); + assert_eq!(info.nullable(0), Some(false)); + + Ok(()) +} From a552fd8a3778425fc6d908f696a82e0fa738ba39 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Fri, 28 Oct 2022 13:52:29 -0600 Subject: [PATCH 02/26] log query paths which were abandoned due to invalid state or looping. Allow instructions to be executed a small number of times to fix nested order by query --- sqlx-sqlite/src/connection/explain.rs | 87 +++++++++++++++++++++------ 1 file changed, 68 insertions(+), 19 deletions(-) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index 850e87f652..5e2b71f8d5 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -123,6 +123,8 @@ const OP_CONCAT: &str = "Concat"; const OP_RESULT_ROW: &str = "ResultRow"; const OP_HALT: &str = "Halt"; +const MAX_LOOP_COUNT: u8 = 2; + #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] struct ColumnType { pub datatype: DataType, @@ -331,7 +333,9 @@ fn root_block_columns( #[derive(Debug, Clone, PartialEq)] struct QueryState { - pub visited: Vec, + // The number of times each instruction has been visited + pub visited: Vec, + // A log of the order of execution of each instruction pub history: Vec, // Registers pub r: HashMap, @@ -412,7 +416,7 @@ pub(super) fn explain( crate::logger::QueryPlanLogger::new(query, &program, conn.log_settings.clone()); let mut states = vec![QueryState { - visited: vec![false; program_size], + visited: vec![0; program_size], history: Vec::new(), r: HashMap::with_capacity(6), p: HashMap::with_capacity(6), @@ -426,25 +430,32 @@ pub(super) fn explain( while let Some(mut state) = states.pop() { while state.program_i < program_size { - if state.visited[state.program_i] { - state.program_i += 1; + let (_, ref opcode, p1, p2, p3, ref p4) = program[state.program_i]; + state.history.push(state.program_i); + + if state.visited[state.program_i] > MAX_LOOP_COUNT { + if logger.log_enabled() { + let program_history: Vec<&(i64, String, i64, i64, i64, Vec)> = + state.history.iter().map(|i| &program[*i]).collect(); + logger.add_result((program_history, None)); + } + //avoid (infinite) loops by breaking if we ever hit the same instruction twice break; } - let (_, ref opcode, p1, p2, p3, ref p4) = program[state.program_i]; - state.history.push(state.program_i); + + state.visited[state.program_i] += 1; match &**opcode { OP_INIT => { // start at - state.visited[state.program_i] = true; state.program_i = p2 as usize; continue; } OP_GOTO => { // goto - state.visited[state.program_i] = true; + state.program_i = p2 as usize; continue; } @@ -459,7 +470,6 @@ pub(super) fn explain( | 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; let mut branch_state = state.clone(); branch_state.program_i = p2 as usize; @@ -469,14 +479,12 @@ pub(super) fn explain( visited_branch_state.insert(bs_hash); states.push(branch_state); } - state.program_i += 1; 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)) { @@ -500,12 +508,18 @@ pub(super) fn explain( } } + if logger.log_enabled() { + let program_history: Vec<&(i64, String, i64, i64, i64, Vec)> = + state.history.iter().map(|i| &program[*i]).collect(); + logger.add_result((program_history, None)); + } + break; } OP_INIT_COROUTINE => { // goto or next instruction (depending on actual values) - state.visited[state.program_i] = true; + state.r.insert(p1, RegDataType::Int(p3)); if p2 != 0 { @@ -518,7 +532,7 @@ pub(super) fn explain( OP_END_COROUTINE => { // jump to p2 of the yield instruction pointed at by register p1 - state.visited[state.program_i] = true; + if let Some(RegDataType::Int(yield_i)) = state.r.get(&p1) { if let Some((_, yield_op, _, yield_p2, _, _)) = program.get(*yield_i as usize) @@ -528,31 +542,58 @@ pub(super) fn explain( state.r.remove(&p1); continue; } else { + if logger.log_enabled() { + let program_history: Vec<&( + i64, + String, + i64, + i64, + i64, + Vec, + )> = state.history.iter().map(|i| &program[*i]).collect(); + logger.add_result((program_history, None)); + } + break; } } else { + if logger.log_enabled() { + let program_history: Vec<&(i64, String, i64, i64, i64, Vec)> = + state.history.iter().map(|i| &program[*i]).collect(); + logger.add_result((program_history, None)); + } break; } } else { + if logger.log_enabled() { + let program_history: Vec<&(i64, String, i64, i64, i64, Vec)> = + state.history.iter().map(|i| &program[*i]).collect(); + logger.add_result((program_history, None)); + } break; } } OP_RETURN => { // jump to the instruction after the instruction pointed at by register p1 - state.visited[state.program_i] = true; + if let Some(RegDataType::Int(return_i)) = state.r.get(&p1) { state.program_i = (*return_i + 1) as usize; state.r.remove(&p1); continue; } else { + if logger.log_enabled() { + let program_history: Vec<&(i64, String, i64, i64, i64, Vec)> = + state.history.iter().map(|i| &program[*i]).collect(); + logger.add_result((program_history, None)); + } break; } } OP_YIELD => { // jump to p2 of the yield instruction pointed at by register p1, store prior instruction in p1 - state.visited[state.program_i] = true; + if let Some(RegDataType::Int(yield_i)) = state.r.get_mut(&p1) { let program_i: usize = state.program_i; @@ -571,13 +612,17 @@ pub(super) fn explain( continue; } } else { + if logger.log_enabled() { + let program_history: Vec<&(i64, String, i64, i64, i64, Vec)> = + state.history.iter().map(|i| &program[*i]).collect(); + logger.add_result((program_history, None)); + } break; } } OP_JUMP => { // goto one of , , or based on the result of a prior compare - state.visited[state.program_i] = true; let mut branch_state = state.clone(); branch_state.program_i = p1 as usize; @@ -909,7 +954,7 @@ pub(super) fn explain( OP_RESULT_ROW => { // output = r[p1 .. p1 + p2] - state.visited[state.program_i] = true; + state.result = Some( (p1..p1 + p2) .map(|i| { @@ -928,13 +973,18 @@ pub(super) fn explain( if logger.log_enabled() { let program_history: Vec<&(i64, String, i64, i64, i64, Vec)> = state.history.iter().map(|i| &program[*i]).collect(); - logger.add_result((program_history, state.result.clone())); + logger.add_result((program_history, Some(state.result.clone()))); } result_states.push(state.clone()); } OP_HALT => { + if logger.log_enabled() { + let program_history: Vec<&(i64, String, i64, i64, i64, Vec)> = + state.history.iter().map(|i| &program[*i]).collect(); + logger.add_result((program_history, None)); + } break; } @@ -945,7 +995,6 @@ pub(super) fn explain( } } - state.visited[state.program_i] = true; state.program_i += 1; } } From edcf2da4bec2348c6e25d41c03d79fbc52594dd2 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Sat, 26 Nov 2022 10:49:24 -0600 Subject: [PATCH 03/26] add failing testcase using nested orderby --- tests/sqlite/describe.rs | 57 +++++++++++++++++++++++++++++++++++----- 1 file changed, 51 insertions(+), 6 deletions(-) diff --git a/tests/sqlite/describe.rs b/tests/sqlite/describe.rs index 224cb6fa52..3ffa6a7ad6 100644 --- a/tests/sqlite/describe.rs +++ b/tests/sqlite/describe.rs @@ -421,6 +421,16 @@ async fn it_describes_table_order_by() -> anyhow::Result<()> { .await?; assert_literal_order_by_described(&mut conn, "SELECT 'a', text FROM tweet ORDER BY text") .await?; + assert_literal_order_by_described( + &mut conn, + "SELECT 'a', text FROM tweet ORDER BY text NULLS LAST", + ) + .await?; + assert_literal_order_by_described( + &mut conn, + "SELECT 'a', text FROM tweet ORDER BY text DESC NULLS LAST", + ) + .await?; Ok(()) } @@ -470,16 +480,51 @@ async fn it_describes_union() -> anyhow::Result<()> { Ok(()) } -//documents a failure originally found through property testing +//documents failures originally found through property testing #[sqlx_macros::test] async fn it_describes_nested_ordered() -> anyhow::Result<()> { + async fn assert_single_true_column_described( + conn: &mut sqlx::SqliteConnection, + query: &str, + ) -> anyhow::Result<()> { + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "INTEGER", "{}", query); + assert_eq!(info.nullable(0), Some(false), "{}", query); + + Ok(()) + } + let mut conn = new::().await?; - let info = conn - .describe("SELECT true FROM (SELECT true) a ORDER BY true") - .await?; - assert_eq!(info.column(0).type_info().name(), "INTEGER"); - assert_eq!(info.nullable(0), Some(false)); + assert_single_true_column_described( + &mut conn, + "SELECT true FROM (SELECT true) a ORDER BY true", + ) + .await?; + assert_single_true_column_described( + &mut conn, + " + SELECT true + FROM ( + SELECT 'a' + ) + CROSS JOIN ( + SELECT 'b' + FROM (SELECT 'c') + CROSS JOIN accounts + ORDER BY id + LIMIT 1 + ) + ", + ) + .await?; + + assert_single_true_column_described( + &mut conn, + "SELECT true FROM tweet + ORDER BY true ASC NULLS LAST", + ) + .await?; Ok(()) } From 5b5f2772c6dc2046ff291490f91d03d086e2709f Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Sat, 26 Nov 2022 10:51:02 -0600 Subject: [PATCH 04/26] fix handling of sequence/offset and rewind --- sqlx-sqlite/src/connection/explain.rs | 39 +++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index 5e2b71f8d5..6b917af046 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -67,6 +67,7 @@ const OP_SEEK_LE: &str = "SeekLE"; const OP_SEEK_LT: &str = "SeekLT"; const OP_SEEK_ROW_ID: &str = "SeekRowId"; const OP_SEEK_SCAN: &str = "SeekScan"; +const OP_SEQUENCE: &str = "Sequence"; const OP_SEQUENCE_TEST: &str = "SequenceTest"; const OP_SORT: &str = "Sort"; const OP_SORTER_DATA: &str = "SorterData"; @@ -120,6 +121,7 @@ const OP_MULTIPLY: &str = "Multiply"; const OP_DIVIDE: &str = "Divide"; const OP_REMAINDER: &str = "Remainder"; const OP_CONCAT: &str = "Concat"; +const OP_OFFSET_LIMIT: &str = "OffsetLimit"; const OP_RESULT_ROW: &str = "ResultRow"; const OP_HALT: &str = "Halt"; @@ -464,7 +466,7 @@ 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_LT | OP_MUST_BE_INT | OP_NE | OP_NEXT + | OP_IS_NULL_OR_TYPE | OP_MUST_BE_INT | OP_LE | OP_LT | 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 @@ -479,12 +481,18 @@ pub(super) fn explain( visited_branch_state.insert(bs_hash); states.push(branch_state); } + state.program_i += 1; continue; } OP_REWIND | OP_LAST | OP_SORT | OP_SORTER_SORT => { - // goto if cursor p1 is empty, else next instruction + // goto if cursor p1 is empty and p2 != 0, else next instruction + + if p2 == 0 { + state.program_i += 1; + continue; + } if let Some(cursor) = state.p.get(&p1) { if matches!(cursor.is_empty(), None | Some(true)) { @@ -505,6 +513,8 @@ pub(super) fn explain( //only take this branch if the cursor is non-empty state.program_i += 1; continue; + } else { + break; } } @@ -668,6 +678,19 @@ pub(super) fn explain( } } + OP_SEQUENCE => { + //Copy sequence number from cursor p1 to register p2, increment cursor p1 sequence number + + //Cursor emulation doesn't sequence value, but it is an int + state.r.insert( + p2, + RegDataType::Single(ColumnType { + datatype: DataType::Int64, + nullable: Some(false), + }), + ); + } + OP_ROW_DATA | OP_SORTER_DATA => { //Get entire row from cursor p1, store it into register p2 if let Some(record) = state.p.get(&p1) { @@ -710,6 +733,7 @@ pub(super) fn explain( // Create a cursor p1 aliasing the record from register p2 state.p.insert(p1, CursorDataType::Pseudo(p2)); } + OP_OPEN_READ | OP_OPEN_WRITE => { //Create a new pointer which is referenced by p1, take column metadata from db schema if found if p3 == 0 { @@ -952,6 +976,17 @@ pub(super) fn explain( } } + OP_OFFSET_LIMIT => { + // r[p2] = if r[p2] < 0 { r[p1] } else if r[p1]<0 { -1 } else { r[p1] + r[p3] } + state.r.insert( + p2, + RegDataType::Single(ColumnType { + datatype: DataType::Int64, + nullable: Some(false), + }), + ); + } + OP_RESULT_ROW => { // output = r[p1 .. p1 + p2] From 3c6eff9a7ca7d69228ac4ce91a455aded621cba5 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Fri, 18 Nov 2022 22:34:31 -0600 Subject: [PATCH 05/26] fix handling when sqlite nests records inside of records --- sqlx-sqlite/src/connection/explain.rs | 132 +++++++++++++++----------- 1 file changed, 77 insertions(+), 55 deletions(-) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index 6b917af046..9f8ea2bf64 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -127,15 +127,18 @@ const OP_HALT: &str = "Halt"; const MAX_LOOP_COUNT: u8 = 2; -#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] -struct ColumnType { - pub datatype: DataType, - pub nullable: Option, +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +enum ColumnType { + Single { + datatype: DataType, + nullable: Option, + }, + Record(Vec), } impl Default for ColumnType { fn default() -> Self { - Self { + Self::Single { datatype: DataType::Null, nullable: None, } @@ -144,43 +147,48 @@ impl Default for ColumnType { impl ColumnType { fn null() -> Self { - Self { + Self::Single { datatype: DataType::Null, nullable: Some(true), } } + fn map_to_datatype(&self) -> DataType { + match self { + Self::Single { datatype, .. } => datatype.clone(), + Self::Record(_) => DataType::Null, //If we're trying to coerce to a regular Datatype, we can assume a Record is invalid for the context + } + } + fn map_to_nullable(&self) -> Option { + match self { + Self::Single { nullable, .. } => *nullable, + Self::Record(_) => None, //If we're trying to coerce to a regular Datatype, we can assume a Record is invalid for the context + } + } } #[derive(Debug, Clone, Eq, PartialEq, Hash)] enum RegDataType { Single(ColumnType), - Record(Vec), Int(i64), } impl RegDataType { fn map_to_datatype(&self) -> DataType { match self { - RegDataType::Single(d) => d.datatype, - RegDataType::Record(_) => DataType::Null, //If we're trying to coerce to a regular Datatype, we can assume a Record is invalid for the context + RegDataType::Single(d) => d.map_to_datatype(), RegDataType::Int(_) => DataType::Int, } } fn map_to_nullable(&self) -> Option { match self { - RegDataType::Single(d) => d.nullable, - RegDataType::Record(_) => None, //If we're trying to coerce to a regular Datatype, we can assume a Record is invalid for the context + RegDataType::Single(d) => d.map_to_nullable(), RegDataType::Int(_) => Some(false), } } fn map_to_columntype(&self) -> ColumnType { match self { - RegDataType::Single(d) => *d, - RegDataType::Record(_) => ColumnType { - datatype: DataType::Null, - nullable: None, - }, //If we're trying to coerce to a regular Datatype, we can assume a Record is invalid for the context - RegDataType::Int(_) => ColumnType { + RegDataType::Single(d) => d.clone(), + RegDataType::Int(_) => ColumnType::Single { datatype: DataType::Int, nullable: Some(false), }, @@ -202,7 +210,7 @@ impl CursorDataType { Self::Normal { cols: record .iter() - .map(|(&colnum, &datatype)| (colnum, datatype)) + .map(|(colnum, datatype)| (*colnum, datatype.clone())) .collect(), is_empty, } @@ -210,7 +218,7 @@ impl CursorDataType { fn from_dense_record(record: &Vec, is_empty: Option) -> Self { Self::Normal { - cols: (0..).zip(record.iter().copied()).collect(), + cols: (0..).zip(record.iter().cloned()).collect(), is_empty, } } @@ -225,7 +233,7 @@ impl CursorDataType { rowdata } Self::Pseudo(i) => match registers.get(i) { - Some(RegDataType::Record(r)) => r.clone(), + Some(RegDataType::Single(ColumnType::Record(r))) => r.clone(), _ => Vec::new(), }, } @@ -238,7 +246,9 @@ impl CursorDataType { match self { Self::Normal { cols, .. } => cols.clone(), Self::Pseudo(i) => match registers.get(i) { - Some(RegDataType::Record(r)) => (0..).zip(r.iter().copied()).collect(), + Some(RegDataType::Single(ColumnType::Record(r))) => { + (0..).zip(r.iter().cloned()).collect() + } _ => HashMap::new(), }, } @@ -313,7 +323,7 @@ fn root_block_columns( let row_info = row_info.entry(block).or_default(); row_info.insert( colnum, - ColumnType { + ColumnType::Single { datatype: datatype.parse().unwrap_or(DataType::Null), nullable: Some(!notnull), }, @@ -323,7 +333,7 @@ fn root_block_columns( let row_info = row_info.entry(block).or_default(); row_info.insert( colnum, - ColumnType { + ColumnType::Single { datatype: datatype.parse().unwrap_or(DataType::Null), nullable: Some(!notnull), }, @@ -665,7 +675,7 @@ pub(super) fn explain( { if let Some(col) = record.get(&p2) { // insert into p3 the datatype of the col - state.r.insert(p3, RegDataType::Single(*col)); + state.r.insert(p3, RegDataType::Single(col.clone())); } else { state .r @@ -684,7 +694,7 @@ pub(super) fn explain( //Cursor emulation doesn't sequence value, but it is an int state.r.insert( p2, - RegDataType::Single(ColumnType { + RegDataType::Single(ColumnType::Single { datatype: DataType::Int64, nullable: Some(false), }), @@ -695,9 +705,13 @@ pub(super) fn explain( //Get entire row from cursor p1, store it into register p2 if let Some(record) = state.p.get(&p1) { let rowdata = record.map_to_dense_record(&state.r); - state.r.insert(p2, RegDataType::Record(rowdata)); + state + .r + .insert(p2, RegDataType::Single(ColumnType::Record(rowdata))); } else { - state.r.insert(p2, RegDataType::Record(Vec::new())); + state + .r + .insert(p2, RegDataType::Single(ColumnType::Record(Vec::new()))); } } @@ -713,16 +727,19 @@ pub(super) fn explain( .unwrap_or(ColumnType::default()), ); } - state.r.insert(p3, RegDataType::Record(record)); + state + .r + .insert(p3, RegDataType::Single(ColumnType::Record(record))); } OP_INSERT | OP_IDX_INSERT | OP_SORTER_INSERT => { - if let Some(RegDataType::Record(record)) = state.r.get(&p2) { + if let Some(RegDataType::Single(ColumnType::Record(record))) = state.r.get(&p2) + { if let Some(CursorDataType::Normal { cols, is_empty }) = state.p.get_mut(&p1) { // Insert the record into wherever pointer p1 is - *cols = (0..).zip(record.iter().copied()).collect(); + *cols = (0..).zip(record.iter().cloned()).collect(); *is_empty = Some(false); } } @@ -784,7 +801,7 @@ pub(super) fn explain( // last_insert_rowid() -> INTEGER state.r.insert( p3, - RegDataType::Single(ColumnType { + RegDataType::Single(ColumnType::Single { datatype: DataType::Int64, nullable: Some(false), }), @@ -799,8 +816,13 @@ pub(super) fn explain( // all columns in cursor X are potentially nullable 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); + for col in cols.values_mut() { + if let ColumnType::Single { + ref mut nullable, .. + } = col + { + *nullable = Some(true); + } } } //else we don't know about the cursor @@ -819,7 +841,7 @@ pub(super) fn explain( // count(_) -> INTEGER state.r.insert( p3, - RegDataType::Single(ColumnType { + RegDataType::Single(ColumnType::Single { datatype: DataType::Int64, nullable: Some(false), }), @@ -842,7 +864,7 @@ pub(super) fn explain( // count(_) -> INTEGER state.r.insert( p1, - RegDataType::Single(ColumnType { + RegDataType::Single(ColumnType::Single { datatype: DataType::Int64, nullable: Some(false), }), @@ -856,7 +878,7 @@ pub(super) fn explain( OP_CAST => { // affinity(r[p1]) if let Some(v) = state.r.get_mut(&p1) { - *v = RegDataType::Single(ColumnType { + *v = RegDataType::Single(ColumnType::Single { datatype: affinity_to_type(p2 as u8), nullable: v.map_to_nullable(), }); @@ -906,7 +928,7 @@ pub(super) fn explain( // r[p2] = state.r.insert( p2, - RegDataType::Single(ColumnType { + RegDataType::Single(ColumnType::Single { datatype: opcode_to_type(&opcode), nullable: Some(false), }), @@ -936,7 +958,7 @@ pub(super) fn explain( (Some(a), Some(b)) => { state.r.insert( p3, - RegDataType::Single(ColumnType { + RegDataType::Single(ColumnType::Single { datatype: if matches!(a.map_to_datatype(), DataType::Null) { b.map_to_datatype() } else { @@ -955,7 +977,7 @@ pub(super) fn explain( (Some(v), None) => { state.r.insert( p3, - RegDataType::Single(ColumnType { + RegDataType::Single(ColumnType::Single { datatype: v.map_to_datatype(), nullable: None, }), @@ -965,7 +987,7 @@ pub(super) fn explain( (None, Some(v)) => { state.r.insert( p3, - RegDataType::Single(ColumnType { + RegDataType::Single(ColumnType::Single { datatype: v.map_to_datatype(), nullable: None, }), @@ -980,7 +1002,7 @@ pub(super) fn explain( // r[p2] = if r[p2] < 0 { r[p1] } else if r[p1]<0 { -1 } else { r[p1] + r[p3] } state.r.insert( p2, - RegDataType::Single(ColumnType { + RegDataType::Single(ColumnType::Single { datatype: DataType::Int64, nullable: Some(false), }), @@ -1160,21 +1182,21 @@ fn test_root_block_columns_has_types() { { let blocknum = table_block_nums["t"]; assert_eq!( - ColumnType { + ColumnType::Single { datatype: DataType::Int64, nullable: Some(true) //sqlite primary key columns are nullable unless declared not null }, root_block_cols[&blocknum][&0] ); assert_eq!( - ColumnType { + ColumnType::Single { datatype: DataType::Text, nullable: Some(true) }, root_block_cols[&blocknum][&1] ); assert_eq!( - ColumnType { + ColumnType::Single { datatype: DataType::Text, nullable: Some(false) }, @@ -1185,14 +1207,14 @@ fn test_root_block_columns_has_types() { { let blocknum = table_block_nums["i1"]; assert_eq!( - ColumnType { + ColumnType::Single { datatype: DataType::Int64, nullable: Some(true) //sqlite primary key columns are nullable unless declared not null }, root_block_cols[&blocknum][&0] ); assert_eq!( - ColumnType { + ColumnType::Single { datatype: DataType::Text, nullable: Some(true) }, @@ -1203,14 +1225,14 @@ fn test_root_block_columns_has_types() { { let blocknum = table_block_nums["i2"]; assert_eq!( - ColumnType { + ColumnType::Single { datatype: DataType::Int64, nullable: Some(true) //sqlite primary key columns are nullable unless declared not null }, root_block_cols[&blocknum][&0] ); assert_eq!( - ColumnType { + ColumnType::Single { datatype: DataType::Text, nullable: Some(true) }, @@ -1221,21 +1243,21 @@ fn test_root_block_columns_has_types() { { let blocknum = table_block_nums["t2"]; assert_eq!( - ColumnType { + ColumnType::Single { datatype: DataType::Int64, nullable: Some(false) }, root_block_cols[&blocknum][&0] ); assert_eq!( - ColumnType { + ColumnType::Single { datatype: DataType::Null, nullable: Some(true) }, root_block_cols[&blocknum][&1] ); assert_eq!( - ColumnType { + ColumnType::Single { datatype: DataType::Null, nullable: Some(false) }, @@ -1246,14 +1268,14 @@ fn test_root_block_columns_has_types() { { let blocknum = table_block_nums["t2i1"]; assert_eq!( - ColumnType { + ColumnType::Single { datatype: DataType::Int64, nullable: Some(false) }, root_block_cols[&blocknum][&0] ); assert_eq!( - ColumnType { + ColumnType::Single { datatype: DataType::Null, nullable: Some(true) }, @@ -1264,14 +1286,14 @@ fn test_root_block_columns_has_types() { { let blocknum = table_block_nums["t2i2"]; assert_eq!( - ColumnType { + ColumnType::Single { datatype: DataType::Int64, nullable: Some(false) }, root_block_cols[&blocknum][&0] ); assert_eq!( - ColumnType { + ColumnType::Single { datatype: DataType::Null, nullable: Some(false) }, From b67df7fe31b293298706484d61cde5536956d7b6 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Sun, 20 Nov 2022 22:09:08 -0600 Subject: [PATCH 06/26] add test of temporary table handling --- tests/sqlite/describe.rs | 50 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/sqlite/describe.rs b/tests/sqlite/describe.rs index 3ffa6a7ad6..b526e85489 100644 --- a/tests/sqlite/describe.rs +++ b/tests/sqlite/describe.rs @@ -88,6 +88,56 @@ async fn it_describes_expression() -> anyhow::Result<()> { Ok(()) } +#[sqlx_macros::test] +async fn it_describes_temporary_table() -> anyhow::Result<()> { + let mut conn = new::().await?; + + conn.execute( + "CREATE TEMPORARY TABLE IF NOT EXISTS empty_all_types_and_nulls( + i1 integer NULL, + r1 real NULL, + t1 text NULL, + b1 blob NULL, + i2 INTEGER NOT NULL, + r2 REAL NOT NULL, + t2 TEXT NOT NULL, + b2 BLOB NOT NULL + )", + ) + .await?; + + let d = conn + .describe("SELECT * FROM empty_all_types_and_nulls") + .await?; + assert_eq!(d.columns().len(), 8); + + assert_eq!(d.column(0).type_info().name(), "INTEGER"); + assert_eq!(d.nullable(0), Some(false)); + + assert_eq!(d.column(1).type_info().name(), "TEXT"); + assert_eq!(d.nullable(1), Some(false)); + + assert_eq!(d.column(2).type_info().name(), "REAL"); + assert_eq!(d.nullable(2), Some(false)); + + assert_eq!(d.column(3).type_info().name(), "BLOB"); + assert_eq!(d.nullable(3), Some(false)); + + assert_eq!(d.column(4).type_info().name(), "INTEGER"); + assert_eq!(d.nullable(4), Some(true)); + + assert_eq!(d.column(5).type_info().name(), "TEXT"); + assert_eq!(d.nullable(5), Some(true)); + + assert_eq!(d.column(6).type_info().name(), "REAL"); + assert_eq!(d.nullable(6), Some(true)); + + assert_eq!(d.column(7).type_info().name(), "BLOB"); + assert_eq!(d.nullable(7), Some(true)); + + Ok(()) +} + #[sqlx_macros::test] async fn it_describes_expression_from_empty_table() -> anyhow::Result<()> { let mut conn = new::().await?; From a8204ccb2660b28b8ed718217165b924c75755d7 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Mon, 21 Nov 2022 01:57:57 -0600 Subject: [PATCH 07/26] WIP add test failure for temp table access --- sqlx-sqlite/src/connection/explain.rs | 65 ++++++++++++++++++++++----- tests/sqlite/describe.rs | 24 +++++----- 2 files changed, 66 insertions(+), 23 deletions(-) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index 9f8ea2bf64..77bae970a6 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -1157,30 +1157,48 @@ fn test_root_block_columns_has_types() { .next() .is_some()); - let table_block_nums: HashMap = execute::iter( + assert!(execute::iter( + &mut conn, + r"CREATE TEMPORARY TABLE t3(a TEXT PRIMARY KEY, b REAL NOT NULL, b_null REAL NULL);", + None, + false + ) + .unwrap() + .next() + .is_some()); + + let table_block_nums: HashMap = execute::iter( &mut conn, - r"select name, rootpage from sqlite_master", + r"select name, 0 db_seq, rootpage from main.sqlite_schema + UNION ALL select name, 1 db_seq, rootpage from temp.sqlite_schema", None, false, ) .unwrap() .filter_map(|res| res.map(|either| either.right()).transpose()) .map(|row| FromRow::from_row(row.as_ref().unwrap())) + .map(|row| row.map(|(name, seq, block)| (name, (seq, block)))) .collect::, Error>>() .unwrap(); let root_block_cols = root_block_columns(&mut conn).unwrap(); - assert_eq!(6, root_block_cols.len()); + //assert_eq!(7, root_block_cols.len()); //prove that we have some information for each table & index - for blocknum in table_block_nums.values() { - assert!(root_block_cols.contains_key(blocknum)); + for (name, (seqnum, blocknum)) in dbg!(&table_block_nums) { + assert!( + root_block_cols.contains_key(blocknum), + "name:{} seq_num:{} key:{}", + name, + seqnum, + blocknum + ); } //prove that each block has the correct information { - let blocknum = table_block_nums["t"]; + let (dbnum, blocknum) = table_block_nums["t"]; assert_eq!( ColumnType::Single { datatype: DataType::Int64, @@ -1205,7 +1223,7 @@ fn test_root_block_columns_has_types() { } { - let blocknum = table_block_nums["i1"]; + let (seqnum, blocknum) = table_block_nums["i1"]; assert_eq!( ColumnType::Single { datatype: DataType::Int64, @@ -1223,7 +1241,7 @@ fn test_root_block_columns_has_types() { } { - let blocknum = table_block_nums["i2"]; + let (seqnum, blocknum) = table_block_nums["i2"]; assert_eq!( ColumnType::Single { datatype: DataType::Int64, @@ -1241,7 +1259,7 @@ fn test_root_block_columns_has_types() { } { - let blocknum = table_block_nums["t2"]; + let (seqnum, blocknum) = table_block_nums["t2"]; assert_eq!( ColumnType::Single { datatype: DataType::Int64, @@ -1266,7 +1284,7 @@ fn test_root_block_columns_has_types() { } { - let blocknum = table_block_nums["t2i1"]; + let (seqnum, blocknum) = table_block_nums["t2i1"]; assert_eq!( ColumnType::Single { datatype: DataType::Int64, @@ -1284,7 +1302,7 @@ fn test_root_block_columns_has_types() { } { - let blocknum = table_block_nums["t2i2"]; + let (seqnum, blocknum) = table_block_nums["t2i2"]; assert_eq!( ColumnType::Single { datatype: DataType::Int64, @@ -1300,4 +1318,29 @@ fn test_root_block_columns_has_types() { root_block_cols[&blocknum][&1] ); } + + { + let (seqnum, blocknum) = table_block_nums["t3"]; + assert_eq!( + ColumnType::Single { + datatype: DataType::Text, + nullable: Some(false) + }, + root_block_cols[&blocknum][&0] + ); + assert_eq!( + ColumnType::Single { + datatype: DataType::Float, + nullable: Some(false) + }, + root_block_cols[&blocknum][&1] + ); + assert_eq!( + ColumnType::Single { + datatype: DataType::Float, + nullable: Some(true) + }, + root_block_cols[&blocknum][&2] + ); + } } diff --git a/tests/sqlite/describe.rs b/tests/sqlite/describe.rs index b526e85489..0022c5cb87 100644 --- a/tests/sqlite/describe.rs +++ b/tests/sqlite/describe.rs @@ -112,28 +112,28 @@ async fn it_describes_temporary_table() -> anyhow::Result<()> { assert_eq!(d.columns().len(), 8); assert_eq!(d.column(0).type_info().name(), "INTEGER"); - assert_eq!(d.nullable(0), Some(false)); + assert_eq!(d.nullable(0), Some(true)); - assert_eq!(d.column(1).type_info().name(), "TEXT"); - assert_eq!(d.nullable(1), Some(false)); + assert_eq!(d.column(1).type_info().name(), "REAL"); + assert_eq!(d.nullable(1), Some(true)); - assert_eq!(d.column(2).type_info().name(), "REAL"); - assert_eq!(d.nullable(2), Some(false)); + assert_eq!(d.column(2).type_info().name(), "TEXT"); + assert_eq!(d.nullable(2), Some(true)); assert_eq!(d.column(3).type_info().name(), "BLOB"); - assert_eq!(d.nullable(3), Some(false)); + assert_eq!(d.nullable(3), Some(true)); assert_eq!(d.column(4).type_info().name(), "INTEGER"); - assert_eq!(d.nullable(4), Some(true)); + assert_eq!(d.nullable(4), Some(false)); - assert_eq!(d.column(5).type_info().name(), "TEXT"); - assert_eq!(d.nullable(5), Some(true)); + assert_eq!(d.column(5).type_info().name(), "REAL"); + assert_eq!(d.nullable(5), Some(false)); - assert_eq!(d.column(6).type_info().name(), "REAL"); - assert_eq!(d.nullable(6), Some(true)); + assert_eq!(d.column(6).type_info().name(), "TEXT"); + assert_eq!(d.nullable(6), Some(false)); assert_eq!(d.column(7).type_info().name(), "BLOB"); - assert_eq!(d.nullable(7), Some(true)); + assert_eq!(d.nullable(7), Some(false)); Ok(()) } From 7cd37ec36f071e04762680959dd719a6c71159a9 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Mon, 21 Nov 2022 02:20:44 -0600 Subject: [PATCH 08/26] fix support for temp tables --- sqlx-sqlite/src/connection/explain.rs | 120 ++++++++++++-------------- 1 file changed, 53 insertions(+), 67 deletions(-) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index 77bae970a6..cac8082f90 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -289,24 +289,22 @@ fn opcode_to_type(op: &str) -> DataType { fn root_block_columns( conn: &mut ConnectionState, -) -> Result>, Error> { - let table_block_columns: Vec<(i64, i64, String, bool)> = execute::iter( +) -> Result>, Error> { + let table_block_columns: Vec<(i64, i64, i64, String, bool)> = execute::iter( conn, - "SELECT s.rootpage, col.cid as colnum, col.type, col.\"notnull\" - FROM (select * from sqlite_temp_schema UNION select * from sqlite_schema) s + "SELECT s.dbnum, s.rootpage, col.cid as colnum, col.type, col.\"notnull\" + FROM ( + select 1 dbnum, tss.* from temp.sqlite_schema tss + UNION ALL select 0 dbnum, mss.* from main.sqlite_schema mss + ) s JOIN pragma_table_info(s.name) AS col - WHERE s.type = 'table'", - None, - false, - )? - .filter_map(|res| res.map(|either| either.right()).transpose()) - .map(|row| FromRow::from_row(&row?)) - .collect::, Error>>()?; - - let index_block_columns: Vec<(i64, i64, String, bool)> = execute::iter( - conn, - "SELECT s.rootpage, idx.seqno as colnum, col.type, col.\"notnull\" - FROM (select * from sqlite_temp_schema UNION select * from sqlite_schema) s + WHERE s.type = 'table' + UNION ALL + SELECT s.dbnum, s.rootpage, idx.seqno as colnum, col.type, col.\"notnull\" + FROM ( + select 1 dbnum, tss.* from temp.sqlite_schema tss + UNION ALL select 0 dbnum, mss.* from main.sqlite_schema mss + ) s JOIN pragma_index_info(s.name) AS idx LEFT JOIN pragma_table_info(s.tbl_name) as col ON col.cid = idx.cid @@ -318,19 +316,9 @@ fn root_block_columns( .map(|row| FromRow::from_row(&row?)) .collect::, Error>>()?; - let mut row_info: HashMap> = HashMap::new(); - for (block, colnum, datatype, notnull) in table_block_columns { - let row_info = row_info.entry(block).or_default(); - row_info.insert( - colnum, - ColumnType::Single { - datatype: datatype.parse().unwrap_or(DataType::Null), - nullable: Some(!notnull), - }, - ); - } - for (block, colnum, datatype, notnull) in index_block_columns { - let row_info = row_info.entry(block).or_default(); + let mut row_info: HashMap<(i64, i64), HashMap> = HashMap::new(); + for (dbnum, block, colnum, datatype, notnull) in table_block_columns { + let row_info = row_info.entry((dbnum, block)).or_default(); row_info.insert( colnum, ColumnType::Single { @@ -753,8 +741,8 @@ pub(super) fn explain( OP_OPEN_READ | OP_OPEN_WRITE => { //Create a new pointer which is referenced by p1, take column metadata from db schema if found - if p3 == 0 { - if let Some(columns) = root_block_cols.get(&p2) { + if p3 == 0 || p3 == 1 { + if let Some(columns) = root_block_cols.get(&(p3, p2)) { state .p .insert(p1, CursorDataType::from_sparse_record(columns, None)); @@ -1167,180 +1155,178 @@ fn test_root_block_columns_has_types() { .next() .is_some()); - let table_block_nums: HashMap = execute::iter( + let table_block_nums: HashMap = execute::iter( &mut conn, - r"select name, 0 db_seq, rootpage from main.sqlite_schema - UNION ALL select name, 1 db_seq, rootpage from temp.sqlite_schema", + r"select name, 0 db_seq, rootpage from main.sqlite_schema UNION ALL select name, 1 db_seq, rootpage from temp.sqlite_schema", None, false, ) .unwrap() .filter_map(|res| res.map(|either| either.right()).transpose()) .map(|row| FromRow::from_row(row.as_ref().unwrap())) - .map(|row| row.map(|(name, seq, block)| (name, (seq, block)))) + .map(|row| row.map(|(name,seq,block)|(name,(seq,block)))) .collect::, Error>>() .unwrap(); let root_block_cols = root_block_columns(&mut conn).unwrap(); - //assert_eq!(7, root_block_cols.len()); + // there should be 7 tables/indexes created explicitly, plus 1 autoindex for t3 + assert_eq!(8, root_block_cols.len()); //prove that we have some information for each table & index - for (name, (seqnum, blocknum)) in dbg!(&table_block_nums) { + for (name, db_seq_block) in dbg!(&table_block_nums) { assert!( - root_block_cols.contains_key(blocknum), - "name:{} seq_num:{} key:{}", - name, - seqnum, - blocknum + root_block_cols.contains_key(db_seq_block), + "{:?}", + (name, db_seq_block) ); } //prove that each block has the correct information { - let (dbnum, blocknum) = table_block_nums["t"]; + let table_db_block = table_block_nums["t"]; assert_eq!( ColumnType::Single { datatype: DataType::Int64, nullable: Some(true) //sqlite primary key columns are nullable unless declared not null }, - root_block_cols[&blocknum][&0] + root_block_cols[&table_db_block][&0] ); assert_eq!( ColumnType::Single { datatype: DataType::Text, nullable: Some(true) }, - root_block_cols[&blocknum][&1] + root_block_cols[&table_db_block][&1] ); assert_eq!( ColumnType::Single { datatype: DataType::Text, nullable: Some(false) }, - root_block_cols[&blocknum][&2] + root_block_cols[&table_db_block][&2] ); } { - let (seqnum, blocknum) = table_block_nums["i1"]; + let table_db_block = table_block_nums["i1"]; assert_eq!( ColumnType::Single { datatype: DataType::Int64, nullable: Some(true) //sqlite primary key columns are nullable unless declared not null }, - root_block_cols[&blocknum][&0] + root_block_cols[&table_db_block][&0] ); assert_eq!( ColumnType::Single { datatype: DataType::Text, nullable: Some(true) }, - root_block_cols[&blocknum][&1] + root_block_cols[&table_db_block][&1] ); } { - let (seqnum, blocknum) = table_block_nums["i2"]; + let table_db_block = table_block_nums["i2"]; assert_eq!( ColumnType::Single { datatype: DataType::Int64, nullable: Some(true) //sqlite primary key columns are nullable unless declared not null }, - root_block_cols[&blocknum][&0] + root_block_cols[&table_db_block][&0] ); assert_eq!( ColumnType::Single { datatype: DataType::Text, nullable: Some(true) }, - root_block_cols[&blocknum][&1] + root_block_cols[&table_db_block][&1] ); } { - let (seqnum, blocknum) = table_block_nums["t2"]; + let table_db_block = table_block_nums["t2"]; assert_eq!( ColumnType::Single { datatype: DataType::Int64, nullable: Some(false) }, - root_block_cols[&blocknum][&0] + root_block_cols[&table_db_block][&0] ); assert_eq!( ColumnType::Single { datatype: DataType::Null, nullable: Some(true) }, - root_block_cols[&blocknum][&1] + root_block_cols[&table_db_block][&1] ); assert_eq!( ColumnType::Single { datatype: DataType::Null, nullable: Some(false) }, - root_block_cols[&blocknum][&2] + root_block_cols[&table_db_block][&2] ); } { - let (seqnum, blocknum) = table_block_nums["t2i1"]; + let table_db_block = table_block_nums["t2i1"]; assert_eq!( ColumnType::Single { datatype: DataType::Int64, nullable: Some(false) }, - root_block_cols[&blocknum][&0] + root_block_cols[&table_db_block][&0] ); assert_eq!( ColumnType::Single { datatype: DataType::Null, nullable: Some(true) }, - root_block_cols[&blocknum][&1] + root_block_cols[&table_db_block][&1] ); } { - let (seqnum, blocknum) = table_block_nums["t2i2"]; + let table_db_block = table_block_nums["t2i2"]; assert_eq!( ColumnType::Single { datatype: DataType::Int64, nullable: Some(false) }, - root_block_cols[&blocknum][&0] + root_block_cols[&table_db_block][&0] ); assert_eq!( ColumnType::Single { datatype: DataType::Null, nullable: Some(false) }, - root_block_cols[&blocknum][&1] + root_block_cols[&table_db_block][&1] ); } { - let (seqnum, blocknum) = table_block_nums["t3"]; + let table_db_block = table_block_nums["t3"]; assert_eq!( ColumnType::Single { datatype: DataType::Text, - nullable: Some(false) + nullable: Some(true) }, - root_block_cols[&blocknum][&0] + root_block_cols[&table_db_block][&0] ); assert_eq!( ColumnType::Single { datatype: DataType::Float, nullable: Some(false) }, - root_block_cols[&blocknum][&1] + root_block_cols[&table_db_block][&1] ); assert_eq!( ColumnType::Single { datatype: DataType::Float, nullable: Some(true) }, - root_block_cols[&blocknum][&2] + root_block_cols[&table_db_block][&2] ); } } From 3fd6965e154e462a156d1a7e3e296aee0afabdbb Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Sun, 27 Nov 2022 16:01:17 -0600 Subject: [PATCH 09/26] add tests for sqlite datetime functions --- tests/sqlite/describe.rs | 125 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/tests/sqlite/describe.rs b/tests/sqlite/describe.rs index 0022c5cb87..ec4a678a4d 100644 --- a/tests/sqlite/describe.rs +++ b/tests/sqlite/describe.rs @@ -578,3 +578,128 @@ async fn it_describes_nested_ordered() -> anyhow::Result<()> { Ok(()) } + +#[sqlx_macros::test] +async fn it_describes_func_date() -> anyhow::Result<()> { + let mut conn = new::().await?; + + let query = "SELECT date();"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(false), "{}", query); + + let query = "SELECT date('now');"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); //can't prove that it's not-null yet + + let query = "SELECT date('now', 'start of month');"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); //can't prove that it's not-null yet + + let query = "SELECT date(:datebind);"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); + Ok(()) +} + +#[sqlx_macros::test] +async fn it_describes_func_time() -> anyhow::Result<()> { + let mut conn = new::().await?; + + let query = "SELECT time();"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(false), "{}", query); + + let query = "SELECT time('now');"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); //can't prove that it's not-null yet + + let query = "SELECT time('now', 'start of month');"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); //can't prove that it's not-null yet + + let query = "SELECT time(:datebind);"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); + Ok(()) +} + +#[sqlx_macros::test] +async fn it_describes_func_datetime() -> anyhow::Result<()> { + let mut conn = new::().await?; + + let query = "SELECT datetime();"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(false), "{}", query); + + let query = "SELECT datetime('now');"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); //can't prove that it's not-null yet + + let query = "SELECT datetime('now', 'start of month');"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); //can't prove that it's not-null yet + + let query = "SELECT datetime(:datebind);"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); + Ok(()) +} + +#[sqlx_macros::test] +async fn it_describes_func_julianday() -> anyhow::Result<()> { + let mut conn = new::().await?; + + let query = "SELECT julianday();"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "REAL", "{}", query); + assert_eq!(info.nullable(0), Some(false), "{}", query); + + let query = "SELECT julianday('now');"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "REAL", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); //can't prove that it's not-null yet + + let query = "SELECT julianday('now', 'start of month');"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "REAL", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); //can't prove that it's not-null yet + + let query = "SELECT julianday(:datebind);"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "REAL", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); + Ok(()) +} + +#[sqlx_macros::test] +async fn it_describes_func_strftime() -> anyhow::Result<()> { + let mut conn = new::().await?; + + let query = "SELECT strftime('%s','now');"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); //can't prove that it's not-null yet + + let query = "SELECT strftime('%s', 'now', 'start of month');"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); //can't prove that it's not-null yet + + let query = "SELECT strftime('%s',:datebind);"; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); + Ok(()) +} From bd49b5fd60fa5ea5b2fba002421a52abd2bdd628 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Sun, 27 Nov 2022 20:18:33 -0600 Subject: [PATCH 10/26] add basic date and time function support --- sqlx-sqlite/src/connection/explain.rs | 30 +++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index cac8082f90..013bba53f0 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -795,6 +795,36 @@ pub(super) fn explain( }), ); } + "date(-1)" | "time(-1)" | "datetime(-1)" | "strftime(-1)" => { + // date|time|datetime|strftime(...) -> TEXT + state.r.insert( + p3, + RegDataType::Single(ColumnType::Single { + datatype: DataType::Text, + nullable: Some(p2 != 0), //never a null result if no argument provided + }), + ); + } + "julianday(-1)" => { + // julianday(...) -> REAL + state.r.insert( + p3, + RegDataType::Single(ColumnType::Single { + datatype: DataType::Float, + nullable: Some(p2 != 0), //never a null result if no argument provided + }), + ); + } + "unixepoch(-1)" => { + // unixepoch(p2...) -> INTEGER + state.r.insert( + p3, + RegDataType::Single(ColumnType::Single { + datatype: DataType::Int64, + nullable: Some(p2 != 0), //never a null result if no argument provided + }), + ); + } _ => logger.add_unknown_operation(&program[state.program_i]), } From 232865833ec8e0ff9d1bb1ebf12002774037cfdc Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Mon, 5 Dec 2022 11:20:28 -0600 Subject: [PATCH 11/26] handle gosub opcode correctly --- sqlx-sqlite/src/connection/explain.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index 013bba53f0..cc70b8bb77 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -460,12 +460,19 @@ pub(super) fn explain( continue; } + OP_GO_SUB => { + // store current instruction in r[p1], goto + state.r.insert(p1, RegDataType::Int(state.program_i as i64)); + state.program_i = p2 as usize; + continue; + } + OP_DECR_JUMP_ZERO | OP_ELSE_EQ | OP_EQ | OP_FILTER | OP_FK_IF_ZERO | OP_FOUND - | 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_MUST_BE_INT | OP_LE | OP_LT | OP_NE | OP_NEXT - | OP_NO_CONFLICT | OP_NOT_EXISTS | OP_NOT_NULL | OP_ONCE | OP_PREV | OP_PROGRAM + | OP_GE | 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_MUST_BE_INT | OP_LE | OP_LT | 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 => { From 4fc692e320618c421ad457ebe39d19fcbaa7ec51 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Tue, 6 Dec 2022 22:28:21 -0600 Subject: [PATCH 12/26] add group by test --- tests/sqlite/describe.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/sqlite/describe.rs b/tests/sqlite/describe.rs index ec4a678a4d..e56a8f34d0 100644 --- a/tests/sqlite/describe.rs +++ b/tests/sqlite/describe.rs @@ -351,6 +351,17 @@ async fn it_describes_left_join() -> anyhow::Result<()> { Ok(()) } +#[sqlx_macros::test] +async fn it_describes_group_by() -> anyhow::Result<()> { + let mut conn = new::().await?; + + let d = conn.describe("select id from accounts group by id").await?; + assert_eq!(d.column(0).type_info().name(), "INTEGER"); + assert_eq!(d.nullable(0), Some(false)); + + Ok(()) +} + #[sqlx_macros::test] async fn it_describes_literal_subquery() -> anyhow::Result<()> { async fn assert_literal_described( From d9472eb556773394c92b12f75f72dbc3bb2a3dc9 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Tue, 6 Dec 2022 22:29:23 -0600 Subject: [PATCH 13/26] fix group by handling --- sqlx-sqlite/src/connection/explain.rs | 78 ++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 8 deletions(-) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index cc70b8bb77..82fa60e226 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -468,14 +468,13 @@ pub(super) fn explain( } OP_DECR_JUMP_ZERO | OP_ELSE_EQ | OP_EQ | OP_FILTER | OP_FK_IF_ZERO | OP_FOUND - | OP_GE | 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_MUST_BE_INT | OP_LE | OP_LT | 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 => { + | OP_GE | OP_GT | OP_IDX_GE | OP_IDX_GT | OP_IDX_LE | OP_IDX_LT | OP_IF_NO_HOPE + | OP_IF_NOT | OP_IF_NOT_OPEN | OP_IF_NOT_ZERO | OP_IF_NULL_ROW | OP_IF_SMALLER + | OP_INCR_VACUUM | OP_IS_NULL | OP_IS_NULL_OR_TYPE | OP_MUST_BE_INT | OP_LE + | OP_LT | 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) let mut branch_state = state.clone(); @@ -491,6 +490,69 @@ pub(super) fn explain( continue; } + OP_IF => { + // goto if r[p1] is true (1) or r[p1] is null and p3 is nonzero + + let might_branch = match state.r.get(&p1) { + Some(RegDataType::Int(r_p1)) => *r_p1 != 0, + _ => true, + }; + + let might_not_branch = match state.r.get(&p1) { + Some(RegDataType::Int(r_p1)) => *r_p1 == 0, + _ => true, + }; + + if might_branch { + let mut branch_state = state.clone(); + branch_state.program_i = p2 as usize; + if p3 == 0 { + state.r.insert(p1, RegDataType::Int(1)); + } + states.push(branch_state); + } + + if might_not_branch { + state.program_i += 1; + if p3 == 0 { + state.r.insert(p1, RegDataType::Int(0)); + } + continue; + } else { + break; + } + } + + OP_IF_POS => { + // goto if r[p1] is true (1) or r[p1] is null and p3 is nonzero + + let might_branch = match state.r.get(&p1) { + Some(RegDataType::Int(r_p1)) => *r_p1 != 0, + _ => true, + }; + + let might_not_branch = match state.r.get(&p1) { + Some(RegDataType::Int(r_p1)) => *r_p1 == 0, + _ => true, + }; + + if might_branch { + let mut branch_state = state.clone(); + branch_state.program_i = p2 as usize; + if let Some(RegDataType::Int(r_p1)) = state.r.get_mut(&p1) { + *r_p1 -= 1; + } + states.push(branch_state); + } + + if might_not_branch { + state.program_i += 1; + continue; + } else { + break; + } + } + OP_REWIND | OP_LAST | OP_SORT | OP_SORTER_SORT => { // goto if cursor p1 is empty and p2 != 0, else next instruction From e7c316ee0f9144e7555dad8022f9985fa47ef8f6 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Tue, 6 Dec 2022 23:14:37 -0600 Subject: [PATCH 14/26] add additional passing group by test --- tests/sqlite/describe.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/sqlite/describe.rs b/tests/sqlite/describe.rs index e56a8f34d0..80f31333c1 100644 --- a/tests/sqlite/describe.rs +++ b/tests/sqlite/describe.rs @@ -359,6 +359,12 @@ async fn it_describes_group_by() -> anyhow::Result<()> { assert_eq!(d.column(0).type_info().name(), "INTEGER"); assert_eq!(d.nullable(0), Some(false)); + let d = conn + .describe("SELECT name from accounts GROUP BY 1 LIMIT -1 OFFSET 1") + .await?; + assert_eq!(d.column(0).type_info().name(), "TEXT"); + assert_eq!(d.nullable(0), Some(false)); + Ok(()) } From 4bb58008b847ae8a5a548202729304c9c9b42027 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Wed, 7 Dec 2022 01:11:10 -0600 Subject: [PATCH 15/26] add test case for simple limit query --- tests/sqlite/describe.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/sqlite/describe.rs b/tests/sqlite/describe.rs index 80f31333c1..67f41396b2 100644 --- a/tests/sqlite/describe.rs +++ b/tests/sqlite/describe.rs @@ -568,6 +568,7 @@ async fn it_describes_nested_ordered() -> anyhow::Result<()> { "SELECT true FROM (SELECT true) a ORDER BY true", ) .await?; + assert_single_true_column_described( &mut conn, " @@ -593,6 +594,8 @@ async fn it_describes_nested_ordered() -> anyhow::Result<()> { ) .await?; + assert_single_true_column_described(&mut conn, "SELECT true LIMIT -1 OFFSET -1").await?; + Ok(()) } From 508e23dffdfd86c378fbb37cbfff3130917a383e Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Wed, 7 Dec 2022 01:12:50 -0600 Subject: [PATCH 16/26] fix IfPos & If touching wrong branches state, fix IfPos using wrong branch criteria --- sqlx-sqlite/src/connection/explain.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index 82fa60e226..c14f194c96 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -507,7 +507,7 @@ pub(super) fn explain( let mut branch_state = state.clone(); branch_state.program_i = p2 as usize; if p3 == 0 { - state.r.insert(p1, RegDataType::Int(1)); + branch_state.r.insert(p1, RegDataType::Int(1)); } states.push(branch_state); } @@ -527,19 +527,19 @@ pub(super) fn explain( // goto if r[p1] is true (1) or r[p1] is null and p3 is nonzero let might_branch = match state.r.get(&p1) { - Some(RegDataType::Int(r_p1)) => *r_p1 != 0, + Some(RegDataType::Int(r_p1)) => *r_p1 >= 1, _ => true, }; let might_not_branch = match state.r.get(&p1) { - Some(RegDataType::Int(r_p1)) => *r_p1 == 0, + Some(RegDataType::Int(r_p1)) => *r_p1 < 1, _ => true, }; if might_branch { let mut branch_state = state.clone(); branch_state.program_i = p2 as usize; - if let Some(RegDataType::Int(r_p1)) = state.r.get_mut(&p1) { + if let Some(RegDataType::Int(r_p1)) = branch_state.r.get_mut(&p1) { *r_p1 -= 1; } states.push(branch_state); From 36797979c0692c2576427a51f1a3eefda49d1a5f Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Wed, 7 Dec 2022 22:08:22 -0600 Subject: [PATCH 17/26] add test for large offsets --- tests/sqlite/describe.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/sqlite/describe.rs b/tests/sqlite/describe.rs index 67f41396b2..acb821eed7 100644 --- a/tests/sqlite/describe.rs +++ b/tests/sqlite/describe.rs @@ -596,6 +596,12 @@ async fn it_describes_nested_ordered() -> anyhow::Result<()> { assert_single_true_column_described(&mut conn, "SELECT true LIMIT -1 OFFSET -1").await?; + assert_single_true_column_described( + &mut conn, + "SELECT true FROM tweet J LIMIT 10 OFFSET 1000000", + ) + .await?; + Ok(()) } From 84c81a94b3dbacdf20f09e339e7dabd5aefd4096 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Wed, 7 Dec 2022 22:10:56 -0600 Subject: [PATCH 18/26] add short-circuit for possible query offset loops --- sqlx-sqlite/src/connection/explain.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index c14f194c96..864927dbf4 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -526,6 +526,8 @@ pub(super) fn explain( OP_IF_POS => { // goto if r[p1] is true (1) or r[p1] is null and p3 is nonzero + // as a workaround for large offset clauses, both branches will be attempted after 1 loop + let might_branch = match state.r.get(&p1) { Some(RegDataType::Int(r_p1)) => *r_p1 >= 1, _ => true, @@ -536,7 +538,8 @@ pub(super) fn explain( _ => true, }; - if might_branch { + let loop_detected = state.visited[state.program_i] > 1; + if might_branch || loop_detected { let mut branch_state = state.clone(); branch_state.program_i = p2 as usize; if let Some(RegDataType::Int(r_p1)) = branch_state.r.get_mut(&p1) { @@ -548,6 +551,19 @@ pub(super) fn explain( if might_not_branch { state.program_i += 1; continue; + } else if loop_detected { + state.program_i += 1; + if matches!(state.r.get_mut(&p1), Some(RegDataType::Int(..))) { + //forget the exact value, in case some later cares + state.r.insert( + p1, + RegDataType::Single(ColumnType::Single { + datatype: DataType::Int64, + nullable: Some(false), + }), + ); + } + continue; } else { break; } From 77c13b0e9eeab9a6699a46c780596f0997a436f3 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Fri, 9 Dec 2022 13:38:03 -0600 Subject: [PATCH 19/26] add groupby query that is predicted incorrectly --- tests/sqlite/describe.rs | 49 ++++++++++++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/tests/sqlite/describe.rs b/tests/sqlite/describe.rs index acb821eed7..dcfc5f9fba 100644 --- a/tests/sqlite/describe.rs +++ b/tests/sqlite/describe.rs @@ -549,27 +549,31 @@ async fn it_describes_union() -> anyhow::Result<()> { //documents failures originally found through property testing #[sqlx_macros::test] -async fn it_describes_nested_ordered() -> anyhow::Result<()> { - async fn assert_single_true_column_described( +async fn it_describes_strange_queries() -> anyhow::Result<()> { + async fn assert_single_column_described( conn: &mut sqlx::SqliteConnection, query: &str, + typename: &str, + nullable: bool, ) -> anyhow::Result<()> { let info = conn.describe(query).await?; - assert_eq!(info.column(0).type_info().name(), "INTEGER", "{}", query); - assert_eq!(info.nullable(0), Some(false), "{}", query); + assert_eq!(info.column(0).type_info().name(), typename, "{}", query); + assert_eq!(info.nullable(0), Some(nullable), "{}", query); Ok(()) } let mut conn = new::().await?; - assert_single_true_column_described( + assert_single_column_described( &mut conn, "SELECT true FROM (SELECT true) a ORDER BY true", + "INTEGER", + false, ) .await?; - assert_single_true_column_described( + assert_single_column_described( &mut conn, " SELECT true @@ -584,21 +588,48 @@ async fn it_describes_nested_ordered() -> anyhow::Result<()> { LIMIT 1 ) ", + "INTEGER", + false, ) .await?; - assert_single_true_column_described( + assert_single_column_described( &mut conn, "SELECT true FROM tweet ORDER BY true ASC NULLS LAST", + "INTEGER", + false, ) .await?; - assert_single_true_column_described(&mut conn, "SELECT true LIMIT -1 OFFSET -1").await?; + assert_single_column_described( + &mut conn, + "SELECT true LIMIT -1 OFFSET -1", + "INTEGER", + false, + ) + .await?; - assert_single_true_column_described( + assert_single_column_described( &mut conn, "SELECT true FROM tweet J LIMIT 10 OFFSET 1000000", + "INTEGER", + false, + ) + .await?; + + assert_single_column_described( + &mut conn, + "SELECT text + FROM (SELECT null) + CROSS JOIN ( + SELECT text + FROM tweet + GROUP BY text + ) + LIMIT -1 OFFSET -1", + "TEXT", + false, ) .await?; From 76f3658157ee0be51fd699787ecdc7f6e74ae991 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Fri, 9 Dec 2022 13:39:27 -0600 Subject: [PATCH 20/26] fix handling of integer cast failures --- sqlx-sqlite/src/connection/explain.rs | 30 ++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index 864927dbf4..d7da2db431 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -470,11 +470,11 @@ pub(super) fn explain( OP_DECR_JUMP_ZERO | OP_ELSE_EQ | OP_EQ | OP_FILTER | OP_FK_IF_ZERO | OP_FOUND | OP_GE | OP_GT | OP_IDX_GE | OP_IDX_GT | OP_IDX_LE | OP_IDX_LT | OP_IF_NO_HOPE | OP_IF_NOT | OP_IF_NOT_OPEN | OP_IF_NOT_ZERO | OP_IF_NULL_ROW | OP_IF_SMALLER - | OP_INCR_VACUUM | OP_IS_NULL | OP_IS_NULL_OR_TYPE | OP_MUST_BE_INT | OP_LE - | OP_LT | 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 => { + | OP_INCR_VACUUM | OP_IS_NULL | OP_IS_NULL_OR_TYPE | OP_LE | OP_LT | 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) let mut branch_state = state.clone(); @@ -490,6 +490,26 @@ pub(super) fn explain( continue; } + OP_MUST_BE_INT => { + // if p1 can be coerced to int, continue + // if p1 cannot be coerced to int, error if p2 == 0, else jump to p2 + + //don't bother checking actual types, just don't branch to instruction 0 + if p2 != 0 { + let mut branch_state = state.clone(); + branch_state.program_i = p2 as usize; + + let bs_hash = BranchStateHash::from_query_state(&branch_state); + if !visited_branch_state.contains(&bs_hash) { + visited_branch_state.insert(bs_hash); + states.push(branch_state); + } + } + + state.program_i += 1; + continue; + } + OP_IF => { // goto if r[p1] is true (1) or r[p1] is null and p3 is nonzero From fc4fdf3ec00b5738af327bad77293d4176de7ebd Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Wed, 14 Dec 2022 02:16:37 -0600 Subject: [PATCH 21/26] add tests for single-row aggregate results --- tests/sqlite/describe.rs | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/sqlite/describe.rs b/tests/sqlite/describe.rs index dcfc5f9fba..402012167f 100644 --- a/tests/sqlite/describe.rs +++ b/tests/sqlite/describe.rs @@ -365,6 +365,35 @@ async fn it_describes_group_by() -> anyhow::Result<()> { assert_eq!(d.column(0).type_info().name(), "TEXT"); assert_eq!(d.nullable(0), Some(false)); + let d = conn + .describe("SELECT sum(id), sum(is_sent) from tweet GROUP BY owner_id") + .await?; + assert_eq!(d.column(0).type_info().name(), "INTEGER"); + assert_eq!(d.nullable(0), Some(false)); + assert_eq!(d.column(1).type_info().name(), "INTEGER"); + assert_eq!(d.nullable(1), Some(false)); + + Ok(()) +} + +#[sqlx_macros::test] +async fn it_describes_ungrouped_aggregate() -> anyhow::Result<()> { + let mut conn = new::().await?; + + let d = conn.describe("select count(1) from accounts").await?; + assert_eq!(d.column(0).type_info().name(), "INTEGER"); + assert_eq!(d.nullable(0), Some(false)); + + let d = conn.describe("SELECT sum(is_sent) from tweet").await?; + assert_eq!(d.column(0).type_info().name(), "INTEGER"); + assert_eq!(d.nullable(0), Some(true)); + + let d = conn + .describe("SELECT coalesce(sum(is_sent),0) from tweet") + .await?; + assert_eq!(d.column(0).type_info().name(), "INTEGER"); + assert_eq!(d.nullable(0), Some(false)); + Ok(()) } @@ -633,6 +662,15 @@ async fn it_describes_strange_queries() -> anyhow::Result<()> { ) .await?; + assert_single_column_described( + &mut conn, + "SELECT EYH.id,COUNT(EYH.id) + FROM accounts EYH", + "INTEGER", + true, + ) + .await?; + Ok(()) } From 8d914c518a762c18a066761744583be59be35ed3 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Wed, 14 Dec 2022 02:17:33 -0600 Subject: [PATCH 22/26] fix handling of null-based branching --- sqlx-sqlite/src/connection/explain.rs | 53 +++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index d7da2db431..f657bfc31f 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -471,9 +471,9 @@ pub(super) fn explain( | OP_GE | OP_GT | OP_IDX_GE | OP_IDX_GT | OP_IDX_LE | OP_IDX_LT | OP_IF_NO_HOPE | OP_IF_NOT | OP_IF_NOT_OPEN | OP_IF_NOT_ZERO | OP_IF_NULL_ROW | OP_IF_SMALLER | OP_INCR_VACUUM | OP_IS_NULL | OP_IS_NULL_OR_TYPE | OP_LE | OP_LT | 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_NEXT | OP_NO_CONFLICT | OP_NOT_EXISTS | 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) @@ -490,6 +490,46 @@ pub(super) fn explain( continue; } + OP_NOT_NULL => { + // goto or next instruction (depending on actual values) + + let might_branch = match state.r.get(&p1) { + Some(r_p1) => !matches!(r_p1.map_to_datatype(), DataType::Null), + _ => false, + }; + + let might_not_branch = match state.r.get(&p1) { + Some(r_p1) => !matches!(r_p1.map_to_nullable(), Some(false)), + _ => false, + }; + + if might_branch { + let mut branch_state = state.clone(); + branch_state.program_i = p2 as usize; + if let Some(RegDataType::Single(ColumnType::Single { nullable, .. })) = + branch_state.r.get_mut(&p1) + { + *nullable = Some(false); + } + + let bs_hash = BranchStateHash::from_query_state(&branch_state); + if !visited_branch_state.contains(&bs_hash) { + visited_branch_state.insert(bs_hash); + states.push(branch_state); + } + } + + if might_not_branch { + state.program_i += 1; + state + .r + .insert(p1, RegDataType::Single(ColumnType::default())); + continue; + } else { + break; + } + } + OP_MUST_BE_INT => { // if p1 can be coerced to int, continue // if p1 cannot be coerced to int, error if p2 == 0, else jump to p2 @@ -529,7 +569,12 @@ pub(super) fn explain( if p3 == 0 { branch_state.r.insert(p1, RegDataType::Int(1)); } - states.push(branch_state); + + let bs_hash = BranchStateHash::from_query_state(&branch_state); + if !visited_branch_state.contains(&bs_hash) { + visited_branch_state.insert(bs_hash); + states.push(branch_state); + } } if might_not_branch { From ef61dbe0fd12f3f88dd0a05a8b5e10f66e954a38 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Fri, 16 Dec 2022 17:23:55 -0600 Subject: [PATCH 23/26] add test for coercion of text by sum --- tests/sqlite/describe.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/sqlite/describe.rs b/tests/sqlite/describe.rs index 402012167f..74d1bd9042 100644 --- a/tests/sqlite/describe.rs +++ b/tests/sqlite/describe.rs @@ -671,6 +671,14 @@ async fn it_describes_strange_queries() -> anyhow::Result<()> { ) .await?; + assert_single_column_described( + &mut conn, + "SELECT SUM(tweet.text) FROM (SELECT NULL FROM accounts_view LIMIT -1 OFFSET 1) CROSS JOIN tweet", + "REAL", + true, // null if accounts view has fewer rows than the offset + ) + .await?; + Ok(()) } From 51c54009ff8676b5ee3d3d82864c4d2b53f6a2e4 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Fri, 16 Dec 2022 17:28:39 -0600 Subject: [PATCH 24/26] fix calculation of sum value coercion --- sqlx-sqlite/src/connection/explain.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index f657bfc31f..32175f9f73 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -1014,6 +1014,20 @@ pub(super) fn explain( nullable: Some(false), }), ); + } else if p4.starts_with("sum(") { + if let Some(r_p2) = state.r.get(&p2) { + let datatype = match r_p2.map_to_datatype() { + DataType::Int64 => DataType::Int64, + DataType::Int => DataType::Int, + DataType::Bool => DataType::Int, + _ => DataType::Float, + }; + let nullable = r_p2.map_to_nullable(); + state.r.insert( + p3, + RegDataType::Single(ColumnType::Single { datatype, nullable }), + ); + } } else if let Some(v) = state.r.get(&p2).cloned() { // r[p3] = AGG ( r[p2] ) state.r.insert(p3, v); @@ -1037,9 +1051,6 @@ pub(super) fn explain( nullable: Some(false), }), ); - } else if let Some(v) = state.r.get(&p2).cloned() { - // r[p3] = AGG ( r[p2] ) - state.r.insert(p3, v); } } From a38b5c478beb915f2b6460b1de2d737f7aa33364 Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Fri, 23 Dec 2022 21:49:44 -0600 Subject: [PATCH 25/26] add failing test for recursive with query --- tests/sqlite/describe.rs | 43 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/sqlite/describe.rs b/tests/sqlite/describe.rs index 74d1bd9042..22121bbd1e 100644 --- a/tests/sqlite/describe.rs +++ b/tests/sqlite/describe.rs @@ -806,3 +806,46 @@ async fn it_describes_func_strftime() -> anyhow::Result<()> { assert_eq!(info.nullable(0), Some(true), "{}", query); Ok(()) } + +#[sqlx_macros::test] +async fn it_describes_with_recursive() -> anyhow::Result<()> { + let mut conn = new::().await?; + + let query = " + WITH RECURSIVE schedule(begin_date) AS ( + SELECT datetime('2022-10-01') + WHERE datetime('2022-10-01') < datetime('2022-11-03') + UNION ALL + SELECT datetime(begin_date,'+1 day') + FROM schedule + WHERE datetime(begin_date) < datetime(?2) + ) + SELECT + begin_date + FROM schedule + GROUP BY begin_date + "; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); + + let query = " + WITH RECURSIVE schedule(begin_date) AS MATERIALIZED ( + SELECT datetime('2022-10-01') + WHERE datetime('2022-10-01') < datetime('2022-11-03') + UNION ALL + SELECT datetime(begin_date,'+1 day') + FROM schedule + WHERE datetime(begin_date) < datetime(?2) + ) + SELECT + begin_date + FROM schedule + GROUP BY begin_date + "; + let info = conn.describe(query).await?; + assert_eq!(info.column(0).type_info().name(), "TEXT", "{}", query); + assert_eq!(info.nullable(0), Some(true), "{}", query); + + Ok(()) +} From 1d7d50af5b4e8c440b2d29887e28545666b801fc Mon Sep 17 00:00:00 2001 From: Tyrel Rink <44035897+tyrelr@users.noreply.github.com> Date: Fri, 23 Dec 2022 21:59:26 -0600 Subject: [PATCH 26/26] add logic for delete operation to fix queries grouping by columns from a recursive query --- sqlx-sqlite/src/connection/explain.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index 32175f9f73..c9247b4ef2 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -19,6 +19,7 @@ const SQLITE_AFF_REAL: u8 = 0x45; /* 'E' */ const OP_INIT: &str = "Init"; const OP_GOTO: &str = "Goto"; const OP_DECR_JUMP_ZERO: &str = "DecrJumpZero"; +const OP_DELETE: &str = "Delete"; const OP_ELSE_EQ: &str = "ElseEq"; const OP_EQ: &str = "Eq"; const OP_END_COROUTINE: &str = "EndCoroutine"; @@ -884,6 +885,15 @@ pub(super) fn explain( //Noop if the register p2 isn't a record, or if pointer p1 does not exist } + OP_DELETE => { + // delete a record from cursor p1 + if let Some(CursorDataType::Normal { is_empty, .. }) = state.p.get_mut(&p1) { + if *is_empty == Some(false) { + *is_empty = None; //the cursor might be empty now + } + } + } + OP_OPEN_PSEUDO => { // Create a cursor p1 aliasing the record from register p2 state.p.insert(p1, CursorDataType::Pseudo(p2));