diff --git a/core/crates/core/src/column/column_origin.rs b/core/crates/core/src/column/column_origin.rs new file mode 100644 index 0000000000..2fef7f9787 --- /dev/null +++ b/core/crates/core/src/column/column_origin.rs @@ -0,0 +1,17 @@ +use crate::meta::TableRef; + +// Set only for a direct column of a permanent base table, qualified so the window's connection names the same table. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct ColumnOrigin { + pub table: TableRef, + pub column: String, +} + +impl ColumnOrigin { + pub fn new(table: TableRef, column: impl Into) -> Self { + Self { + table, + column: column.into(), + } + } +} diff --git a/core/crates/core/src/column/mod.rs b/core/crates/core/src/column/mod.rs index 682da48187..46344ffd22 100644 --- a/core/crates/core/src/column/mod.rs +++ b/core/crates/core/src/column/mod.rs @@ -1,6 +1,7 @@ mod catalog_type; mod classify; mod column_info; +mod column_origin; mod column_type; mod default; mod integer_kind; @@ -13,6 +14,7 @@ mod sql_type_expr; pub use catalog_type::CatalogType; pub use classify::{classify_type_name, has_dynamic_storage}; pub use column_info::ColumnInfo; +pub use column_origin::ColumnOrigin; pub use column_type::ColumnType; pub use default::ColumnDefault; pub use integer_kind::{IntegerKind, IntegerWidth}; diff --git a/core/crates/core/src/column/result_column.rs b/core/crates/core/src/column/result_column.rs index 9f1e7d010f..0509caa405 100644 --- a/core/crates/core/src/column/result_column.rs +++ b/core/crates/core/src/column/result_column.rs @@ -1,15 +1,11 @@ -use super::ColumnType; +use super::{ColumnOrigin, ColumnType}; -/// A column of a result set, which is all a driver can say about an -/// arbitrary query's output. -/// -/// Browse uses `ColumnInfo` from `fetch_columns`, which knows about -/// keys and defaults. A result column knows only its name and type, -/// because that is all a `SELECT` over an expression has. +// Browse reads keys and defaults from `ColumnInfo`; an arbitrary query's output has only a name, a type and maybe an origin. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ResultColumn { pub name: String, pub column_type: ColumnType, + pub origin: Option, } impl ResultColumn { @@ -17,6 +13,12 @@ impl ResultColumn { Self { name: name.into(), column_type, + origin: None, } } + + pub fn with_origin(mut self, origin: Option) -> Self { + self.origin = origin; + self + } } diff --git a/core/crates/core/src/edit/mod.rs b/core/crates/core/src/edit/mod.rs index 077d499a3e..fb4ba8a9b3 100644 --- a/core/crates/core/src/edit/mod.rs +++ b/core/crates/core/src/edit/mod.rs @@ -11,6 +11,7 @@ mod is_unchanged; mod new_draft_cell; mod parse_edit_text; mod parse_literal_text; +mod result_target; mod temporal_parts; pub use cell_expression::{CellExpression, MomentKind, cell_expressions, typed_cell_expression}; @@ -25,4 +26,7 @@ pub use parse_edit_text::parse_edit_text; pub use parse_literal_text::{ parse_filter_text, parse_literal_text, parse_time_with_offset, parse_timestamp_with_offset, }; +pub use result_target::{ + ResultLock, ResultSource, ResultTarget, parsed_origins, result_source, result_target, value_for_column, +}; pub use temporal_parts::{TemporalParts, temporal_parts}; diff --git a/core/crates/core/src/edit/result_target.rs b/core/crates/core/src/edit/result_target.rs new file mode 100644 index 0000000000..a13bb21c43 --- /dev/null +++ b/core/crates/core/src/edit/result_target.rs @@ -0,0 +1,463 @@ +use crate::column::{ColumnInfo, ColumnKind, ColumnOrigin, ColumnType, ReadForm, ResultColumn, TemporalZone}; +use crate::meta::TableRef; +use crate::sql_syntax::{ + SourceItem, SourceRefusal, SourceTable, SqlGrammar, same_name, source_columns_for_result, source_table, +}; +use crate::value::Value; + +use super::parse_literal_text; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum ResultLock { + NotOneSelect, + MoreThanOneTable, + Collapsed, + // No origin, or one that is not the table the statement names: a view, a temporary table, an engine that reports none. + Untraced, + NoPrimaryKey, + KeyNotShown, + ColumnsChanged, +} + +impl From for ResultLock { + fn from(refusal: SourceRefusal) -> Self { + match refusal { + SourceRefusal::NotOneSelect => Self::NotOneSelect, + SourceRefusal::MoreThanOneTable => Self::MoreThanOneTable, + SourceRefusal::Collapsed => Self::Collapsed, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ResultSource { + pub table: TableRef, + statement: SourceTable, + grammar: SqlGrammar, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ResultTarget { + pub table: TableRef, + // For each result column, the catalog column it writes; `None` locks the cell. + pub positions: Vec>, +} + +// The statement decides whether its rows are one table's; the driver's origins prove which table. +pub fn result_source(sql: &str, grammar: SqlGrammar, columns: &[ResultColumn]) -> Result { + let statement = source_table(sql, grammar)?; + let mut origins = columns.iter().filter_map(|column| column.origin.as_ref()); + let table = origins.next().map(|origin| &origin.table).ok_or(ResultLock::Untraced)?; + if origins.any(|origin| origin.table != *table) || !names_table(&statement.table, table, grammar) { + return Err(ResultLock::Untraced); + } + Ok(ResultSource { + table: table.clone(), + statement, + grammar, + }) +} + +pub fn result_target( + source: &ResultSource, + result: &[ResultColumn], + catalog: &[ColumnInfo], +) -> Result { + let names: Vec = result.iter().map(|column| column.name.clone()).collect(); + let parsed = source_columns_for_result(&source.statement, catalog, &names).ok_or(ResultLock::ColumnsChanged)?; + let mut positions = Vec::with_capacity(result.len()); + for (column, parsed) in result.iter().zip(parsed) { + let traced = match (parsed, &column.origin) { + (Some(position), Some(origin)) => { + let named = catalog + .get(position) + .is_some_and(|info| same_name(&info.name, &origin.column, source.grammar)); + if !named { + return Err(ResultLock::Untraced); + } + Some(position) + } + _ => None, + }; + // The second copy of a column would write over the first one's edit in the same statement. + let first = traced.filter(|position| !positions.contains(&Some(*position))); + positions.push(first); + } + let mut keys = catalog + .iter() + .enumerate() + .filter(|(_, info)| info.primary_key) + .peekable(); + if keys.peek().is_none() { + return Err(ResultLock::NoPrimaryKey); + } + if keys.any(|(index, _)| !positions.contains(&Some(index))) { + return Err(ResultLock::KeyNotShown); + } + Ok(ResultTarget { + table: source.table.clone(), + positions, + }) +} + +// For an engine that reports no origins: once the driver resolved the statement's table on its own connection, the statement says which columns are that table's. +pub fn parsed_origins(statement: &SourceTable, table: &TableRef, names: &[String]) -> Vec> { + let wildcards = statement + .items + .iter() + .filter(|item| matches!(item, SourceItem::Wildcard)) + .count(); + let others = statement.items.len() - wildcards; + let spread = match names.len().checked_sub(others) { + Some(spread) if wildcards == 0 && spread == 0 => 0, + Some(spread) if wildcards > 0 && spread % wildcards == 0 => spread / wildcards, + _ => return vec![None; names.len()], + }; + let mut origins = Vec::with_capacity(names.len()); + for item in &statement.items { + match item { + SourceItem::Column(name) => origins.push(Some(ColumnOrigin::new(table.clone(), name.name.clone()))), + SourceItem::Expression => origins.push(None), + SourceItem::Wildcard => { + let start = origins.len(); + origins.extend( + names[start..start + spread] + .iter() + .map(|name| Some(ColumnOrigin::new(table.clone(), name.clone()))), + ); + } + } + } + origins +} + +// A driver that reads a column as the server's text, as PostgreSQL's editor does, leaves its type to the catalog; a key +// or a copied value has to bind as that type. A value any other driver read is kept as it was stored. +pub fn value_for_column(value: &Value, read: &ColumnType, column: &ColumnType) -> Option { + // MySQL's editor reads TIMESTAMP in the session's zone, which names no instant the column's offset binding takes. + if matches!( + (value, &column.kind), + ( + Value::Timestamp(_), + ColumnKind::Timestamp { + zone: TemporalZone::Offset, + .. + } + ) + ) { + return None; + } + let Value::Text(text) = value else { + return Some(value.clone()); + }; + if !matches!(read.kind, ColumnKind::Other) || read.read_form != ReadForm::ServerText { + return Some(value.clone()); + } + // The session's extra_float_digits, DateStyle, IntervalStyle and lc_monetary shape how floats, times and money read, + // so their text may not name the stored value; only kinds whose text is exact are read back. + match &column.kind { + ColumnKind::Text { .. } | ColumnKind::Enum { .. } | ColumnKind::Other => Some(value.clone()), + ColumnKind::Bytes { .. } => postgres_bytea(text).map(Value::Bytes), + ColumnKind::Integer(_) + | ColumnKind::Decimal { .. } + | ColumnKind::Bool(_) + | ColumnKind::Uuid + | ColumnKind::Json { .. } => parse_literal_text(text, column).ok(), + _ => None, + } +} + +fn postgres_bytea(text: &str) -> Option> { + let (pairs, odd) = text.strip_prefix("\\x")?.as_bytes().as_chunks::<2>(); + if !odd.is_empty() { + return None; + } + let digit = |byte: u8| char::from(byte).to_digit(16); + pairs + .iter() + .map(|[high, low]| u8::try_from(digit(*high)? * 16 + digit(*low)?).ok()) + .collect() +} + +fn names_table(statement: &TableRef, origin: &TableRef, grammar: SqlGrammar) -> bool { + if !same_name(&statement.name, &origin.name, grammar) { + return false; + } + match (&statement.schema, &origin.schema) { + (None, _) => true, + (Some(written), Some(resolved)) => same_name(written, resolved, grammar), + (Some(written), None) => { + matches!(grammar, SqlGrammar::Sqlite | SqlGrammar::DuckDb) && written.eq_ignore_ascii_case("main") + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::column::{ColumnDefault, IntegerKind, IntegerWidth, ReadForm, TextLength}; + + fn typed(name: &str, kind: ColumnKind) -> ColumnType { + ColumnType { + name: name.to_owned(), + kind, + catalog: None, + dynamic_storage: false, + read_form: ReadForm::Native, + } + } + + fn int() -> ColumnType { + typed( + "integer", + ColumnKind::Integer(IntegerKind { + width: IntegerWidth::W32, + signed: true, + }), + ) + } + + fn text() -> ColumnType { + typed( + "text", + ColumnKind::Text { + length: TextLength::Unbounded, + }, + ) + } + + fn table() -> TableRef { + TableRef::new(Some("public".to_owned()), "t") + } + + fn traced(name: &str, column: &str) -> ResultColumn { + ResultColumn::new(name, text()).with_origin(Some(ColumnOrigin::new(table(), column))) + } + + fn untraced(name: &str) -> ResultColumn { + ResultColumn::new(name, text()) + } + + fn info(name: &str, primary_key: bool) -> ColumnInfo { + ColumnInfo { + name: name.to_owned(), + column_type: text(), + nullable: !primary_key, + primary_key, + is_auto_increment: false, + is_generated: false, + partition_scoped: false, + default: ColumnDefault::None, + comment: None, + } + } + + fn catalog() -> Vec { + vec![info("id", true), info("a", false), info("b", false)] + } + + fn target(sql: &str, result: &[ResultColumn]) -> Result { + let source = result_source(sql, SqlGrammar::PostgreSql, result)?; + result_target(&source, result, &catalog()) + } + + #[test] + fn a_whole_table_maps_every_column() { + let result = [traced("id", "id"), traced("a", "a"), traced("b", "b")]; + + let found = target("SELECT * FROM t", &result).expect("editable"); + + assert_eq!(found.table, table()); + assert_eq!(found.positions, vec![Some(0), Some(1), Some(2)]); + } + + #[test] + fn a_renamed_column_writes_its_own_column_and_an_expression_locks() { + let result = [traced("x", "a"), traced("id", "id"), untraced("?column?")]; + + let found = target("SELECT a AS x, id, b + 1 FROM t", &result).expect("editable"); + + assert_eq!(found.positions, vec![Some(1), Some(0), None]); + } + + #[test] + fn a_column_shown_twice_is_written_once() { + let result = [traced("id", "id"), traced("a", "a"), traced("a", "a")]; + + let found = target("SELECT id, a, a FROM t", &result).expect("editable"); + + assert_eq!(found.positions, vec![Some(0), Some(1), None]); + } + + #[test] + fn rows_without_their_whole_key_are_locked() { + let result = [traced("a", "a")]; + + assert_eq!(target("SELECT a FROM t", &result), Err(ResultLock::KeyNotShown)); + let keyless = vec![info("a", false)]; + let source = result_source("SELECT a FROM t", SqlGrammar::PostgreSql, &result).expect("traced"); + assert_eq!(result_target(&source, &result, &keyless), Err(ResultLock::NoPrimaryKey)); + } + + #[test] + fn a_result_without_origins_is_untraced() { + let result = [untraced("id"), untraced("a")]; + + assert_eq!( + result_source("SELECT id, a FROM t", SqlGrammar::PostgreSql, &result), + Err(ResultLock::Untraced) + ); + } + + #[test] + fn origins_that_name_another_table_than_the_statement_are_untraced() { + let through_view = [traced("id", "id"), traced("a", "a")]; + assert_eq!( + result_source("SELECT * FROM v", SqlGrammar::Sqlite, &through_view), + Err(ResultLock::Untraced) + ); + let mixed = [ + traced("id", "id"), + ResultColumn::new("c", text()).with_origin(Some(ColumnOrigin::new(TableRef::unqualified("u"), "c"))), + ]; + assert_eq!( + result_source("SELECT * FROM t", SqlGrammar::PostgreSql, &mixed), + Err(ResultLock::Untraced) + ); + assert_eq!( + result_source("SELECT * FROM other.t", SqlGrammar::PostgreSql, &through_view), + Err(ResultLock::Untraced) + ); + } + + #[test] + fn the_statement_refuses_before_the_origins_are_read() { + let result = [traced("id", "id")]; + + for (sql, lock) in [ + ("SELECT t.id FROM t JOIN u ON u.id = t.id", ResultLock::MoreThanOneTable), + ("SELECT DISTINCT id FROM t", ResultLock::Collapsed), + ("SELECT id FROM t; SELECT 1", ResultLock::NotOneSelect), + ] { + assert_eq!(result_source(sql, SqlGrammar::PostgreSql, &result), Err(lock), "{sql}"); + } + } + + #[test] + fn a_schema_the_statement_leaves_out_is_the_one_the_server_resolved() { + let result = [traced("id", "id")]; + + assert!(result_source("SELECT id FROM public.t", SqlGrammar::PostgreSql, &result).is_ok()); + assert!(result_source("SELECT id FROM t", SqlGrammar::PostgreSql, &result).is_ok()); + let main = + [ResultColumn::new("id", text()).with_origin(Some(ColumnOrigin::new(TableRef::unqualified("t"), "id")))]; + assert!(result_source("SELECT id FROM main.t", SqlGrammar::Sqlite, &main).is_ok()); + assert!(result_source("SELECT id FROM other.t", SqlGrammar::Sqlite, &main).is_err()); + } + + #[test] + fn a_table_changed_since_the_run_is_noticed() { + let result = [traced("id", "id"), traced("a", "a"), traced("b", "b")]; + let source = result_source("SELECT * FROM t", SqlGrammar::PostgreSql, &result).expect("traced"); + + let renamed = vec![info("id", true), info("renamed", false), info("b", false)]; + + assert_eq!( + result_target(&source, &result, &renamed), + Err(ResultLock::ColumnsChanged) + ); + } + + #[test] + fn an_origin_that_disagrees_with_the_statement_is_untraced() { + let result = [traced("id", "a"), traced("a", "id")]; + + assert_eq!(target("SELECT id, a FROM t", &result), Err(ResultLock::Untraced)); + let swapped = [traced("a", "id"), traced("id", "a")]; + assert_eq!( + target("SELECT id AS a, a AS id FROM t", &swapped).map(|found| found.positions), + Ok(vec![Some(0), Some(1)]) + ); + } + + #[test] + fn parsed_origins_spread_a_wildcard_over_the_columns_it_read() { + let statement = source_table("SELECT a AS x, *, b + 1 FROM t", SqlGrammar::DuckDb).expect("one table"); + let names = ["x", "id", "a", "b", "next"].map(str::to_owned); + + let origins = parsed_origins(&statement, &table(), &names); + + let columns: Vec> = origins + .iter() + .map(|origin| origin.as_ref().map(|origin| origin.column.as_str())) + .collect(); + assert_eq!(columns, vec![Some("a"), Some("id"), Some("a"), Some("b"), None]); + let unsplit = source_table("SELECT *, * FROM t", SqlGrammar::DuckDb).expect("one table"); + assert_eq!(parsed_origins(&unsplit, &table(), &names), vec![None; 5]); + } + + #[test] + fn server_text_becomes_the_columns_type() { + let read = ColumnType::untyped_text("text"); + assert_eq!( + value_for_column(&Value::Text("42".to_owned()), &read, &int()), + Some(Value::Int(42)) + ); + assert_eq!( + value_for_column(&Value::Text("x".to_owned()), &read, &text()), + Some(Value::Text("x".to_owned())) + ); + assert_eq!(value_for_column(&Value::Text("forty".to_owned()), &read, &int()), None); + let bytes = typed( + "bytea", + ColumnKind::Bytes { + length: TextLength::Unbounded, + }, + ); + assert_eq!( + value_for_column(&Value::Text("\\x01ff".to_owned()), &read, &bytes), + Some(Value::Bytes(vec![1, 255])) + ); + assert_eq!(value_for_column(&Value::Int(7), &read, &int()), Some(Value::Int(7))); + } + + #[test] + fn text_a_session_setting_can_round_is_not_read_back() { + let read = ColumnType::untyped_text("text"); + let float = typed("double precision", ColumnKind::Float(crate::column::FloatWidth::F64)); + let date = typed("date", ColumnKind::Date); + + assert_eq!(value_for_column(&Value::Text("1".to_owned()), &read, &float), None); + assert_eq!( + value_for_column(&Value::Text("05/11/2026".to_owned()), &read, &date), + None + ); + } + + #[test] + fn a_wall_clock_time_is_not_a_value_of_an_offset_column() { + let stamp = typed( + "timestamp", + ColumnKind::Timestamp { + zone: TemporalZone::Offset, + fractional_digits: None, + }, + ); + let naive = Value::Timestamp(crate::value::Temporal::Finite( + chrono::NaiveDate::from_ymd_opt(2026, 11, 5) + .and_then(|date| date.and_hms_opt(12, 0, 0)) + .expect("a time"), + )); + + assert_eq!(value_for_column(&naive, &stamp, &stamp), None); + } + + #[test] + fn text_a_driver_read_as_stored_is_never_reparsed() { + let date = typed("DATE", ColumnKind::Date); + let padded = Value::Text(" 2024-01-01 ".to_owned()); + + assert_eq!(value_for_column(&padded, &date, &date), Some(padded.clone())); + assert_eq!(value_for_column(&padded, &text(), &date), Some(padded)); + } +} diff --git a/core/crates/core/src/sql_syntax/mod.rs b/core/crates/core/src/sql_syntax/mod.rs index 8c63f6e1c8..ae9dbe4dfc 100644 --- a/core/crates/core/src/sql_syntax/mod.rs +++ b/core/crates/core/src/sql_syntax/mod.rs @@ -16,6 +16,7 @@ pub mod redis; mod reserved_words; pub mod script; mod sort_reference; +mod source_table; mod state_changing_functions; mod statement_access; mod statement_tokens; @@ -33,4 +34,7 @@ pub use order_by::with_order_by; pub use parse_expression::{MAX_EXPRESSION_LEN, ValidatedExpression, parse_expression}; pub use parse_type::{MAX_TYPE_LEN, TypeShape, ValidatedType, parse_type}; pub use sort_reference::{SortReference, sort_reference}; +pub use source_table::{ + SourceItem, SourceName, SourceRefusal, SourceTable, same_name, source_columns_for_result, source_table, +}; pub use statement_access::{StatementAccess, sqlite_pragma_reports, statement_access}; diff --git a/core/crates/core/src/sql_syntax/source_table.rs b/core/crates/core/src/sql_syntax/source_table.rs new file mode 100644 index 0000000000..dee43f5925 --- /dev/null +++ b/core/crates/core/src/sql_syntax/source_table.rs @@ -0,0 +1,799 @@ +use sqlparser::ast::{ + Expr, GroupByExpr, Ident, ObjectName, ObjectNamePart, OrderByKind, Query, Select, SelectFlavor, SelectItem, + SelectItemQualifiedWildcardKind, SetExpr, Statement, TableFactor, WildcardAdditionalOptions, +}; +use sqlparser::parser::Parser; + +use super::SqlGrammar; +use super::parse_type::RECURSION_LIMIT; +use super::script::{LexicalSettings, ScriptPlan, ScriptTokenKind}; +use crate::column::ColumnInfo; +use crate::meta::TableRef; + +// Editing needs a table read row for row; shapes whose identity cannot be proved are refused. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SourceTable { + pub table: TableRef, + pub items: Vec, + grammar: SqlGrammar, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SourceItem { + Column(SourceName), + Wildcard, + Expression, +} + +// Unquoted names are resolved using the grammar before matching catalog columns. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SourceName { + pub name: String, + pub quoted: bool, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum SourceRefusal { + NotOneSelect, + MoreThanOneTable, + Collapsed, +} + +pub fn source_table(sql: &str, grammar: SqlGrammar) -> Result { + let query = parse_query(sql, grammar)?; + single_table_query(&query, grammar) +} + +fn parse_query(sql: &str, grammar: SqlGrammar) -> Result, SourceRefusal> { + let dialect = grammar.parser_dialect().ok_or(SourceRefusal::NotOneSelect)?; + let plan = ScriptPlan::build(sql, grammar, LexicalSettings::default_for(grammar)); + if !plan.diagnostics().is_empty() + || plan.tokens(0..sql.len()).any(|token| { + matches!( + token.kind, + ScriptTokenKind::ExecutableMarker | ScriptTokenKind::Directive | ScriptTokenKind::Unterminated + ) + }) + { + return Err(SourceRefusal::NotOneSelect); + } + let mut statements = Parser::new(dialect) + .with_recursion_limit(RECURSION_LIMIT) + .try_with_sql(sql) + .and_then(|mut parser| parser.parse_statements()) + .map_err(|_| SourceRefusal::NotOneSelect)? + .into_iter(); + let Some(Statement::Query(query)) = statements.next() else { + return Err(SourceRefusal::NotOneSelect); + }; + match statements.next() { + None => Ok(query), + Some(_) => Err(SourceRefusal::NotOneSelect), + } +} + +fn plain_select(query: &Query) -> Result<&Select, SourceRefusal> { + if query.with.is_some() { + return Err(SourceRefusal::MoreThanOneTable); + } + if query.for_clause.is_some() || query.format_clause.is_some() || !query.pipe_operators.is_empty() { + return Err(SourceRefusal::NotOneSelect); + } + if let Some(order) = &query.order_by { + let OrderByKind::Expressions(expressions) = &order.kind else { + return Err(SourceRefusal::Collapsed); + }; + if order.interpolate.is_some() + || expressions + .iter() + .any(|item| item.with_fill.is_some() || !row_expression(&item.expr)) + { + return Err(SourceRefusal::Collapsed); + } + } + let select = match query.body.as_ref() { + SetExpr::Select(select) => select, + SetExpr::SetOperation { .. } | SetExpr::Query(_) => return Err(SourceRefusal::MoreThanOneTable), + _ => return Err(SourceRefusal::NotOneSelect), + }; + let plain_grouping = matches!(&select.group_by, GroupByExpr::Expressions(expressions, modifiers) + if expressions.is_empty() && modifiers.is_empty()); + if select.distinct.is_some() || !plain_grouping || select.having.is_some() || select.qualify.is_some() { + return Err(SourceRefusal::Collapsed); + } + if select.into.is_some() + || select.exclude.is_some() + || !select.lateral_views.is_empty() + || !select.connect_by.is_empty() + || select.value_table_mode.is_some() + || select.flavor != SelectFlavor::Standard + { + return Err(SourceRefusal::NotOneSelect); + } + Ok(select) +} + +fn single_table_query(query: &Query, grammar: SqlGrammar) -> Result { + let select = plain_select(query)?; + let [from] = select.from.as_slice() else { + return Err(match select.from.is_empty() { + true => SourceRefusal::NotOneSelect, + false => SourceRefusal::MoreThanOneTable, + }); + }; + if !from.joins.is_empty() { + return Err(SourceRefusal::MoreThanOneTable); + } + let TableFactor::Table { + name, + alias, + args: None, + version: None, + with_ordinality: false, + partitions, + json_path: None, + sample: None, + .. + } = &from.relation + else { + return Err(SourceRefusal::MoreThanOneTable); + }; + if !partitions.is_empty() || alias.as_ref().is_some_and(|alias| !alias.columns.is_empty()) { + return Err(SourceRefusal::MoreThanOneTable); + } + let parts = identifiers(name).ok_or(SourceRefusal::MoreThanOneTable)?; + let (schema, table) = match parts.as_slice() { + [table] => (None, (*table).clone()), + [schema, table] => (Some((*schema).clone()), (*table).clone()), + _ => return Err(SourceRefusal::MoreThanOneTable), + }; + let qualifiers: Vec<&Ident> = alias.as_ref().map_or_else(|| vec![&table], |alias| vec![&alias.name]); + let items = select + .projection + .iter() + .map(|item| source_item(item, &qualifiers, schema.as_ref(), grammar)) + .collect::, _>>()?; + if grammar == SqlGrammar::ClickHouse && alias_shadows_a_column(&select.projection, &items, grammar) { + return Err(SourceRefusal::Collapsed); + } + Ok(SourceTable { + table: TableRef::new( + schema.map(|ident| resolved_name(&ident, grammar)), + resolved_name(&table, grammar), + ), + items, + grammar, + }) +} + +fn identifiers(name: &ObjectName) -> Option> { + name.0 + .iter() + .map(|part| match part { + ObjectNamePart::Identifier(ident) => Some(ident), + ObjectNamePart::Function(_) => None, + }) + .collect() +} + +// A `*` of another relation would put columns the table does not have in the grid, so it refuses the statement. +fn source_item( + item: &SelectItem, + qualifiers: &[&Ident], + schema: Option<&Ident>, + grammar: SqlGrammar, +) -> Result { + match item { + SelectItem::UnnamedExpr(expr) | SelectItem::ExprWithAlias { expr, .. } => match row_expression(expr) { + true => Ok(column(expr, qualifiers, schema, grammar)), + false => Err(SourceRefusal::Collapsed), + }, + SelectItem::ExprWithAliases { .. } => Err(SourceRefusal::Collapsed), + SelectItem::Wildcard(options) => match plain_wildcard(options) { + true => Ok(SourceItem::Wildcard), + false => Err(SourceRefusal::NotOneSelect), + }, + SelectItem::QualifiedWildcard(SelectItemQualifiedWildcardKind::ObjectName(name), options) => { + let parts = identifiers(name).ok_or(SourceRefusal::MoreThanOneTable)?; + match ( + plain_wildcard(options), + names_the_table(&parts, qualifiers, schema, grammar), + ) { + (true, true) => Ok(SourceItem::Wildcard), + (false, _) => Err(SourceRefusal::NotOneSelect), + (true, false) => Err(SourceRefusal::MoreThanOneTable), + } + } + SelectItem::QualifiedWildcard(SelectItemQualifiedWildcardKind::Expr(_), _) => { + Err(SourceRefusal::MoreThanOneTable) + } + } +} + +fn column(expr: &Expr, qualifiers: &[&Ident], schema: Option<&Ident>, grammar: SqlGrammar) -> SourceItem { + let (qualifier, name): (&[Ident], &Ident) = match expr { + Expr::Identifier(ident) => (&[], ident), + Expr::CompoundIdentifier(parts) => match parts.split_last() { + Some((name, qualifier)) => (qualifier, name), + None => return SourceItem::Expression, + }, + Expr::Nested(inner) => return column(inner, qualifiers, schema, grammar), + _ => return SourceItem::Expression, + }; + let qualifier: Vec<&Ident> = qualifier.iter().collect(); + if !qualifier.is_empty() && !names_the_table(&qualifier, qualifiers, schema, grammar) { + return SourceItem::Expression; + } + if qualifier.is_empty() && name.quote_style.is_none() && value_keyword(&name.value, grammar) { + return SourceItem::Expression; + } + SourceItem::Column(SourceName { + name: resolved_name(name, grammar), + quoted: name.quote_style.is_some(), + }) +} + +// ClickHouse substitutes a select-list alias for a column of the same name anywhere in the query, so `2 AS id, id AS k` +// reads 2 into k. +fn alias_shadows_a_column(projection: &[SelectItem], items: &[SourceItem], grammar: SqlGrammar) -> bool { + projection.iter().enumerate().any(|(index, item)| { + let SelectItem::ExprWithAlias { alias, .. } = item else { + return false; + }; + let alias = resolved_name(alias, grammar); + items + .iter() + .enumerate() + .any(|(other, item)| other != index && matches!(item, SourceItem::Column(column) if column.name == alias)) + }) +} + +// PostgreSQL reads these bare words as functions even where the table has a column of that name. +fn value_keyword(name: &str, grammar: SqlGrammar) -> bool { + const POSTGRES: [&str; 12] = [ + "current_catalog", + "current_date", + "current_role", + "current_schema", + "current_time", + "current_timestamp", + "current_user", + "localtime", + "localtimestamp", + "session_user", + "system_user", + "user", + ]; + grammar == SqlGrammar::PostgreSql && POSTGRES.iter().any(|keyword| keyword.eq_ignore_ascii_case(name)) +} + +fn names_the_table(parts: &[&Ident], qualifiers: &[&Ident], schema: Option<&Ident>, grammar: SqlGrammar) -> bool { + match parts { + [name] => qualifiers.iter().any(|qualifier| same_ident(qualifier, name, grammar)), + [part_schema, name] => { + schema.is_some_and(|schema| same_ident(schema, part_schema, grammar)) + && qualifiers.iter().any(|qualifier| same_ident(qualifier, name, grammar)) + } + _ => false, + } +} + +fn resolved_name(ident: &Ident, grammar: SqlGrammar) -> String { + if ident.quote_style.is_none() && matches!(grammar, SqlGrammar::PostgreSql | SqlGrammar::Cql) { + ident.value.to_ascii_lowercase() + } else { + ident.value.clone() + } +} + +// Names the server already resolved: an engine that folds case or compares by collation finds either spelling. +pub fn same_name(left: &str, right: &str, grammar: SqlGrammar) -> bool { + match grammar { + SqlGrammar::MySql | SqlGrammar::Sqlite | SqlGrammar::DuckDb | SqlGrammar::MsSql => { + left.eq_ignore_ascii_case(right) + } + SqlGrammar::PostgreSql | SqlGrammar::ClickHouse | SqlGrammar::Cql | SqlGrammar::Redis | SqlGrammar::Mongo => { + left == right + } + } +} + +fn same_ident(left: &Ident, right: &Ident, grammar: SqlGrammar) -> bool { + let (left, right) = (resolved_name(left, grammar), resolved_name(right, grammar)); + match grammar { + SqlGrammar::Sqlite | SqlGrammar::DuckDb => left.eq_ignore_ascii_case(&right), + _ => left == right, + } +} + +// A function may aggregate or return several rows, including one whose name looks scalar. +fn row_expression(expr: &Expr) -> bool { + match expr { + Expr::Identifier(_) | Expr::CompoundIdentifier(_) | Expr::Value(_) | Expr::TypedString(_) => true, + Expr::Nested(expr) + | Expr::UnaryOp { expr, .. } + | Expr::IsFalse(expr) + | Expr::IsNotFalse(expr) + | Expr::IsTrue(expr) + | Expr::IsNotTrue(expr) + | Expr::IsNull(expr) + | Expr::IsNotNull(expr) + | Expr::IsUnknown(expr) + | Expr::IsNotUnknown(expr) => row_expression(expr), + Expr::BinaryOp { left, right, .. } + | Expr::IsDistinctFrom(left, right) + | Expr::IsNotDistinctFrom(left, right) => row_expression(left) && row_expression(right), + Expr::Between { expr, low, high, .. } => row_expression(expr) && row_expression(low) && row_expression(high), + Expr::InList { expr, list, .. } => row_expression(expr) && list.iter().all(row_expression), + Expr::Case { + operand, + conditions, + else_result, + .. + } => { + operand.as_deref().is_none_or(row_expression) + && conditions + .iter() + .all(|branch| row_expression(&branch.condition) && row_expression(&branch.result)) + && else_result.as_deref().is_none_or(row_expression) + } + _ => false, + } +} + +// REPLACE, RENAME, EXCLUDE and the like change which columns a `*` stands for. +fn plain_wildcard(options: &WildcardAdditionalOptions) -> bool { + options.opt_ilike.is_none() + && options.opt_exclude.is_none() + && options.opt_except.is_none() + && options.opt_replace.is_none() + && options.opt_rename.is_none() + && options.opt_alias.is_none() +} + +// A wildcard reads the columns a `*` shows, which leaves out MySQL's INVISIBLE and ClickHouse's MATERIALIZED and ALIAS +// ones, so it maps by the exact names it read; a name the catalog no longer has means the table changed since. +pub fn source_columns_for_result( + source: &SourceTable, + columns: &[ColumnInfo], + result_names: &[String], +) -> Option>> { + let wildcards = source + .items + .iter() + .filter(|item| matches!(item, SourceItem::Wildcard)) + .count(); + let spread = result_names.len().checked_sub(source.items.len() - wildcards)?; + let per_wildcard = match wildcards { + 0 if spread == 0 => 0, + 0 => return None, + _ if spread % wildcards == 0 => spread / wildcards, + _ => return None, + }; + let mut mapped = Vec::with_capacity(result_names.len()); + for item in &source.items { + match item { + SourceItem::Wildcard => { + let names = result_names.get(mapped.len()..mapped.len() + per_wildcard)?; + for name in names { + let mut named = columns.iter().enumerate().filter(|(_, column)| column.name == *name); + match (named.next(), named.next()) { + (Some((position, _)), None) => mapped.push(Some(position)), + _ => return None, + } + } + } + SourceItem::Expression => mapped.push(None), + // A name the table lacks may be a word the engine reads as a function; the driver's origin says which. + SourceItem::Column(name) => mapped.push(column_position(columns, name, source.grammar)), + } + } + Some(mapped) +} + +// SQL Server's catalog collation is not available here, so only exact names can be proved. +fn column_position(columns: &[ColumnInfo], name: &SourceName, grammar: SqlGrammar) -> Option { + let insensitive = matches!(grammar, SqlGrammar::MySql | SqlGrammar::Sqlite | SqlGrammar::DuckDb); + let mut matches = columns.iter().enumerate().filter(|(_, column)| { + if insensitive { + column.name.eq_ignore_ascii_case(&name.name) + } else { + column.name == name.name + } + }); + match (matches.next(), matches.next()) { + (Some((position, _)), None) => Some(position), + _ => None, + } +} + +#[cfg(test)] +mod tests { + use crate::column::{ColumnDefault, ColumnType}; + + use super::*; + + fn source(sql: &str) -> Option { + source_table(sql, SqlGrammar::PostgreSql).ok() + } + + fn named(name: &str) -> SourceItem { + SourceItem::Column(SourceName { + name: name.to_owned(), + quoted: false, + }) + } + + fn source_columns(source: &SourceTable, columns: &[ColumnInfo], width: usize) -> Option>> { + let names: Vec = (0..width).map(|index| format!("column {index}")).collect(); + source_columns_for_result(source, columns, &names) + } + + fn info(name: &str) -> ColumnInfo { + ColumnInfo { + name: name.to_owned(), + column_type: ColumnType::untyped_text("text"), + nullable: true, + primary_key: false, + is_auto_increment: false, + is_generated: false, + partition_scoped: false, + default: ColumnDefault::None, + comment: None, + } + } + + #[test] + fn a_select_from_one_table_names_it_and_its_columns() { + let found = source("SELECT id, name AS who, 1 + 1 FROM people WHERE id > 1 ORDER BY name LIMIT 10"); + + assert_eq!( + found, + Some(SourceTable { + table: TableRef::unqualified("people"), + items: vec![named("id"), named("name"), SourceItem::Expression], + grammar: SqlGrammar::PostgreSql, + }) + ); + } + + #[test] + fn a_schema_and_an_alias_qualify_the_same_table() { + let found = source("SELECT p.id, app.people.note, p.* FROM app.people AS p").expect("one table"); + + assert_eq!(found.table, TableRef::new(Some("app".to_owned()), "people")); + assert_eq!( + found.items, + vec![named("id"), SourceItem::Expression, SourceItem::Wildcard] + ); + assert_eq!( + source("SELECT people.id FROM app.people").map(|found| found.items), + Some(vec![named("id")]) + ); + assert_eq!( + source("SELECT other.id FROM people").map(|found| found.items), + Some(vec![SourceItem::Expression]), + "a column of another relation" + ); + } + + #[test] + fn every_shape_that_reads_more_than_one_table_row_for_row_has_none() { + for sql in [ + "SELECT * FROM a JOIN b ON a.id = b.id", + "SELECT * FROM a, b", + "SELECT * FROM (SELECT * FROM a) AS s", + "WITH s AS (SELECT * FROM a) SELECT * FROM s", + "SELECT * FROM a UNION SELECT * FROM b", + "SELECT DISTINCT name FROM a", + "SELECT name, count(*) FROM a GROUP BY name", + "SELECT id FROM a HAVING id > 1", + "SELECT 1", + "SELECT * FROM generate_series(1, 3)", + "SELECT * FROM a AS t(x, y)", + "SELECT * FROM a; SELECT * FROM b", + "UPDATE a SET id = 1", + "SELECT b.* FROM a", + "SELECT * FROM cat.app.people", + "SELEC * FROM a", + ] { + assert_eq!(source(sql), None, "{sql}"); + } + } + + #[test] + fn each_refusal_names_why_the_rows_are_not_one_tables() { + let refusal = |sql| source_table(sql, SqlGrammar::PostgreSql).err(); + for sql in [ + "SELECT * FROM a JOIN b ON a.id = b.id", + "SELECT * FROM a, b", + "SELECT * FROM (SELECT * FROM a) AS s", + "WITH s AS (SELECT * FROM a) SELECT * FROM s", + "SELECT * FROM a UNION SELECT * FROM b", + "SELECT * FROM generate_series(1, 3)", + "SELECT b.* FROM a", + ] { + assert_eq!(refusal(sql), Some(SourceRefusal::MoreThanOneTable), "{sql}"); + } + for sql in [ + "SELECT DISTINCT name FROM a", + "SELECT name, count(*) FROM a GROUP BY name", + "SELECT id, upper(name) FROM a", + ] { + assert_eq!(refusal(sql), Some(SourceRefusal::Collapsed), "{sql}"); + } + for sql in [ + "SELECT 1", + "UPDATE a SET id = 1", + "SELECT * FROM a; SELECT * FROM b", + "SELEC * FROM a", + ] { + assert_eq!(refusal(sql), Some(SourceRefusal::NotOneSelect), "{sql}"); + } + } + + #[test] + fn words_postgresql_reads_as_functions_are_expressions() { + let found = source("SELECT id, current_role, \"user\", CURRENT_SCHEMA FROM people").expect("one table"); + + assert_eq!( + found.items, + vec![ + named("id"), + SourceItem::Expression, + SourceItem::Column(SourceName { + name: "user".to_owned(), + quoted: true, + }), + SourceItem::Expression, + ] + ); + } + + #[test] + fn a_clickhouse_alias_that_names_another_items_column_refuses_the_read() { + let clickhouse = |sql| source_table(sql, SqlGrammar::ClickHouse); + + assert_eq!( + clickhouse("SELECT 2 AS id, id AS k, a FROM t"), + Err(SourceRefusal::Collapsed) + ); + assert!(clickhouse("SELECT id AS id, a AS x FROM t").is_ok()); + assert!(source_table("SELECT 2 AS id, id AS k FROM t", SqlGrammar::PostgreSql).is_ok()); + } + + #[test] + fn a_locking_read_still_reads_one_table() { + assert!(source("SELECT * FROM a WHERE id = 1 FOR UPDATE").is_some()); + assert!(source_table("SELECT TOP 5 * FROM a", SqlGrammar::MsSql).is_ok()); + assert_eq!( + source_table("SELECT * FROM a FOR JSON AUTO", SqlGrammar::MsSql), + Err(SourceRefusal::NotOneSelect) + ); + } + + #[test] + fn engines_without_sql_have_no_source() { + assert_eq!( + source_table("SELECT * FROM a", SqlGrammar::Redis), + Err(SourceRefusal::NotOneSelect) + ); + assert_eq!( + source_table("db.a.find()", SqlGrammar::Mongo), + Err(SourceRefusal::NotOneSelect) + ); + } + + #[test] + fn a_wildcard_maps_the_columns_it_read_by_name() { + let columns = [info("id"), info("name"), info("hidden"), info("note")]; + let star = source("SELECT *, 1 + 1 FROM people").expect("one table"); + let picked = source("SELECT NAME, \"id\" FROM people").expect("one table"); + let read = ["id", "name", "note", "?column?"].map(str::to_owned); + + assert_eq!( + source_columns_for_result(&star, &columns, &read), + Some(vec![Some(0), Some(1), Some(3), None]), + "a column a star leaves out is skipped" + ); + let renamed = ["id", "label", "note", "?column?"].map(str::to_owned); + assert_eq!( + source_columns_for_result(&star, &columns, &renamed), + None, + "the table changed since" + ); + assert_eq!(source_columns(&picked, &columns, 2), Some(vec![Some(1), Some(0)])); + } + + #[test] + fn a_quoted_name_matches_only_as_spelled() { + let columns = [info("Name"), info("name")]; + let quoted = source("SELECT \"Name\" FROM people").expect("one table"); + let folded = source("SELECT NAME FROM people").expect("one table"); + let missing = source("SELECT \"NAME\" FROM people").expect("one table"); + + assert_eq!(source_columns("ed, &columns, 1), Some(vec![Some(0)])); + assert_eq!( + source_columns(&folded, &columns, 1), + Some(vec![Some(1)]), + "unquoted names fold before lookup" + ); + assert_eq!(source_columns(&missing, &columns, 1), Some(vec![None])); + } + + #[test] + fn unquoted_identifiers_fold_before_catalog_lookup() { + for grammar in [SqlGrammar::PostgreSql, SqlGrammar::Cql] { + let found = source_table("SELECT Name FROM App.People", grammar).expect("one table"); + assert_eq!(found.table, TableRef::new(Some("app".to_owned()), "people")); + assert_eq!( + source_columns(&found, &[info("Name"), info("name")], 1), + Some(vec![Some(1)]) + ); + assert_eq!(source_columns(&found, &[info("Name")], 1), Some(vec![None])); + + let quoted = source_table("SELECT \"Name\" FROM \"App\".\"People\"", grammar).expect("one table"); + assert_eq!(quoted.table, TableRef::new(Some("App".to_owned()), "People")); + assert_eq!( + source_columns("ed, &[info("Name"), info("name")], 1), + Some(vec![Some(0)]) + ); + } + } + + #[test] + fn qualifiers_follow_the_same_folding_as_their_declarations() { + let found = source("SELECT \"p\".Name FROM People AS P").expect("one table"); + assert_eq!(found.items, vec![named("name")]); + let found = source("SELECT P.Name FROM People AS \"P\"").expect("one table"); + assert_eq!(found.items, vec![SourceItem::Expression]); + } + + #[test] + fn case_insensitive_columns_match_even_when_quoted() { + for grammar in [SqlGrammar::Sqlite, SqlGrammar::DuckDb] { + let found = source_table("SELECT \"NAME\" FROM people", grammar).expect("one table"); + assert_eq!(source_columns(&found, &[info("name")], 1), Some(vec![Some(0)])); + assert_eq!( + source_columns(&found, &[info("name"), info("Name")], 1), + Some(vec![None]) + ); + } + let found = source_table("SELECT `NAME` FROM people", SqlGrammar::MySql).expect("one table"); + assert_eq!(source_columns(&found, &[info("name")], 1), Some(vec![Some(0)])); + } + + #[test] + fn case_sensitive_and_unknown_collations_never_guess() { + for grammar in [SqlGrammar::ClickHouse, SqlGrammar::MsSql] { + let found = source_table("SELECT Name FROM people", grammar).expect("one table"); + assert_eq!(source_columns(&found, &[info("name")], 1), Some(vec![None])); + assert_eq!( + source_columns(&found, &[info("Name"), info("name")], 1), + Some(vec![Some(0)]) + ); + } + } + + #[test] + fn expressions_that_may_collapse_or_expand_rows_are_refused() { + for grammar in [SqlGrammar::Sqlite, SqlGrammar::PostgreSql, SqlGrammar::MySql] { + for sql in [ + "SELECT id, count(*) FROM people", + "SELECT id, 1 + count(*) FROM people", + "SELECT id, CASE WHEN id = 1 THEN count(*) ELSE 0 END FROM people", + "SELECT id, custom_aggregate(name) FROM people", + "SELECT id, generate_series(1, 2) FROM people", + "SELECT id, upper(name) FROM people", + "SELECT id, (SELECT count(*) FROM people) FROM people", + "SELECT id FROM people ORDER BY count(*)", + "SELECT id, CAST(id AS custom_type) FROM people", + ] { + assert_eq!( + source_table(sql, grammar), + Err(SourceRefusal::Collapsed), + "{grammar:?}: {sql}" + ); + } + } + } + + #[test] + fn scalar_operators_do_not_change_row_identity() { + for sql in [ + "SELECT id, name, 1 + 1 FROM people", + "SELECT id, CASE WHEN id = 1 THEN 'one' ELSE 'other' END FROM people", + "SELECT id, -id FROM people", + "SELECT id, name IS NULL FROM people", + ] { + let found = source(sql).expect("scalar expressions preserve rows"); + assert_eq!(found.items.last(), Some(&SourceItem::Expression)); + } + } + + #[test] + fn gap_filling_cannot_invent_editable_rows() { + assert_eq!( + source_table("SELECT id FROM people ORDER BY id WITH FILL", SqlGrammar::ClickHouse), + Err(SourceRefusal::Collapsed), + ); + } + + #[test] + fn executable_comments_do_not_hide_a_different_read() { + for sql in [ + "SELECT id /*! , count(*) */ FROM people", + "SELECT id /*M! , count(*) */ FROM people", + "SELECT 1 /*! INTO @x */", + ] { + assert!(source_table(sql, SqlGrammar::MySql).is_err(), "{sql}"); + } + } + + #[test] + fn wildcard_names_reject_same_width_reordering_and_renaming() { + let found = source("SELECT * FROM people").expect("one table"); + let read = ["id", "name", "note"].map(str::to_owned); + + assert_eq!( + source_columns_for_result(&found, &[info("id"), info("name"), info("note")], &read), + Some(vec![Some(0), Some(1), Some(2)]), + ); + assert_eq!( + source_columns_for_result(&found, &[info("name"), info("id"), info("note")], &read), + Some(vec![Some(1), Some(0), Some(2)]), + "a reordered table maps each cell to the column it was read from", + ); + assert_eq!( + source_columns_for_result(&found, &[info("id"), info("label"), info("note")], &read), + None, + "the old name cannot be mapped to a newly named column", + ); + assert_eq!( + source_columns_for_result(&found, &[info("id"), info("Name"), info("note")], &read), + None, + "wildcard headers carry the exact catalog spelling", + ); + } + + #[test] + fn every_wildcard_segment_is_checked_without_restricting_explicit_aliases() { + let found = source("SELECT name AS person, p.*, id + 1 AS next, p.* FROM people AS p").expect("one table"); + let columns = [info("id"), info("name"), info("note")]; + let mut read = ["person", "id", "name", "note", "next", "id", "name", "note"].map(str::to_owned); + assert_eq!( + source_columns_for_result(&found, &columns, &read), + Some(vec![ + Some(1), + Some(0), + Some(1), + Some(2), + None, + Some(0), + Some(1), + Some(2) + ]), + ); + read[0] = "display name".to_owned(); + read[4] = "computed alias".to_owned(); + assert!(source_columns_for_result(&found, &columns, &read).is_some()); + read.swap(5, 6); + assert_eq!( + source_columns_for_result(&found, &columns, &read).map(|mapped| mapped[5..7].to_vec()), + Some(vec![Some(1), Some(0)]), + "each cell maps to the column its header names" + ); + } + + #[test] + fn a_result_without_wildcards_keeps_its_aliases_and_checks_its_width() { + let found = source("SELECT name AS id, id AS name, 1 + 1 AS note FROM people").expect("one table"); + let columns = [info("id"), info("name")]; + let read = ["id", "name", "note"].map(str::to_owned); + assert_eq!( + source_columns_for_result(&found, &columns, &read), + Some(vec![Some(1), Some(0), None]), + ); + assert_eq!(source_columns_for_result(&found, &columns, &read[..2]), None); + } +} diff --git a/core/crates/drivers/cassandra/src/editor.rs b/core/crates/drivers/cassandra/src/editor.rs index 84c6351594..b3cc4a8251 100644 --- a/core/crates/drivers/cassandra/src/editor.rs +++ b/core/crates/drivers/cassandra/src/editor.rs @@ -22,6 +22,7 @@ use tokio::time::Instant; use tokio_util::sync::CancellationToken; use crate::errors::{execution_error, use_keyspace_error}; +use crate::result_origin::SpecTable; use crate::rows::{ColumnsRead, MAX_PAGE_ROWS, next_state, page_size}; use crate::session::{CqlLink, LinkSlot}; @@ -275,9 +276,7 @@ impl Run { }) } - /// One statement, page by page. A page is asked for only once the - /// sink took the last, and once the set passes the row limit no - /// further page is asked for, so the server never reads past it. + // A page is asked for once the sink took the last, and none past the row limit, so the server never reads past it. async fn statement( &self, text: &str, @@ -317,7 +316,14 @@ impl Run { .into_rows_result() .map_err(|error| DriverError::Protocol(error.to_string()))?; if read.is_none() { - let columns = ColumnsRead::of(rows.column_specs()); + let mut columns = ColumnsRead::of(rows.column_specs()); + if let Some(spec) = SpecTable::of(text, rows.column_specs()) { + tokio::select! { + biased; + interrupt = stop.triggered() => return Ok(StatementEnd::Stopped(interrupt.end())), + traced = spec.columns(&self.link, columns.columns.clone()) => columns.columns = traced, + } + } emitter .send(ResultEvent::ResultSetStarted { columns: columns.columns.clone(), diff --git a/core/crates/drivers/cassandra/src/lib.rs b/core/crates/drivers/cassandra/src/lib.rs index 1115f9f927..ce9ec3e153 100644 --- a/core/crates/drivers/cassandra/src/lib.rs +++ b/core/crates/drivers/cassandra/src/lib.rs @@ -16,6 +16,7 @@ mod editor; mod errors; mod pin; mod relay; +mod result_origin; mod rows; mod session; mod stream_read; diff --git a/core/crates/drivers/cassandra/src/result_origin.rs b/core/crates/drivers/cassandra/src/result_origin.rs new file mode 100644 index 0000000000..767d59a0d8 --- /dev/null +++ b/core/crates/drivers/cassandra/src/result_origin.rs @@ -0,0 +1,101 @@ +use std::sync::Arc; + +use scylla::response::query_result::ColumnSpecs; +use tablepro_core::DriverError; +use tablepro_core::column::ResultColumn; +use tablepro_core::edit::parsed_origins; +use tablepro_core::meta::TableRef; +use tablepro_core::sql_syntax::{SourceTable, SqlGrammar, source_table}; + +use crate::catalog::is_system_keyspace; +use crate::session::CqlLink; + +// A materialized view is not in it: the view has a table of its own, which takes no writes. +const BASE_TABLE: &str = "SELECT table_name FROM system_schema.tables WHERE keyspace_name = ? AND table_name = ?"; + +// The RESULT metadata names the keyspace and table every column was read from, but a column by its alias or its +// function's text, so the statement says which columns are the table's own. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct SpecTable { + statement: SourceTable, + table: TableRef, +} + +impl SpecTable { + pub(crate) fn of(sql: &str, specs: ColumnSpecs<'_, '_>) -> Option { + let statement = source_table(sql, SqlGrammar::Cql).ok()?; + let mut tables = specs.iter().map(|spec| spec.table_spec()); + let first = tables.next()?; + if tables.any(|table| table != first) { + return None; + } + let (keyspace, name) = (first.ks_name(), first.table_name()); + if is_system_keyspace(keyspace) { + return None; + } + Some(Self { + statement, + table: TableRef::new(Some(keyspace.to_owned()), name), + }) + } + + pub(crate) async fn columns(&self, link: &CqlLink, columns: Arc<[ResultColumn]>) -> Arc<[ResultColumn]> { + match self.is_base_table(link).await { + Ok(true) => self.traced(&columns), + Ok(false) => columns, + Err(error) => { + tracing::debug!(%error, "could not read where an editor result comes from"); + columns + } + } + } + + async fn is_base_table(&self, link: &CqlLink) -> Result { + let keyspace = self.table.schema.as_deref().unwrap_or_default(); + let result = link.catalogue(BASE_TABLE, (keyspace, self.table.name.as_str())).await?; + let rows = result + .into_rows_result() + .map_err(|error| DriverError::Protocol(error.to_string()))?; + Ok(rows.rows_num() > 0) + } + + fn traced(&self, columns: &[ResultColumn]) -> Arc<[ResultColumn]> { + let names: Vec = columns.iter().map(|column| column.name.clone()).collect(); + columns + .iter() + .zip(parsed_origins(&self.statement, &self.table, &names)) + .map(|(column, origin)| column.clone().with_origin(origin)) + .collect() + } +} + +#[cfg(test)] +mod tests { + use tablepro_core::column::ColumnType; + + use super::*; + + #[test] + fn an_alias_traces_to_the_column_it_names() { + let spec = SpecTable { + statement: source_table("SELECT a AS x, \"Some Col\", Id, writetime FROM t", SqlGrammar::Cql) + .expect("one table"), + table: TableRef::new(Some("shop".to_owned()), "t"), + }; + let columns: Vec = ["x", "Some Col", "id", "writetime"] + .iter() + .map(|name| ResultColumn::new(*name, ColumnType::untyped_text("text"))) + .collect(); + + let origins: Vec> = spec + .traced(&columns) + .iter() + .map(|column| column.origin.as_ref().map(|origin| origin.column.clone())) + .collect(); + + assert_eq!( + origins, + ["a", "Some Col", "id", "writetime"].map(|name| Some(name.to_owned())) + ); + } +} diff --git a/core/crates/drivers/cassandra/tests/integration/main.rs b/core/crates/drivers/cassandra/tests/integration/main.rs index e8eaa9dc42..8e72fb6ca9 100644 --- a/core/crates/drivers/cassandra/tests/integration/main.rs +++ b/core/crates/drivers/cassandra/tests/integration/main.rs @@ -10,6 +10,7 @@ mod conformance_engine; mod editor; mod metadata; mod paging; +mod result_origins; mod round_trip; mod support; mod tls; diff --git a/core/crates/drivers/cassandra/tests/integration/result_origins.rs b/core/crates/drivers/cassandra/tests/integration/result_origins.rs new file mode 100644 index 0000000000..631bad3dd4 --- /dev/null +++ b/core/crates/drivers/cassandra/tests/integration/result_origins.rs @@ -0,0 +1,92 @@ +//! Where an editor result's columns come from: the RESULT metadata names the table, and the statement says which +//! columns are its own. + +use tablepro_core::column::ColumnOrigin; +use tablepro_core::{EditorSession, ResultEvent, TableRef}; + +use crate::support::{CassandraServer, ScyllaServer, Server, connect, run_ok, session, setup, start, table}; + +const T: &str = "CREATE TABLE tablepro.t (id int PRIMARY KEY, a text, b int)"; +const MIXED: &str = "CREATE TABLE tablepro.\"MyTable\" (\"Id\" int PRIMARY KEY, \"Some Col\" text)"; +const BY_B: &str = "CREATE MATERIALIZED VIEW tablepro.t_by_b AS SELECT * FROM tablepro.t \ + WHERE b IS NOT NULL AND id IS NOT NULL PRIMARY KEY (b, id)"; +const ROW: &str = "INSERT INTO tablepro.t (id, a, b) VALUES (1, 'one', 10)"; + +fn origin(table: &TableRef, column: &str) -> Option { + Some(ColumnOrigin::new(table.clone(), column)) +} + +async fn traced(editor: &dyn EditorSession, sql: &str) -> Vec> { + run_ok(editor, sql) + .await + .all() + .into_iter() + .find_map(|event| match event { + ResultEvent::ResultSetStarted { columns } => { + Some(columns.iter().map(|column| column.origin.clone()).collect()) + } + _ => None, + }) + .expect("a result set") +} + +async fn columns_of_a_table_name_it_and_others_do_not() { + let fixture = start::().await; + let connection = connect(&fixture, false).await; + setup(connection.as_ref(), &[T, MIXED, BY_B, ROW]).await; + fixture + .cql( + "CREATE KEYSPACE tp_other WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 1};\n\ + CREATE TABLE tp_other.t2 (id int PRIMARY KEY, c text);", + ) + .await + .expect("a second keyspace"); + let editor = session(connection.as_ref()).await; + let t = table("t"); + let mixed = table("MyTable"); + let t2 = TableRef::new(Some("tp_other".to_owned()), "t2"); + + assert_eq!( + traced(editor.as_ref(), "SELECT * FROM t").await, + vec![origin(&t, "id"), origin(&t, "a"), origin(&t, "b")] + ); + assert_eq!( + traced(editor.as_ref(), "SELECT a AS x, ID FROM t WHERE id = 1").await, + vec![origin(&t, "a"), origin(&t, "id")] + ); + assert_eq!( + traced(editor.as_ref(), "SELECT * FROM \"MyTable\"").await, + vec![origin(&mixed, "Id"), origin(&mixed, "Some Col")] + ); + assert_eq!( + traced(editor.as_ref(), "SELECT * FROM tp_other.t2").await, + vec![origin(&t2, "id"), origin(&t2, "c")] + ); + assert_eq!( + traced(editor.as_ref(), "SELECT id, writetime(a) FROM t").await, + vec![None, None], + "a function is no column, and its statement reads the table through it" + ); + assert_eq!( + traced(editor.as_ref(), "SELECT * FROM t_by_b").await, + vec![None, None, None], + "a materialized view takes no writes" + ); + assert_eq!( + traced(editor.as_ref(), "SELECT cluster_name FROM system.local").await, + vec![None] + ); + editor.close().await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn cassandra_result_columns_name_their_table() { + columns_of_a_table_name_it_and_others_do_not::().await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn scylla_result_columns_name_their_table() { + columns_of_a_table_name_it_and_others_do_not::().await; +} diff --git a/core/crates/drivers/clickhouse/src/editor_run.rs b/core/crates/drivers/clickhouse/src/editor_run.rs index a319898b4e..255895468d 100644 --- a/core/crates/drivers/clickhouse/src/editor_run.rs +++ b/core/crates/drivers/clickhouse/src/editor_run.rs @@ -14,6 +14,7 @@ use crate::call_client::whole_seconds; use crate::cancel::{acknowledges_stop, kill_query, wait_until_ended}; use crate::errors::{code, server_code}; use crate::lingering_query::LingeringQuery; +use crate::result_origin::{TracedTable, source, traced_table, with_origins}; use crate::result_reader::{ReadEvent, ResultReader}; /// Rows per `ResultEvent::Rows`. The sink is awaited per batch, so a slow @@ -43,11 +44,8 @@ pub(crate) struct EditorRun { pub(crate) type RunOutcome = Result; impl EditorRun { - /// Send the statement and stream what comes back. - /// - /// A stop sends KILL QUERY for this run's query id and keeps reading, - /// because the server answers the KILL by ending the response; only - /// a server that has not answered within the grace is cut off. + // A stop sends KILL QUERY and keeps reading, because the server answers the KILL by ending the response; only a + // server that has not answered within the grace is cut off. pub(crate) async fn run(self, sql: UserSql, options: RunOptions, sink: Box) -> RunOutcome { let started = Instant::now(); if sql.is_empty() { @@ -69,6 +67,10 @@ impl EditorRun { return Ok(summary(started, unsent(interrupt))); } } + let traced = match self.trace(&sql, &mut stop).await { + Ok(traced) => traced, + Err(interrupt) => return Ok(summary(started, unsent(interrupt))), + }; let query_id = Uuid::new_v4(); let mut reader = match self.open(&sql, query_id, bound) { Ok(reader) => reader, @@ -88,7 +90,7 @@ impl EditorRun { } }; match event { - Ok(ReadEvent::Columns(columns)) => emitter.start(columns).await, + Ok(ReadEvent::Columns(columns)) => emitter.start(with_origins(traced.as_ref(), columns)).await, Ok(ReadEvent::Row(row)) => { if emitter.row(row).await { emitter.limit_reached().await; @@ -173,6 +175,22 @@ impl EditorRun { None } + // A lookup the stop cuts short may still hold the session, so the next run waits for it as for a stopped statement. + async fn trace(&self, sql: &UserSql, stop: &mut Stop) -> Result, Interrupt> { + let Some(statement) = source(sql.as_str()) else { + return Ok(None); + }; + let lookup_id = Uuid::new_v4(); + tokio::select! { + biased; + interrupt = stop.triggered() => { + self.fire(stop, interrupt, lookup_id); + Err(interrupt) + } + traced = traced_table(&self.session, statement, lookup_id) => Ok(traced), + } + } + /// Send `sql` as `query_id`, which the server ends at `bound`. fn open(&self, sql: &UserSql, query_id: Uuid, bound: Option) -> Result { let mut query = self diff --git a/core/crates/drivers/clickhouse/src/lib.rs b/core/crates/drivers/clickhouse/src/lib.rs index 769cd1b56e..acd1d1e18c 100644 --- a/core/crates/drivers/clickhouse/src/lib.rs +++ b/core/crates/drivers/clickhouse/src/lib.rs @@ -30,6 +30,7 @@ mod line_reader; mod lingering_query; mod metadata; mod param_value; +mod result_origin; mod result_reader; mod row_seed; mod session_status; diff --git a/core/crates/drivers/clickhouse/src/result_origin.rs b/core/crates/drivers/clickhouse/src/result_origin.rs new file mode 100644 index 0000000000..b3d8adb693 --- /dev/null +++ b/core/crates/drivers/clickhouse/src/result_origin.rs @@ -0,0 +1,166 @@ +use std::sync::Arc; + +use tablepro_core::Value; +use tablepro_core::column::ResultColumn; +use tablepro_core::edit::parsed_origins; +use tablepro_core::meta::TableRef; +use tablepro_core::sql_syntax::{SourceTable, SqlGrammar, source_table}; +use uuid::Uuid; + +use crate::result_reader::{CatalogRow, read_table}; + +// ClickHouse reports no origin, so the session that runs the statement is asked how it resolves the name: an +// unqualified one is a temporary table of the session's first, then a table of its current database. Only the two +// databases that can hold the name are listed, so no other database's engine is asked for its tables. The answer is one +// row, which a `limit` the session set cannot cut short. +const LOOKUP: &str = "SELECT countIf(is_temporary), groupArrayIf(database, NOT is_temporary), \ + groupArrayIf(engine, NOT is_temporary), \ + (SELECT groupArray(name) FROM system.columns \ + WHERE database = if({db:String} = '', currentDatabase(), {db:String}) AND table = {tbl:String}) \ + FROM system.tables \ + WHERE name = {tbl:String} AND database IN ('', if({db:String} = '', currentDatabase(), {db:String}))"; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct TracedTable { + statement: SourceTable, + table: TableRef, + columns: Vec, +} + +impl TracedTable { + pub(crate) fn columns(&self, columns: Arc<[ResultColumn]>) -> Arc<[ResultColumn]> { + let names: Vec = columns.iter().map(|column| column.name.clone()).collect(); + let origins = parsed_origins(&self.statement, &self.table, &names); + columns + .iter() + .zip(origins) + .map(|(column, origin)| { + // A virtual column such as `_part` is read from the table but is none of its columns. + let origin = origin.filter(|origin| self.columns.contains(&origin.column)); + column.clone().with_origin(origin) + }) + .collect() + } +} + +pub(crate) fn with_origins(traced: Option<&TracedTable>, columns: Arc<[ResultColumn]>) -> Arc<[ResultColumn]> { + match traced { + Some(traced) => traced.columns(columns), + None => columns, + } +} + +pub(crate) fn source(sql: &str) -> Option { + source_table(sql, SqlGrammar::ClickHouse).ok() +} + +// Sent on the editor's session before the statement, which the server would refuse while the session runs another. +pub(crate) async fn traced_table( + session: &clickhouse::Client, + statement: SourceTable, + query_id: Uuid, +) -> Option { + let query = session + .query_raw(LOOKUP) + .with_setting("query_id", query_id.to_string()) + .param("db", statement.table.schema.as_deref().unwrap_or_default()) + .param("tbl", statement.table.name.as_str()); + let rows = match read_table(query, None).await { + Ok(result) => result.rows, + Err(error) => { + tracing::debug!(%error, "could not read where an editor result comes from"); + return None; + } + }; + let (table, columns) = resolve(&statement.table, rows.first()?)?; + Some(TracedTable { + statement, + table, + columns, + }) +} + +fn resolve(written: &TableRef, row: &[Value]) -> Option<(TableRef, Vec)> { + let row = CatalogRow(row); + // A temporary table of the name is the one an unqualified name reads. + if row.flag(0).ok()? { + return None; + } + let [database] = <[String; 1]>::try_from(row.texts(1).ok()?).ok()?; + let [engine] = <[String; 1]>::try_from(row.texts(2).ok()?).ok()?; + let columns = row.texts(3).ok()?; + edits_rows(&engine).then(|| (TableRef::new(Some(database), written.name.clone()), columns)) +} + +// The table tab keys a row by its sorting key and writes it as a mutation, which only the MergeTree family has. Views, +// dictionaries, system tables and engines that hold another server's rows are left out with them. +fn edits_rows(engine: &str) -> bool { + engine.ends_with("MergeTree") +} + +#[cfg(test)] +mod tests { + use tablepro_core::column::{ColumnOrigin, ColumnType}; + + use super::*; + + fn texts(texts: &[&str]) -> Value { + Value::Array(texts.iter().map(|text| Value::Text((*text).to_owned())).collect()) + } + + fn row(temporaries: u64, databases: &[&str], engines: &[&str]) -> Vec { + vec![ + Value::UInt(temporaries), + texts(databases), + texts(engines), + texts(&["id", "a"]), + ] + } + + #[test] + fn only_one_mergetree_table_of_the_name_is_traced() { + let written = TableRef::unqualified("t"); + + assert_eq!( + resolve(&written, &row(0, &["shop"], &["ReplicatedMergeTree"])), + Some(( + TableRef::new(Some("shop".to_owned()), "t"), + vec!["id".to_owned(), "a".to_owned()] + )) + ); + assert_eq!(resolve(&written, &row(1, &["shop"], &["MergeTree"])), None); + assert_eq!(resolve(&written, &row(0, &[], &[])), None); + for engine in [ + "View", + "MaterializedView", + "Dictionary", + "Memory", + "MySQL", + "Distributed", + ] { + assert_eq!(resolve(&written, &row(0, &["shop"], &[engine])), None, "{engine}"); + } + } + + #[test] + fn origins_name_only_the_tables_own_columns() { + let traced = TracedTable { + statement: source("SELECT _part, a AS x, *, id + 1 FROM t").expect("one table"), + table: TableRef::new(Some("shop".to_owned()), "t"), + columns: vec!["id".to_owned(), "a".to_owned()], + }; + let result: Arc<[ResultColumn]> = ["_part", "x", "id", "a", "plus(id, 1)"] + .iter() + .map(|name| ResultColumn::new(*name, ColumnType::untyped_text("String"))) + .collect(); + + let origins: Vec> = traced + .columns(result) + .iter() + .map(|column| column.origin.clone()) + .collect(); + + let origin = |column: &str| Some(ColumnOrigin::new(TableRef::new(Some("shop".to_owned()), "t"), column)); + assert_eq!(origins, vec![None, origin("a"), origin("id"), origin("a"), None]); + } +} diff --git a/core/crates/drivers/clickhouse/tests/integration/main.rs b/core/crates/drivers/clickhouse/tests/integration/main.rs index 725152b4b1..28ed0cd691 100644 --- a/core/crates/drivers/clickhouse/tests/integration/main.rs +++ b/core/crates/drivers/clickhouse/tests/integration/main.rs @@ -12,6 +12,7 @@ mod ddl; mod dump; mod keys_and_decimals; mod metadata; +mod result_origins; mod round_trip; mod sessions; mod support; diff --git a/core/crates/drivers/clickhouse/tests/integration/result_origins.rs b/core/crates/drivers/clickhouse/tests/integration/result_origins.rs new file mode 100644 index 0000000000..64d65e439a --- /dev/null +++ b/core/crates/drivers/clickhouse/tests/integration/result_origins.rs @@ -0,0 +1,133 @@ +//! Where an editor result's columns come from: ClickHouse reports nothing, so the driver resolves the statement's table +//! in the tab's own session. + +use tablepro_core::column::ColumnOrigin; +use tablepro_core::{EditorSession, ResultEvent, TableRef}; + +use crate::support::{DATABASE, Events, connect, quoted, run_ok, session, setup, start, table, unique}; + +fn origin(table: &TableRef, column: &str) -> Option { + Some(ColumnOrigin::new(table.clone(), column)) +} + +fn origins(events: &Events) -> Vec> { + events + .all() + .into_iter() + .find_map(|event| match event { + ResultEvent::ResultSetStarted { columns } => { + Some(columns.iter().map(|column| column.origin.clone()).collect()) + } + _ => None, + }) + .expect("a result set") +} + +async fn traced(editor: &dyn EditorSession, sql: &str) -> Vec> { + origins(&run_ok(editor, sql).await) +} + +#[tokio::test] +#[ignore = "requires docker"] +async fn columns_read_straight_from_a_table_name_it() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + let name = unique("origin"); + let other = unique("elsewhere"); + setup( + connection.as_ref(), + &[ + &format!( + "CREATE TABLE {} (id UInt64, a String, b Int32) ENGINE = MergeTree ORDER BY id", + quoted(&name) + ), + &format!("INSERT INTO {} VALUES (1, 'one', 10)", quoted(&name)), + &format!("CREATE DATABASE {}", quoted(&other)), + &format!( + "CREATE TABLE {}.t2 (id UInt64, c String) ENGINE = MergeTree ORDER BY id", + quoted(&other) + ), + ], + ) + .await; + let editor = session(connection.as_ref()).await; + let t = table(&name); + let t2 = TableRef::new(Some(other.clone()), "t2"); + let from = quoted(&name); + + assert_eq!( + traced(editor.as_ref(), &format!("SELECT * FROM {from}")).await, + vec![origin(&t, "id"), origin(&t, "a"), origin(&t, "b")] + ); + assert_eq!( + traced(editor.as_ref(), &format!("SELECT a AS x, id FROM {from}")).await, + vec![origin(&t, "a"), origin(&t, "id")] + ); + assert_eq!( + traced(editor.as_ref(), &format!("SELECT id, b + 1 AS next, _part FROM {from}")).await, + vec![origin(&t, "id"), None, None], + "an expression and a virtual column are none of the table's columns" + ); + assert_eq!( + traced(editor.as_ref(), &format!("SELECT * FROM {DATABASE}.{from}")).await, + vec![origin(&t, "id"), origin(&t, "a"), origin(&t, "b")] + ); + assert_eq!( + traced(editor.as_ref(), &format!("SELECT * FROM {}.t2", quoted(&other))).await, + vec![origin(&t2, "id"), origin(&t2, "c")] + ); + run_ok(editor.as_ref(), &format!("USE {}", quoted(&other))).await; + assert_eq!( + traced(editor.as_ref(), "SELECT c, id FROM t2").await, + vec![origin(&t2, "c"), origin(&t2, "id")], + "an unqualified name is the session's current database's" + ); + editor.close().await; + connection.close().await.expect("a close"); +} + +#[tokio::test] +#[ignore = "requires docker"] +async fn a_view_a_temporary_table_or_an_unkeyed_engine_is_not_traced() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + let name = unique("origin"); + let view = unique("view"); + let memory = unique("memory"); + setup( + connection.as_ref(), + &[ + &format!( + "CREATE TABLE {} (id UInt64, a String) ENGINE = MergeTree ORDER BY id", + quoted(&name) + ), + &format!("CREATE VIEW {} AS SELECT id, a FROM {}", quoted(&view), quoted(&name)), + &format!("CREATE TABLE {} (id UInt64) ENGINE = Memory", quoted(&memory)), + ], + ) + .await; + let editor = session(connection.as_ref()).await; + + assert_eq!( + traced(editor.as_ref(), &format!("SELECT * FROM {}", quoted(&view))).await, + vec![None, None] + ); + assert_eq!( + traced(editor.as_ref(), &format!("SELECT * FROM {}", quoted(&memory))).await, + vec![None] + ); + run_ok(editor.as_ref(), "CREATE TEMPORARY TABLE scratch (x UInt8)").await; + assert_eq!(traced(editor.as_ref(), "SELECT x FROM scratch").await, vec![None]); + // A temporary table takes an unqualified name before the database's table of the same name. + run_ok( + editor.as_ref(), + &format!("CREATE TEMPORARY TABLE {} (z UInt8)", quoted(&name)), + ) + .await; + assert_eq!( + traced(editor.as_ref(), &format!("SELECT * FROM {}", quoted(&name))).await, + vec![None] + ); + editor.close().await; + connection.close().await.expect("a close"); +} diff --git a/core/crates/drivers/duckdb/src/connection.rs b/core/crates/drivers/duckdb/src/connection.rs index 09b85a593c..788d858ffb 100644 --- a/core/crates/drivers/duckdb/src/connection.rs +++ b/core/crates/drivers/duckdb/src/connection.rs @@ -388,8 +388,7 @@ impl Connection for DuckDbConnection { self.read(options, move |link, _| count(link, &statement)).await } - /// Every row, on a connection of the read's own, inside a read-only - /// transaction, so user SQL that writes is refused by DuckDB itself. + // Inside a read-only transaction, so user SQL that writes is refused by DuckDB itself. async fn stream_read( &self, read: &ReadStatement, @@ -414,7 +413,7 @@ impl Connection for DuckDbConnection { batch(link, "BEGIN TRANSACTION READ ONLY")?; let streamed = match &read { ReadStatement::Generated(statement) => run_bound(link, statement, events), - ReadStatement::User(sql) => run_script(link, sql.as_str(), None, events, &|| false), + ReadStatement::User(sql) => run_script(link, sql.as_str(), None, events, &|| false, None), }; link.hold_stops(); let _ = batch(link, "ROLLBACK"); diff --git a/core/crates/drivers/duckdb/src/editor_session.rs b/core/crates/drivers/duckdb/src/editor_session.rs index 0f3dd52f84..0cb9f6f1fe 100644 --- a/core/crates/drivers/duckdb/src/editor_session.rs +++ b/core/crates/drivers/duckdb/src/editor_session.rs @@ -30,16 +30,19 @@ pub(crate) struct DuckDbEditorSession { shared: Arc, } -/// What a run needs once its caller is gone. A run goes on a task of -/// its own, so a dropped caller is a cancel that still interrupts the -/// statement and settles the session. +// A run goes on a task of its own, so a dropped caller is a cancel that still interrupts the statement and settles the +// session. struct Shared { call: Arc>>, stop: StopHandle, running: AtomicBool, lost: AtomicBool, - /// Raised to keep the run from starting another statement. + // Raised to keep the run from starting another statement. halt: Arc, + // The file's catalogue, which the table tab's names are qualified with. + catalog: String, + // Where the last run or state read left the transaction; a new connection starts outside one. + idle: AtomicBool, statement_timeout: Option, cancel_grace: Duration, close_timeout: Duration, @@ -59,6 +62,7 @@ impl DuckDbEditorSession { liveness: &LivenessPolicy, cancel: CancellationToken, ) -> Result { + let catalog = instance.catalog().to_owned(); let connecting = blocking(move || instance.link()); let link = tokio::select! { biased; @@ -82,6 +86,8 @@ impl DuckDbEditorSession { running: AtomicBool::new(false), lost: AtomicBool::new(false), halt: Arc::new(AtomicBool::new(false)), + catalog, + idle: AtomicBool::new(true), statement_timeout: options.statement_timeout, cancel_grace: liveness.cancel_grace, close_timeout: liveness.close_timeout, @@ -108,8 +114,7 @@ impl Shared { interrupt_until_settled(&self.stop).await; } - /// One run on the connection `held` holds, which it gives up when - /// the connection cannot take another. + // The run gives the connection up when it cannot take another. async fn run_held( &self, mut held: OwnedMutexGuard>, @@ -134,6 +139,8 @@ impl Shared { Ok(_) => self.state_after_run(call).await, Err(_) => TransactionState::Unknown, }; + self.idle + .store(transaction_state == TransactionState::Idle, Ordering::SeqCst); let kept = call.holds_connection(); self.running.store(false, Ordering::SeqCst); let end = match ended { @@ -155,9 +162,7 @@ impl Shared { }) } - /// Run the script, stopping it on a cancel or at the deadline. A - /// stopped statement gets the cancel grace to give way; past that - /// the connection is given up. + // A stopped statement gets the cancel grace to give way; past that the connection is given up. async fn supervise( &self, call: &mut DuckCall, @@ -185,8 +190,9 @@ impl Shared { }; let text = sql.as_str().to_owned(); let limit = options.row_limit; + let traced = self.idle.load(Ordering::SeqCst).then(|| self.catalog.clone()); let running = stream_into(call, None, sink, move |link, events| { - run_script(link, &text, Some(limit), events, &halted) + run_script(link, &text, Some(limit), events, &halted, traced.as_deref()) }); tokio::pin!(running); let stop = tokio::select! { @@ -367,6 +373,9 @@ impl EditorSession for DuckDbEditorSession { let given_up = LoseOnDrop(Some(&shared.lost)); let state = call.run(None, |link| Ok(transaction_state(link))).await; given_up.disarm(); + shared + .idle + .store(matches!(state, Ok(TransactionState::Idle)), Ordering::SeqCst); state } diff --git a/core/crates/drivers/duckdb/src/lib.rs b/core/crates/drivers/duckdb/src/lib.rs index c5b11a72f4..366656f6f1 100644 --- a/core/crates/drivers/duckdb/src/lib.rs +++ b/core/crates/drivers/duckdb/src/lib.rs @@ -17,6 +17,7 @@ mod encode; mod errors; mod instance; mod pool; +mod result_origin; mod result_stream; mod stop; mod transaction_state; diff --git a/core/crates/drivers/duckdb/src/result_origin.rs b/core/crates/drivers/duckdb/src/result_origin.rs new file mode 100644 index 0000000000..6127fd0463 --- /dev/null +++ b/core/crates/drivers/duckdb/src/result_origin.rs @@ -0,0 +1,269 @@ +use tablepro_core::DriverError; +use tablepro_core::column::ColumnOrigin; +use tablepro_core::edit::parsed_origins; +use tablepro_core::meta::TableRef; +use tablepro_core::sql_syntax::{SourceTable, SqlGrammar, source_table}; + +use crate::errors::map_error; +use crate::instance::Link; + +// DuckDB reports no origin, so one query reads what its binder resolves the name against: the session's database, +// schema and search path, a temporary object or a catalogue the name could mean instead, and each base table of that +// name with its columns. Names compare case-insensitively, as DuckDB's catalogue does. +const LOOKUP: &str = "SELECT 'scope', current_database(), current_schema(), current_setting('search_path'), NULL \ + UNION ALL \ + SELECT 'catalog', database_name, NULL, NULL, NULL FROM duckdb_databases() \ + WHERE lower(database_name) = lower(?) \ + UNION ALL \ + SELECT 'temporary', database_name, schema_name, table_name, NULL FROM duckdb_tables() \ + WHERE temporary AND lower(table_name) = lower(?) \ + UNION ALL \ + SELECT 'temporary', database_name, schema_name, view_name, NULL FROM duckdb_views() \ + WHERE temporary AND lower(view_name) = lower(?) \ + UNION ALL \ + SELECT 'column', t.database_name, t.schema_name, t.table_name, c.column_name \ + FROM duckdb_tables() AS t \ + JOIN duckdb_columns() AS c ON c.database_oid = t.database_oid AND c.table_oid = t.table_oid \ + WHERE NOT t.temporary AND lower(t.table_name) = lower(?)"; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct TracedTable { + statement: SourceTable, + table: TableRef, + columns: Vec, +} + +impl TracedTable { + pub(crate) fn origins(&self, names: &[String]) -> Vec> { + parsed_origins(&self.statement, &self.table, names) + .into_iter() + .map(|origin| origin.and_then(|origin| self.cataloged(origin))) + .collect() + } + + // DuckDB also binds names the table has no column for, such as `rowid`. + fn cataloged(&self, origin: ColumnOrigin) -> Option { + let mut named = self + .columns + .iter() + .filter(|column| column.eq_ignore_ascii_case(&origin.column)); + match (named.next(), named.next()) { + (Some(column), None) => Some(ColumnOrigin::new(origin.table, column.clone())), + _ => None, + } + } +} + +// Asked before the statement: a query on the connection would close the statement's streaming result. +pub(crate) fn traced_table(link: &Link, catalog: &str, sql: &str) -> Option { + let statement = source_table(sql, SqlGrammar::DuckDb).ok()?; + let found = match lookup(link, &statement.table) { + Ok(found) => found, + Err(error) => { + tracing::debug!(%error, "could not read where an editor result comes from"); + return None; + } + }; + let (table, columns) = resolve(catalog, &statement.table, &found)?; + Some(TracedTable { + statement, + table, + columns, + }) +} + +#[derive(Debug, Clone, PartialEq, Eq)] +enum Found { + Scope { + database: String, + schema: String, + search_path: String, + }, + Catalog, + Temporary, + Column { + database: String, + schema: String, + table: String, + column: String, + }, +} + +fn lookup(link: &Link, written: &TableRef) -> Result, DriverError> { + let schema = written.schema.as_deref().unwrap_or_default(); + let name = written.name.as_str(); + let mut statement = link.connection.prepare(LOOKUP).map_err(map_error)?; + let mut rows = statement + .query(duckdb::params![schema, name, name, name]) + .map_err(map_error)?; + let mut found = Vec::new(); + while let Some(row) = rows.next().map_err(map_error)? { + let cell = |index: usize| { + row.get::<_, Option>(index) + .map(Option::unwrap_or_default) + .map_err(map_error) + }; + found.push(match cell(0)?.as_str() { + "scope" => Found::Scope { + database: cell(1)?, + schema: cell(2)?, + search_path: cell(3)?, + }, + "catalog" => Found::Catalog, + "temporary" => Found::Temporary, + _ => Found::Column { + database: cell(1)?, + schema: cell(2)?, + table: cell(3)?, + column: cell(4)?, + }, + }); + } + Ok(found) +} + +// The table tab names the file's tables by schema and table, and the dialect puts the file's catalogue in front, so +// only a table of that catalogue is traced. +fn resolve(catalog: &str, written: &TableRef, found: &[Found]) -> Option<(TableRef, Vec)> { + let (database, current_schema, search_path) = found.iter().find_map(|found| match found { + Found::Scope { + database, + schema, + search_path, + } => Some((database, schema, search_path)), + _ => None, + })?; + // A temporary object comes first even for `main.t`, and with more than one search path entry a name can resolve + // past the current schema or to another catalogue's schema. + if !database.eq_ignore_ascii_case(catalog) || search_path.contains(',') || found.contains(&Found::Temporary) { + return None; + } + let schema = match &written.schema { + // DuckDB reads `a.t` as catalogue `a` when there is one. + Some(_) if found.contains(&Found::Catalog) => return None, + Some(schema) => schema, + None => current_schema, + }; + let mut table: Option = None; + let mut columns = Vec::new(); + for entry in found { + let Found::Column { + database, + schema: column_schema, + table: column_table, + column, + } = entry + else { + continue; + }; + if !database.eq_ignore_ascii_case(catalog) + || !column_schema.eq_ignore_ascii_case(schema) + || !column_table.eq_ignore_ascii_case(&written.name) + { + continue; + } + let named = TableRef::new(Some(column_schema.clone()), column_table.clone()); + match &table { + Some(seen) if *seen != named => return None, + _ => table = Some(named), + } + columns.push(column.clone()); + } + Some((table?, columns)) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn scope(schema: &str, search_path: &str) -> Found { + Found::Scope { + database: "sales".to_owned(), + schema: schema.to_owned(), + search_path: search_path.to_owned(), + } + } + + fn column(database: &str, schema: &str, table: &str, column: &str) -> Found { + Found::Column { + database: database.to_owned(), + schema: schema.to_owned(), + table: table.to_owned(), + column: column.to_owned(), + } + } + + fn orders() -> Vec { + vec![ + scope("main", ""), + column("sales", "main", "Orders", "id"), + column("sales", "main", "Orders", "total"), + column("sales", "audit", "Orders", "id"), + column("archive", "main", "Orders", "id"), + ] + } + + #[test] + fn a_name_resolves_to_the_catalogs_spelling_in_the_current_schema() { + let resolved = resolve("sales", &TableRef::unqualified("orders"), &orders()); + + assert_eq!( + resolved, + Some(( + TableRef::new(Some("main".to_owned()), "Orders"), + vec!["id".to_owned(), "total".to_owned()] + )) + ); + let audit = resolve("sales", &TableRef::new(Some("AUDIT".to_owned()), "orders"), &orders()); + assert_eq!(audit.map(|(table, _)| table.schema), Some(Some("audit".to_owned()))); + } + + #[test] + fn a_name_that_may_mean_something_else_is_not_traced() { + let mut shadowed = orders(); + shadowed.push(Found::Temporary); + assert_eq!(resolve("sales", &TableRef::unqualified("orders"), &shadowed), None); + + let mut catalog = orders(); + catalog.push(Found::Catalog); + assert_eq!( + resolve("sales", &TableRef::new(Some("archive".to_owned()), "orders"), &catalog), + None + ); + + let mut searched = orders(); + searched[0] = scope("main", "sales.main,archive.main"); + assert_eq!(resolve("sales", &TableRef::unqualified("orders"), &searched), None); + + assert_eq!(resolve("archive", &TableRef::unqualified("orders"), &orders()), None); + assert_eq!(resolve("sales", &TableRef::unqualified("missing"), &orders()), None); + } + + #[test] + fn origins_name_only_columns_the_table_has() { + let traced = TracedTable { + statement: source_table("SELECT ROWID, ID AS k, *, total + 1 FROM orders", SqlGrammar::DuckDb) + .expect("one table"), + table: TableRef::new(Some("main".to_owned()), "Orders"), + columns: vec!["id".to_owned(), "total".to_owned()], + }; + let names = ["rowid", "k", "id", "total", "(total + 1)"].map(str::to_owned); + + let columns: Vec> = traced + .origins(&names) + .into_iter() + .map(|origin| origin.map(|origin| origin.column)) + .collect(); + + assert_eq!( + columns, + vec![ + None, + Some("id".to_owned()), + Some("id".to_owned()), + Some("total".to_owned()), + None + ] + ); + } +} diff --git a/core/crates/drivers/duckdb/src/result_stream.rs b/core/crates/drivers/duckdb/src/result_stream.rs index a3df73bf5d..5a0205f0c6 100644 --- a/core/crates/drivers/duckdb/src/result_stream.rs +++ b/core/crates/drivers/duckdb/src/result_stream.rs @@ -10,7 +10,7 @@ use std::path::PathBuf; use std::sync::Arc; use duckdb::arrow::array::{Array, StructArray}; -use tablepro_core::column::ResultColumn; +use tablepro_core::column::{ColumnOrigin, ResultColumn}; use tablepro_core::sql_syntax::SqlGrammar; use tablepro_core::sql_syntax::script::{LexicalSettings, ScriptDiagnostic, ScriptPlan, ScriptToken, ScriptTokenKind}; use tablepro_core::statement::{BoundParam, Statement}; @@ -23,6 +23,7 @@ use crate::duck_type::DuckType; use crate::encode::bind_all; use crate::errors::map_error; use crate::instance::{Link, attach_unless_open}; +use crate::result_origin::{TracedTable, traced_table}; /// How many rows travel in one `Rows` event: enough that a large read /// is not one await per row, few enough that the first rows show at @@ -40,10 +41,10 @@ pub(crate) struct StreamOutcome { pub halted: bool, } -/// The columns a statement returns, by name and by DuckDB type. struct Shape { names: Vec, types: Vec, + origins: Vec>, } impl Shape { @@ -53,14 +54,22 @@ impl Shape { let types = (0..names.len()) .map(|index| DuckType::from_logical(&statement.column_logical_type(index))) .collect(); - Self { names, types } + Self { + names, + types, + origins: Vec::new(), + } } fn columns(&self) -> Arc<[ResultColumn]> { self.names .iter() .zip(&self.types) - .map(|(name, duck)| ResultColumn::new(name.clone(), duck.column_type())) + .enumerate() + .map(|(index, (name, duck))| { + ResultColumn::new(name.clone(), duck.column_type()) + .with_origin(self.origins.get(index).cloned().flatten()) + }) .collect() } @@ -163,15 +172,15 @@ impl Events { } } -/// Run SQL the user wrote, one statement after another, until one -/// fails, the reader goes or `halt` is raised. Each statement goes to -/// DuckDB alone, so each result set streams by itself. +// Each statement goes to DuckDB alone, so each result set streams by itself. `traced` is the file's catalogue when the +// session is outside a transaction: a lookup inside one could fail and abort it. pub(crate) fn run_script( link: &Link, sql: &str, limit: Option, events: &mut Events, halt: &dyn Fn() -> bool, + mut traced: Option<&str>, ) -> Result { let mut outcome = StreamOutcome::default(); for statement in split_statements(sql) { @@ -179,6 +188,18 @@ pub(crate) fn run_script( outcome.halted = true; break; } + // Only a SELECT can read one table row for row, and parsing every statement of a long script would slow it. + let source = traced + .filter(|_| main_verb(statement).as_deref() == Some("SELECT")) + .and_then(|catalog| traced_table(link, catalog, statement)); + // A stop during the lookup interrupted the lookup, and DuckDB clears an interrupt as a statement starts. + if traced.is_some() && halt() { + outcome.halted = true; + break; + } + if traced.is_some() && controls_transaction(statement) { + traced = None; + } match attach_literal(statement) { Some(literal) => { // DuckDB leaves a HIDDEN database out of what it lists, so @@ -195,14 +216,14 @@ pub(crate) fn run_script( if halt() { return Ok(false); } - run_statement(link, statement, &[], limit, events, &mut outcome).map(|()| true) + run_statement(link, statement, &[], limit, None, events, &mut outcome).map(|()| true) })?; if !ran { outcome.halted = true; break; } } - None => run_statement(link, statement, &[], limit, events, &mut outcome)?, + None => run_statement(link, statement, &[], limit, source.as_ref(), events, &mut outcome)?, } if outcome.consumer_gone { break; @@ -214,7 +235,15 @@ pub(crate) fn run_script( /// Run a read the app built, every row of it. pub(crate) fn run_bound(link: &Link, statement: &Statement, events: &mut Events) -> Result { let mut outcome = StreamOutcome::default(); - run_statement(link, statement.sql(), statement.params(), None, events, &mut outcome)?; + run_statement( + link, + statement.sql(), + statement.params(), + None, + None, + events, + &mut outcome, + )?; Ok(outcome) } @@ -223,10 +252,14 @@ fn run_statement( sql: &str, params: &[BoundParam], limit: Option, + source: Option<&TracedTable>, events: &mut Events, outcome: &mut StreamOutcome, ) -> Result<(), DriverError> { - let (running, shape) = start(link, sql, params)?; + let (running, mut shape) = start(link, sql, params)?; + if let Some(source) = source { + shape.origins = source.origins(&shape.names); + } match command_result(sql, &shape) { Some(Command::ChangedRows) => { let changed = match next_chunk(&running)? { @@ -872,6 +905,7 @@ mod tests { width: tablepro_core::column::IntegerWidth::W64, signed: true, }], + origins: Vec::new(), }; assert_eq!(command_result("EXECUTE q", &count), None); diff --git a/core/crates/drivers/duckdb/tests/integration/main.rs b/core/crates/drivers/duckdb/tests/integration/main.rs index ad4d92dc03..5ccb01294e 100644 --- a/core/crates/drivers/duckdb/tests/integration/main.rs +++ b/core/crates/drivers/duckdb/tests/integration/main.rs @@ -23,6 +23,8 @@ mod open_semantics; #[cfg(test)] mod paging; #[cfg(test)] +mod result_origins; +#[cfg(test)] mod round_trip; #[cfg(test)] mod sessions; diff --git a/core/crates/drivers/duckdb/tests/integration/result_origins.rs b/core/crates/drivers/duckdb/tests/integration/result_origins.rs new file mode 100644 index 0000000000..0e161837e5 --- /dev/null +++ b/core/crates/drivers/duckdb/tests/integration/result_origins.rs @@ -0,0 +1,163 @@ +//! Where an editor result's columns come from: DuckDB reports nothing, so the driver resolves the statement's table on +//! the tab's own connection. + +use std::sync::Arc; + +use tablepro_core::column::ColumnOrigin; +use tablepro_core::meta::TableRef; +use tablepro_core::{Connection, EditorSession, ResultEvent, RunEnd, TransactionState}; + +use crate::support::{Collected, TestFile, run, session}; + +const SCHEMA: &str = "CREATE TABLE t (id INTEGER PRIMARY KEY, a VARCHAR, b INTEGER); \ + INSERT INTO t VALUES (1, 'one', 10); \ + CREATE VIEW v AS SELECT * FROM t; \ + CREATE SCHEMA sales; \ + CREATE TABLE sales.orders (id INTEGER PRIMARY KEY, total INTEGER); \ + CREATE TABLE \"MixedCase\" (\"Id\" INTEGER PRIMARY KEY, \"Some Col\" VARCHAR)"; + +fn main_table(name: &str) -> TableRef { + TableRef::new(Some("main".to_owned()), name) +} + +fn origin(table: &TableRef, column: &str) -> Option { + Some(ColumnOrigin::new(table.clone(), column)) +} + +// The columns of the run's last result set. +fn origins(collected: &Collected) -> Vec> { + collected + .events() + .into_iter() + .filter_map(|event| match event { + ResultEvent::ResultSetStarted { columns } => { + Some(columns.iter().map(|column| column.origin.clone()).collect()) + } + _ => None, + }) + .next_back() + .expect("a result set") +} + +async fn traced(editor: &dyn EditorSession, sql: &str) -> Vec> { + let (summary, collected) = run(editor, sql).await; + assert_eq!(summary.end, RunEnd::Completed, "{sql}"); + origins(&collected) +} + +async fn ready(file: &TestFile) -> (Arc, Arc) { + let connection = file.open().await; + let editor = session(connection.as_ref()).await; + let (summary, _) = run(editor.as_ref(), SCHEMA).await; + assert_eq!(summary.end, RunEnd::Completed); + (connection, editor) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn columns_read_straight_from_a_table_name_it() { + let file = TestFile::new(); + let (connection, editor) = ready(&file).await; + let t = main_table("t"); + + assert_eq!( + traced(editor.as_ref(), "SELECT * FROM t").await, + vec![origin(&t, "id"), origin(&t, "a"), origin(&t, "b")] + ); + assert_eq!( + traced(editor.as_ref(), "SELECT a AS x, id FROM t").await, + vec![origin(&t, "a"), origin(&t, "id")] + ); + assert_eq!( + traced(editor.as_ref(), "SELECT id, b + 1 AS next FROM t").await, + vec![origin(&t, "id"), None] + ); + assert_eq!( + traced(editor.as_ref(), "SELECT rowid, ID FROM T").await, + vec![None, origin(&t, "id")], + "rowid is no column of the table, and a name in another case is the catalogue's column" + ); + let mixed = main_table("MixedCase"); + assert_eq!( + traced(editor.as_ref(), "SELECT * FROM mixedcase").await, + vec![origin(&mixed, "Id"), origin(&mixed, "Some Col")] + ); + editor.close().await; + connection.close().await.unwrap(); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn a_table_in_another_schema_is_named_with_it() { + let file = TestFile::new(); + let (connection, editor) = ready(&file).await; + let orders = TableRef::new(Some("sales".to_owned()), "orders"); + + assert_eq!( + traced(editor.as_ref(), "SELECT * FROM sales.orders").await, + vec![origin(&orders, "id"), origin(&orders, "total")] + ); + assert_eq!( + traced(editor.as_ref(), "USE sales; SELECT total, id FROM orders").await, + vec![origin(&orders, "total"), origin(&orders, "id")] + ); + editor.close().await; + connection.close().await.unwrap(); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn a_view_a_temporary_table_or_another_database_is_not_traced() { + let file = TestFile::new(); + let (connection, editor) = ready(&file).await; + + assert_eq!(traced(editor.as_ref(), "SELECT * FROM v").await, vec![None; 3]); + assert_eq!( + traced( + editor.as_ref(), + "CREATE TEMP TABLE scratch (id INTEGER); SELECT * FROM scratch" + ) + .await, + vec![None] + ); + // A temporary table takes the name first, also as `main.t`. + let (shadowing, _) = run(editor.as_ref(), "CREATE TEMP TABLE t (z INTEGER)").await; + assert_eq!(shadowing.end, RunEnd::Completed); + assert_eq!(traced(editor.as_ref(), "SELECT * FROM t").await, vec![None]); + assert_eq!(traced(editor.as_ref(), "SELECT * FROM main.t").await, vec![None]); + let (dropped, _) = run(editor.as_ref(), "DROP TABLE temp.main.t").await; + assert_eq!(dropped.end, RunEnd::Completed); + assert_eq!( + traced(editor.as_ref(), "SELECT id FROM t").await, + vec![origin(&main_table("t"), "id")] + ); + let other = file.path.with_file_name("other.duckdb"); + let attaching = format!( + "ATTACH '{}' AS other; CREATE TABLE other.w (id INTEGER PRIMARY KEY); SELECT * FROM other.w", + other.display() + ); + assert_eq!(traced(editor.as_ref(), &attaching).await, vec![None]); + assert_eq!( + traced(editor.as_ref(), "USE other; SELECT * FROM w").await, + vec![None], + "the table tab names the file's own catalogue" + ); + editor.close().await; + connection.close().await.unwrap(); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn nothing_is_looked_up_inside_a_transaction() { + let file = TestFile::new(); + let (connection, editor) = ready(&file).await; + + assert_eq!(traced(editor.as_ref(), "BEGIN; SELECT id FROM t").await, vec![None]); + let (open, collected) = run(editor.as_ref(), "SELECT id FROM t").await; + assert_eq!(open.transaction_state, TransactionState::InTransaction); + assert_eq!(origins(&collected), vec![None]); + let (ended, _) = run(editor.as_ref(), "ROLLBACK").await; + assert_eq!(ended.transaction_state, TransactionState::Idle); + assert_eq!( + traced(editor.as_ref(), "SELECT id FROM t").await, + vec![origin(&main_table("t"), "id")] + ); + editor.close().await; + connection.close().await.unwrap(); +} diff --git a/core/crates/drivers/mssql/src/editor_run.rs b/core/crates/drivers/mssql/src/editor_run.rs index 6e7a9df7de..7e4a9c0f86 100644 --- a/core/crates/drivers/mssql/src/editor_run.rs +++ b/core/crates/drivers/mssql/src/editor_run.rs @@ -15,6 +15,7 @@ use crate::decode::decode_row; use crate::errors::{ends_connection, map_error}; use crate::in_band_request::{Interrupt, StopPolicy, Watch, conclude, cut_short}; use crate::physical::MssqlClient; +use crate::result_origin::describe; use crate::transport_io::ReadTally; /// Rows per `ResultEvent::Rows`. The sink is awaited per batch, so a slow @@ -118,6 +119,7 @@ impl EditorRun<'_> { async fn batch(&mut self, watch: &Watch, text: &str) -> Result { let limit = self.limit.get(); + let mut origins = describe(self.client, watch, text).await?; let sink = &mut *self.sink; let mut stream = watch.start(self.client.simple_query(text)).await?; let mut open: Option = None; @@ -145,7 +147,12 @@ impl EditorRun<'_> { return Ok(BatchEnd::ConsumerGone); } open = Some(OpenSet::default()); - let columns: Arc<[ResultColumn]> = metadata.columns().iter().map(result_column).collect(); + let columns: Vec = metadata.columns().iter().map(result_column).collect(); + let columns: Arc<[ResultColumn]> = match origins.take() { + Some(origins) => origins.attach(columns), + None => columns, + } + .into(); sink.accept(ResultEvent::ResultSetStarted { columns }).await } QueryItem::Row(row) => match open.as_mut() { diff --git a/core/crates/drivers/mssql/src/lib.rs b/core/crates/drivers/mssql/src/lib.rs index e72d3e0759..9c7a2b91c6 100644 --- a/core/crates/drivers/mssql/src/lib.rs +++ b/core/crates/drivers/mssql/src/lib.rs @@ -31,6 +31,7 @@ mod mssql_pooled; mod physical; mod probe; mod read_only_guard; +mod result_origin; mod standalone; mod stream_read; mod tds_message; diff --git a/core/crates/drivers/mssql/src/result_origin.rs b/core/crates/drivers/mssql/src/result_origin.rs new file mode 100644 index 0000000000..122853a56b --- /dev/null +++ b/core/crates/drivers/mssql/src/result_origin.rs @@ -0,0 +1,237 @@ +use tablepro_core::column::{ColumnOrigin, ResultColumn}; +use tablepro_core::meta::TableRef; +use tablepro_core::sql_syntax::{SqlGrammar, source_table}; +use tablepro_core::value::Value; +use tracing::debug; + +use crate::catalog::Cells; +use crate::dialect::quote; +use crate::errors::ends_connection; +use crate::in_band_request::{Interrupt, Watch}; +use crate::physical::MssqlClient; +use crate::workspace_call::{catalogue_rows, text_param}; + +// Mode 1 traces a view or a synonym through to its base table, so a column counts only when the statement's own name resolves to that table. +// Inside a transaction, or where IMPLICIT_TRANSACTIONS would open one, nothing is described: the describe must neither fail inside the user's work nor start it. +const DESCRIBE_SQL: &str = "IF @@TRANCOUNT = 0 AND (@@OPTIONS & 2) = 0 \ + SELECT d.column_ordinal, d.name, d.is_hidden, d.source_schema, d.source_table, d.source_column, \ + CAST(CASE WHEN d.source_server IS NULL AND DB_ID(d.source_database) = DB_ID() \ + AND OBJECT_ID(QUOTENAME(d.source_schema) + N'.' + QUOTENAME(d.source_table), N'U') = OBJECT_ID(@P2) \ + THEN 1 ELSE 0 END AS bit) \ + FROM sys.dm_exec_describe_first_result_set(@P1, NULL, 1) AS d"; + +const DESCRIBED_WIDTH: usize = 7; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct Origins(Vec<(String, Option)>); + +impl Origins { + // A describe whose columns are not the result's saw another statement: the schema changed in between, or a session setting reshaped one of them. + pub(crate) fn attach(self, columns: Vec) -> Vec { + let same = self.0.len() == columns.len() + && self + .0 + .iter() + .zip(&columns) + .all(|((name, _), column)| *name == column.name); + if !same { + return columns; + } + columns + .into_iter() + .zip(self.0) + .map(|(column, (_, origin))| column.with_origin(origin)) + .collect() + } +} + +pub(crate) async fn describe( + client: &mut MssqlClient, + watch: &Watch, + batch: &str, +) -> Result, Interrupt> { + let Ok(statement) = source_table(batch, SqlGrammar::MsSql) else { + return Ok(None); + }; + // A temporary table lives in tempdb under a name only this session resolves. + if statement.table.name.starts_with('#') { + return Ok(None); + } + let params = [text_param(batch), text_param(&written_name(&statement.table))]; + match catalogue_rows(client, watch, DESCRIBE_SQL, ¶ms).await { + Ok(rows) => Ok(origins(&rows)), + // The user's batch has not been sent yet, so a refused describe only leaves its result untraced. + Err(Interrupt::Failed(error)) if !ends_connection(&error) => { + debug!(%error, "an editor result could not be described"); + Ok(None) + } + Err(interrupt) => Err(interrupt), + } +} + +fn written_name(table: &TableRef) -> String { + match &table.schema { + Some(schema) => format!("{}.{}", quote(schema), quote(&table.name)), + None => quote(&table.name), + } +} + +fn origins(rows: &[Vec]) -> Option { + let mut shown = Vec::with_capacity(rows.len()); + for row in rows { + if row.len() != DESCRIBED_WIDTH { + return None; + } + let cells = Cells::new(row, "sys.dm_exec_describe_first_result_set"); + // A batch the server cannot describe comes back as one row with an error number and no ordinal. + let ordinal = cells.int(0)?; + if cells.flag(2) { + continue; + } + shown.push((ordinal, cells.text(1).unwrap_or_default(), origin(&cells))); + } + shown.sort_by_key(|(ordinal, _, _)| *ordinal); + let numbered = shown + .iter() + .zip(1_i64..) + .all(|((ordinal, _, _), position)| *ordinal == position); + (numbered && !shown.is_empty()) + .then(|| Origins(shown.into_iter().map(|(_, name, origin)| (name, origin)).collect())) +} + +fn origin(cells: &Cells<'_>) -> Option { + if !cells.flag(6) { + return None; + } + let (schema, table, column) = (cells.text(3)?, cells.text(4)?, cells.text(5)?); + if table.starts_with('#') { + return None; + } + Some(ColumnOrigin::new(TableRef::new(Some(schema), table), column)) +} + +#[cfg(test)] +mod tests { + use tablepro_core::column::{ColumnKind, ColumnType, ReadForm, TextLength}; + + use super::*; + + fn text(value: &str) -> Value { + Value::Text(value.to_owned()) + } + + fn row(ordinal: i64, name: &str, hidden: bool, source: Option<(&str, &str, &str)>, base: bool) -> Vec { + let (schema, table, column) = match source { + Some((schema, table, column)) => (text(schema), text(table), text(column)), + None => (Value::Null, Value::Null, Value::Null), + }; + vec![ + Value::Int(ordinal), + text(name), + Value::Bool(hidden), + schema, + table, + column, + Value::Bool(base), + ] + } + + fn column(name: &str) -> ResultColumn { + ResultColumn::new( + name, + ColumnType { + name: "nvarchar".to_owned(), + kind: ColumnKind::Text { + length: TextLength::Unbounded, + }, + catalog: None, + dynamic_storage: false, + read_form: ReadForm::Native, + }, + ) + } + + fn traced(table: &str, column: &str) -> Option { + Some(ColumnOrigin::new(TableRef::new(Some("dbo".to_owned()), table), column)) + } + + fn attached(rows: &[Vec], names: &[&str]) -> Vec> { + let columns: Vec = names.iter().copied().map(column).collect(); + let columns = match origins(rows) { + Some(origins) => origins.attach(columns), + None => columns, + }; + columns.into_iter().map(|column| column.origin).collect() + } + + #[test] + fn a_renamed_column_traces_to_its_base_column_and_an_expression_to_nothing() { + let rows = [ + row(1, "x", false, Some(("dbo", "t", "a")), true), + row(2, "id", false, Some(("dbo", "t", "id")), true), + row(3, "e", false, None, false), + ]; + + assert_eq!( + attached(&rows, &["x", "id", "e"]), + vec![traced("t", "a"), traced("t", "id"), None] + ); + } + + #[test] + fn a_hidden_key_row_is_not_a_column_of_the_result() { + let rows = [ + row(2, "id", true, Some(("dbo", "t", "id")), true), + row(1, "a", false, Some(("dbo", "t", "a")), true), + ]; + + assert_eq!(attached(&rows, &["a"]), vec![traced("t", "a")]); + } + + #[test] + fn a_source_that_is_not_the_named_base_table_or_is_temporary_is_untraced() { + let rows = [ + row(1, "id", false, Some(("dbo", "t", "id")), false), + row(2, "tid", false, Some(("dbo", "#tmp_____2", "tid")), true), + ]; + + assert_eq!(attached(&rows, &["id", "tid"]), vec![None, None]); + } + + #[test] + fn a_describe_that_does_not_match_the_result_traces_nothing() { + let rows = [ + row(1, "id", false, Some(("dbo", "t", "id")), true), + row(2, "a", false, Some(("dbo", "t", "a")), true), + ]; + let error = vec![ + Value::Null, + Value::Null, + Value::Bool(false), + Value::Null, + Value::Null, + Value::Null, + Value::Bool(false), + ]; + + assert_eq!(attached(&rows, &["id", "b"]), vec![None, None], "another name"); + assert_eq!(attached(&rows, &["id"]), vec![None], "another width"); + assert_eq!(attached(&rows[1..], &["a"]), vec![None], "a gap in the ordinals"); + assert_eq!(attached(&[error], &["id"]), vec![None], "an error row"); + assert_eq!( + attached(&[vec![text("plan")]], &["id"]), + vec![None], + "another result set" + ); + assert_eq!(attached(&[], &["id"]), vec![None], "nothing described"); + } + + #[test] + fn the_statement_name_is_bracketed_as_written() { + assert_eq!(written_name(&TableRef::unqualified("t")), "[t]"); + assert_eq!( + written_name(&TableRef::new(Some("Mixed Case".to_owned()), "odd]name")), + "[Mixed Case].[odd]]name]" + ); + } +} diff --git a/core/crates/drivers/mssql/tests/integration/sessions.rs b/core/crates/drivers/mssql/tests/integration/sessions.rs index 4df5fae92b..dd0b4c5e5d 100644 --- a/core/crates/drivers/mssql/tests/integration/sessions.rs +++ b/core/crates/drivers/mssql/tests/integration/sessions.rs @@ -1,10 +1,11 @@ //! Editor sessions: one connection per tab, batches as SQL batches, so //! USE, SET and an open transaction last from one run to the next. -use tablepro_core::{ResultEvent, RowLimit, RunOptions, TransactionState, Truncation, Value}; +use tablepro_core::column::ColumnOrigin; +use tablepro_core::{ResultEvent, RowLimit, RunEnd, RunOptions, TableRef, TransactionState, Truncation, Value}; use tokio_util::sync::CancellationToken; -use crate::support::{connect, run, run_ok, run_with, scalar, session, setup, start}; +use crate::support::{Events, connect, run, run_ok, run_with, scalar, session, setup, start}; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[ignore = "requires docker"] @@ -141,3 +142,151 @@ async fn go_batches_run_in_order_and_continue_after_an_error() { assert!(!summary.succeeded()); assert_eq!(events.rows(), vec![vec![Value::Int(1)], vec![Value::Int(3)]]); } + +const TRACED_SCHEMA: &str = "CREATE TABLE dbo.t (id int PRIMARY KEY, a nvarchar(20), b int) +GO +INSERT INTO dbo.t VALUES (1, N'one', 10) +GO +CREATE SCHEMA [Sales Data] +GO +CREATE TABLE [Sales Data].[Orders] (order_id int PRIMARY KEY, total int) +GO +CREATE VIEW dbo.v_t AS SELECT id, a FROM dbo.t +GO +CREATE SYNONYM dbo.syn_t FOR dbo.t"; + +fn origins(events: &Events) -> Vec> { + let sets: Vec>> = events + .all() + .into_iter() + .filter_map(|event| match event { + ResultEvent::ResultSetStarted { columns } => { + Some(columns.iter().map(|column| column.origin.clone()).collect()) + } + _ => None, + }) + .collect(); + assert_eq!(sets.len(), 1, "one result set, and nothing from the describe"); + sets.into_iter().next().unwrap_or_default() +} + +fn from(schema: &str, table: &str, column: &str) -> Option { + Some(ColumnOrigin::new(TableRef::new(Some(schema.to_owned()), table), column)) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn editor_results_carry_the_base_table_column_each_column_reads() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + let session = session(connection.as_ref()).await; + run_ok(session.as_ref(), TRACED_SCHEMA).await; + + let every = run_ok(session.as_ref(), "SELECT * FROM t").await; + assert_eq!( + origins(&every), + vec![from("dbo", "t", "id"), from("dbo", "t", "a"), from("dbo", "t", "b")] + ); + assert_eq!( + every.rows(), + vec![vec![Value::Int(1), Value::Text("one".to_owned()), Value::Int(10)]] + ); + + let renamed = run_ok(session.as_ref(), "SELECT a AS x, id FROM t").await; + assert_eq!(origins(&renamed), vec![from("dbo", "t", "a"), from("dbo", "t", "id")]); + + let expression = run_ok(session.as_ref(), "SELECT id, a + N'!' AS e FROM t").await; + assert_eq!(origins(&expression), vec![from("dbo", "t", "id"), None]); + + // The describe adds the key the select list leaves out as a hidden row; the host finds the key missing. + let keyless = run_ok(session.as_ref(), "SELECT a FROM t").await; + assert_eq!(origins(&keyless), vec![from("dbo", "t", "a")]); + + let other_schema = run_ok(session.as_ref(), "SELECT * FROM [Sales Data].[Orders]").await; + assert_eq!( + origins(&other_schema), + vec![ + from("Sales Data", "Orders", "order_id"), + from("Sales Data", "Orders", "total") + ] + ); + + run_ok( + session.as_ref(), + "CREATE DATABASE other\nGO\nCREATE TABLE other.dbo.t2 (id int PRIMARY KEY, b int)", + ) + .await; + // TableRef has no database part, so a table named through another database stays untraced. + let three_part = run_ok(session.as_ref(), "SELECT * FROM other.dbo.t2").await; + assert_eq!(origins(&three_part), vec![None, None]); + // After a USE the origin is the session's own database's table; the host refuses a session in another database than the tab's. + run_ok(session.as_ref(), "USE other").await; + let moved = run_ok(session.as_ref(), "SELECT * FROM t2").await; + assert_eq!(origins(&moved), vec![from("dbo", "t2", "id"), from("dbo", "t2", "b")]); + assert_eq!(session.database().as_deref(), Some("other")); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn editor_results_of_a_view_synonym_temp_table_or_open_transaction_carry_no_origin() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + let session = session(connection.as_ref()).await; + run_ok(session.as_ref(), TRACED_SCHEMA).await; + + // Browse mode traces both to dbo.t; the statement names another object, so the driver reports nothing. + let view = run_ok(session.as_ref(), "SELECT * FROM v_t").await; + assert_eq!(origins(&view), vec![None, None]); + let synonym = run_ok(session.as_ref(), "SELECT * FROM syn_t").await; + assert_eq!(origins(&synonym), vec![None, None, None]); + + run_ok( + session.as_ref(), + "CREATE TABLE #tmp (tid int PRIMARY KEY, tv nvarchar(20))", + ) + .await; + let temporary = run_ok(session.as_ref(), "SELECT * FROM #tmp").await; + assert_eq!(origins(&temporary), vec![None, None]); + + run_ok(session.as_ref(), "BEGIN TRANSACTION").await; + let (inside, events) = run(session.as_ref(), "SELECT * FROM t").await; + assert_eq!(inside.end, RunEnd::Completed); + assert_eq!(inside.transaction_state, TransactionState::InTransaction); + assert_eq!(origins(&events), vec![None, None, None]); + run_ok(session.as_ref(), "ROLLBACK TRANSACTION").await; + + run_ok(session.as_ref(), "SET IMPLICIT_TRANSACTIONS ON").await; + let implicit = run_ok(session.as_ref(), "SELECT * FROM t").await; + assert_eq!(origins(&implicit), vec![None, None, None]); + run_ok( + session.as_ref(), + "IF @@TRANCOUNT > 0 ROLLBACK TRANSACTION\nGO\nSET IMPLICIT_TRANSACTIONS OFF", + ) + .await; + + let control = run_ok(session.as_ref(), "SELECT id FROM t").await; + assert_eq!(origins(&control), vec![from("dbo", "t", "id")]); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn a_stop_while_a_result_is_described_cancels_the_run_and_keeps_the_session() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + let session = session(connection.as_ref()).await; + run_ok( + session.as_ref(), + "CREATE TABLE dbo.t (id int PRIMARY KEY)\nGO\nINSERT INTO dbo.t VALUES (1)", + ) + .await; + let cancel = CancellationToken::new(); + cancel.cancel(); + + let (stopped, events) = run_with(session.as_ref(), "SELECT * FROM t", RunOptions::new(cancel)).await; + + assert_eq!(stopped.end, RunEnd::Cancelled); + assert!(events.rows().is_empty(), "{:?}", events.all()); + let after = run_ok(session.as_ref(), "SELECT * FROM t").await; + assert_eq!(origins(&after), vec![from("dbo", "t", "id")]); + assert_eq!(after.rows(), vec![vec![Value::Int(1)]]); +} diff --git a/core/crates/drivers/mysql/src/column_origins.rs b/core/crates/drivers/mysql/src/column_origins.rs new file mode 100644 index 0000000000..45faa8cc8c --- /dev/null +++ b/core/crates/drivers/mysql/src/column_origins.rs @@ -0,0 +1,149 @@ +use std::sync::Arc; + +use sqlx::mysql::{MySqlConnection, MySqlRow}; +use sqlx::{AssertSqlSafe, Column as _, Executor as _, Row as _}; +use tablepro_core::ResultEvent; +use tablepro_core::column::{ColumnOrigin, ResultColumn}; +use tablepro_core::meta::TableRef; +use tablepro_core::sql_syntax::SourceTable; + +use crate::dialect::quote; + +// performance_schema's tables show as CREATE TABLE, and no row of theirs is a stored one. +const SYSTEM_SCHEMAS: [&str; 2] = ["information_schema", "performance_schema"]; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct TracedSet { + set_index: usize, + columns: Arc<[ResultColumn]>, + // As sqlx gives it: the schema and table joined with an unquoted dot, and the column. + wire: Vec, Arc)>>, + statement: SourceTable, +} + +impl TracedSet { + pub(crate) fn new(set_index: usize, columns: Arc<[ResultColumn]>, row: &MySqlRow, statement: SourceTable) -> Self { + let wire = row + .columns() + .iter() + .map(|column| { + column + .origin() + .table_column() + .map(|origin| (origin.table.clone(), origin.name.clone())) + }) + .collect(); + Self { + set_index, + columns, + wire, + statement, + } + } +} + +// A merge view names itself, a TEMPTABLE view its base table, and a temporary table looks like the base table it shadows, so SHOW CREATE TABLE on the name the statement read decides. +pub(crate) async fn resolved_columns(connection: &mut MySqlConnection, set: TracedSet) -> Option { + let origins: Vec> = set + .wire + .iter() + .map(|wire| { + wire.as_ref() + .and_then(|(table, column)| origin(&set.statement.table, table, column)) + }) + .collect(); + let mut tables = origins.iter().flatten().map(|origin| &origin.table); + let first = tables.next()?; + if tables.any(|table| table != first) || !permanent_table(connection, &set.statement.table).await { + return None; + } + let columns = set + .columns + .iter() + .zip(origins) + .map(|(column, origin)| column.clone().with_origin(origin)) + .collect(); + Some(ResultEvent::ColumnsResolved { + set_index: set.set_index, + columns, + }) +} + +// The name the statement wrote splits the joined text, whatever dots the schema or table holds. +fn origin(statement: &TableRef, wire: &str, column: &str) -> Option { + let schema_end = wire.len().checked_sub(statement.name.len() + 1)?; + let schema = wire.get(..schema_end)?; + let table = wire.get(schema_end..)?.strip_prefix('.')?; + let named = table.eq_ignore_ascii_case(&statement.name) + && statement + .schema + .as_ref() + .is_none_or(|written| written.eq_ignore_ascii_case(schema)); + let system = SYSTEM_SCHEMAS.iter().any(|system| system.eq_ignore_ascii_case(schema)); + (named && !system && !schema.is_empty() && !column.is_empty()) + .then(|| ColumnOrigin::new(TableRef::new(Some(schema.to_owned()), table), column)) +} + +async fn permanent_table(connection: &mut MySqlConnection, table: &TableRef) -> bool { + let name = match &table.schema { + Some(schema) => format!("{}.{}", quote(schema), quote(&table.name)), + None => quote(&table.name), + }; + let shown = connection + .fetch_all(sqlx::raw_sql(AssertSqlSafe(format!("SHOW CREATE TABLE {name}")))) + .await; + let rows = match shown { + Ok(rows) => rows, + Err(error) => { + tracing::debug!(%error, "the result's table was not shown"); + return false; + } + }; + let definition = rows + .first() + .and_then(|row| row.try_get_unchecked::, _>(1).ok().flatten()); + // A temporary table, a view and a MariaDB sequence each show otherwise. + definition + .map(String::from_utf8_lossy) + .is_some_and(|definition| definition.starts_with("CREATE TABLE ") && !definition.contains(" SEQUENCE=1")) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn traced(schema: &str, table: &str, column: &str) -> Option { + Some(ColumnOrigin::new(TableRef::new(Some(schema.to_owned()), table), column)) + } + + #[test] + fn the_written_name_splits_a_dotted_schema_or_table() { + let unqualified = TableRef::unqualified("t"); + let dotted = TableRef::unqualified("b.c"); + + assert_eq!(origin(&unqualified, "p.t", "id"), traced("p", "t", "id")); + assert_eq!(origin(&unqualified, "a.b.t", "id"), traced("a.b", "t", "id")); + assert_eq!(origin(&dotted, "a.b.c", "id"), traced("a", "b.c", "id")); + assert_eq!( + origin(&TableRef::new(Some("P".to_owned()), "T"), "p.t", "id"), + traced("p", "t", "id") + ); + } + + #[test] + fn another_table_a_bare_name_or_a_system_schema_has_no_origin() { + let unqualified = TableRef::unqualified("vt"); + + assert_eq!(origin(&unqualified, "p.t", "id"), None); + assert_eq!(origin(&TableRef::unqualified("t"), "t", "id"), None); + assert_eq!(origin(&TableRef::new(Some("q".to_owned()), "t"), "p.t", "id"), None); + assert_eq!( + origin( + &TableRef::unqualified("accounts"), + "performance_schema.accounts", + "USER" + ), + None + ); + } +} diff --git a/core/crates/drivers/mysql/src/editor_run.rs b/core/crates/drivers/mysql/src/editor_run.rs index f80bb3c71e..5dd874a4c7 100644 --- a/core/crates/drivers/mysql/src/editor_run.rs +++ b/core/crates/drivers/mysql/src/editor_run.rs @@ -12,10 +12,12 @@ use futures_util::TryStreamExt; use sqlx::mysql::{MySqlConnection, MySqlRow}; use sqlx::{AssertSqlSafe, Either, Executor as _, Row as _}; use tablepro_core::column::ResultColumn; +use tablepro_core::sql_syntax::{SourceTable, SqlGrammar, source_table}; use tablepro_core::{DriverError, ResultEvent, ResultSink, RowLimit, SinkFlow, Truncation, Value}; use tokio_util::sync::CancellationToken; use crate::catalog::result_column; +use crate::column_origins::TracedSet; use crate::decode::decode_row; use crate::errors::map_error; use crate::flavour::MySqlFlavour; @@ -26,19 +28,15 @@ use crate::script_split; /// How many rows travel in one `Rows` event. const ROWS_PER_EVENT: usize = 256; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +#[derive(Debug, Clone, PartialEq, Eq, Default)] pub(crate) struct RunOutcome { - /// The sink stopped reading, so the run was cut short. pub consumer_gone: bool, - /// A stop came between two statements, so the rest was not sent. + // A stop came between two statements, so the rest was not sent. pub halted: bool, + pub traced: Option, } -/// Run the script one text per round trip (`script_split`), sending the -/// next only while `halt` says no stop has come. When the sink stops -/// reading, `stopped` is cancelled so the caller can stop the server. -/// The text already sent is still read to its end, unshown, since only -/// that frees the connection. +// A sink that stops reading cancels `stopped`; the text already sent is still read to its end, unshown, since only that frees the connection. pub(crate) async fn run_script( connection: &mut MySqlConnection, flavour: MySqlFlavour, @@ -48,15 +46,23 @@ pub(crate) async fn run_script( stopped: &CancellationToken, halt: impl Fn() -> bool, ) -> Result { + let texts = script_split::texts(sql); + // The table is looked up once the run is over, when only a run of one statement still has what that statement read. + let trace = match texts.as_slice() { + [text] => source_table(text, SqlGrammar::MySql).ok(), + _ => None, + }; let mut emitter = Emitter { sink, limit: limit.get(), open: None, sets: 0, gone: false, + trace, + traced: None, }; let mut halted = false; - for text in script_split::texts(sql) { + for text in texts { if emitter.gone { break; } @@ -79,6 +85,7 @@ pub(crate) async fn run_script( Ok(RunOutcome { consumer_gone: emitter.gone, halted, + traced: emitter.traced, }) } @@ -94,6 +101,8 @@ struct Emitter<'a> { open: Option, sets: usize, gone: bool, + trace: Option, + traced: Option, } impl Emitter<'_> { @@ -107,6 +116,12 @@ impl Emitter<'_> { .iter() .map(|column| result_column(column, MySqlTimestampMode::SessionLocal)) .collect(); + // Taken at the first set, so a second one leaves none. + let set_index = self.sets; + self.traced = self + .trace + .take() + .map(|statement| TracedSet::new(set_index, columns.clone(), row, statement)); self.sets += 1; self.open = Some(OpenSet { batch: Vec::new(), diff --git a/core/crates/drivers/mysql/src/editor_session.rs b/core/crates/drivers/mysql/src/editor_session.rs index 681243cc05..b278728e6f 100644 --- a/core/crates/drivers/mysql/src/editor_session.rs +++ b/core/crates/drivers/mysql/src/editor_session.rs @@ -15,6 +15,7 @@ use tokio::time::Instant; use tokio_util::sync::CancellationToken; use crate::cancel::ControlLink; +use crate::column_origins::{TracedSet, resolved_columns}; use crate::decode::decode_row; use crate::dialect::quote; use crate::editor_run::{RunOutcome, run_script}; @@ -113,8 +114,7 @@ impl Shared { } } - /// One run on the connection `guard` holds, which it gives up when - /// the connection cannot take another. + // The connection `guard` holds is given up when it cannot take another run. async fn run_held( &self, mut guard: OwnedMutexGuard>, @@ -131,7 +131,8 @@ impl Shared { }; let started = Instant::now(); self.running.store(true, Ordering::SeqCst); - let supervised = self.supervise(held, &sql, &options, sink.as_mut()).await; + let mut traced = None; + let supervised = self.supervise(held, &sql, &options, sink.as_mut(), &mut traced).await; // A session in a database nothing knows would run the next script // in the wrong one, so it ends instead. let placed = match &supervised { @@ -149,9 +150,16 @@ impl Shared { } _ => TransactionState::Unknown, }; + let settled = match (&supervised, placed, traced) { + (Ok((RunEnd::Completed, true)), true, Some(set)) => { + self.trace(&mut held.connection, set, transaction_state, sink.as_mut()) + .await + } + _ => true, + }; self.running.store(false, Ordering::SeqCst); let end = match supervised { - Ok((end, true)) if placed => end, + Ok((end, true)) if placed && settled => end, Ok((end, _)) => { self.lost.store(true, Ordering::SeqCst); *guard = None; @@ -185,16 +193,14 @@ impl Shared { true } - /// Run the script, stopping it on a cancel, at the deadline, when - /// the session closes or when the sink stops reading. A stop is KILL - /// QUERY on a connection of the session's own; the statement then - /// has the cancel grace to end, and nothing after it is sent. + // A stop is KILL QUERY on a connection of the session's own; the statement then has the cancel grace to end, and nothing after it is sent. async fn supervise( &self, held: &mut Held, sql: &UserSql, options: &RunOptions, sink: &mut dyn ResultSink, + traced: &mut Option, ) -> Result<(RunEnd, bool), DriverError> { let limit = options.row_limit; let deadline = options.deadline.map(|limit| Instant::now() + limit); @@ -216,7 +222,10 @@ impl Shared { let stop = self.stop_due(options, deadline).unwrap_or(Stop::Cancelled); Ok((stopped_end(stop, false), true)) } - outcome => finished(outcome).map(|end| (end, true)), + mut outcome => { + *traced = outcome.as_mut().ok().and_then(|outcome| outcome.traced.take()); + finished(outcome).map(|end| (end, true)) + } }; } () = options.cancel.cancelled() => Stop::Cancelled, @@ -245,6 +254,33 @@ impl Shared { } } + // Only a session known to be outside a transaction is traced, since the host edits no other. False when the lookup + // did not finish: its answer may still be due on the connection, which then takes no other statement. + async fn trace( + &self, + connection: &mut MySqlConnection, + set: TracedSet, + state: TransactionState, + sink: &mut dyn ResultSink, + ) -> bool { + if state != TransactionState::Idle { + return true; + } + let resolving = tokio::time::timeout(self.liveness.cancel_grace, resolved_columns(connection, set)); + let resolved = tokio::select! { + biased; + () = self.closing.cancelled() => return false, + resolved = resolving => match resolved { + Ok(resolved) => resolved, + Err(_) => return false, + }, + }; + if let Some(event) = resolved { + sink.accept(event).await; + } + true + } + fn stop_due(&self, options: &RunOptions, deadline: Option) -> Option { if options.cancel.is_cancelled() || self.closing.is_cancelled() { Some(Stop::Cancelled) diff --git a/core/crates/drivers/mysql/src/lib.rs b/core/crates/drivers/mysql/src/lib.rs index e774985393..5a246f76a6 100644 --- a/core/crates/drivers/mysql/src/lib.rs +++ b/core/crates/drivers/mysql/src/lib.rs @@ -9,6 +9,7 @@ mod cancel; mod capabilities; mod catalog; +mod column_origins; mod connection; mod ddl; mod decode; diff --git a/core/crates/drivers/mysql/tests/integration/editor_session.rs b/core/crates/drivers/mysql/tests/integration/editor_session.rs index 6701d7132d..4b09b574ef 100644 --- a/core/crates/drivers/mysql/tests/integration/editor_session.rs +++ b/core/crates/drivers/mysql/tests/integration/editor_session.rs @@ -4,8 +4,10 @@ use std::time::Duration; +use tablepro_core::column::ColumnOrigin; use tablepro_core::{ - CallOptions, EditorSession, EditorSessionState, RowLimit, RunEnd, RunOptions, RunSummary, TransactionState, UserSql, + CallOptions, EditorSession, EditorSessionState, ResultEvent, RowLimit, RunEnd, RunOptions, RunSummary, TableRef, + TransactionState, UserSql, }; use tablepro_test_fixtures::MysqlFlavour; use tokio_util::sync::CancellationToken; @@ -288,3 +290,145 @@ async fn a_use_moves_the_session_and_it_says_where() { connection.close().await.expect("a close"); } } + +fn traced(schema: &str, table: &str, column: &str) -> Option { + Some(ColumnOrigin::new(TableRef::new(Some(schema.to_owned()), table), column)) +} + +// The first set's columns, as the last `ColumnsResolved` left them. +fn origins_in(events: &Events) -> Vec> { + let mut columns = None; + for event in events.all() { + match event { + ResultEvent::ResultSetStarted { columns: started } if columns.is_none() => columns = Some(started), + ResultEvent::ColumnsResolved { + set_index: 0, + columns: resolved, + } => columns = Some(resolved), + _ => {} + } + } + columns + .expect("a result set") + .iter() + .map(|column| column.origin.clone()) + .collect() +} + +async fn origins(editor: &dyn EditorSession, sql: &str) -> Vec> { + origins_in(&run_ok(editor, sql).await) +} + +// Root, so MySQL's performance_schema says where the transaction stands. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn only_a_plain_read_of_a_permanent_table_traces_its_columns() { + for flavour in FLAVOURS { + let fixture = start(flavour).await; + let root = connect_root(&fixture).await; + setup( + root.as_ref(), + &[ + "CREATE TABLE t (id int PRIMARY KEY, a varchar(10), b int)", + "INSERT INTO t VALUES (1, 'x', 2)", + "CREATE VIEW v AS SELECT * FROM t", + "CREATE ALGORITHM = TEMPTABLE VIEW vt AS SELECT * FROM t", + "CREATE DATABASE IF NOT EXISTS other", + "CREATE TABLE other.w (id int PRIMARY KEY, c int)", + "INSERT INTO other.w VALUES (1, 3)", + ], + ) + .await; + let editor = session(root.as_ref()).await; + let database = fixture.database(); + let table = |column: &str| traced(database, "t", column); + let editor = editor.as_ref(); + + let plain = run_ok(editor, "SELECT * FROM t").await; + assert_eq!( + plain.started(), + 1, + "{flavour:?}: the lookup's own result reached the run" + ); + assert_eq!(plain.rows().len(), 1, "{flavour:?}"); + assert!( + !plain.all().iter().any(|event| matches!(event, ResultEvent::Notice(_))), + "{flavour:?}" + ); + assert_eq!(origins_in(&plain), [table("id"), table("a"), table("b")], "{flavour:?}"); + assert_eq!( + origins(editor, "SELECT a AS x, id FROM t").await, + [table("a"), table("id")], + "{flavour:?}" + ); + assert_eq!( + origins(editor, "SELECT id, b + 1 AS c FROM t").await, + [table("id"), None], + "{flavour:?}" + ); + assert_eq!( + origins(editor, "SELECT * FROM v").await, + [None, None, None], + "{flavour:?}" + ); + assert_eq!( + origins(editor, "SELECT * FROM vt").await, + [None, None, None], + "{flavour:?}" + ); + assert_eq!( + origins(editor, "SELECT * FROM other.w").await, + [traced("other", "w", "id"), traced("other", "w", "c")], + "{flavour:?}" + ); + assert_eq!( + origins(editor, "SELECT * FROM t; SELECT 1").await, + [None, None, None], + "{flavour:?}: a run of two statements" + ); + + run_ok(editor, "START TRANSACTION; INSERT INTO t VALUES (2, 'y', 3)").await; + let (inside, events) = crate::support::run(editor, "SELECT * FROM t").await; + assert_eq!(inside.transaction_state, TransactionState::InTransaction, "{flavour:?}"); + assert_eq!(origins_in(&events), [None, None, None], "{flavour:?}"); + run_ok(editor, "ROLLBACK").await; + + run_ok(editor, "CREATE TEMPORARY TABLE tt (id int PRIMARY KEY)").await; + run_ok(editor, "INSERT INTO tt VALUES (1)").await; + assert_eq!(origins(editor, "SELECT * FROM tt").await, [None], "{flavour:?}"); + run_ok( + editor, + "CREATE TEMPORARY TABLE t (id int PRIMARY KEY, a varchar(10), b int)", + ) + .await; + run_ok(editor, "INSERT INTO t VALUES (1, 'x', 2)").await; + assert_eq!( + origins(editor, "SELECT * FROM t").await, + [None, None, None], + "{flavour:?}: a temporary table over t" + ); + editor.close().await; + root.close().await.expect("a close"); + } +} + +// The host edits only rows read outside a transaction, so a session that cannot say is not traced. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn mysql_user_without_ps_select_is_not_traced() { + let fixture = start(MysqlFlavour::Mysql).await; + let connection = connect(&fixture, false).await; + setup( + connection.as_ref(), + &["CREATE TABLE t (id int PRIMARY KEY)", "INSERT INTO t VALUES (1)"], + ) + .await; + let editor = session(connection.as_ref()).await; + + let (summary, events) = crate::support::run(editor.as_ref(), "SELECT * FROM t").await; + + assert_eq!(summary.transaction_state, TransactionState::Unknown); + assert_eq!(origins_in(&events), [None]); + editor.close().await; + connection.close().await.expect("a close"); +} diff --git a/core/crates/drivers/postgres/src/editor/mod.rs b/core/crates/drivers/postgres/src/editor/mod.rs index 570d453aeb..a84d8e4b58 100644 --- a/core/crates/drivers/postgres/src/editor/mod.rs +++ b/core/crates/drivers/postgres/src/editor/mod.rs @@ -1,11 +1,11 @@ -//! Editor sessions in psql's execution model: the script is split on the -//! client and sent one statement per message, with the transaction -//! status followed statement by statement. +// Editor sessions in psql's execution model: the script is split on the client and sent one statement per message, +// with the transaction status followed statement by statement. mod actor; mod cursor_flight; mod editor_dispatch; mod editor_session; +mod result_origin; mod run; mod session_core; mod session_status; diff --git a/core/crates/drivers/postgres/src/editor/result_origin.rs b/core/crates/drivers/postgres/src/editor/result_origin.rs new file mode 100644 index 0000000000..aef73940d0 --- /dev/null +++ b/core/crates/drivers/postgres/src/editor/result_origin.rs @@ -0,0 +1,160 @@ +use postgres_protocol::escape::escape_identifier; +use tablepro_core::DriverError; +use tablepro_core::column::ColumnOrigin; +use tablepro_core::edit::parsed_origins; +use tablepro_core::meta::TableRef; +use tablepro_core::sql_syntax::{SourceTable, SqlGrammar, source_table}; +use tokio_postgres::Client; + +use super::editor_dispatch::CursorOwner; +use crate::metadata::{decode_error, text_rows}; + +pub(super) const SAVEPOINT: &str = "SAVEPOINT tablepro_origin_savepoint"; +pub(super) const RELEASE_SAVEPOINT: &str = "RELEASE SAVEPOINT tablepro_origin_savepoint"; +pub(super) const ROLLBACK_TO_SAVEPOINT: &str = + "ROLLBACK TO SAVEPOINT tablepro_origin_savepoint; RELEASE SAVEPOINT tablepro_origin_savepoint"; + +// The open cursor holds a lock on the relation its statement read, so a relation of the same name +// created since the DECLARE is not taken for it. +const ORIGIN_SQL: &str = "\ +SELECT n.nspname::text, c.relname::text, a.attname::text +FROM pg_catalog.pg_class c +JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace +JOIN pg_catalog.pg_attribute a ON a.attrelid = c.oid AND a.attnum > 0 AND NOT a.attisdropped +WHERE c.oid = pg_catalog.to_regclass($1) + AND c.relkind IN ('r', 'p') + AND c.relpersistence IN ('p', 'u') + AND n.nspname <> 'information_schema' + AND NOT pg_catalog.starts_with(n.nspname, 'pg_') + AND EXISTS ( + SELECT 1 FROM pg_catalog.pg_locks l + WHERE l.locktype = 'relation' AND l.relation = c.oid AND l.pid = pg_catalog.pg_backend_pid() + ) +ORDER BY a.attnum"; + +pub(super) struct TracedTable { + statement: SourceTable, + table: TableRef, + columns: Vec, +} + +impl TracedTable { + pub(super) fn origins(&self, names: &[String]) -> Vec> { + parsed_origins(&self.statement, &self.table, names) + .into_iter() + .map(|origin| origin.filter(|origin| self.columns.contains(&origin.column))) + .collect() + } +} + +// Inside the user's transaction a failed lookup would abort it, so only the driver's own transaction is traced. +pub(super) fn traceable(text: &str, owner: CursorOwner) -> Option { + match owner { + CursorOwner::Driver { .. } => source_table(text, SqlGrammar::PostgreSql).ok(), + CursorOwner::Savepoint => None, + } +} + +// tokio-postgres would prepare a lookup of any non-built-in column type under a name, so the statement is not +// described; its table is resolved instead, with every projected type built in. +pub(super) async fn lookup(client: &Client, statement: SourceTable) -> Result, DriverError> { + let relation = relation(&statement.table); + let rows = text_rows(client, ORIGIN_SQL, &[relation.as_str()]).await?; + let Some(first) = rows.first() else { + return Ok(None); + }; + let field = decode_error("pg_attribute"); + let schema: String = first.try_get(0).map_err(&field)?; + let name: String = first.try_get(1).map_err(&field)?; + let columns = rows + .iter() + .map(|row| row.try_get(2).map_err(&field)) + .collect::, _>>()?; + Ok(Some(TracedTable { + statement, + table: TableRef::new(Some(schema), name), + columns, + })) +} + +fn relation(table: &TableRef) -> String { + match table.schema.as_deref() { + Some(schema) => format!("{}.{}", escape_identifier(schema), escape_identifier(&table.name)), + None => escape_identifier(&table.name), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const DRIVER: CursorOwner = CursorOwner::Driver { read_only: false }; + + fn traced(sql: &str, columns: &[&str]) -> TracedTable { + TracedTable { + statement: traceable(sql, DRIVER).expect("one table"), + table: TableRef::new(Some("public".to_owned()), "t"), + columns: columns.iter().map(|column| (*column).to_owned()).collect(), + } + } + + fn columns(origins: &[Option]) -> Vec> { + origins + .iter() + .map(|origin| origin.as_ref().map(|origin| origin.column.as_str())) + .collect() + } + + fn names(names: &[&str]) -> Vec { + names.iter().map(|name| (*name).to_owned()).collect() + } + + #[test] + fn only_a_one_table_select_in_the_drivers_own_transaction_is_traced() { + assert!(traceable("SELECT * FROM t", DRIVER).is_some()); + assert!(traceable("SELECT * FROM t", CursorOwner::Driver { read_only: true }).is_some()); + assert!(traceable("SELECT * FROM t", CursorOwner::Savepoint).is_none()); + assert!(traceable("SELECT t.id, u.c FROM t JOIN u ON u.t_id = t.id", DRIVER).is_none()); + assert!(traceable("SELECT count(*) FROM t", DRIVER).is_none()); + } + + #[test] + fn a_column_the_table_lacks_has_no_origin() { + let table = traced("SELECT ctid, id, a AS x, id + 1 FROM t", &["id", "a"]); + + let origins = table.origins(&names(&["ctid", "id", "x", "?column?"])); + + assert_eq!(columns(&origins), vec![None, Some("id"), Some("a"), None]); + assert!(origins.iter().flatten().all(|origin| origin.table == table.table)); + } + + #[test] + fn a_wildcard_traces_the_columns_it_read() { + let table = traced("SELECT *, 1 AS one FROM t", &["id", "a"]); + + let origins = table.origins(&names(&["id", "a", "one"])); + + assert_eq!(columns(&origins), vec![Some("id"), Some("a"), None]); + } + + #[test] + fn a_bare_value_keyword_is_never_the_column_of_that_name() { + let table = traced( + "SELECT current_role, \"current_role\", CURRENT_SCHEMA, id FROM t", + &["current_role", "current_schema", "id"], + ); + + let origins = table.origins(&names(&["current_role", "current_role", "current_schema", "id"])); + + assert_eq!(columns(&origins), vec![None, Some("current_role"), None, Some("id")]); + } + + #[test] + fn the_relation_is_quoted_as_the_statement_resolved_it() { + assert_eq!( + relation(&TableRef::new(Some("Other".to_owned()), "my\"t")), + "\"Other\".\"my\"\"t\"" + ); + assert_eq!(relation(&TableRef::unqualified("t")), "\"t\""); + } +} diff --git a/core/crates/drivers/postgres/src/editor/run.rs b/core/crates/drivers/postgres/src/editor/run.rs index cca1a904cb..533e79623d 100644 --- a/core/crates/drivers/postgres/src/editor/run.rs +++ b/core/crates/drivers/postgres/src/editor/run.rs @@ -2,17 +2,19 @@ use std::sync::Arc; use std::sync::atomic::Ordering; use futures_util::StreamExt; -use tablepro_core::sql_syntax::SqlGrammar; use tablepro_core::sql_syntax::script::{LexicalSettings, ScriptPlan, ScriptToken}; +use tablepro_core::sql_syntax::{SourceTable, SqlGrammar}; use tablepro_core::{ DriverError, ResultEvent, ResultSink, RowLimit, RunEnd, ServerCode, ServerNotice, SinkFlow, Truncation, Value, }; use tokio_postgres::SimpleQueryMessage; +use tracing::debug; use super::cursor_flight::{ CLOSE, RELEASE_SAVEPOINT, ROLLBACK_TO_SAVEPOINT, SAVEPOINT, correct_position, declare, fetch, needs_plain_statement, }; use super::editor_dispatch::{CursorOwner, Flight, Wrap, fallback, plan}; +use super::result_origin::{self, TracedTable, traceable}; use super::session_core::{Lost, SessionCore}; use super::statement_class::{StatementClass, classify}; use super::stop_watch::{Interrupt, StopWatch}; @@ -37,13 +39,14 @@ enum Role { }, } -/// One run of user SQL on the session. pub(crate) struct Run<'a> { core: &'a mut SessionCore, sink: &'a mut dyn ResultSink, watch: StopWatch, limit: RowLimit, sink_gone: bool, + // Set only while a cursor's FETCH is read, for its RowDescription. + traced: Option, } /// The result set an exchange is reading. @@ -69,6 +72,7 @@ impl<'a> Run<'a> { watch, limit, sink_gone: false, + traced: None, } } @@ -158,9 +162,8 @@ impl<'a> Run<'a> { } } - /// A row source read through a cursor, so a result larger than the - /// limit stops on the server. The DECLARE and the FETCH go as - /// separate messages. + // A row source goes through a cursor, so a result larger than the limit stops on the server. The DECLARE and the + // FETCH go as separate messages. async fn cursor( &mut self, text: &str, @@ -184,9 +187,15 @@ impl<'a> Run<'a> { self.abandon_cursor(owner, false).await?; return Ok(Some(error)); } + // With backslash escapes on, the statement the parser reads may not be the one the server ran. + self.traced = match traceable(text, owner) { + Some(statement) if self.core.standard_conforming_strings() => self.trace(statement).await?, + _ => None, + }; let fetched = self .exchange(&fetch(self.limit), Role::User { class, cursor: true }) .await?; + self.traced = None; if let Some(error) = fetched { self.abandon_cursor(owner, false).await?; return Ok(Some(error)); @@ -219,6 +228,49 @@ impl<'a> Run<'a> { Ok(()) } + // The lookup goes in one round trip between a savepoint of the driver's own and its release, so one that fails + // is rolled back to the savepoint and leaves the cursor's transaction and the user's rows as they were. + async fn trace(&mut self, statement: SourceTable) -> Result, Lost> { + if self.stopped() || self.watch.due().is_some() { + return Ok(None); + } + self.watch.settle().await; + let (saved, found, released) = { + let client = &self.core.client; + let mut flight = std::pin::pin!(async move { + futures_util::join!( + client.batch_execute(result_origin::SAVEPOINT), + result_origin::lookup(client, statement), + client.batch_execute(result_origin::RELEASE_SAVEPOINT), + ) + }); + loop { + tokio::select! { + biased; + answers = &mut flight => break answers, + interrupt = self.watch.triggered(), if self.watch.fired().is_none() => { + let running = Some((Arc::clone(&self.core.endpoint), self.core.cancel_token.clone())); + self.watch.fire(interrupt, running); + } + () = self.watch.gave_up(), if self.watch.fired().is_some() => return Err(Lost), + } + } + }; + self.core.discard_notices(); + if saved.is_ok() && released.is_err() && !self.stopped() { + self.exchange(result_origin::ROLLBACK_TO_SAVEPOINT, Role::Control) + .await?; + } + match (found, released) { + (Ok(traced), Ok(())) => Ok(traced), + (Ok(_), Err(_)) => Ok(None), + (Err(error), _) => { + debug!(%error, "the origin lookup of a result failed"); + Ok(None) + } + } + } + async fn track(&mut self, class: StatementClass, succeeded: bool) -> Result<(), Lost> { match self.core.tracker.after(class, succeeded) { Next::Known(state) => { @@ -304,7 +356,17 @@ impl<'a> Run<'a> { match message { SimpleQueryMessage::RowDescription(columns) => { set.open = true; - let columns = columns.iter().map(|column| text_column(column.name())).collect(); + let names: Vec = columns.iter().map(|column| column.name().to_owned()).collect(); + let mut origins = self + .traced + .take() + .map(|traced| traced.origins(&names)) + .unwrap_or_default() + .into_iter(); + let columns = names + .iter() + .map(|name| text_column(name).with_origin(origins.next().flatten())) + .collect(); self.send(ResultEvent::ResultSetStarted { columns }).await; } SimpleQueryMessage::Row(row) => { diff --git a/core/crates/drivers/postgres/tests/integration/editor_session.rs b/core/crates/drivers/postgres/tests/integration/editor_session.rs index 63eb936270..2ad8d529fa 100644 --- a/core/crates/drivers/postgres/tests/integration/editor_session.rs +++ b/core/crates/drivers/postgres/tests/integration/editor_session.rs @@ -4,13 +4,18 @@ use std::time::Duration; +use secrecy::SecretString; +use tablepro_core::column::ColumnOrigin; use tablepro_core::{ - DriverError, EditorSessionState, ResultEvent, RowLimit, RunEnd, RunOptions, TransactionState, Truncation, Value, + Credentials, DriverError, EditorSessionState, ResultEvent, RowLimit, RunEnd, RunOptions, TableRef, TlsConfig, + TransactionState, Truncation, Value, WorkspaceSettings, }; +use tablepro_test_support::conformance::test_liveness; use tokio_util::sync::CancellationToken; use crate::support::{ - CALL, EventSink, Events, connect, run, run_ok, run_with, scalar, session, setup, start, text, within, + CALL, EventSink, Events, connect, connect_with, direct, options_over, run, run_ok, run_with, scalar, session, + setup, start, text, within, }; fn limited(rows: u64) -> RunOptions { @@ -570,3 +575,195 @@ async fn a_session_opened_in_another_database_runs_there() { ); elsewhere.close().await; } + +const TRACED_TABLES: &[&str] = &[ + "CREATE TYPE mood AS ENUM ('sad', 'happy')", + "CREATE TABLE t (id int PRIMARY KEY, a text, b int, feeling mood)", + "INSERT INTO t VALUES (1, 'x', 2, 'happy')", + "CREATE TABLE u (id int PRIMARY KEY, t_id int, c text)", + "CREATE VIEW v AS SELECT id, a FROM t", + "CREATE SCHEMA other", + "CREATE TABLE other.w (id int PRIMARY KEY, label text)", + "CREATE TABLE \"MixedCase\" (\"Id\" int PRIMARY KEY)", +]; + +fn origins(events: &Events) -> Vec> { + events + .all() + .into_iter() + .find_map(|event| match event { + ResultEvent::ResultSetStarted { columns } => { + Some(columns.iter().map(|column| column.origin.clone()).collect()) + } + _ => None, + }) + .expect("a result set") +} + +fn origin(schema: &str, table: &str, column: &str) -> Option { + Some(ColumnOrigin::new(TableRef::new(Some(schema.to_owned()), table), column)) +} + +fn of_t(columns: &[&str]) -> Vec> { + columns.iter().map(|column| origin("public", "t", column)).collect() +} + +// The lookup projects built-in types only, so the enum column makes tokio-postgres prepare nothing under a name. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn a_whole_table_traces_every_column_and_prepares_nothing() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + setup(connection.as_ref(), TRACED_TABLES).await; + let session = session(connection.as_ref()).await; + + let (summary, whole) = run(session.as_ref(), "SELECT * FROM t").await; + let prepared = run_ok(session.as_ref(), "SELECT name FROM pg_prepared_statements").await; + + assert_eq!(summary.end, RunEnd::Completed); + assert_eq!(summary.transaction_state, TransactionState::Idle); + assert_eq!(origins(&whole), of_t(&["id", "a", "b", "feeling"])); + assert_eq!(texts(&whole), ["1", "x", "2", "happy"]); + assert!(prepared.rows().is_empty(), "{:?}", prepared.rows()); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn a_renamed_column_traces_to_the_column_it_reads() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + setup(connection.as_ref(), TRACED_TABLES).await; + let session = session(connection.as_ref()).await; + + let renamed = run_ok(session.as_ref(), "SELECT a AS x, id, b + 1 AS bumped, 'k' AS k FROM t").await; + let swapped = run_ok(session.as_ref(), "SELECT id AS a, a AS id FROM t").await; + let qualified = run_ok(session.as_ref(), "SELECT t.b, public.t.id FROM public.t").await; + + assert_eq!( + origins(&renamed), + vec![origin("public", "t", "a"), origin("public", "t", "id"), None, None] + ); + assert_eq!(origins(&swapped), of_t(&["id", "a"])); + assert_eq!(origins(&qualified), of_t(&["b", "id"])); +} + +// A join is refused by the statement's shape before any lookup, so its columns carry no origin either. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn views_temporary_tables_joins_and_catalogs_are_not_traced() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + setup(connection.as_ref(), TRACED_TABLES).await; + let session = session(connection.as_ref()).await; + run_ok(session.as_ref(), "CREATE TEMP TABLE scratch (id int PRIMARY KEY)").await; + + let view = run_ok(session.as_ref(), "SELECT * FROM v").await; + let temporary = run_ok(session.as_ref(), "SELECT * FROM scratch").await; + let joined = run_ok(session.as_ref(), "SELECT t.id, u.c FROM t JOIN u ON u.t_id = t.id").await; + let catalog = run_ok( + session.as_ref(), + "SELECT oid, relname FROM pg_catalog.pg_class WHERE relname = 'pg_class'", + ) + .await; + run_ok(session.as_ref(), "CREATE TEMP TABLE t (id int PRIMARY KEY)").await; + let shadowed = run_ok(session.as_ref(), "SELECT * FROM t").await; + + assert_eq!(origins(&view), vec![None, None]); + assert_eq!(origins(&temporary), vec![None]); + assert_eq!(origins(&joined), vec![None, None]); + assert_eq!(origins(&catalog), vec![None, None]); + assert_eq!(origins(&shadowed), vec![None]); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn a_table_in_another_schema_traces_qualified() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + setup(connection.as_ref(), TRACED_TABLES).await; + let session = session(connection.as_ref()).await; + + let named = run_ok(session.as_ref(), "SELECT * FROM other.w").await; + let mixed = run_ok(session.as_ref(), "SELECT * FROM \"MixedCase\"").await; + run_ok(session.as_ref(), "SET search_path TO other, public").await; + let found = run_ok(session.as_ref(), "SELECT label FROM w").await; + + assert_eq!( + origins(&named), + vec![origin("other", "w", "id"), origin("other", "w", "label")] + ); + assert_eq!(origins(&mixed), vec![origin("public", "MixedCase", "Id")]); + assert_eq!(origins(&found), vec![origin("other", "w", "label")]); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn a_select_inside_the_users_transaction_is_not_traced() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + setup(connection.as_ref(), TRACED_TABLES).await; + let session = session(connection.as_ref()).await; + + let (summary, events) = run(session.as_ref(), "BEGIN; SELECT * FROM t").await; + + assert_eq!(summary.end, RunEnd::Completed); + assert_eq!(summary.transaction_state, TransactionState::InTransaction); + assert_eq!(origins(&events), vec![None; 4]); + run_ok(session.as_ref(), "ROLLBACK").await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn a_read_only_connection_traces_its_reads() { + let fixture = start().await; + setup(connect(&fixture, false).await.as_ref(), TRACED_TABLES).await; + let reader = connect(&fixture, true).await; + let session = session(reader.as_ref()).await; + + let (summary, events) = run(session.as_ref(), "SELECT id, a FROM t").await; + + assert_eq!(summary.end, RunEnd::Completed); + assert_eq!(summary.transaction_state, TransactionState::Idle); + assert_eq!(origins(&events), of_t(&["id", "a"])); +} + +// A role that cannot read pg_locks fails the lookup inside the driver's own transaction. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn a_failed_lookup_leaves_the_rows_and_the_session_as_they_were() { + let fixture = start().await; + let admin = connect(&fixture, false).await; + setup(admin.as_ref(), TRACED_TABLES).await; + let granting = session(admin.as_ref()).await; + run_ok( + granting.as_ref(), + "CREATE ROLE reader LOGIN PASSWORD 'reader'; GRANT SELECT ON t TO reader; \ + REVOKE SELECT ON pg_catalog.pg_locks FROM PUBLIC", + ) + .await; + let liveness = test_liveness(); + let reader = connect_with(options_over( + direct(fixture.host(), fixture.port(), &liveness), + Credentials::Password { + username: "reader".to_owned(), + password: SecretString::from("reader".to_owned()), + }, + fixture.database(), + TlsConfig::disabled(), + WorkspaceSettings::workspace(false), + liveness, + )) + .await; + let session = session(reader.as_ref()).await; + + let (denied, _) = run(session.as_ref(), "SELECT 1 FROM pg_catalog.pg_locks").await; + let (summary, events) = run(session.as_ref(), "SELECT id, a FROM t").await; + let after = run_ok(session.as_ref(), "SELECT 1").await; + + assert!(matches!(denied.end, RunEnd::Failed(_)), "{:?}", denied.end); + assert_eq!(summary.end, RunEnd::Completed); + assert_eq!(summary.transaction_state, TransactionState::Idle); + assert_eq!(origins(&events), vec![None, None]); + assert_eq!(texts(&events), ["1", "x"]); + assert_eq!(texts(&after), ["1"]); +} diff --git a/core/crates/drivers/sqlite/src/column_origins.rs b/core/crates/drivers/sqlite/src/column_origins.rs new file mode 100644 index 0000000000..503737dbaa --- /dev/null +++ b/core/crates/drivers/sqlite/src/column_origins.rs @@ -0,0 +1,159 @@ +use std::ffi::{CStr, CString, c_char}; + +use sqlx::sqlite::{LockedSqliteHandle, SqliteConnection as SqlxConnection}; +use tablepro_core::column::ColumnOrigin; +use tablepro_core::meta::TableRef; +use tablepro_core::sql_syntax::{SqlGrammar, source_table}; + +use crate::autocommit::is_autocommit; +use crate::catalog::MAIN_SCHEMA; + +const TEMP_SCHEMA: &str = "temp"; + +struct NamedColumn { + database: String, + table: String, + column: String, +} + +// sqlx joins the database and table with an unquoted dot, so table `a.b` in main reads as table `b` in `a`; SQLite's own calls keep them apart. +pub(crate) async fn column_origins(connection: &mut SqlxConnection, sql: &str) -> Option>> { + let statement = source_table(sql, SqlGrammar::Sqlite).ok()?; + let table = CString::new(statement.table.name.as_str()).ok()?; + let schema = statement.table.schema.as_deref().map(CString::new).transpose().ok()?; + let mut handle = connection.lock_handle().await.ok()?; + if !is_autocommit(&mut handle) || !names_a_table(&mut handle, schema.as_deref(), &table) { + return None; + } + let named = named_columns(&mut handle, sql)?; + Some( + named + .into_iter() + .map(|named| named.and_then(|named| origin(named, &statement.table))) + .collect(), + ) +} + +// A view reports its base table's origins, so the statement's own name has to resolve, as SQLite resolves it, to a table. +fn names_a_table(handle: &mut LockedSqliteHandle<'_>, schema: Option<&CStr>, table: &CStr) -> bool { + let raw = handle.as_raw_handle().as_ptr(); + // SAFETY: `raw` comes from the locked handle, so the connection is + // open and nothing runs on it meanwhile. Both names are + // NUL-terminated and outlive the call; every output may be NULL. + // The statement was just compiled, so the schema is loaded. + let code = unsafe { + libsqlite3_sys::sqlite3_table_column_metadata( + raw, + schema.map_or(std::ptr::null(), CStr::as_ptr), + table.as_ptr(), + std::ptr::null(), + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + ) + }; + code == libsqlite3_sys::SQLITE_OK +} + +fn named_columns(handle: &mut LockedSqliteHandle<'_>, sql: &str) -> Option>> { + let raw = handle.as_raw_handle().as_ptr(); + let length = std::ffi::c_int::try_from(sql.len()).ok()?; + let mut statement = std::ptr::null_mut(); + // SAFETY: `raw` is the locked, open connection; `sql` outlives the + // call and `length` is its length, so SQLite reads inside it. + let code = unsafe { + libsqlite3_sys::sqlite3_prepare_v2(raw, sql.as_ptr().cast(), length, &mut statement, std::ptr::null_mut()) + }; + if statement.is_null() { + return None; + } + // SAFETY: `statement` was prepared above and is finalized after the + // last read; the names SQLite returns stay valid until then. + let named = unsafe { + let text = |name: *const c_char| match name.is_null() { + true => None, + false => CStr::from_ptr(name).to_str().ok().map(str::to_owned), + }; + let named: Vec> = (0..libsqlite3_sys::sqlite3_column_count(statement)) + .map(|index| { + Some(NamedColumn { + database: text(libsqlite3_sys::sqlite3_column_database_name(statement, index))?, + table: text(libsqlite3_sys::sqlite3_column_table_name(statement, index))?, + column: text(libsqlite3_sys::sqlite3_column_origin_name(statement, index))?, + }) + }) + .collect(); + libsqlite3_sys::sqlite3_finalize(statement); + named + }; + (code == libsqlite3_sys::SQLITE_OK).then_some(named) +} + +// A temp table lives on this connection only, and `sqlite_` names are SQLite's own tables. +fn origin(named: NamedColumn, statement: &TableRef) -> Option { + let internal = named + .table + .get(..7) + .is_some_and(|prefix| prefix.eq_ignore_ascii_case("sqlite_")); + let written = statement + .schema + .as_ref() + .is_none_or(|schema| schema.eq_ignore_ascii_case(&named.database)); + if named.database == TEMP_SCHEMA || internal || !written || !named.table.eq_ignore_ascii_case(&statement.name) { + return None; + } + let schema = (named.database != MAIN_SCHEMA).then_some(named.database); + Some(ColumnOrigin::new(TableRef::new(schema, named.table), named.column)) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn named(database: &str, table: &str, column: &str) -> NamedColumn { + NamedColumn { + database: database.to_owned(), + table: table.to_owned(), + column: column.to_owned(), + } + } + + #[test] + fn main_carries_no_schema_and_an_attached_database_its_name() { + let unqualified = TableRef::unqualified("t"); + + assert_eq!( + origin(named("main", "t", "id"), &unqualified), + Some(ColumnOrigin::new(TableRef::unqualified("t"), "id")) + ); + assert_eq!( + origin(named("other", "t", "id"), &unqualified), + Some(ColumnOrigin::new(TableRef::new(Some("other".to_owned()), "t"), "id")) + ); + assert_eq!( + origin(named("main", "a.b", "id"), &TableRef::unqualified("a.b")), + Some(ColumnOrigin::new(TableRef::unqualified("a.b"), "id")) + ); + } + + #[test] + fn temp_internal_and_other_tables_have_no_origin() { + let unqualified = TableRef::unqualified("t"); + + assert_eq!(origin(named("temp", "t", "id"), &unqualified), None); + assert_eq!( + origin( + named("main", "sqlite_master", "name"), + &TableRef::unqualified("sqlite_master") + ), + None + ); + assert_eq!(origin(named("main", "u", "id"), &unqualified), None); + assert_eq!( + origin(named("main", "t", "id"), &TableRef::new(Some("other".to_owned()), "t")), + None + ); + } +} diff --git a/core/crates/drivers/sqlite/src/lib.rs b/core/crates/drivers/sqlite/src/lib.rs index 6c25f2501e..f2468cb3a1 100644 --- a/core/crates/drivers/sqlite/src/lib.rs +++ b/core/crates/drivers/sqlite/src/lib.rs @@ -5,6 +5,7 @@ mod attach_guard; mod autocommit; mod capabilities; mod catalog; +mod column_origins; mod connection; mod create_file; mod ddl; diff --git a/core/crates/drivers/sqlite/src/result_stream.rs b/core/crates/drivers/sqlite/src/result_stream.rs index 5dccc89731..b4b7ca0575 100644 --- a/core/crates/drivers/sqlite/src/result_stream.rs +++ b/core/crates/drivers/sqlite/src/result_stream.rs @@ -10,7 +10,7 @@ use std::sync::Arc; use futures::TryStreamExt; use sqlx::sqlite::{SqliteColumn, SqliteConnection as SqlxConnection}; use sqlx::{AssertSqlSafe, Column, Executor, SqlSafeStr, Statement as _, TypeInfo}; -use tablepro_core::column::{ColumnType, ResultColumn}; +use tablepro_core::column::{ColumnOrigin, ColumnType, ResultColumn}; use tablepro_core::sql_syntax::SqlGrammar; use tablepro_core::sql_syntax::script::{ LexicalSettings, OpenConstruct, ScriptDiagnostic, ScriptPlan, ScriptTokenKind, @@ -20,6 +20,7 @@ use tablepro_core::value::Value; use tablepro_core::{DriverError, QueryResult, ResultEvent, ResultSink, RowLimit, SinkFlow, Truncation}; use crate::catalog::{result_column_type, storage_class_type}; +use crate::column_origins::column_origins; use crate::decode::decode_row; use crate::encode::bind_all; use crate::errors::map_error; @@ -77,10 +78,7 @@ pub(crate) async fn count(connection: &mut SqlxConnection, statement: &Statement } } -/// Run SQL the user wrote, one statement after another, until one fails, -/// the sink stops reading or `halt` says a stop came. The interrupt -/// reaches a statement only every thousand instructions, so a short one -/// would otherwise run after the stop. +// `halt` is read before each statement: the interrupt reaches one only every thousand instructions, so a short one would otherwise run after the stop. pub(crate) async fn run_script( connection: &mut SqlxConnection, sql: &str, @@ -106,7 +104,16 @@ pub(crate) async fn run_script( outcome.halted = true; break; } - run_statement(connection, statement, &[], limit, sink, &mut outcome).await?; + let mut columns = prepare(connection, statement).await?; + if !columns.is_empty() + && let Some(origins) = column_origins(connection, statement).await + && origins.len() == columns.len() + { + for (column, origin) in columns.iter_mut().zip(origins) { + column.origin = origin; + } + } + run_statement(connection, statement, &[], columns, limit, sink, &mut outcome).await?; if outcome.consumer_gone { break; } @@ -126,17 +133,18 @@ pub(crate) fn refuse_nul(sql: &str) -> Result<(), DriverError> { } } -/// Run a read the app built, every row of it. pub(crate) async fn run_bound( connection: &mut SqlxConnection, statement: &Statement, sink: &mut dyn ResultSink, ) -> Result { let mut outcome = StreamOutcome::default(); + let columns = prepare(connection, statement.sql()).await?; run_statement( connection, statement.sql(), statement.params(), + columns, None, sink, &mut outcome, @@ -213,11 +221,11 @@ async fn run_statement( connection: &mut SqlxConnection, sql: &str, params: &[BoundParam], + mut columns: Vec, limit: Option, sink: &mut dyn ResultSink, outcome: &mut StreamOutcome, ) -> Result<(), DriverError> { - let mut columns = prepare(connection, sql).await?; let query = bind_all(sqlx::query(AssertSqlSafe(sql.to_owned())), params)?; if columns.is_empty() { let done = query.execute(&mut *connection).await.map_err(map_error)?; @@ -295,11 +303,12 @@ fn gone(flow: SinkFlow) -> bool { flow == SinkFlow::ConsumerGone } -/// A result column whose type may wait for the first row. +// The type may wait for the first row. #[derive(Debug, Clone)] struct PendingColumn { name: String, column_type: Option, + origin: Option, } async fn prepare(connection: &mut SqlxConnection, sql: &str) -> Result, DriverError> { @@ -314,6 +323,7 @@ fn pending(column: &SqliteColumn) -> PendingColumn { PendingColumn { name: column.name().to_owned(), column_type: result_column_type(column.type_info().name()), + origin: None, } } @@ -332,7 +342,7 @@ fn finish(columns: Vec) -> Vec { .into_iter() .map(|column| { let column_type = column.column_type.unwrap_or_else(|| storage_class_type(&Value::Null)); - ResultColumn::new(column.name, column_type) + ResultColumn::new(column.name, column_type).with_origin(column.origin) }) .collect() } diff --git a/core/crates/drivers/sqlite/tests/integration/sessions.rs b/core/crates/drivers/sqlite/tests/integration/sessions.rs index 7c38f68789..391992226b 100644 --- a/core/crates/drivers/sqlite/tests/integration/sessions.rs +++ b/core/crates/drivers/sqlite/tests/integration/sessions.rs @@ -2,14 +2,17 @@ use std::time::Duration; +use tablepro_core::column::ColumnOrigin; +use tablepro_core::meta::TableRef; use tablepro_core::{ - BusyReason, CallOptions, DriverError, EditorSessionOptions, EditorSessionState, ErrorCategory, ReadStatement, - ResultEvent, RowLimit, RunEnd, RunOptions, TransactionState, Truncation, UserSql, Value, WorkspaceSettings, - WriteKind, + BusyReason, CallOptions, DriverError, EditorSession, EditorSessionOptions, EditorSessionState, ErrorCategory, + ReadStatement, ResultEvent, RowLimit, RunEnd, RunOptions, TransactionState, Truncation, UserSql, Value, + WorkspaceSettings, WriteKind, }; use tokio::time::Instant; use tokio_util::sync::CancellationToken; +use crate::sqlite_engine::create_database; use crate::support::{Collected, TestFile, apply, batch, call, connect, query, run, run_with, statement}; const UNBOUNDED: &str = "WITH RECURSIVE r(n) AS (SELECT 1 UNION ALL SELECT n + 1 FROM r) SELECT count(*) FROM r"; @@ -527,3 +530,123 @@ async fn a_statement_holding_a_nul_is_refused_at_once() { session.close().await; connection.close().await.unwrap(); } + +fn traced(schema: Option<&str>, table: &str, column: &str) -> Option { + Some(ColumnOrigin::new( + TableRef::new(schema.map(str::to_owned), table), + column, + )) +} + +async fn origins(session: &dyn EditorSession, sql: &str) -> Vec> { + let (summary, collected) = run(session, sql).await; + assert_eq!(summary.end, RunEnd::Completed, "{sql}"); + collected + .events() + .into_iter() + .find_map(|event| match event { + ResultEvent::ResultSetStarted { columns } => { + Some(columns.iter().map(|column| column.origin.clone()).collect()) + } + _ => None, + }) + .unwrap() +} + +#[tokio::test] +async fn only_a_plain_read_of_a_permanent_table_traces_its_columns() { + let file = TestFile::new().await; + let connection = file.open().await; + let session = connection + .open_editor_session(EditorSessionOptions::default(), CancellationToken::new()) + .await + .unwrap(); + let other = file.directory().join("other.sqlite3"); + create_database(&other).await.unwrap(); + let other = other.display().to_string().replace('\'', "''"); + let setup = format!( + "CREATE TABLE t (id INTEGER PRIMARY KEY, a TEXT, b INTEGER); INSERT INTO t VALUES (1, 'x', 2); \ + CREATE VIEW v AS SELECT * FROM t; CREATE TABLE \"a.b\" (id INTEGER PRIMARY KEY); \ + CREATE TEMP TABLE tt (id INTEGER PRIMARY KEY); \ + CREATE TABLE s (id INTEGER PRIMARY KEY); CREATE TEMP VIEW s AS SELECT * FROM main.s; \ + ATTACH DATABASE '{other}' AS other; CREATE TABLE other.w (id INTEGER PRIMARY KEY, c INTEGER)" + ); + let (made, _) = run(session.as_ref(), &setup).await; + assert_eq!(made.end, RunEnd::Completed); + let table = |column: &str| traced(None, "t", column); + let session = session.as_ref(); + + assert_eq!( + origins(session, "SELECT * FROM t").await, + [table("id"), table("a"), table("b")] + ); + assert_eq!( + origins(session, "SELECT * FROM main.t").await, + [table("id"), table("a"), table("b")] + ); + assert_eq!( + origins(session, "SELECT a AS x, id FROM t").await, + [table("a"), table("id")] + ); + assert_eq!( + origins(session, "SELECT id, b + 1 AS c FROM t").await, + [table("id"), None] + ); + assert_eq!(origins(session, "SELECT * FROM v").await, [None, None, None]); + assert_eq!(origins(session, "SELECT * FROM tt").await, [None]); + assert_eq!( + origins(session, "SELECT * FROM s").await, + [None], + "a temp view over main.s" + ); + assert_eq!( + origins(session, "SELECT * FROM main.s").await, + [traced(None, "s", "id")] + ); + assert_eq!( + origins(session, "SELECT * FROM \"a.b\"").await, + [traced(None, "a.b", "id")] + ); + assert_eq!( + origins(session, "SELECT * FROM other.w").await, + [traced(Some("other"), "w", "id"), traced(Some("other"), "w", "c")] + ); + assert_eq!( + origins(session, "SELECT id, a FROM t UNION ALL SELECT id, c FROM other.w").await, + [None, None] + ); + assert_eq!( + origins(session, "SELECT (SELECT a FROM t) AS s, id FROM t").await, + [None, None] + ); + session.close().await; + connection.close().await.unwrap(); +} + +#[tokio::test] +async fn a_read_inside_a_transaction_traces_nothing() { + let file = TestFile::new().await; + let connection = file.open().await; + apply( + connection.as_ref(), + &["CREATE TABLE t (id INTEGER PRIMARY KEY, a TEXT)"], + ) + .await + .unwrap(); + let session = connection + .open_editor_session(EditorSessionOptions::default(), CancellationToken::new()) + .await + .unwrap(); + + let (begun, _) = run(session.as_ref(), "BEGIN").await; + let inside = origins(session.as_ref(), "SELECT * FROM t").await; + let (committed, _) = run(session.as_ref(), "COMMIT").await; + let after = origins(session.as_ref(), "SELECT * FROM t").await; + + assert_eq!(begun.transaction_state, TransactionState::InTransaction); + assert_eq!(inside, [None, None]); + assert_eq!(committed.transaction_state, TransactionState::Idle); + assert_eq!(after, [traced(None, "t", "id"), traced(None, "t", "a")]); + session.close().await; + connection.close().await.unwrap(); +} diff --git a/core/docs/decisions/0015-editing-a-query-result.md b/core/docs/decisions/0015-editing-a-query-result.md new file mode 100644 index 0000000000..0234f148d8 --- /dev/null +++ b/core/docs/decisions/0015-editing-a-query-result.md @@ -0,0 +1,46 @@ +# 0015: A query result edits when its statement reads one table and its driver traces the columns + +- **Status**: Accepted +- **Date**: 2026-11-05 + +## Context + +A result in a query tab is rows of an arbitrary statement. Editing one writes back to a table, so the app has to know which table and which column each cell came from, and that every row is one row of that table. A save that guesses writes another table's rows, or another row of the right table. + +Measured on PostgreSQL 17.11, MySQL 8.4.13, MariaDB 11.8.9, SQLite 3.46.1, DuckDB 2.0.0, SQL Server 2022, ClickHouse 26.10, Cassandra 5.0.9, ScyllaDB 2026.3 and MongoDB 8.3 on 2026-11-05: + +- No engine's metadata proves a result is one table's rows. PostgreSQL, MySQL, MariaDB and SQLite name both copies of a self-join as the same table; SQLite names a table for UNION arms, scalar subqueries and table-valued functions; MariaDB names a derived table's or CTE's alias; a MySQL temporary table looks like a base table; SQL Server's describe maps a view to its base table. +- What each engine can say about a column: PostgreSQL's RowDescription carries the table OID and attribute number, which tokio-postgres drops on the simple protocol and keeps on a `prepare` of the cursor's FETCH; sqlx gives MySQL's `org_table`/`org_name` and SQLite's `sqlite3_column_*_name` as `Column::origin()`, and every SQLite build we ship has column metadata; SQL Server answers `sys.dm_exec_describe_first_result_set` in browse mode; Cassandra's RESULT metadata names the table but gives an alias or a function's text as the column; DuckDB and ClickHouse say nothing; MongoDB's cursor names its collection after `$group` and `$unionWith` too. + +## Decision + +The statement decides whether a result is one table's rows; the driver proves which table and columns, on the connection that ran it. + +- `sql_syntax::source_table` accepts one `SELECT` over one table, optionally qualified and aliased, with `WHERE`, `ORDER BY` on row expressions, `LIMIT` and locking clauses. It refuses joins, comma lists, subqueries and functions in `FROM`, CTEs, set operations, `DISTINCT`, `GROUP BY`, `HAVING`, and any function or subquery in the select list, since a function's name cannot say whether it aggregates. On ClickHouse it also refuses an alias that names another item's column, since ClickHouse substitutes the alias there. PostgreSQL's bare `current_role`, `current_schema` and their kin are expressions. +- `ResultColumn::origin` is a `ColumnOrigin { table, column }`, set by the driver only for a direct column, renamed or not, of a permanent base table, with the table qualified so any connection to the same database names it. A view, a temporary table, an expression and an untraced column carry none. + - PostgreSQL: after the DECLARE, the statement's table resolves with `to_regclass` on the tab's connection, and counts only when the open cursor holds a lock on it, so it is the relation the statement read; relkind `r` or `p`, persistence permanent or unlogged. A `prepare` of the FETCH gives the OIDs too, but makes tokio-postgres look up every non-built-in column type by name, a round trip each. + - MySQL and MariaDB: `Column::origin()` for a run of one statement, confirmed after it by `SHOW CREATE TABLE` on the same connection, which names a temporary table that shadows the base one. + - SQLite: `Column::origin()`; the `temp` database carries none. + - SQL Server: the describe, for a batch of one statement outside a transaction, with the statement's table resolving to the same object, so a view or a synonym carries none. + - DuckDB, ClickHouse, Cassandra and ScyllaDB: the driver resolves the statement's table on its own connection, refusing a temporary table or a view, and `edit::parsed_origins` maps the columns. Cassandra's RESULT metadata names the table. + - MongoDB and Redis: none. +- `edit::result_source` requires every origin to name one table, the one the statement names. `edit::result_target` maps each result column to the table's catalog column, refuses a table without a primary key and a result that leaves out part of it, locks expressions and a second copy of a column, and refuses a result whose wildcard no longer matches the catalog. +- A driver looks nothing up inside the user's transaction, and a lookup that fails leaves every origin empty and the run unchanged. +- PostgreSQL's editor reads text; `edit::value_for_column` turns a value of a column the driver left as server text into the catalog's type where that text is exact: integers, decimals, booleans, UUIDs, JSON, text, enums and hex bytea. Session settings shape how floats, dates, times, intervals and money read, so a key of those kinds leaves its row read-only, and a result holding such a value is not duplicated. A value any other driver read stays as it was stored, so SQLite's ` 2024-01-01 ` is not the key `2024-01-01`. + +## Rationale + +- The parser is strict where the metadata is wrong, and the metadata is exact where the parser would guess, such as which schema an unqualified name resolved to on that connection. +- The save keeps the table tab's path: `save::plan_save` with its row guards over the catalog's columns, so a query result writes nothing a table tab would not. + +## Consequences + +- An app saves through the connection it saves a table tab with, so it refuses a result read inside an open transaction or in another database than that connection's. +- `SELECT id, upper(name) FROM t` is read-only, though its rows are one table's. +- Cassandra statements the generic SQL parser cannot read, such as `ALLOW FILTERING`, are read-only. + +## Alternatives considered + +- **Metadata alone.** Self-joins, UNION arms in SQLite and aliases in MariaDB would write the wrong rows. +- **The parser alone, resolving names on the save connection.** A tab's search path, database and temporary tables are not that connection's. +- **Rewriting the user's statement to qualify its table.** It changes the statement the user runs and moves its error positions; the driver's own lookup proves the same table without touching it. diff --git a/core/docs/decisions/README.md b/core/docs/decisions/README.md index f2e3564723..45ae1b195a 100644 --- a/core/docs/decisions/README.md +++ b/core/docs/decisions/README.md @@ -19,3 +19,4 @@ The format, the statuses and how a later ADR changes an earlier one follow [the | [0009](0009-in-process-ssh-on-windows.md) | SSH on Windows runs in process on russh | Accepted | russh behind `tablepro-ssh`'s `in-process` feature, on every target; Windows uses it, Linux keeps OpenSSH. Forwards are channels with no address, so MySQL and MongoDB relay them through a private listener that serves only this process. | | [0010](0010-macos-on-the-core.md) | The macOS app moves to the core in slices, starting with the connection library | Accepted | UniFFI Swift bindings in an XCFramework of the static library, without the engines, at the app's deployment target; UI, Keychain and the CloudKit transport stay Swift; the core maps the library's CloudKit records both ways with the existing fields and merges a pull field by field. | | [0011](0011-sql-dump-and-import-in-process.md) | SQL dumps and SQL file imports run in process | Accepted | Export Database writes one `.sql` file from each driver's catalog read in `pg_dump` and `mysqldump` order, and Import SQL File splits a file of any size as the engine's client does and runs it on an editor session; no app calls `pg_dump` or `mysqldump`. | +| [0015](0015-editing-a-query-result.md) | A query result edits when its statement reads one table and its driver traces the columns | Accepted | The parser accepts one row-preserving SELECT over one table; each driver sets `ResultColumn::origin` only for a direct column of a permanent base table, from the wire or a lookup on its own connection; the result maps to the catalog with its full primary key and saves through `plan_save`. | diff --git a/linux/ROADMAP.md b/linux/ROADMAP.md index 966c5a482c..19ffcb7a68 100644 --- a/linux/ROADMAP.md +++ b/linux/ROADMAP.md @@ -48,7 +48,7 @@ The app runs on GNOME 50 ([ADR 0015](docs/decisions/0015-gnome-50-platform-floor | Data grid | A virtualized `GtkColumnView` with keyboard-focusable cells, pages of 100 to 10,000 rows, and a pager with an exact or estimated row count; a table of about 100,000 rows or fewer is counted on its own, a larger one offers Count Rows | | Filters | Per-column operators, a raw WHERE fragment checked as one expression, match any or all; binary columns compare to `0x` hex. Kept per table, and per tab across a restart or a reopen. On Cassandra and ScyllaDB only the operators the server evaluates, matched all, with `ALLOW FILTERING` | | Sorting | A header click sorts tables on the server and query results in the grid, one column at a time. A Cassandra or ScyllaDB table keeps the server's order, and pages by its paging state | -| Cell editing | Editors by column kind: inline text, a multi-line editor, a calendar for dates, a toggle for booleans, JSON checked before saving; values are edited from their exact text, never from what the cell displays. Table tabs only, on tables with a primary key; key, generated and auto-increment columns, and binary, spatial, array and composite values, are read-only | +| Cell editing | Editors by column kind: inline text, a multi-line editor, a calendar for dates, a toggle for booleans, JSON checked before saving; values are edited from their exact text, never from what the cell displays. Tables with a primary key, in a table tab or as a query result that reads one table with its whole key ([ADR 0036](docs/decisions/0036-editing-query-results.md)); key, generated and auto-increment columns, and binary, spatial, array and composite values, are read-only | | Rows | Insert, duplicate and delete; Paste (Ctrl+V) adds tab-separated rows from the clipboard, filling the columns from the focused one or by a header's names, or edits the focused cell with one value, and asks first above 1,000 rows, up to 100,000; Set NULL and Empty, and DEFAULT on a new row; undo and redo; save or discard pending changes, and stop a save. On ClickHouse, Cassandra and ScyllaDB a save is not atomic, and Stop takes effect between statements | | Server-computed values | A date, time or timestamp cell takes the engine's own `NOW()`, `CURRENT_TIMESTAMP`, `CURRENT_DATE` and the like, typed in any letter case or picked from the cell menu, and a saved row's cell takes DEFAULT where the engine has `SET c = DEFAULT` (PostgreSQL, MySQL and MariaDB, SQL Server, DuckDB). Only each engine's own list is offered, which on DuckDB is `now()` alone, since the bundled build has no ICU extension; the save writes the expression into the statement, so the Confirm Writes review and the history show it as written | | Bulk delete | Deleting 10 rows or more asks first, unless Confirm Before Deleting Rows is off in Preferences | diff --git a/linux/changelog.d/4055.added.md b/linux/changelog.d/4055.added.md new file mode 100644 index 0000000000..6ac33bbe8c --- /dev/null +++ b/linux/changelog.d/4055.added.md @@ -0,0 +1 @@ +Rows of a query result that reads one table with its whole primary key can be edited and saved from the SQL editor. diff --git a/linux/crates/app/src/application/accelerators.rs b/linux/crates/app/src/application/accelerators.rs index d0b114f5e4..1b0e0794e4 100644 --- a/linux/crates/app/src/application/accelerators.rs +++ b/linux/crates/app/src/application/accelerators.rs @@ -7,8 +7,9 @@ use super::{Accelerator, AcceleratorScope, ShortcutListing, ShortcutOwner, Short use AcceleratorScope::{Application, Component, Local, Stock}; use ShortcutListing::{Listed, Unlisted}; use ShortcutOwner::{ - AssistantComposer, BrowseTab, CellEditor, ConnectionTree, DiagramCanvas, DiagramTab, FilterStrip, Grid, GridCell, - HistoryRow, InlineSuggestion, PickerPage, PickerRow, SqlEditor, SqlEditorCursors, SqlEditorTab, StructureTab, + AssistantComposer, BrowseTab, CellEditor, ConnectionTree, DiagramCanvas, DiagramTab, EditorResults, FilterStrip, + Grid, GridCell, HistoryRow, InlineSuggestion, PickerPage, PickerRow, SqlEditor, SqlEditorCursors, SqlEditorTab, + StructureTab, }; use ShortcutSection::{BrowseTable, DataGrid, ErDiagram, General, TableStructure, Tabs}; @@ -348,6 +349,15 @@ pub const ACCELERATORS: &[Accelerator] = &[ &["s"], Listed(ShortcutSection::SqlEditor), ), + local(EditorResults, BUBBLE, "results.save", &["s"], Unlisted), + local(EditorResults, BUBBLE, "results.undo", &["z"], Unlisted), + local( + EditorResults, + BUBBLE, + "results.redo", + &["z", "y"], + Unlisted, + ), local( SqlEditorTab, BUBBLE, @@ -551,6 +561,7 @@ pub const fn controller_name(owner: ShortcutOwner) -> &'static str { SqlEditor => "tablepro-sql-editor-shortcuts", SqlEditorCursors => "tablepro-sql-editor-cursor-shortcuts", SqlEditorTab => "tablepro-sql-editor-tab-shortcuts", + EditorResults => "tablepro-editor-results-shortcuts", StructureTab => "tablepro-structure-tab-shortcuts", FilterStrip => "tablepro-filter-strip-shortcuts", ConnectionTree => "tablepro-connection-tree-shortcuts", diff --git a/linux/crates/app/src/application/shortcut_owner.rs b/linux/crates/app/src/application/shortcut_owner.rs index 5dd720720a..4b94b25b06 100644 --- a/linux/crates/app/src/application/shortcut_owner.rs +++ b/linux/crates/app/src/application/shortcut_owner.rs @@ -14,6 +14,8 @@ pub enum ShortcutOwner { SqlEditorCursors, /// The whole SQL editor tab, its find bar included. SqlEditorTab, + // The editor's results, ahead of the tab, so Ctrl+S saves changed rows before the file. + EditorResults, StructureTab, FilterStrip, /// A row of the window's connection tree. diff --git a/linux/crates/app/src/services/grid/cell_editability.rs b/linux/crates/app/src/services/grid/cell_editability.rs index e53f230a8a..e0f4800dea 100644 --- a/linux/crates/app/src/services/grid/cell_editability.rs +++ b/linux/crates/app/src/services/grid/cell_editability.rs @@ -29,7 +29,7 @@ pub fn cell_editability( return Err(CellLock::Unaddressable); } let Some(catalog) = column.catalog.as_ref() else { - return Err(CellLock::ResultRow); + return Err(CellLock::NotTableColumn); }; if catalog.is_generated { return Err(CellLock::Generated); @@ -178,7 +178,7 @@ mod tests { false, false ), - Err(CellLock::ResultRow), + Err(CellLock::NotTableColumn), "a column the catalogue does not know" ); assert!(cell_editability(&dotted, &GridRowId::Draft(0), &identity, false, false).is_err()); diff --git a/linux/crates/app/src/services/grid/cell_lock.rs b/linux/crates/app/src/services/grid/cell_lock.rs index a1b5aa8e26..357fec7bf1 100644 --- a/linux/crates/app/src/services/grid/cell_lock.rs +++ b/linux/crates/app/src/services/grid/cell_lock.rs @@ -1,5 +1,6 @@ use tablepro_core::edit::{EditParseError, ReadOnlyReason}; +use super::ResultRefusal; use crate::i18n::gettext; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -13,6 +14,9 @@ pub enum CellLock { AutoIncrement, UnkeyedRow, ResultRow, + Result(ResultRefusal), + // An expression, or a second copy of a column the result already writes. + NotTableColumn, ReadOnlySession, /// The row is marked for deletion. PendingDelete, @@ -34,6 +38,8 @@ impl CellLock { Self::AutoIncrement => gettext("The database numbers this column's values"), Self::UnkeyedRow => gettext("Rows of a table without a primary key can't be edited"), Self::ResultRow => gettext("Query results can't be edited here"), + Self::Result(refusal) => refusal.explanation(), + Self::NotTableColumn => gettext("This column can't be written back to the table"), Self::ReadOnlySession => gettext("This connection is read-only."), Self::PendingDelete => gettext("Rows marked for deletion can't be edited"), Self::Unaddressable => { @@ -60,6 +66,7 @@ mod tests { CellLock::AutoIncrement, CellLock::UnkeyedRow, CellLock::ResultRow, + CellLock::NotTableColumn, CellLock::ReadOnlySession, CellLock::PendingDelete, CellLock::Unaddressable, diff --git a/linux/crates/app/src/services/grid/edit_policy.rs b/linux/crates/app/src/services/grid/edit_policy.rs index b148b55073..886c8cd7c5 100644 --- a/linux/crates/app/src/services/grid/edit_policy.rs +++ b/linux/crates/app/src/services/grid/edit_policy.rs @@ -9,6 +9,10 @@ pub enum EditRefusal { NoPrimaryKey, /// The table's columns have not been read yet. NoColumns, + // A query result's rows wait for its save, and are read again after it. + Saving, + // A query result holds a value read as the server's text, which a copied row would write as text. + NotCopyable, } impl EditRefusal { @@ -18,6 +22,10 @@ impl EditRefusal { Self::ReadOnly => Some(gettext("This connection is read-only.")), Self::NoPrimaryKey => Some(gettext("This table has no primary key, so editing is turned off.")), Self::NoColumns => None, + Self::Saving => Some(gettext("Wait for the changes to finish saving")), + Self::NotCopyable => Some(gettext( + "Rows of this result can't be duplicated, since some values were read as text", + )), } } } @@ -33,6 +41,17 @@ pub struct EditPolicy { pub paste: Result<(), EditRefusal>, } +pub fn saving_policy() -> EditPolicy { + let verdict = Err(EditRefusal::Saving); + EditPolicy { + cells: verdict, + insert: verdict, + duplicate: verdict, + delete: verdict, + paste: verdict, + } +} + pub fn edit_policy(read_only: bool, has_primary_key: bool, has_columns: bool) -> EditPolicy { let verdict = match (read_only, has_columns, has_primary_key) { (true, _, _) => Err(EditRefusal::ReadOnly), diff --git a/linux/crates/app/src/services/grid/mod.rs b/linux/crates/app/src/services/grid/mod.rs index 25bdda424b..ba31b78469 100644 --- a/linux/crates/app/src/services/grid/mod.rs +++ b/linux/crates/app/src/services/grid/mod.rs @@ -18,6 +18,7 @@ mod find_matches; mod grid_column; mod grid_row_id; mod hex_dump; +mod result_refusal; mod row_key_builder; mod row_snapshot; mod selection_summary; @@ -40,11 +41,12 @@ pub use self::edit_navigation::{EditDirection, next_edit_target}; pub use self::edit_parse::{ CommitResult, EditStart, commit_edit, commit_input, edit_context, empty_placeholder, opening_text, parse_cell_text, }; -pub use self::edit_policy::{EditPolicy, EditRefusal, edit_policy, selection_hint}; +pub use self::edit_policy::{EditPolicy, EditRefusal, edit_policy, saving_policy, selection_hint}; pub use self::find_matches::{FindMatch, find_matches}; pub use self::grid_column::{CellDisplay, GridColumn, grid_columns}; pub use self::grid_row_id::GridRowId; pub use self::hex_dump::hex_dump; +pub use self::result_refusal::ResultRefusal; pub use self::row_key_builder::RowKeyBuilder; pub use self::row_snapshot::RowSnapshot; pub use self::selection_summary::{SelectionSummary, SummaryRule, summarize}; diff --git a/linux/crates/app/src/services/grid/result_refusal.rs b/linux/crates/app/src/services/grid/result_refusal.rs new file mode 100644 index 0000000000..4c2efd727e --- /dev/null +++ b/linux/crates/app/src/services/grid/result_refusal.rs @@ -0,0 +1,91 @@ +use tablepro_core::edit::ResultLock; + +use crate::i18n::gettext; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ResultRefusal { + NotOneSelect, + MoreThanOneTable, + Collapsed, + Untraced, + NoPrimaryKey, + KeyNotShown, + ColumnsChanged, + ReadOnlySession, + OpenTransaction, + TransactionUnknown, + OtherDatabase, + SeveralResults, + Checking, + ColumnsUnread, +} + +impl From for ResultRefusal { + fn from(lock: ResultLock) -> Self { + match lock { + ResultLock::NotOneSelect => Self::NotOneSelect, + ResultLock::MoreThanOneTable => Self::MoreThanOneTable, + ResultLock::Collapsed => Self::Collapsed, + ResultLock::Untraced => Self::Untraced, + ResultLock::NoPrimaryKey => Self::NoPrimaryKey, + ResultLock::KeyNotShown => Self::KeyNotShown, + ResultLock::ColumnsChanged => Self::ColumnsChanged, + } + } +} + +impl ResultRefusal { + pub fn explanation(self) -> String { + match self { + Self::NotOneSelect => gettext("Only the rows of one SELECT can be edited"), + Self::MoreThanOneTable => gettext("Rows from more than one table, a subquery or a CTE can't be edited"), + Self::Collapsed => gettext("Grouped, distinct or computed rows can't be edited"), + Self::Untraced => gettext("Only rows read straight from a table can be edited"), + Self::NoPrimaryKey => gettext("Rows of a table without a primary key can't be edited"), + Self::KeyNotShown => { + gettext("The result leaves out part of the table's primary key, so its rows can't be edited") + } + Self::ColumnsChanged => gettext("The table changed since the query ran. Run it again to edit its rows."), + Self::ReadOnlySession => gettext("This connection is read-only."), + Self::OpenTransaction => gettext("Rows read inside an open transaction can't be edited here"), + Self::TransactionUnknown => { + gettext("The server doesn't say whether a transaction is open, so these rows can't be edited here") + } + Self::OtherDatabase => gettext("Rows from another database than the connection's can't be edited here"), + Self::SeveralResults => gettext("Rows can be edited when the run returns one result"), + Self::Checking => gettext("Checking whether these rows can be edited…"), + Self::ColumnsUnread => gettext("The table's columns couldn't be read, so its rows can't be edited"), + } + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashSet; + + use super::*; + + #[test] + fn every_refusal_explains_itself_once() { + let all = [ + ResultRefusal::NotOneSelect, + ResultRefusal::MoreThanOneTable, + ResultRefusal::Collapsed, + ResultRefusal::Untraced, + ResultRefusal::NoPrimaryKey, + ResultRefusal::KeyNotShown, + ResultRefusal::ColumnsChanged, + ResultRefusal::ReadOnlySession, + ResultRefusal::OpenTransaction, + ResultRefusal::TransactionUnknown, + ResultRefusal::OtherDatabase, + ResultRefusal::SeveralResults, + ResultRefusal::Checking, + ResultRefusal::ColumnsUnread, + ]; + + let texts: HashSet = all.iter().map(|refusal| refusal.explanation()).collect(); + + assert_eq!(texts.len(), all.len(), "two refusals share a sentence"); + } +} diff --git a/linux/crates/app/src/ui/browse_tab/layout.rs b/linux/crates/app/src/ui/browse_tab/layout.rs index 6ce7d49a1b..50076171f7 100644 --- a/linux/crates/app/src/ui/browse_tab/layout.rs +++ b/linux/crates/app/src/ui/browse_tab/layout.rs @@ -137,6 +137,7 @@ pub(super) fn grid(tab: &BrowseTab, init: GridInit) -> DataGrid { sorting: init.sorting, rules: None, tracker: Some(init.tracker), + result_lock: None, settings: init.settings, layout, references: Some(init.keys), diff --git a/linux/crates/app/src/ui/browse_tab/mod.rs b/linux/crates/app/src/ui/browse_tab/mod.rs index 2dbeb6e28f..0904faf590 100644 --- a/linux/crates/app/src/ui/browse_tab/mod.rs +++ b/linux/crates/app/src/ui/browse_tab/mod.rs @@ -68,6 +68,7 @@ pub(crate) use self::browse_request_kind::BrowseRequestKind; pub(crate) use self::browse_tab_input::BrowseTabInput; use self::browse_tab_widgets::BrowseTabWidgets; use self::column_recheck::ColumnRecheck; +pub(super) use self::columns_fetch::{ColumnFreshness, load_columns}; pub(crate) use self::data_input::DataInput; use self::grid_input::GridInput; pub(crate) use self::page_reply::PageReply; @@ -75,8 +76,7 @@ use self::page_state::PageState; #[cfg(test)] pub(super) use self::pager_bar::PagerBar; pub(crate) use self::paging_input::PagingInput; -#[cfg(test)] -pub(super) use self::pending_changes_bar::PendingChangesBar; +pub(super) use self::pending_changes_bar::{PendingChangesBar, PendingChangesRequest}; pub(crate) use self::row_total::RowTotal; pub(crate) use self::save_input::SaveInput; pub(crate) use self::table_filter::TableFilter; diff --git a/linux/crates/app/src/ui/browse_tab/pending_changes_bar.rs b/linux/crates/app/src/ui/browse_tab/pending_changes_bar.rs index 64d62899d3..ae47683553 100644 --- a/linux/crates/app/src/ui/browse_tab/pending_changes_bar.rs +++ b/linux/crates/app/src/ui/browse_tab/pending_changes_bar.rs @@ -12,7 +12,7 @@ use crate::ui::a11y::announce_key_shortcuts; /// What a click in the bar asks the tab for, one signal each. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(super) enum PendingChangesRequest { +pub(crate) enum PendingChangesRequest { Save, Discard, StopSaving, @@ -102,7 +102,7 @@ impl Default for PendingChangesBar { } impl PendingChangesBar { - pub(super) fn connect_request( + pub(crate) fn connect_request( &self, request: PendingChangesRequest, f: F, @@ -110,11 +110,11 @@ impl PendingChangesBar { self.connect_closure(request.signal(), false, glib::closure_local!(move |bar: &Self| f(bar))) } - pub(super) fn announce_save_keys(&self, group: &str, save: &gio::SimpleAction) { + pub(crate) fn announce_save_keys(&self, group: &str, save: &gio::SimpleAction) { announce_key_shortcuts(&self.imp().save_button.get(), &format!("{group}.{}", save.name())); } - pub(super) fn show_pending(&self, count: usize) { + pub(crate) fn show_pending(&self, count: usize) { let imp = self.imp(); let visible = count > 0; if visible { @@ -136,7 +136,7 @@ impl PendingChangesBar { /// While a save runs, Save and Discard are off, so a double click /// cannot send two, and Stop Saving takes their place, the focus /// with it. - pub(super) fn show_saving(&self, saving: bool) { + pub(crate) fn show_saving(&self, saving: bool) { let imp = self.imp(); let save = imp.save_button.get(); let stop = imp.stop_button.get(); diff --git a/linux/crates/app/src/ui/data_grid/edit/mod.rs b/linux/crates/app/src/ui/data_grid/edit/mod.rs index a5e4bf8d89..240b88ad48 100644 --- a/linux/crates/app/src/ui/data_grid/edit/mod.rs +++ b/linux/crates/app/src/ui/data_grid/edit/mod.rs @@ -63,13 +63,14 @@ impl EditController { /// A grid edits only with a tracker and the rules of its column set. pub fn set_rules(&self, grid: &DataGrid, rules: Option) { let tracker = grid.config().and_then(|config| config.tracker.clone()); + let result_lock = grid.config().and_then(|config| config.result_lock); let (tracked, policy) = match (tracker, rules) { (Some(tracker), Some(rules)) => { let read_only = rules.policy.cells == Err(EditRefusal::ReadOnly); let tracked = TrackedValues::tracked(tracker, rules.identity).on_read_only_connection(read_only); (tracked, Some(rules.policy)) } - _ => (TrackedValues::untracked(), None), + _ => (TrackedValues::untracked().locked_by(result_lock), None), }; self.tracked.replace(tracked); self.policy.set(policy); diff --git a/linux/crates/app/src/ui/data_grid/grid_config.rs b/linux/crates/app/src/ui/data_grid/grid_config.rs index ba7bd645c9..af32a0200a 100644 --- a/linux/crates/app/src/ui/data_grid/grid_config.rs +++ b/linux/crates/app/src/ui/data_grid/grid_config.rs @@ -7,7 +7,7 @@ use tablepro_linux_platform::AppSettings; use tablepro_session::runtime::Tasks; use super::{KeysHandle, LayoutBinding}; -use crate::services::grid::{EditPolicy, GridColumn}; +use crate::services::grid::{EditPolicy, GridColumn, ResultRefusal}; use crate::services::tab_tracker_handle::TabTrackerHandle; /// How a grid's headers sort its rows. @@ -45,6 +45,8 @@ pub struct GridConfig { pub rules: Option, /// Where edits go. `None` for a grid that only shows rows. pub tracker: Option, + // Why a query result's rows can't be edited, shown where a cell would open. + pub result_lock: Option, /// Read when it is needed, so a change in Preferences reaches open /// grids. pub settings: Rc, diff --git a/linux/crates/app/src/ui/data_grid/test_grid.rs b/linux/crates/app/src/ui/data_grid/test_grid.rs index 9cf0a78518..8393abd3de 100644 --- a/linux/crates/app/src/ui/data_grid/test_grid.rs +++ b/linux/crates/app/src/ui/data_grid/test_grid.rs @@ -186,6 +186,7 @@ impl Grid { policy: spec.policy.unwrap_or_else(|| edit_policy(false, has_key, true)), }), tracker: spec.editable.then(|| tracker.clone()), + result_lock: None, settings: settings.get().clone(), layout: spec.layout, references: spec.references, diff --git a/linux/crates/app/src/ui/data_grid/tracked_values.rs b/linux/crates/app/src/ui/data_grid/tracked_values.rs index 18fb5af153..fbc83996d2 100644 --- a/linux/crates/app/src/ui/data_grid/tracked_values.rs +++ b/linux/crates/app/src/ui/data_grid/tracked_values.rs @@ -6,7 +6,7 @@ use tablepro_core::meta::RowIdentity; use super::GridRow; use crate::services::change_tracker::{CellState, RowKey, RowState}; -use crate::services::grid::{CellLock, GridColumn, GridRowId, RowSnapshot, cell_editability}; +use crate::services::grid::{CellLock, GridColumn, GridRowId, ResultRefusal, RowSnapshot, cell_editability}; use crate::services::tab_tracker_handle::TabTrackerHandle; use crate::ui::icons; @@ -92,6 +92,7 @@ pub struct TrackedValues { tracker: Option, identity: RowIdentity, read_only: bool, + result_lock: Option, } impl Default for TrackedValues { @@ -106,6 +107,7 @@ impl TrackedValues { tracker: None, identity: RowIdentity::Unordered, read_only: true, + result_lock: None, } } @@ -114,9 +116,15 @@ impl TrackedValues { tracker: Some(tracker), identity, read_only: false, + result_lock: None, } } + pub fn locked_by(mut self, result_lock: Option) -> Self { + self.result_lock = result_lock; + self + } + /// On a read-only connection every cell is locked, and says why. pub fn on_read_only_connection(mut self, read_only: bool) -> Self { self.read_only = read_only; @@ -147,6 +155,9 @@ impl TrackedValues { /// `lock` for a cell whose mark the caller already read. pub fn lock_marked(&self, column: &GridColumn, row: &GridRow, row_deleted: bool) -> Option { + if let (Some(refusal), GridRowId::Result) = (self.result_lock, row.id()) { + return Some(CellLock::Result(refusal)); + } if let Err(lock) = cell_editability(column, &row.id(), &self.identity, self.read_only, row_deleted) { return Some(lock); } diff --git a/linux/crates/app/src/ui/editor/imp.rs b/linux/crates/app/src/ui/editor/imp.rs index 7cf2bfe202..1d30be8356 100644 --- a/linux/crates/app/src/ui/editor/imp.rs +++ b/linux/crates/app/src/ui/editor/imp.rs @@ -13,6 +13,7 @@ use super::EditorDeps; use super::editor_session_slot::EditorSessionSlot; use super::editor_view::EditorView; use super::find_bar::FindBar; +use super::result_editing::ResultEditing; use super::results_pane::ResultsPane; use super::run_log::RunLog; use super::transaction_dirty::OpenTransaction; @@ -103,6 +104,7 @@ pub struct SqlEditor { /// completion list closes itself on Escape before the editor's keys /// run, and lets the key go on. pub(super) escape_found_popup: Cell, + pub(super) editing: ResultEditing, } #[glib::object_subclass] @@ -156,6 +158,9 @@ impl ObjectImpl for SqlEditor { Signal::builder("assistant-requested") .param_types([glib::BoxedAnyObject::static_type()]) .build(), + Signal::builder("save-rows").build(), + Signal::builder("stop-saving-rows").build(), + Signal::builder("rows-dirty-changed").build(), ] }) } diff --git a/linux/crates/app/src/ui/editor/mod.rs b/linux/crates/app/src/ui/editor/mod.rs index 15f28fb72d..abae55eac3 100644 --- a/linux/crates/app/src/ui/editor/mod.rs +++ b/linux/crates/app/src/ui/editor/mod.rs @@ -14,6 +14,7 @@ mod find_state; mod format_plan; mod imp; mod inline_suggestion; +mod result_editing; mod result_item; mod result_widgets; mod results_pane; @@ -422,6 +423,10 @@ impl SqlEditor { if self.is_running() || imp.shut_down.get() || imp.confirming.get() { return; } + let retry = scoped.clone(); + if !self.results_may_change(move |editor| editor.execute(retry.clone())) { + return; + } let Some(deps) = self.deps() else { return; }; @@ -491,6 +496,11 @@ impl SqlEditor { .iter() .any(|batch| tablepro_core::sql_syntax::changes_catalog(&batch.text, grammar)), ); + self.note_run_moves_database( + batches + .iter() + .any(|batch| tablepro_core::sql_syntax::may_change_database(&batch.text, grammar)), + ); let statements = batches.iter().map(|batch| UserSql::new(batch.text.as_str())).collect(); let run = ScriptRun { batches, @@ -520,6 +530,8 @@ impl SqlEditor { events, ))); + self.forget_result_edits(); + self.starts_quiet_run(); imp.results.clear(); imp.results.set_running(true); imp.results.set_summary(&crate::i18n::gettext("Running…")); @@ -606,7 +618,11 @@ impl SqlEditor { available, }), }; - let page = result_widgets::entry_widget(usize::try_from(index).ok()?, entry, &grid_deps, actions); + let shown = match &entry.kind { + EntryKind::ResultSet(result) => self.result_grid(usize::try_from(index).ok()?, result), + _ => None, + }; + let page = result_widgets::entry_widget(usize::try_from(index).ok()?, entry, &grid_deps, shown, actions); if let Some(grid) = &page.grid { let editor = self.downgrade(); grid.connect_request(move |_, request| { @@ -614,6 +630,12 @@ impl SqlEditor { editor.grid_request(index, request); } }); + let editor = self.downgrade(); + grid.connect_editing_changed(move |_| { + if let Some(editor) = editor.upgrade() { + editor.refresh_result_actions(); + } + }); } Some(page) } @@ -642,10 +664,8 @@ impl SqlEditor { match request { GridRequest::Export(shown) => self.offer_export(index, shown), GridRequest::Toast(text) => self.emit_by_name::<()>("toast", &[&text]), - GridRequest::Sort(_) - | GridRequest::FollowReference(_) - | GridRequest::FilterByValue(_) - | GridRequest::PendingChanged => {} + GridRequest::PendingChanged => self.on_rows_pending_changed(), + GridRequest::Sort(_) | GridRequest::FollowReference(_) | GridRequest::FilterByValue(_) => {} } } @@ -694,7 +714,10 @@ impl SqlEditor { if dirty_changed || open_changed { self.emit_by_name::<()>("transaction-changed", &[&dirty]); } - self.record_history(entry, dialect); + if !self.ends_quiet_run() { + self.record_history(entry, dialect); + } + self.decide_result_edits(); self.show_scope(); if imp.changes_catalog.get() { self.list_databases(); @@ -852,6 +875,26 @@ impl SqlEditor { .set_page_builder(move |index| weak.upgrade().and_then(|editor| editor.result_page(index))); } + pub(crate) fn connect_save_rows(&self, f: F) -> glib::SignalHandlerId { + self.connect_closure("save-rows", false, glib::closure_local!(move |editor: &Self| f(editor))) + } + + pub(crate) fn connect_rows_dirty_changed(&self, f: F) -> glib::SignalHandlerId { + self.connect_closure( + "rows-dirty-changed", + false, + glib::closure_local!(move |editor: &Self| f(editor)), + ) + } + + pub(crate) fn connect_stop_saving_rows(&self, f: F) -> glib::SignalHandlerId { + self.connect_closure( + "stop-saving-rows", + false, + glib::closure_local!(move |editor: &Self| f(editor)), + ) + } + /// Ask the window to export the result on line `index`: the rows /// its grid shows, or every row its statement returns when run /// again. diff --git a/linux/crates/app/src/ui/editor/result_editing.rs b/linux/crates/app/src/ui/editor/result_editing.rs new file mode 100644 index 0000000000..505691f281 --- /dev/null +++ b/linux/crates/app/src/ui/editor/result_editing.rs @@ -0,0 +1,702 @@ +use std::cell::{Cell, OnceCell, RefCell}; +use std::collections::HashMap; +use std::rc::Rc; +use std::sync::Arc; + +use gtk4::gio; +use gtk4::prelude::*; +use gtk4::subclass::prelude::*; +use tablepro_core::browse::PageLayout; +use tablepro_core::column::{ColumnDefault, ColumnInfo, ResultColumn}; +use tablepro_core::edit::{ResultSource, result_source, result_target, value_for_column}; +use tablepro_core::meta::{RowIdentity, TableRef}; +use tablepro_core::{TransactionState, Value}; + +use super::SqlEditor; +use super::run_log::EntryKind; +use super::run_scope::ScopedText; +use crate::application::{ShortcutOwner, local_shortcut_controller}; +use crate::services::change_tracker::{RowKey, StatementSource, pk_identity}; +use crate::services::grid::{ + EditPolicy, EditRefusal, GridColumn, ResultRefusal, RowKeyBuilder, edit_policy, saving_policy, +}; +use crate::services::save_batch::SaveTarget; +use crate::services::tab_tracker_handle::TabTrackerHandle; +use crate::ui::browse_tab::{ColumnFreshness, PendingChangesRequest, load_columns}; +use crate::ui::data_grid::{DataGrid, EditRules, RowData}; +use crate::ui::discard_alert::confirm_discard; +use crate::ui::result_pane::{ResultGrid, ResultSetView}; +use crate::workspace::PendingState; + +#[derive(Debug, Clone)] +pub(super) enum ResultEdit { + Locked(ResultRefusal), + Checking, + Editable(Rc), +} + +#[derive(Debug)] +pub(super) struct EditTarget { + table: TableRef, + // One per result column, in result order; a column the result can't write back is generated, so no statement names it. + save_columns: Vec, + // Which result columns are the table's own, so only their values are retyped. + traced: Vec, + // A value kept as the server's text would bind as text in a copied row. + copyable: Cell, + grid_columns: Arc<[GridColumn]>, + identity: RowIdentity, +} + +#[derive(Debug)] +struct ResultActions { + save: gio::SimpleAction, + undo: gio::SimpleAction, + redo: gio::SimpleAction, +} + +#[derive(Debug, Default)] +pub struct ResultEditing { + tracker: OnceCell, + actions: OnceCell, + states: RefCell>, + // The one result of the run, once its table's columns proved it editable; the tracker holds its target. + owner: Cell>, + // Bumped by every run, so a catalog read for an older one is dropped. + run: Cell, + dirty: Cell, + // A result read after a USE in the same run came from a database the tab may have left again. + moves_database: Cell, + // The run that reads a result again after its save; history keeps what the user ran, not this. + rereading: Cell, + quiet_run: Cell, + // No row changes while a save runs, so the result read after it loses none. + saving: Cell, +} + +impl SqlEditor { + pub(crate) fn attach_row_tracker(&self, tracker: TabTrackerHandle) { + if self.imp().editing.tracker.set(tracker).is_err() { + return; + } + let results = self.imp().results.get(); + let group = gio::SimpleActionGroup::new(); + let action = |name: &str, act: fn(&SqlEditor)| { + let action = gio::SimpleAction::new(name, None); + action.set_enabled(false); + let editor = self.downgrade(); + action.connect_activate(move |_, _| { + if let Some(editor) = editor.upgrade() { + act(&editor); + } + }); + group.add_action(&action); + action + }; + let actions = ResultActions { + save: action("save", SqlEditor::save_rows), + undo: action("undo", |editor| { + if let Some(grid) = editor.owner_grid() { + grid.undo(); + } + }), + redo: action("redo", |editor| { + if let Some(grid) = editor.owner_grid() { + grid.redo(); + } + }), + }; + results.insert_action_group("results", Some(&group)); + results.add_controller(local_shortcut_controller(ShortcutOwner::EditorResults)); + let _ = self.imp().editing.actions.set(actions); + let bar = results.pending_bar(); + for request in [ + PendingChangesRequest::Save, + PendingChangesRequest::Discard, + PendingChangesRequest::StopSaving, + ] { + let editor = self.downgrade(); + bar.connect_request(request, move |_| { + if let Some(editor) = editor.upgrade() { + editor.on_pending_request(request); + } + }); + } + } + + // A value still in a result cell's editor is an unsaved change once the window asks about closing. + pub(crate) fn commit_result_edit(&self) { + if let Some(grid) = self.owner_grid() { + grid.commit_edit(); + } + } + + fn row_tracker(&self) -> Option { + self.imp().editing.tracker.get().cloned() + } + + pub(super) fn note_run_moves_database(&self, moves: bool) { + self.imp().editing.moves_database.set(moves); + } + + // Called as a run starts: whether it is the read after a save, which history leaves out. + pub(super) fn starts_quiet_run(&self) { + let editing = &self.imp().editing; + editing.quiet_run.set(editing.rereading.get()); + } + + pub(super) fn ends_quiet_run(&self) -> bool { + self.imp().editing.quiet_run.replace(false) + } + + // The rows of the last run are about to be replaced, which the caller made sure loses no change. + pub(super) fn forget_result_edits(&self) { + let editing = &self.imp().editing; + editing.run.set(editing.run.get().wrapping_add(1)); + editing.states.borrow_mut().clear(); + editing.owner.set(None); + if let Some(tracker) = self.row_tracker() { + let _ = tracker.discard(); + } + self.refresh_pending_bar(); + } + + // Before a run replaces the results: false when the run has to wait, after asking where it may. + pub(super) fn results_may_change(&self, then: impl Fn(&Self) + 'static) -> bool { + let Some(tracker) = self.row_tracker() else { + return true; + }; + if let Some(grid) = self.owner_grid() { + grid.commit_edit(); + } + if tracker.pending_state() == PendingState::Saving { + self.emit_by_name::<()>( + "toast", + &[&crate::i18n::gettext("Wait for the changes to finish saving")], + ); + return false; + } + let pending = tracker.pending_count(); + if pending == 0 && tracker.pending_state() == PendingState::Clean { + return true; + } + let editor = self.downgrade(); + confirm_discard(self, pending.max(1), move || { + if let Some(editor) = editor.upgrade() { + // The run about to start replaces these rows, so they are not read again first. + editor.drop_rows(); + then(&editor); + } + }); + false + } + + pub(super) fn decide_result_edits(&self) { + if self.row_tracker().is_none() { + return; + } + let imp = self.imp(); + let Some(deps) = self.deps() else { + return; + }; + let Some(session) = deps.session.get() else { + return; + }; + let grammar = self.grammar(); + // A shell's documents or a key store's replies are not a table's rows; the row details say so already. + if !grammar.is_sql() { + return; + } + let (results, transaction) = { + let log = imp.log.borrow(); + let Some(log) = log.as_ref() else { + return; + }; + let results: Vec = log + .entries() + .iter() + .enumerate() + .filter(|(_, entry)| matches!(entry.kind, EntryKind::ResultSet(_))) + .map(|(index, _)| index) + .collect(); + (results, log.transaction()) + }; + // The save runs on the window's connection, so rows read inside the tab's own transaction would be written outside it. + // These come before any parse: a batch of many SELECTs hands each result the whole batch. + let common = if session.read_only() { + Some(ResultRefusal::ReadOnlySession) + } else if matches!( + transaction, + Some(TransactionState::InTransaction | TransactionState::Failed) + ) || imp.open_transaction.get().is_some() + { + Some(ResultRefusal::OpenTransaction) + } else if transaction != Some(TransactionState::Idle) { + Some(ResultRefusal::TransactionUnknown) + } else if self.tab_database().is_some() || imp.editing.moves_database.get() { + Some(ResultRefusal::OtherDatabase) + } else if results.len() > 1 { + Some(ResultRefusal::SeveralResults) + } else { + None + }; + let run = imp.editing.run.get(); + for index in results { + let state = match common { + Some(refusal) => ResultEdit::Locked(refusal), + None => { + let source = imp + .log + .borrow() + .as_ref() + .and_then(|log| log.result(index)) + .map(|result| result_source(result.statement.as_str(), grammar, &result.columns)); + match source { + Some(Ok(source)) => { + self.read_target_columns(run, index, source); + ResultEdit::Checking + } + Some(Err(lock)) => ResultEdit::Locked(lock.into()), + None => ResultEdit::Locked(ResultRefusal::Untraced), + } + } + }; + imp.editing.states.borrow_mut().insert(index, state); + self.refresh_result_page(index); + } + } + + fn read_target_columns(&self, run: u64, index: usize, source: ResultSource) { + let Some(deps) = self.deps() else { + return; + }; + let Some(session) = deps.session.get() else { + return; + }; + let editor = self.downgrade(); + let table = source.table.clone(); + let _read = crate::runtime::spawn_and_deliver( + &deps.tasks, + load_columns(session, table, ColumnFreshness::Refetch), + move |reply| { + let Some(editor) = editor.upgrade() else { + return; + }; + let catalog = reply.ok().and_then(Result::ok); + editor.on_target_columns(run, index, &source, catalog.as_deref()); + }, + ); + } + + // The run's one result takes the tracker the moment it proves editable; nothing else can hold it before the next run. + fn on_target_columns(&self, run: u64, index: usize, source: &ResultSource, catalog: Option<&[ColumnInfo]>) { + let imp = self.imp(); + if imp.editing.run.get() != run { + return; + } + let state = { + let mut log = imp.log.borrow_mut(); + let Some(result) = log.as_mut().and_then(|log| log.result_mut(index)) else { + return; + }; + match catalog { + None => ResultEdit::Locked(ResultRefusal::ColumnsUnread), + Some(catalog) => match result_target(source, &result.columns, catalog) { + Err(lock) => ResultEdit::Locked(lock.into()), + Ok(target) => { + let target = edit_target(&target.table, &result.columns, &target.positions, catalog); + result.rows = typed_rows(&target, &result.columns, std::mem::take(&mut result.rows)); + ResultEdit::Editable(Rc::new(target)) + } + }, + } + }; + if let (ResultEdit::Editable(target), Some(tracker)) = (&state, self.row_tracker()) { + let _ = tracker.discard(); + let _ = tracker.set_target(SaveTarget { + table: target.table.clone(), + columns: target.save_columns.clone(), + }); + imp.editing.owner.set(Some(index)); + } + imp.editing.states.borrow_mut().insert(index, state); + self.refresh_result_page(index); + } + + pub(super) fn result_grid(&self, index: usize, view: &ResultSetView) -> Option { + let editing = &self.imp().editing; + let tracker = self.row_tracker()?; + let state = editing.states.borrow().get(&index).cloned(); + let target = match state? { + ResultEdit::Locked(refusal) => return Some(ResultGrid::read_only(view, Some(refusal))), + ResultEdit::Checking => return Some(ResultGrid::read_only(view, Some(ResultRefusal::Checking))), + ResultEdit::Editable(target) => target, + }; + Some(ResultGrid { + columns: target.grid_columns.clone(), + rows: editable_rows(&target, view), + rules: Some(EditRules { + identity: target.identity.clone(), + policy: self.result_policy(&target), + }), + tracker: Some(tracker), + table: Some(target.table.clone()), + result_lock: None, + }) + } + + fn result_policy(&self, target: &EditTarget) -> EditPolicy { + match self.imp().editing.saving.get() { + true => saving_policy(), + false => EditPolicy { + duplicate: match target.copyable.get() { + true => Ok(()), + false => Err(EditRefusal::NotCopyable), + }, + ..edit_policy(false, true, true) + }, + } + } + + fn follow_saving(&self, saving: bool) { + let editing = &self.imp().editing; + editing.saving.set(saving); + let target = match editing + .owner + .get() + .and_then(|owner| editing.states.borrow().get(&owner).cloned()) + { + Some(ResultEdit::Editable(target)) => target, + _ => return, + }; + if let Some(grid) = self.owner_grid() { + grid.edit().set_rules( + &grid, + Some(EditRules { + identity: target.identity.clone(), + policy: self.result_policy(&target), + }), + ); + grid.refresh_actions(); + grid.redraw(); + } + } + + fn refresh_result_page(&self, index: usize) { + let Ok(page) = u32::try_from(index) else { + return; + }; + let results = &self.imp().results; + if results.selected() == Some(page) { + results.rebuild_page(page); + } else { + results.forget_pages(|kept| kept == page); + } + } + + pub(super) fn on_rows_pending_changed(&self) { + self.refresh_pending_bar(); + } + + pub(super) fn refresh_result_actions(&self) { + let Some(actions) = self.imp().editing.actions.get() else { + return; + }; + let tracker = self.row_tracker(); + let grid = self.owner_grid(); + // Held while the run's result edits, whether or not it has changes, so Ctrl+S in the results never saves the file. + actions.save.set_enabled(self.imp().editing.owner.get().is_some()); + let idle = grid.is_some() + && tracker + .as_ref() + .is_some_and(|tracker| tracker.pending_state() != PendingState::Saving); + actions + .undo + .set_enabled(idle && tracker.as_ref().is_some_and(TabTrackerHandle::can_undo)); + actions + .redo + .set_enabled(idle && tracker.as_ref().is_some_and(TabTrackerHandle::can_redo)); + } + + fn refresh_pending_bar(&self) { + self.refresh_result_actions(); + let bar = self.imp().results.pending_bar(); + let count = self.row_tracker().map_or(0, |tracker| tracker.pending_count()); + if self.imp().editing.dirty.replace(count > 0) != (count > 0) { + self.emit_by_name::<()>("rows-dirty-changed", &[]); + } + if count == 0 + && let Some(grid) = self.owner_grid() + { + crate::ui::a11y::pass_focus(&bar, &grid.column_view()); + } + bar.show_pending(count); + } + + fn on_pending_request(&self, request: PendingChangesRequest) { + match request { + PendingChangesRequest::Save => self.save_rows(), + PendingChangesRequest::Discard => self.confirm_rows_discard(), + PendingChangesRequest::StopSaving => self.emit_by_name::<()>("stop-saving-rows", &[]), + } + } + + pub(super) fn save_rows(&self) { + if let Some(grid) = self.owner_grid() { + grid.commit_edit(); + } + let unsaved = self + .row_tracker() + .is_some_and(|tracker| tracker.pending_state() == PendingState::Unsaved && tracker.pending_count() > 0); + if unsaved { + self.emit_by_name::<()>("save-rows", &[]); + } + } + + fn confirm_rows_discard(&self) { + let Some(tracker) = self.row_tracker() else { + return; + }; + if let Some(grid) = self.owner_grid() { + grid.commit_edit(); + } + let pending = tracker.pending_count(); + if pending == 0 { + return; + } + let editor = self.downgrade(); + confirm_discard(self, pending, move || { + if let Some(editor) = editor.upgrade() { + editor.discard_rows(); + } + }); + } + + // The owner's grid, built or not; the tracker's changes are drawn by no other. + fn owner_grid(&self) -> Option { + let owner = u32::try_from(self.imp().editing.owner.get()?).ok()?; + self.imp().results.kept_grid(owner) + } + + // After a save whose outcome is unknown the rows on screen may not be what the table holds, so they are read again. + fn discard_rows(&self) { + let unknown = self + .row_tracker() + .is_some_and(|tracker| matches!(tracker.pending_state(), PendingState::OutcomeUnknown(_))); + self.drop_rows(); + if unknown { + self.reread_result(); + } + } + + fn drop_rows(&self) { + match self.owner_grid() { + Some(grid) => { + grid.discard(); + grid.resync(); + } + None => { + if let Some(tracker) = self.row_tracker() { + let _ = tracker.discard(); + } + } + } + self.on_rows_pending_changed(); + } + + pub(crate) fn rows_save_started(&self) { + self.imp().results.pending_bar().show_saving(true); + self.follow_saving(true); + self.refresh_result_actions(); + } + + fn rows_save_ended(&self) { + self.imp().results.pending_bar().show_saving(false); + self.follow_saving(false); + self.refresh_pending_bar(); + } + + // The tracker let go of what the save wrote, and nothing changed while it ran, so the statement behind the result + // runs again for what the server stored. + pub(crate) fn rows_saved(&self) { + self.rows_save_ended(); + self.reread_result(); + } + + pub(crate) fn rows_save_failed(&self, failing: Option) { + self.rows_save_ended(); + let grid = self.owner_grid(); + if let (Some(source), Some(tracker)) = (failing, self.row_tracker()) { + let key = match source { + StatementSource::Insert { draft_id } => RowKey::Draft(draft_id), + StatementSource::Update { row_key } | StatementSource::Delete { row_key } => row_key, + }; + let delta = tracker.mark_error_row(key.clone()); + if let Some(grid) = &grid { + grid.apply_delta(&delta); + grid.reveal_row(&key); + } + } + if let Some(grid) = grid { + grid.redraw(); + } + } + + pub(crate) fn rows_save_unknown(&self) { + self.rows_save_ended(); + if let Some(grid) = self.owner_grid() { + grid.redraw(); + } + } + + pub(crate) fn rows_discarded(&self, reload: bool) { + if let Some(grid) = self.owner_grid() { + grid.resync(); + } + self.on_rows_pending_changed(); + if reload { + self.reread_result(); + } + } + + // An ordinary run of the result's statement, with its Stop, its Safe Mode and its result checked again from the start. + fn reread_result(&self) { + let imp = self.imp(); + let Some(owner) = imp.editing.owner.get() else { + return; + }; + let Some(statement) = imp + .log + .borrow() + .as_ref() + .and_then(|log| log.result(owner)) + .map(|result| result.statement.as_str().to_owned()) + else { + return; + }; + imp.editing.rereading.set(true); + self.execute(ScopedText { + text: statement, + base_char: 0, + }); + imp.editing.rereading.set(false); + } +} + +fn edit_target( + table: &TableRef, + result: &[ResultColumn], + positions: &[Option], + catalog: &[ColumnInfo], +) -> EditTarget { + let save_columns: Vec = result + .iter() + .zip(positions) + .enumerate() + .map( + |(index, (column, position))| match position.and_then(|position| catalog.get(position)) { + Some(info) => info.clone(), + None => unwritable(index, column), + }, + ) + .collect(); + // Every column carries its entry, a computed one generated, so pasting and deleting see the whole row. + let grid_columns = result + .iter() + .zip(&save_columns) + .enumerate() + .map(|(index, (column, info))| GridColumn { + index, + name: column.name.clone(), + column_type: column.column_type.clone(), + catalog: Some(info.clone()), + addressable: true, + }) + .collect(); + EditTarget { + table: table.clone(), + identity: pk_identity(&save_columns), + traced: positions.iter().map(Option::is_some).collect(), + copyable: Cell::new(true), + save_columns, + grid_columns, + } +} + +// A NUL can't be in an identifier, so no key lookup by name ever lands on this column. +fn unwritable(index: usize, column: &ResultColumn) -> ColumnInfo { + ColumnInfo { + name: format!("\0{index}"), + column_type: column.column_type.clone(), + nullable: true, + primary_key: false, + is_auto_increment: false, + is_generated: true, + partition_scoped: false, + default: ColumnDefault::None, + comment: None, + } +} + +// PostgreSQL's editor reads text; keys and copied values have to be the columns' own types. +fn typed_rows(target: &EditTarget, read: &[ResultColumn], rows: Vec>) -> Vec> { + let retyped = |value: &Value| matches!(value, Value::Text(_) | Value::Timestamp(_)); + if !rows.iter().flat_map(|row| row.iter()).any(retyped) { + return rows; + } + rows.into_iter() + .map(|row| { + row.iter() + .zip(read.iter().zip(target.save_columns.iter().zip(&target.traced))) + .map(|(value, (read, (column, traced)))| match traced { + false => value.clone(), + true => value_for_column(value, &read.column_type, &column.column_type).unwrap_or_else(|| { + target.copyable.set(false); + value.clone() + }), + }) + .collect() + }) + .collect() +} + +// A key value the column's type cannot take, such as a wall-clock TIMESTAMP, shows as read but names no row. +fn editable_rows(target: &EditTarget, view: &ResultSetView) -> Vec { + let width = target.save_columns.len(); + let layout = PageLayout { + visible: 0..width, + identity: width..width, + }; + let keys = RowKeyBuilder::for_page(&target.identity, &target.save_columns, &layout); + let key_columns: Vec = (0..width) + .filter(|index| target.traced[*index] && target.save_columns[*index].primary_key) + .collect(); + view.rows + .iter() + .map(|cells| { + let bindable = key_columns.iter().all(|index| { + let (Some(value), Some(read)) = (cells.get(*index), view.columns.get(*index)) else { + return false; + }; + value_for_column(value, &read.column_type, &target.save_columns[*index].column_type).is_some() + }); + let key = keys + .as_ref() + .filter(|_| bindable) + .and_then(|keys| keys.key_for(cells, &[])); + RowData::persisted(key, cells.clone()) + }) + .collect() +} + +#[cfg(test)] +impl SqlEditor { + // `Some(None)` once the result edits, `Some(Some(why))` while it does not. + pub(super) fn result_refusal(&self, index: usize) -> Option> { + match self.imp().editing.states.borrow().get(&index)? { + ResultEdit::Locked(refusal) => Some(Some(*refusal)), + ResultEdit::Checking => Some(Some(ResultRefusal::Checking)), + ResultEdit::Editable(_) => Some(None), + } + } +} diff --git a/linux/crates/app/src/ui/editor/result_widgets.rs b/linux/crates/app/src/ui/editor/result_widgets.rs index 8475ebf0c2..1c4b05fdda 100644 --- a/linux/crates/app/src/ui/editor/result_widgets.rs +++ b/linux/crates/app/src/ui/editor/result_widgets.rs @@ -5,7 +5,7 @@ use super::results_pane::ResultPage; use super::run_log::{EntryKind, RunLogEntry}; use crate::ui::export::ExportSource; use crate::ui::icons; -use crate::ui::result_pane::{ResultGridDeps, result_set_page}; +use crate::ui::result_pane::{ResultGrid, ResultGridDeps, result_set_page}; /// What a page's buttons reach in the editor. pub struct PageActions { @@ -33,6 +33,7 @@ pub(crate) fn entry_widget( index: usize, entry: &RunLogEntry, grid_deps: &ResultGridDeps, + shown: Option, actions: PageActions, ) -> ResultPage where @@ -47,7 +48,9 @@ where } = actions; let widget = match &entry.kind { EntryKind::ResultSet(result) => { - let (widget, grid) = result_set_page(result, &result.summary.tab_label(index), grid_deps, export_all); + let shown = shown.unwrap_or_else(|| ResultGrid::read_only(result, None)); + let (widget, grid) = + result_set_page(result, shown, &result.summary.tab_label(index), grid_deps, export_all); return ResultPage { widget, grid: Some(grid), @@ -219,7 +222,7 @@ mod tests { let deps = grid_deps(); - let page = entry_widget(1, &entry, &deps.deps, actions(|_| {})); + let page = entry_widget(1, &entry, &deps.deps, None, actions(|_| {})); let grid = page.grid.expect("the grid"); assert!(grid.is_ancestor(&page.widget)); @@ -241,7 +244,7 @@ mod tests { preview: String::new(), }; - let page = entry_widget(0, &entry, &grid_deps().deps, actions(|_| {})); + let page = entry_widget(0, &entry, &grid_deps().deps, None, actions(|_| {})); assert!(page.grid.is_none()); let page = page.widget.downcast::().expect("a status page"); @@ -268,6 +271,7 @@ mod tests { 0, &entry, &grid_deps().deps, + None, actions(move |position| seen.set(Some(position))), ); let page = page.widget.downcast::().expect("a status page"); @@ -295,6 +299,7 @@ mod tests { 0, &entry, &grid_deps().deps, + None, PageActions { go_to_error: |_| {}, export_all: |_| {}, diff --git a/linux/crates/app/src/ui/editor/results_pane.rs b/linux/crates/app/src/ui/editor/results_pane.rs index 22330c8c6f..80799a0117 100644 --- a/linux/crates/app/src/ui/editor/results_pane.rs +++ b/linux/crates/app/src/ui/editor/results_pane.rs @@ -6,6 +6,7 @@ use libadwaita as adw; use super::result_item::ResultItem; use super::run_log::{EntryKind, RunLogEntry}; use super::sidebar_policy::{self, SidebarState}; +use crate::ui::browse_tab::PendingChangesBar; use crate::ui::data_grid::DataGrid; /// How many results keep their page once shown, so going back to one @@ -66,6 +67,8 @@ mod imp { pub list_view: TemplateChild, #[template_child] pub stack: TemplateChild, + #[template_child] + pub pending_bar: TemplateChild, pub items: std::cell::OnceCell, pub selection: std::cell::OnceCell, pub sidebar: Cell, @@ -81,6 +84,7 @@ mod imp { type ParentType = adw::Bin; fn class_init(klass: &mut Self::Class) { + PendingChangesBar::ensure_type(); klass.bind_template(); } @@ -180,6 +184,42 @@ impl ResultsPane { self.imp().page_builder.replace(Some(Box::new(builder))); } + pub(crate) fn pending_bar(&self) -> PendingChangesBar { + self.imp().pending_bar.get() + } + + pub fn forget_pages(&self, forget: impl Fn(u32) -> bool) { + let imp = self.imp(); + let forgotten: std::collections::VecDeque = { + let mut built = imp.built.borrow_mut(); + let (forgotten, kept) = built.drain(..).partition(|page| forget(page.index)); + *built = kept; + forgotten + }; + for page in forgotten { + if let Some(widget) = imp.stack.child_by_name(&page_name(page.index)) { + imp.stack.remove(&widget); + } + } + } + + pub fn rebuild_page(&self, index: u32) { + self.forget_pages(|page| page == index); + if self.selected() == Some(index) { + self.show_page(index); + } + } + + // The grid of line `index` while its page is kept, shown or not. + pub fn kept_grid(&self, index: u32) -> Option { + self.imp() + .built + .borrow() + .iter() + .find(|page| page.index == index) + .and_then(|page| page.grid.clone()) + } + pub fn set_running(&self, running: bool) { self.imp().spinner.set_visible(running); } @@ -567,6 +607,23 @@ mod tests { ); } + #[gtk4::test] + fn a_page_the_host_changed_is_built_again() { + let pane = pane(); + pane.append(&[command(), command()]); + let first = pane.imp().stack.visible_child().expect("the first page"); + + pane.rebuild_page(0); + + let rebuilt = pane.imp().stack.visible_child().expect("the first page again"); + assert_ne!(rebuilt, first, "the page was not built again"); + assert_eq!(built_pages(&pane), 1); + pane.select(1); + pane.forget_pages(|index| index == 0); + assert_eq!(built_pages(&pane), 1, "a forgotten page stayed"); + assert_eq!(pane.imp().stack.visible_child_name().as_deref(), Some("entry-1")); + } + #[gtk4::test] fn going_back_to_a_recent_result_keeps_its_page() { let pane = pane(); diff --git a/linux/crates/app/src/ui/editor/run_log.rs b/linux/crates/app/src/ui/editor/run_log.rs index d57d4bdf71..08ead884b4 100644 --- a/linux/crates/app/src/ui/editor/run_log.rs +++ b/linux/crates/app/src/ui/editor/run_log.rs @@ -383,6 +383,27 @@ impl RunLog { &self.entries } + pub fn result(&self, index: usize) -> Option<&ResultSetView> { + match &self.entries.get(index)?.kind { + EntryKind::ResultSet(result) => Some(result), + _ => None, + } + } + + pub fn result_mut(&mut self, index: usize) -> Option<&mut ResultSetView> { + match &mut self.entries.get_mut(index)?.kind { + EntryKind::ResultSet(result) => Some(result), + _ => None, + } + } + + pub fn first_result(self) -> Option { + self.entries.into_iter().find_map(|entry| match entry.kind { + EntryKind::ResultSet(result) => Some(result), + _ => None, + }) + } + pub fn transaction(&self) -> Option { self.transaction } diff --git a/linux/crates/app/src/ui/editor/tests.rs b/linux/crates/app/src/ui/editor/tests.rs index 199fa66742..431c2a6458 100644 --- a/linux/crates/app/src/ui/editor/tests.rs +++ b/linux/crates/app/src/ui/editor/tests.rs @@ -3045,3 +3045,230 @@ fn completion_reads_only_the_session_of_the_tabs_database() { pick_database(&harness, "shop"); assert_eq!(followed_catalog(&harness), None, "the last database's catalog stayed"); } + +mod result_edits { + use tablepro_core::column::{ColumnDefault, ColumnInfo, ColumnOrigin}; + use tablepro_core::edit::CellInput; + use tablepro_core::meta::TableRef; + + use super::*; + use crate::services::change_tracker::RowKey; + use crate::services::change_tracker_registry::ChangeTrackerRegistry; + use crate::services::grid::ResultRefusal; + use crate::services::tab_tracker_handle::TabTrackerHandle; + use crate::services::tracker_edit::TrackerEdit; + + fn table() -> TableRef { + TableRef::new(Some("public".to_owned()), "t") + } + + fn traced(name: &str, column: &str) -> ResultColumn { + ResultColumn::new(name, ColumnType::untyped_text("text")).with_origin(Some(ColumnOrigin::new(table(), column))) + } + + fn catalog_column(name: &str, primary_key: bool) -> ColumnInfo { + ColumnInfo { + name: name.to_owned(), + column_type: ColumnType::untyped_text("text"), + nullable: !primary_key, + primary_key, + is_auto_increment: false, + is_generated: false, + partition_scoped: false, + default: ColumnDefault::None, + comment: None, + } + } + + fn rows_of(columns: Vec, transaction: TransactionState) -> Replies { + let width = columns.len(); + let rows = (1..=2) + .map(|id| { + (0..width) + .map(|column| match column { + 0 => Value::Int(id), + _ => Value::Text(format!("v{id}")), + }) + .collect() + }) + .collect(); + let mut run = FakeRun::completed(vec![ + ResultEvent::ResultSetStarted { + columns: columns.into(), + }, + ResultEvent::Rows(rows), + ResultEvent::ResultSetFinished { + retained: 2, + total_rows: Some(2), + truncation: Truncation::Complete, + }, + ]); + run.transaction_state = transaction; + let mut runs = Replies::default(); + runs.always(run); + runs + } + + fn editing(runs: Replies, text: &str) -> (Harness, TabTrackerHandle) { + crate::register_test_resources(); + let runtime = crate::test_support::test_runtime(); + let fake = Arc::new(FakeEditorSession::new(runs)); + let mut script = ConnectionScript::default(); + script.fetch_columns.always(FakeReply::ok(vec![ + catalog_column("id", true), + catalog_column("a", false), + ])); + let active = ActiveSession::new(); + active.replace(session_on(fake.clone(), runtime.tasks(), script)); + let settings = MemorySettings::new(); + let editor = SqlEditor::new(SqlEditorInit { + initial_query: Some(text.to_owned()), + settings: settings.get().clone(), + history: None, + tasks: runtime.tasks(), + session: active.clone(), + hold: Rc::new(tablepro_session::runtime::CountingHold::new()), + close_timeout: Duration::from_secs(5), + snippets: crate::test_support::builtin_snippets_only(), + }); + let tracker = ChangeTrackerRegistry::default().register(uuid::Uuid::new_v4()); + editor.attach_row_tracker(tracker.clone()); + let harness = Harness { + editor, + fake, + history: None, + active, + settings, + runtime, + }; + (harness, tracker) + } + + fn settled(harness: &Harness) -> Option { + let mut state = None; + wait_until(WAIT, || { + state = harness.editor.result_refusal(0); + state.is_some_and(|refusal| refusal != Some(ResultRefusal::Checking)) + }) + .expect("the result to settle"); + state.flatten() + } + + fn change_a(tracker: &TabTrackerHandle) { + let _ = tracker.apply(TrackerEdit::Cell { + row_key: RowKey::from_pk_values(&[Value::Int(1)]).expect("a key"), + col: 1, + original: Value::Text("v1".to_owned()), + value: CellInput::Value(Value::Text("changed".to_owned())), + }); + } + + #[gtk4::test] + fn a_result_read_from_one_table_with_its_key_edits_and_saves_through_the_window() { + let (harness, tracker) = editing( + rows_of(vec![traced("id", "id"), traced("x", "a")], TransactionState::Idle), + "SELECT id, a AS x FROM t", + ); + let _window = shown(&harness); + run_and_wait(&harness); + + assert_eq!(settled(&harness), None); + let grid = shown_grid(&harness); + assert!(grid.config().is_some_and(|config| config.tracker.is_some())); + change_a(&tracker); + let saves = SignalLog::connect(&harness.editor, "save-rows", |_| ()); + harness.editor.save_rows(); + + assert_eq!(saves.count(), 1, "Save did not reach the window"); + let (target, _) = tracker.preview_save().expect("a plan").expect("changes"); + assert_eq!(target.table, table()); + let names: Vec<&str> = target.columns.iter().map(|column| column.name.as_str()).collect(); + assert_eq!(names, ["id", "a"], "the renamed column writes its own column"); + } + + #[gtk4::test] + fn a_join_stays_read_only_and_says_why() { + let (harness, _) = editing( + rows_of(vec![traced("id", "id"), traced("a", "a")], TransactionState::Idle), + "SELECT t.id, u.a FROM t JOIN u ON u.id = t.id", + ); + let _window = shown(&harness); + run_and_wait(&harness); + + assert_eq!(settled(&harness), Some(ResultRefusal::MoreThanOneTable)); + let grid = shown_grid(&harness); + assert_eq!( + grid.config().and_then(|config| config.result_lock), + Some(ResultRefusal::MoreThanOneTable) + ); + } + + #[gtk4::test] + fn a_result_without_its_whole_key_is_read_only() { + let (harness, _) = editing( + rows_of(vec![traced("a", "a")], TransactionState::Idle), + "SELECT a FROM t", + ); + run_and_wait(&harness); + + assert_eq!(settled(&harness), Some(ResultRefusal::KeyNotShown)); + } + + #[gtk4::test] + fn rows_read_inside_an_open_transaction_are_read_only() { + let (harness, _) = editing( + rows_of( + vec![traced("id", "id"), traced("a", "a")], + TransactionState::InTransaction, + ), + "SELECT id, a FROM t", + ); + run_and_wait(&harness); + + assert_eq!(settled(&harness), Some(ResultRefusal::OpenTransaction)); + } + + #[gtk4::test] + fn columns_no_driver_traced_are_read_only() { + let (harness, _) = editing( + rows_of( + vec![ + ResultColumn::new("id", ColumnType::untyped_text("text")), + ResultColumn::new("a", ColumnType::untyped_text("text")), + ], + TransactionState::Idle, + ), + "SELECT id, a FROM t", + ); + run_and_wait(&harness); + + assert_eq!(settled(&harness), Some(ResultRefusal::Untraced)); + } + + #[gtk4::test] + fn a_run_asks_before_it_drops_changed_rows() { + adw::init().expect("libadwaita initialises under the test backend"); + let (harness, tracker) = editing( + rows_of(vec![traced("id", "id"), traced("a", "a")], TransactionState::Idle), + "SELECT id, a FROM t", + ); + let window = shown(&harness); + run_and_wait(&harness); + assert_eq!(settled(&harness), None); + change_a(&tracker); + let before = sent(&harness.fake).len(); + + harness.editor.run(); + + assert!(!harness.editor.is_running(), "the run dropped a pending change"); + let mut alert = None; + wait_until(WAIT, || { + alert = crate::workspace::visible_alert(&window); + alert.is_some() + }) + .expect("the discard question"); + crate::test_support::respond(&alert.expect("the alert"), "discard"); + wait_until(WAIT, || sent(&harness.fake).len() > before).expect("the run after the discard"); + assert_eq!(tracker.pending_count(), 0); + } +} diff --git a/linux/crates/app/src/ui/key_tab/mod.rs b/linux/crates/app/src/ui/key_tab/mod.rs index f29a2b6ba1..a1e3c75561 100644 --- a/linux/crates/app/src/ui/key_tab/mod.rs +++ b/linux/crates/app/src/ui/key_tab/mod.rs @@ -258,6 +258,7 @@ fn key_grid(key: &str, settings: Rc, tasks: Tasks) -> DataGrid { sorting: GridSorting::Client, rules: None, tracker: None, + result_lock: None, settings, layout: None, references: None, diff --git a/linux/crates/app/src/ui/result_pane/mod.rs b/linux/crates/app/src/ui/result_pane/mod.rs index fce8ebd947..c8bc659026 100644 --- a/linux/crates/app/src/ui/result_pane/mod.rs +++ b/linux/crates/app/src/ui/result_pane/mod.rs @@ -7,9 +7,11 @@ mod result_set_view; mod result_summary; use std::rc::Rc; +use std::sync::Arc; use gtk4 as gtk; use gtk4::prelude::*; +use tablepro_core::meta::TableRef; use tablepro_linux_platform::AppSettings; use tablepro_session::runtime::Tasks; @@ -17,8 +19,9 @@ pub(crate) use self::result_limit_banner::ResultLimitBanner; pub(crate) use self::result_set_view::ResultSetView; pub(crate) use self::result_summary::ResultSummary; use crate::i18n::ngettext_f; -use crate::services::grid::grid_columns; -use crate::ui::data_grid::{DataGrid, DialectSource, GridConfig, GridSorting, RowData}; +use crate::services::grid::{GridColumn, ResultRefusal, grid_columns}; +use crate::services::tab_tracker_handle::TabTrackerHandle; +use crate::ui::data_grid::{DataGrid, DialectSource, EditRules, GridConfig, GridSorting, RowData}; use crate::ui::export::ExportSource; use crate::ui::format::format_thousands; @@ -31,26 +34,58 @@ pub(crate) struct ResultGridDeps { pub dialect: Option, } +pub(crate) struct ResultGrid { + pub columns: Arc<[GridColumn]>, + pub rows: Vec, + pub rules: Option, + pub tracker: Option, + pub table: Option, + pub result_lock: Option, +} + +impl ResultGrid { + pub(crate) fn read_only(view: &ResultSetView, result_lock: Option) -> Self { + Self { + columns: grid_columns(&view.columns, &[], |_| true), + rows: view.rows.iter().cloned().map(RowData::result).collect(), + rules: None, + tracker: None, + table: None, + result_lock, + } + } +} + /// The page for `view` and the grid on it, named `name` as the results /// list names it. Export All on its banner hands `export_all` the rows /// the grid shows, in the order it shows them, for the dialog's Shown /// Rows. pub(crate) fn result_set_page( view: &ResultSetView, + shown: ResultGrid, name: &str, deps: &ResultGridDeps, export_all: impl Fn(ExportSource) + 'static, ) -> (gtk::Widget, DataGrid) { + let ResultGrid { + columns, + rows, + rules, + tracker, + table, + result_lock, + } = shown; let grid = DataGrid::new(GridConfig { - columns: grid_columns(&view.columns, &[], |_| true), + columns, sorting: GridSorting::Client, - rules: None, - tracker: None, + rules, + tracker, + result_lock, settings: deps.settings.clone(), layout: None, references: None, dialect: deps.dialect.clone(), - table: None, + table, find: true, filter_by_value: false, tasks: deps.tasks.clone(), @@ -58,7 +93,7 @@ pub(crate) fn result_set_page( }); grid.set_hexpand(true); grid.set_vexpand(true); - grid.replace_rows(view.rows.iter().cloned().map(RowData::result).collect(), 0); + grid.replace_rows(rows, 0); let banner = ResultLimitBanner::new(&view.summary); let shown = grid.downgrade(); banner.connect_export_all(move || { diff --git a/linux/crates/app/src/ui/workspace/close_flow.rs b/linux/crates/app/src/ui/workspace/close_flow.rs index c49f3800dd..2257f256c2 100644 --- a/linux/crates/app/src/ui/workspace/close_flow.rs +++ b/linux/crates/app/src/ui/workspace/close_flow.rs @@ -14,6 +14,7 @@ use super::workspace_persistence::WorkspaceTabs; use super::{WorkspaceInput, WorkspaceModel, WorkspaceTab}; use crate::runtime::DeliverInput; use crate::ui::browse_tab::BrowseTab; +use crate::ui::editor::SqlEditor; use crate::workspace::CloseDecision; /// Every editor tab's connection, taken from its tab. Each tab's run is @@ -68,6 +69,17 @@ impl WorkspaceModel { } /// The table tabs, or only `id` when it is one. + pub(super) fn editor_tabs(&self) -> Vec { + self.workspace_tabs + .borrow() + .values() + .filter_map(|tab| match tab { + WorkspaceTab::Editor(slot) => Some(slot.editor.clone()), + _ => None, + }) + .collect() + } + pub(super) fn browse_tabs(&self, id: Option) -> Vec { self.workspace_tabs .borrow() @@ -90,6 +102,9 @@ impl WorkspaceModel { for browse in self.browse_tabs(Some(id)) { browse.commit_cell_edit(); } + if let Some(editor) = self.editor_tab(id) { + editor.commit_result_edit(); + } let tab_view = self.tab_view.clone(); if self.close_guards.may_close_tab(&self.workspace_handle, id) { self.finish_close_workspace_tab(id, &tab_view); diff --git a/linux/crates/app/src/ui/workspace/mod.rs b/linux/crates/app/src/ui/workspace/mod.rs index 3d11128ec7..be5401ce8f 100644 --- a/linux/crates/app/src/ui/workspace/mod.rs +++ b/linux/crates/app/src/ui/workspace/mod.rs @@ -583,6 +583,8 @@ pub(crate) enum WorkspaceInput { /// workspace is saved. WorkspaceTabsChanged, SaveTab(Uuid), + // A query tab's changed rows alone, never its SQL file. + SaveRows(Uuid), StopSaving(Uuid), /// A save started or settled, or tabs' changes were dropped. Workspace(crate::workspace::WorkspaceEvent), @@ -983,6 +985,9 @@ impl Workspace { for browse in self.read(|model| model.browse_tabs(None)).unwrap_or_default() { browse.commit_cell_edit(); } + for editor in self.read(|model| model.editor_tabs()).unwrap_or_default() { + editor.commit_result_edit(); + } } /// The exports running from this workspace. @@ -1272,6 +1277,7 @@ impl WorkspaceModel { } => self.dispatch_select_table(schema, name, open_mode), WorkspaceInput::WorkspaceTabsChanged => self.on_workspace_tabs_changed(), WorkspaceInput::SaveTab(id) => self.request_save(id), + WorkspaceInput::SaveRows(id) => self.request_save_rows(id), WorkspaceInput::StopSaving(id) => self.stop_saving(id), WorkspaceInput::Workspace(event) => self.on_workspace_event(event), WorkspaceInput::SavesInFlightChanged(count) => self.on_saves_in_flight_changed(count), diff --git a/linux/crates/app/src/ui/workspace/saves.rs b/linux/crates/app/src/ui/workspace/saves.rs index e8000e11d1..30c7dac8be 100644 --- a/linux/crates/app/src/ui/workspace/saves.rs +++ b/linux/crates/app/src/ui/workspace/saves.rs @@ -37,6 +37,22 @@ impl WorkspaceModel { }); } + pub(super) fn request_save_rows(&self, tab: Uuid) { + let Some(parent) = self.dialog_parent() else { + return; + }; + let (handle, input) = (self.workspace_handle.clone(), self.input.clone()); + glib::spawn_future_local(async move { + match handle.save_rows_of(&parent, tab).await { + SaveStart::Rejected(rejection) if rejection.needs_alert() => input.deliver(WorkspaceInput::ShowAlert { + title: crate::i18n::gettext("Cannot Save"), + body: rejection.message(), + }), + start => tracing::debug!(?start, %tab, "rows save"), + } + }); + } + pub(super) fn stop_saving(&self, tab: Uuid) { self.workspace_handle.stop_saves(CloseScope::Tab(tab)); } @@ -46,6 +62,9 @@ impl WorkspaceModel { WorkspaceEvent::SaveStarted { tab } => { self.dispatch_to_tab(tab, browse_tab::SaveInput::SaveStarted); self.send_to_structure_tab(tab, structure_tab::SaveInput::SaveStarted); + if let Some(editor) = self.editor_tab(tab) { + editor.rows_save_started(); + } } WorkspaceEvent::SaveSettled { settled, in_prompt } => self.on_save_settled(*settled, in_prompt), WorkspaceEvent::Discarded { tabs, reload } => { @@ -60,6 +79,9 @@ impl WorkspaceModel { false => structure_tab::SaveInput::Discard, }; self.send_to_structure_tab(tab, structure); + if let Some(editor) = self.editor_tab(tab) { + editor.rows_discarded(reload); + } } } } @@ -98,19 +120,34 @@ impl WorkspaceModel { .timeout(4) .build(); self.add_toast(toast); - self.dispatch_to_tab(tab, browse_tab::SaveInput::SaveCommitted(origins)); - } - (WriteKind::RowChanges, SaveOutcome::RolledBack(cause)) => { - // The row behind the failing step is marked before the - // error shows, so the user sees which one it was. - if let Some(source) = failing { - self.dispatch_to_tab(tab, browse_tab::SaveInput::MarkErrorRow(source)); + match self.editor_tab(tab) { + Some(editor) => editor.rows_saved(), + None => self.dispatch_to_tab(tab, browse_tab::SaveInput::SaveCommitted(origins)), } - self.dispatch_to_tab(tab, browse_tab::SaveInput::SaveFailed(cause.message())); } + (WriteKind::RowChanges, SaveOutcome::RolledBack(cause)) => match self.editor_tab(tab) { + Some(editor) => { + editor.rows_save_failed(failing); + self.show_error_alert(&crate::i18n::gettext("Save Failed"), &cause.message()); + } + None => { + // The row behind the failing step is marked before the + // error shows, so the user sees which one it was. + if let Some(source) = failing { + self.dispatch_to_tab(tab, browse_tab::SaveInput::MarkErrorRow(source)); + } + self.dispatch_to_tab(tab, browse_tab::SaveInput::SaveFailed(cause.message())); + } + }, (WriteKind::RowChanges, SaveOutcome::Unknown(cause)) => { let message = cause.sentence(&self.tab_name(tab)); - self.dispatch_to_tab(tab, browse_tab::SaveInput::SaveOutcomeUnknown(message)); + match self.editor_tab(tab) { + Some(editor) => { + editor.rows_save_unknown(); + self.show_error_alert(&crate::i18n::gettext("Save Result Unknown"), &message); + } + None => self.dispatch_to_tab(tab, browse_tab::SaveInput::SaveOutcomeUnknown(message)), + } } (WriteKind::SchemaChanges, SaveOutcome::Committed { .. }) => { self.on_structure_save_completed(tab, saved_table_name); diff --git a/linux/crates/app/src/ui/workspace/tab_factory.rs b/linux/crates/app/src/ui/workspace/tab_factory.rs index f3837f86cb..55b8f44575 100644 --- a/linux/crates/app/src/ui/workspace/tab_factory.rs +++ b/linux/crates/app/src/ui/workspace/tab_factory.rs @@ -189,6 +189,7 @@ impl TabFactory { if let Some(database) = database { editor.move_to_database(&database); } + editor.attach_row_tracker(parts.change_trackers.register(id)); connect_editor(&editor, id, &parts.input, &parts.toasts); // It moved before the workspace heard it, which follows it once // the tab is in. @@ -519,6 +520,12 @@ pub(super) fn connect_editor( editor.connect_transaction_changed(move |_, _| transaction.deliver(WorkspaceInput::EditorTransactionChanged)); let toast = input.clone(); editor.connect_toast(move |_, text| toast.deliver(WorkspaceInput::ShowToast(text))); + let save = input.clone(); + editor.connect_save_rows(move |_| save.deliver(WorkspaceInput::SaveRows(tab_id))); + let stop = input.clone(); + editor.connect_stop_saving_rows(move |_| stop.deliver(WorkspaceInput::StopSaving(tab_id))); + let dirty = input.clone(); + editor.connect_rows_dirty_changed(move |_| dirty.deliver(WorkspaceInput::TabDirtyChanged)); let close = input.clone(); editor.connect_close_requested(move |_| close.deliver(WorkspaceInput::CloseWorkspaceTab(tab_id))); let assistant = input.clone(); diff --git a/linux/crates/app/src/ui/workspace/window_tabs.rs b/linux/crates/app/src/ui/workspace/window_tabs.rs index 8c4a92bc64..65cdcda025 100644 --- a/linux/crates/app/src/ui/workspace/window_tabs.rs +++ b/linux/crates/app/src/ui/workspace/window_tabs.rs @@ -68,4 +68,8 @@ impl TabDirectory for WindowTabs { slot.editor.file()?; Some(Box::pin(slot.editor.save_to_file())) } + + fn has_file(&self, tab: Uuid) -> bool { + matches!(self.tabs.borrow().get(&tab), Some(WorkspaceTab::Editor(slot)) if slot.editor.file().is_some()) + } } diff --git a/linux/crates/app/src/ui/workspace/workspace_tabs.rs b/linux/crates/app/src/ui/workspace/workspace_tabs.rs index 272adcac51..52dd497b8a 100644 --- a/linux/crates/app/src/ui/workspace/workspace_tabs.rs +++ b/linux/crates/app/src/ui/workspace/workspace_tabs.rs @@ -14,6 +14,7 @@ use tablepro_storage::{DraftId, DraftScope}; use uuid::Uuid; use crate::ui::browse_tab::{BrowseTabInput, PagingInput, SaveInput, TableFilter}; +use crate::ui::editor::SqlEditor; use crate::ui::structure_tab::StructureMode; use crate::workspace::{ EditorFileRecord, RestoredDrafts, TabRecord, WorkspaceFileName, WorkspaceLoad, WorkspaceRecord, WorkspaceSaveError, @@ -243,6 +244,7 @@ impl WorkspaceModel { match &removed { WorkspaceTab::Editor(slot) => { slot.editor.shutdown(); + self.change_trackers.unregister(id); // The record no longer names this draft, so the file has // nothing left pointing at it. self.drafts.discard(&self.draft_scope(), slot.draft); @@ -475,9 +477,12 @@ impl WorkspaceModel { /// name rows of the connection going away, so they cannot be saved /// after it. pub(super) fn close_tabs(&mut self) { - for tab in self.workspace_tabs.borrow().values() { + for (id, tab) in self.workspace_tabs.borrow().iter() { match tab { - WorkspaceTab::Editor(slot) => slot.editor.shutdown(), + WorkspaceTab::Editor(slot) => { + slot.editor.shutdown(); + self.change_trackers.unregister(*id); + } WorkspaceTab::Structure(slot) => { slot.structure.shutdown(); self.structure_trackers.unregister(slot.id); @@ -494,6 +499,13 @@ impl WorkspaceModel { self.workspace_tabs.borrow_mut().clear(); } + pub(super) fn editor_tab(&self, tab_id: Uuid) -> Option { + match self.workspace_tabs.borrow().get(&tab_id) { + Some(WorkspaceTab::Editor(slot)) => Some(slot.editor.clone()), + _ => None, + } + } + /// Forward a per-tab Browse input to the `Table` slot's Browse-side /// controller. pub(super) fn dispatch_to_tab(&self, tab_id: Uuid, msg: impl Into) { diff --git a/linux/crates/app/src/workspace/close_guard.rs b/linux/crates/app/src/workspace/close_guard.rs index cd6a682a77..0611239adc 100644 --- a/linux/crates/app/src/workspace/close_guard.rs +++ b/linux/crates/app/src/workspace/close_guard.rs @@ -147,7 +147,9 @@ async fn start_saves( tabs: &[Uuid], ) -> (Vec<(Uuid, SaveRejection)>, bool) { let mut refused = Vec::new(); - for tab in tabs { + let mut started = HashSet::new(); + // A query tab is listed once for its rows and once for its file, and one save takes both. + for tab in tabs.iter().filter(|tab| started.insert(**tab)) { for start in handle.save(parent, &[*tab]).await { match start { SaveStart::Rejected(SaveRejection::ReviewDeclined) => return (refused, true), diff --git a/linux/crates/app/src/workspace/tab_directory.rs b/linux/crates/app/src/workspace/tab_directory.rs index b237a570d5..b23899a993 100644 --- a/linux/crates/app/src/workspace/tab_directory.rs +++ b/linux/crates/app/src/workspace/tab_directory.rs @@ -15,4 +15,8 @@ pub trait TabDirectory { fn save_file(&self, _tab: Uuid) -> Option>> { None } + + fn has_file(&self, _tab: Uuid) -> bool { + false + } } diff --git a/linux/crates/app/src/workspace/workspace_handle.rs b/linux/crates/app/src/workspace/workspace_handle.rs index 4b75b8d680..0fdd3339d7 100644 --- a/linux/crates/app/src/workspace/workspace_handle.rs +++ b/linux/crates/app/src/workspace/workspace_handle.rs @@ -182,6 +182,23 @@ impl WorkspaceHandle { starts } + pub async fn save_rows_of(&self, parent: >k::Widget, tab: Uuid) -> SaveStart { + let Some(tracker) = self.0.deps.rows.get(tab) else { + return SaveStart::NothingToSave; + }; + self.save_tracked_rows(parent, tab, &tracker).await + } + + async fn save_tracked_rows(&self, parent: >k::Widget, tab: Uuid, tracker: &TabTrackerHandle) -> SaveStart { + let Some(_claim) = ReviewClaim::take(&self.0.reviewing, tab) else { + return SaveStart::Rejected(SaveRejection::InReview); + }; + if let Err(start) = self.confirm_rows(parent, tracker).await { + return start; + } + self.save_rows(tab, tracker) + } + /// Stop the saves `scope` covers. Each settles when its driver /// answers the cancel, as rolled back or with an unknown outcome. pub fn stop_saves(&self, scope: CloseScope) { @@ -221,7 +238,9 @@ impl WorkspaceHandle { PendingState::Clean if entry.open_transaction => Some(DirtyKind::OpenTransaction), PendingState::Clean => None, }; - let file = entry.unsaved_file.then_some(DirtyKind::UnsavedFile); + // A query tab's changed rows and its file are saved and discarded together, so they are one choice. + let rows = matches!(pending, Some(DirtyKind::Unsaved | DirtyKind::OutcomeUnknown(_))); + let file = (entry.unsaved_file && !rows).then_some(DirtyKind::UnsavedFile); [pending, file].into_iter().flatten().map(move |kind| DirtyTab { id: entry.id, title: entry.title.clone(), @@ -270,14 +289,34 @@ impl WorkspaceHandle { } async fn save_tab(&self, parent: >k::Widget, tab: Uuid) -> SaveStart { - if let Some(tracker) = self.0.deps.rows.get(tab) { - let Some(_claim) = ReviewClaim::take(&self.0.reviewing, tab) else { - return SaveStart::Rejected(SaveRejection::InReview); - }; - if let Err(start) = self.confirm_rows(parent, &tracker).await { + // An editor tab holds a row tracker beside its file; a clean one leaves the save to the file. + let rows = + self.0.deps.rows.get(tab).filter(|tracker| { + tracker.pending_state() != PendingState::Clean || !self.0.deps.directory.has_file(tab) + }); + if let Some(tracker) = rows { + let start = self.save_tracked_rows(parent, tab, &tracker).await; + if !matches!( + start, + SaveStart::Started(_) | SaveStart::Saved | SaveStart::NothingToSave + ) { return start; } - return self.save_rows(tab, &tracker); + // A file the text has not moved from stays as it is on disk, edits made elsewhere included. + let unsaved = self + .0 + .deps + .directory + .tabs() + .iter() + .any(|entry| entry.id == tab && entry.unsaved_file); + return match self.0.deps.directory.save_file(tab).filter(|_| unsaved) { + Some(saving) => match saving.await { + Ok(()) => start, + Err(message) => SaveStart::Rejected(SaveRejection::FileNotSaved(message)), + }, + None => start, + }; } if let Some(tracker) = self.0.deps.structures.get(tab) { // A close that saves its tabs can ask again while the review is @@ -738,6 +777,21 @@ mod tests { script } + #[gtk4::test] + fn changed_rows_and_an_unsaved_file_of_one_tab_are_one_choice() { + let workspace = Workspace::open(ConnectionScript::default()); + workspace.tabs.0.borrow_mut()[0].unsaved_file = true; + + let dirty = workspace.handle.dirty_tabs(); + + assert_eq!(dirty.len(), 1, "{dirty:?}"); + assert_eq!(dirty[0].kind, DirtyKind::Unsaved); + let _ = workspace.tracker.discard(); + let dirty = workspace.handle.dirty_tabs(); + assert_eq!(dirty.len(), 1); + assert_eq!(dirty[0].kind, DirtyKind::UnsavedFile); + } + #[gtk4::test] fn commit_ack_lost_applied_reloads() { let workspace = Workspace::open(commit_ack_lost(Ok(CommitResolution::Applied))); diff --git a/linux/data/resources/ui/results-pane.ui b/linux/data/resources/ui/results-pane.ui index 303f881c23..398ebe75ae 100644 --- a/linux/data/resources/ui/results-pane.ui +++ b/linux/data/resources/ui/results-pane.ui @@ -98,6 +98,9 @@ + + + false diff --git a/linux/docs/decisions/0036-editing-query-results.md b/linux/docs/decisions/0036-editing-query-results.md new file mode 100644 index 0000000000..66c7da0812 --- /dev/null +++ b/linux/docs/decisions/0036-editing-query-results.md @@ -0,0 +1,36 @@ +# 0036: A query tab edits the one result of its run, through the table tab's save + +- **Status**: Accepted +- **Date**: 2026-11-05 + +## Context + +[Core ADR 0015](../../../core/docs/decisions/0015-editing-a-query-result.md) decides when a query result's rows are one table's. A query tab runs on its own connection ([0030](0030-a-database-is-a-session-of-its-own.md)), shows every result of a run, keeps at most eight result pages built, and builds them again from the run's rows. A table tab saves its changes through the window's connection, with Confirm Writes, the Read-Only level, the row guards, query history and the close prompts keyed by its tab id. + +## Decision + +- A query tab holds one change tracker, registered under its tab id as a table tab's is, so Save, Confirm Writes, history, the ledger and the close prompts treat it as they treat a table tab. +- After a run that returns one result, a result that `result_source` accepts reads its table's columns on the window's connection, becomes editable once `result_target` maps them, and takes the tracker. A run of several results, a Read-Only connection, a run that leaves a transaction open or does not say whether one is, a run that may change database, and a tab in another database than the connection's keep every result read-only. +- The pending changes bar of a table tab sits under the results. Its Save, and Ctrl+S while the results have the focus, save the rows and never the SQL file; Ctrl+S elsewhere saves the file. The close prompt lists a tab's changed rows and its file as one choice, which saves or discards both. Ctrl+Z and Ctrl+Shift+Z undo and redo the rows in the results. +- After a save the statement behind the result runs again as a run of the tab, with its Stop, and its result is checked from the start; history leaves that run out. Changes made while the save ran keep the rows they were made on until they are saved too. +- A run, Commit or Roll Back with changes pending asks to discard them first, and waits while a save runs. +- Why a result or a column is read-only shows where an edit would open, in the row details, as for a table tab's locked cell. + +## Rationale + +- One tracker per tab keeps the window's save, close and history paths as they are; changes in two results would need two saves under one tab id, and handing one tracker between results drew a new race in each review round. +- Reading the result again as an ordinary run reuses its Stop, its busy state and its checks, rather than a second path that has to keep them in step. +- The window's connection is where the table tab's guards, Safe Mode and outcome handling already live. Saving on the tab's connection would commit or roll back the user's own transaction. +- The row details already explain a locked cell; a banner over every read-only result, most of them joins and aggregates, would be noise (HIG, Banners). + +## Consequences + +- A result read in a transaction is read-only until it is committed or rolled back and run again. +- A script that returns several results is read-only; running its one statement makes that result editable. +- The read after a save starts the result over, scrolled to the top. +- MongoDB and Redis results stay read-only; a collection's documents edit in its table tab ([0026](0026-documents-in-the-table-tab.md)). + +## Alternatives considered + +- **A tracker per result.** The window saves and closes by tab id; several dirty results would need several saves, prompts and outcomes per tab. +- **Saving on the tab's connection.** It would join the user's transaction, and the table tab's guards and outcome handling would need a second home. diff --git a/linux/docs/decisions/README.md b/linux/docs/decisions/README.md index 994ff3c7b0..6bb8a4d1fb 100644 --- a/linux/docs/decisions/README.md +++ b/linux/docs/decisions/README.md @@ -81,6 +81,7 @@ A fact the repository keeps elsewhere, such as a workflow trigger, a branch, a v | [0031](0031-duckdb-2.md) | DuckDB 2.0, writing files DuckDB 1.x reads | Accepted | duckdb 1.20000.1 compiles DuckDB 2.0.0, and every database opens with `storage_compatibility_version = 'v0.10.2'` so new and attached files stay readable by DuckDB 1.x. | | [0032](0032-connection-list-tree.md) | The connection list is the library's tree in a list view | Accepted | The picker and the window's tree show groups nested and in the library's order from one outline keyed by id, with Favourites and Recent sections in the picker, drag and drop, and a menu item for every move. | | [0033](0033-no-compatibility-with-pre-release-states.md) | No compatibility with pre-release states | Accepted | Every app-owned format starts at version 1 with no upgrade step and the history schema is one migration; nothing reads what a build before the first release wrote. | +| [0036](0036-editing-query-results.md) | A query tab edits the one result of its run, through the table tab's save | Accepted | One tracker per query tab under its tab id, held by the run's one editable result; saves go through the window's connection with Confirm Writes, history and row guards, and the statement runs again after a save. | | [0037](0037-mysql-ca-bundle-removal-condition.md) | When the MySQL CA bundle goes | Accepted | sqlx #4044/#4045 add rustls-platform-verifier features, not a caller's ClientConfig; the bundle goes once sqlx's own TLS passes system-trust, extra-CA and name tests against MySQL. | | [0040](0040-copyright-file-from-the-source-tarballs.md) | The copyright file describes the source tarballs | Accepted | `debian/copyright` covers every crate `cargo vendor` ships and the code crates carry from other projects, from `cargo metadata` and each crate's license files; the binary inventory keeps the About dialog, the notices and the RPM `License` tag. | | [0041](0041-beta-repositories-and-release-packages.md) | Beta repositories, a Flatpak repository of our own, and packages on every GitHub release | Accepted | A beta tag goes to a beta PPA, COPR project and Flatpak repository that testers add beside the stable ones; a signed Flatpak repository on R2 serves every other distribution with updates; every GitHub release carries the tested .deb and .rpm and a Flatpak bundle that updates from that repository, with SHA256SUMS. | diff --git a/linux/po/POTFILES.in b/linux/po/POTFILES.in index 2da3447268..0ec3c60ff0 100644 --- a/linux/po/POTFILES.in +++ b/linux/po/POTFILES.in @@ -90,6 +90,7 @@ crates/app/src/services/grid/grid_column.rs crates/app/src/services/grid/grid_row_id.rs crates/app/src/services/grid/hex_dump.rs crates/app/src/services/grid/mod.rs +crates/app/src/services/grid/result_refusal.rs crates/app/src/services/grid/row_key_builder.rs crates/app/src/services/grid/row_snapshot.rs crates/app/src/services/grid/selection_summary.rs @@ -350,6 +351,7 @@ crates/app/src/ui/editor/format_plan.rs crates/app/src/ui/editor/imp.rs crates/app/src/ui/editor/inline_suggestion.rs crates/app/src/ui/editor/mod.rs +crates/app/src/ui/editor/result_editing.rs crates/app/src/ui/editor/result_item.rs crates/app/src/ui/editor/result_widgets.rs crates/app/src/ui/editor/results_pane.rs