diff --git a/linux/crates/app/src/ui/browse_tab/clipboard_handlers.rs b/linux/crates/app/src/ui/browse_tab/clipboard_handlers.rs index 05b685bcb3..0c8ef41197 100644 --- a/linux/crates/app/src/ui/browse_tab/clipboard_handlers.rs +++ b/linux/crates/app/src/ui/browse_tab/clipboard_handlers.rs @@ -36,7 +36,13 @@ impl BrowseModel { /// write them back as `INSERT` statements. fn sql_target(&self) -> crate::ui::export_dialog::SqlTarget { crate::ui::export_dialog::SqlTarget { - table: self.table.clone(), + table: self.table_ref(), + generated: self + .current_columns + .iter() + .filter(|column| column.is_generated) + .map(|column| column.name.clone()) + .collect(), } } diff --git a/linux/crates/app/src/ui/clipboard/clipboard_copy.rs b/linux/crates/app/src/ui/clipboard/clipboard_copy.rs index e439fa013f..a2fbb2be01 100644 --- a/linux/crates/app/src/ui/clipboard/clipboard_copy.rs +++ b/linux/crates/app/src/ui/clipboard/clipboard_copy.rs @@ -170,6 +170,10 @@ fn copy_error_text(error: &CopyError) -> String { "Couldn't copy: the column “{column}” holds a value TablePro couldn't read", &[("column", column)], ), + CopyError::Encode(EncodeError::NotXmlText { column }) => gettext_f( + "Couldn't copy: the column “{column}” holds a control character", + &[("column", column)], + ), CopyError::Insert(RenderInsertError::Literal(_)) => { gettext("This row has a value that can't be written as SQL") } diff --git a/linux/crates/app/src/ui/clipboard/copy_renderer.rs b/linux/crates/app/src/ui/clipboard/copy_renderer.rs index 077d0f7d04..2468210b44 100644 --- a/linux/crates/app/src/ui/clipboard/copy_renderer.rs +++ b/linux/crates/app/src/ui/clipboard/copy_renderer.rs @@ -115,18 +115,12 @@ fn render_rows( CopyKind::Rows => encode(TsvEncoder::new(columns, false), rows, ClipboardFormat::Tsv), CopyKind::RowsWithHeaders => encode(TsvEncoder::new(columns, true), rows, ClipboardFormat::Tsv), CopyKind::Csv => encode( - CsvEncoder::new( - columns, - CsvOptions { - header_row: false, - ..CsvOptions::default() - }, - ), + CsvEncoder::new(columns, clipboard_csv(false)), rows, ClipboardFormat::Csv, ), CopyKind::CsvWithHeaders => encode( - CsvEncoder::new(columns, CsvOptions::default()), + CsvEncoder::new(columns, clipboard_csv(true)), rows, ClipboardFormat::Csv, ), @@ -135,6 +129,17 @@ fn render_rows( } } +/// CSV as the clipboard carries it. Copied text goes as the user sees +/// it: the apostrophe an export puts before `-` or `@alice` would be +/// pasted into every other place the text goes too. +fn clipboard_csv(header_row: bool) -> CsvOptions { + CsvOptions { + header_row, + sanitize_formulas: false, + ..CsvOptions::default() + } +} + fn render_in_clause(dialect: &dyn SqlDialect, values: &InClauseRows) -> Result { let InClauseRows { columns, rows, column } = values; let column_type = columns diff --git a/linux/crates/app/src/ui/clipboard/tests.rs b/linux/crates/app/src/ui/clipboard/tests.rs index 5ec4a36b97..0e6110ddcc 100644 --- a/linux/crates/app/src/ui/clipboard/tests.rs +++ b/linux/crates/app/src/ui/clipboard/tests.rs @@ -91,10 +91,17 @@ fn copy_renderer_csv_matches_render_csv() { CopyKind::Csv, CsvOptions { header_row: false, + sanitize_formulas: false, + ..CsvOptions::default() + }, + ), + ( + CopyKind::CsvWithHeaders, + CsvOptions { + sanitize_formulas: false, ..CsvOptions::default() }, ), - (CopyKind::CsvWithHeaders, CsvOptions::default()), ]; for (kind, options) in cases { let payload = render(rows_request(&rows, kind)).expect("the copy"); @@ -106,6 +113,17 @@ fn copy_renderer_csv_matches_render_csv() { } } +#[test] +fn a_copy_keeps_text_that_starts_like_a_formula_as_it_is() { + let rows = vec![vec![Value::Text("-".into()), Value::Text("@alice".into())]]; + + let tsv = render(rows_request(&rows, CopyKind::Rows)).expect("the copy"); + let csv = render(rows_request(&rows, CopyKind::Csv)).expect("the copy"); + + assert_eq!(tsv.text, "-\t@alice\n"); + assert_eq!(csv.text, "-,@alice\n"); +} + #[test] fn copy_insert_uses_visible_columns_only() { let request = CopyRequest::Insert(InsertCopy { @@ -196,7 +214,7 @@ fn a_slower_earlier_copy_gives_way_to_a_later_one() { finish_on_main(WAIT, fast) .expect("the later copy to end") .expect("the later copy"); - assert_eq!(local_text(&clipboard).as_deref(), Some("-5\t'=cmd\n")); + assert_eq!(local_text(&clipboard).as_deref(), Some("-5\t=cmd\n")); let landed = clipboard.content(); finish_on_main(WAIT, slow) .expect("the earlier copy to end") @@ -295,7 +313,7 @@ fn copy_as_csv_offers_text_csv_and_plain_text() { assert!(formats.contains_type(String::static_type()), "{}", formats.to_str()); assert_eq!( local_text(&clipboard).as_deref(), - Some("\"id\",\"name\"\n\"-5\",\"'=cmd\"\n\"7\",\"Ada, \"\"the first\"\"\"\n") + Some("id,name\n-5,=cmd\n7,\"Ada, \"\"the first\"\"\"\n") ); assert_eq!(toasts, ["Copied 2 rows"]); } @@ -312,7 +330,7 @@ fn copy_rows_offers_tab_separated_values() { ); assert_eq!( local_text(&clipboard).as_deref(), - Some("-5\t'=cmd\n7\t\"Ada, \"\"the first\"\"\"\n") + Some("-5\t=cmd\n7\t\"Ada, \"\"the first\"\"\"\n") ); assert_eq!(toasts, ["Copied 2 rows"]); } diff --git a/linux/crates/app/src/ui/export/export_failure_text.rs b/linux/crates/app/src/ui/export/export_failure_text.rs index 0f5705b4c7..12d87717e1 100644 --- a/linux/crates/app/src/ui/export/export_failure_text.rs +++ b/linux/crates/app/src/ui/export/export_failure_text.rs @@ -53,10 +53,7 @@ pub(crate) fn progress_text(rows: u64, expected: Option) -> String /// file. pub(crate) fn failure_text(failure: &ExportFailure, file: Option, name: &str) -> String { let reason = match failure { - ExportFailure::Stream(ExportStreamFailure::Encode(EncodeError::Undecodable { column })) => gettext_f( - "The column “{column}” holds a value TablePro couldn't read, so it can't be written to the file.", - &[("column", column)], - ), + ExportFailure::Stream(ExportStreamFailure::Encode(error)) => encode_text(error), ExportFailure::Stream(ExportStreamFailure::MultipleResultSets) => { gettext("This statement returned more than one result, so it can't be exported as one file.") } @@ -108,8 +105,27 @@ fn rerun_text(error: &OperationError) -> String { } } +/// Why a value could not go into the file, whichever format refused it. +fn encode_text(error: &EncodeError) -> String { + match error { + EncodeError::Undecodable { column } => gettext_f( + "The column “{column}” holds a value TablePro couldn't read, so it can't be written to the file.", + &[("column", column)], + ), + EncodeError::NotXmlText { column } => gettext_f( + "The column “{column}” holds a control character XML can't carry. Export it as CSV or JSON instead.", + &[("column", column)], + ), + } +} + fn document_text(error: &RenderError) -> String { match error { + RenderError::Encode(error) => encode_text(error), + RenderError::TextTooLong { column, limit } => gettext_f( + "The column “{column}” holds a value longer than the {limit} characters an Excel cell holds. Export it as CSV instead.", + &[("column", column), ("limit", &format_thousands(*limit as u64))], + ), RenderError::TooManyRows { got, limit } => gettext_f( "An Excel sheet holds {limit} rows, and this result has {got}.", &[ @@ -273,4 +289,51 @@ mod tests { "An Excel sheet holds 100 rows, and this result has more. Export it as CSV instead." ); } + + fn document(error: RenderError) -> String { + failure_text( + &ExportFailure::Stream(ExportStreamFailure::Document(error)), + None, + "rows.xlsx", + ) + } + + #[test] + fn a_sheet_names_the_column_it_could_not_write() { + let unreadable = document(RenderError::Encode(EncodeError::Undecodable { + column: "body".to_owned(), + })); + let long = document(RenderError::TextTooLong { + column: "body".to_owned(), + limit: 32_767, + }); + + assert_eq!( + unreadable, + "The column “body” holds a value TablePro couldn't read, so it can't be written to the file." + ); + assert_eq!( + long, + format!( + "The column “body” holds a value longer than the {} characters an Excel cell holds. Export it as CSV instead.", + format_thousands(32_767) + ) + ); + } + + #[test] + fn xml_names_the_column_with_a_control_character() { + let text = failure_text( + &ExportFailure::Stream(ExportStreamFailure::Encode(EncodeError::NotXmlText { + column: "note".to_owned(), + })), + None, + "rows.xml", + ); + + assert_eq!( + text, + "The column “note” holds a control character XML can't carry. Export it as CSV or JSON instead." + ); + } } diff --git a/linux/crates/app/src/ui/export/export_format.rs b/linux/crates/app/src/ui/export/export_format.rs index 57800fc061..2696261d8a 100644 --- a/linux/crates/app/src/ui/export/export_format.rs +++ b/linux/crates/app/src/ui/export/export_format.rs @@ -7,6 +7,7 @@ use tablepro_core::export::{ self, CsvEncoder, CsvOptions, HtmlEncoder, JsonArrayEncoder, MarkdownEncoder, RenderError, RowEncoder, SqlInsertEncoder, XlsxSheetWriter, XmlEncoder, }; +use tablepro_core::meta::TableRef; /// What an export writes. #[derive(Debug, Clone)] @@ -38,7 +39,12 @@ impl RowFormat { Self::Markdown => Box::new(MarkdownEncoder::new(columns)), Self::Html { title } => Box::new(HtmlEncoder::new(columns, title.clone())), Self::Xml => Box::new(XmlEncoder::new(columns)), - Self::Sql(target) => Box::new(SqlInsertEncoder::new(target.dialect.clone(), &target.table, columns)), + Self::Sql(target) => Box::new(SqlInsertEncoder::new( + target.dialect.clone(), + &target.table, + columns, + &target.generated, + )), } } } @@ -88,6 +94,8 @@ impl DocumentWriter { /// from. SQL is offered only with both. #[derive(Debug, Clone)] pub(crate) struct InsertTarget { - pub table: String, + pub table: TableRef, + /// The columns the server computes, which the statements leave out. + pub generated: Arc<[String]>, pub dialect: Arc, } diff --git a/linux/crates/app/src/ui/export/export_sink_parts.rs b/linux/crates/app/src/ui/export/export_sink_parts.rs index 7c24bf07cc..0c082e3c3f 100644 --- a/linux/crates/app/src/ui/export/export_sink_parts.rs +++ b/linux/crates/app/src/ui/export/export_sink_parts.rs @@ -47,7 +47,8 @@ mod tests { RowFormat::Html { title: "t".to_owned() }, RowFormat::Xml, RowFormat::Sql(InsertTarget { - table: "t".to_owned(), + table: tablepro_core::meta::TableRef::unqualified("t"), + generated: Arc::from([]), dialect: Arc::new(FakeDialect), }), ]; diff --git a/linux/crates/app/src/ui/export_dialog.rs b/linux/crates/app/src/ui/export_dialog.rs index 256958ccd9..cd6549cf59 100644 --- a/linux/crates/app/src/ui/export_dialog.rs +++ b/linux/crates/app/src/ui/export_dialog.rs @@ -8,6 +8,7 @@ use relm4::{adw, gtk}; use tablepro_core::dialect::SqlDialect; use tablepro_core::export::{CsvDecimal, CsvDelimiter, CsvLineBreak, CsvOptions, CsvQuote, XLSX_MAX_DATA_ROWS}; +use tablepro_core::meta::TableRef; use tablepro_session::Session; use uuid::Uuid; @@ -24,7 +25,9 @@ use crate::ui::format::format_thousands; /// several tables or none, so SQL is not offered for one. #[derive(Debug, Clone)] pub struct SqlTarget { - pub table: String, + pub table: TableRef, + /// The columns the server computes, which the statements leave out. + pub generated: Arc<[String]>, } #[derive(Clone, Copy, PartialEq, Eq)] @@ -347,6 +350,7 @@ pub(crate) fn present(init: ExportDialogInit<'_>) { } = offer; let target = target.zip(dialect).map(|(target, dialect)| InsertTarget { table: target.table, + generated: target.generated, dialect, }); let formats = Format::offered(target.as_ref()); diff --git a/linux/crates/app/src/ui/grid/row_json_dialog.rs b/linux/crates/app/src/ui/grid/row_json_dialog.rs index ca85a19a49..9f60d40cb7 100644 --- a/linux/crates/app/src/ui/grid/row_json_dialog.rs +++ b/linux/crates/app/src/ui/grid/row_json_dialog.rs @@ -7,6 +7,7 @@ use libadwaita::prelude::*; use sourceview5::prelude::*; use tablepro_core::Value; use tablepro_core::column::ResultColumn; +use tablepro_core::export::EncodeError; use tablepro_session::runtime::Tasks; use super::RowSnapshot; @@ -35,16 +36,16 @@ struct RowJson { shortened: bool, } -fn render(request: RowJsonRequest) -> RowJson { +fn render(request: RowJsonRequest) -> Result { let values = request.row.to_values(); - let full = tablepro_core::export::row_to_json(&request.columns, &values); + let full = tablepro_core::export::row_to_json(&request.columns, &values)?; let mut shortened = false; let shown_values: Vec = values.into_iter().map(|value| shown(value, &mut shortened)).collect(); let shown = match shortened { - true => tablepro_core::export::row_to_json(&request.columns, &shown_values), + true => tablepro_core::export::row_to_json(&request.columns, &shown_values)?, false => full.clone(), }; - RowJson { full, shown, shortened } + Ok(RowJson { full, shown, shortened }) } /// `value` as the dialog shows it, with anything long cut short and @@ -118,18 +119,33 @@ pub(crate) async fn present_row_json( tasks: Tasks, request: RowJsonRequest, ) -> Option { - let rendered = match tasks.spawn_blocking_task(move || render(request)).await { - Ok(rendered) => rendered, + let dialog = match tasks.spawn_blocking_task(move || render(request)).await { + Ok(Ok(rendered)) => row_json_dialog(rendered), + Ok(Err(refused)) => refused_alert(&refused).upcast(), Err(failure) => { tracing::warn!(%failure, "a row could not be rendered as JSON"); return None; } }; - let dialog = row_json_dialog(rendered); dialog.present(Some(&parent)); Some(dialog) } +/// Why the row has no JSON to show. Both refusals name the column the +/// grid shows as unreadable. +fn refused_alert(refused: &EncodeError) -> adw::AlertDialog { + let (EncodeError::Undecodable { column } | EncodeError::NotXmlText { column }) = refused; + let alert = adw::AlertDialog::new( + Some(&crate::i18n::gettext("Can't Show Row as JSON")), + Some(&crate::i18n::gettext_f( + "The column “{column}” holds a value TablePro couldn't read, so the row has no JSON to show.", + &[("column", column)], + )), + ); + alert.add_response("close", &crate::i18n::gettext("_Close")); + alert +} + fn row_json_dialog(json: RowJson) -> adw::Dialog { let RowJson { full, shown, shortened } = json; let buffer = sourceview5::Buffer::new(None); @@ -300,7 +316,7 @@ mod tests { ), }; - let rendered = render(request); + let rendered = render(request).expect("a readable row"); assert!(rendered.shortened); let longest = rendered.shown.lines().map(str::len).max().unwrap_or_default(); @@ -321,6 +337,42 @@ mod tests { assert_eq!(full["tags"].as_array().map(Vec::len), Some(LONG)); } + #[gtk4::test] + fn an_unreadable_value_is_refused_by_name_rather_than_shown_as_null() { + adw::init().expect("libadwaita initialises under the test backend"); + let window = OwnedWindow::presented(adw::Window::new()); + let runtime = crate::test_support::test_runtime(); + let unreadable = Value::Undecodable(Box::new(tablepro_core::value::UndecodedValue { + type_name: "text".to_owned(), + reason: tablepro_core::value::UndecodableReason::InvalidEncoding, + })); + let request = RowJsonRequest { + columns: vec![ + ResultColumn::new("id", ColumnType::untyped_text("text")), + ResultColumn::new("body", ColumnType::untyped_text("text")), + ] + .into(), + row: RowSnapshot::new(vec![Value::Int(1), unreadable].into(), Vec::new()), + }; + + let dialog = finish_on_main( + Duration::from_secs(10), + present_row_json(window.clone().upcast(), runtime.tasks(), request), + ) + .expect("the render to end") + .expect("a dialog"); + + let alert = dialog + .downcast::() + .expect("an alert, not the JSON view"); + assert_eq!( + alert.body().as_str(), + "The column “body” holds a value TablePro couldn't read, so the row has no JSON to show." + ); + alert.force_close(); + runtime.shutdown(); + } + #[gtk4::test] fn a_closed_dialog_lets_go_of_the_row() { adw::init().expect("libadwaita initialises under the test backend"); diff --git a/linux/crates/app/src/ui/import_dialog.rs b/linux/crates/app/src/ui/import_dialog.rs index c4a39f6b6e..c8f7ca31b2 100644 --- a/linux/crates/app/src/ui/import_dialog.rs +++ b/linux/crates/app/src/ui/import_dialog.rs @@ -5,9 +5,10 @@ use relm4::adw::prelude::*; use relm4::prelude::*; use relm4::{adw, gtk}; -use tablepro_core::ColumnInfo; use tablepro_core::export::CsvDelimiter; -use tablepro_core::import::{CsvImportOptions, CsvSheet, MAX_PREVIEW_ROWS, read_csv, suggest_mapping}; +use tablepro_core::import::{ + CsvImportOptions, CsvSheet, ImportColumn, ImportError, MAX_PREVIEW_ROWS, read_csv, suggest_mapping, +}; use crate::runtime::DeliverInput; @@ -28,8 +29,10 @@ pub struct ImportCsvDialog { schema: Option, table: String, bytes: Vec, - columns: Vec, + columns: Vec, sheet: CsvSheet, + /// Why the file could not be read under the options as they stand. + read_error: Option, options: CsvImportOptions, /// One row per table column, in column order. The selected index is /// 0 for "Skip" and the field number plus one otherwise. @@ -45,7 +48,7 @@ pub struct ImportCsvDialog { pub struct ImportCsvDialogInit { pub schema: Option, pub table: String, - pub columns: Vec, + pub columns: Vec, pub bytes: Vec, } @@ -68,9 +71,9 @@ pub enum ImportCsvDialogInput { pub struct CsvImport { pub schema: Option, pub table: String, - pub columns: Vec, + pub columns: Vec, /// One entry per table column: the CSV field it takes, or `None` - /// for a column the user set to Skip. + /// for a column the user set to Skip or the server fills. pub mapping: Vec>, pub options: CsvImportOptions, pub bytes: Vec, @@ -79,7 +82,6 @@ pub struct CsvImport { #[derive(Debug)] pub enum ImportCsvDialogOutput { Import(Box), - ShowToast(String), } impl Component for ImportCsvDialog { @@ -150,8 +152,14 @@ impl Component for ImportCsvDialog { .build(); page.add(&mapping_group); - let summary = gtk::Label::builder().xalign(0.0).wrap(true).build(); - summary.add_css_class("dim-label"); + // Problems with the file are told here, inside the dialog that + // covers the window a toast would go to, and long enough to read. + let summary = gtk::Label::builder() + .xalign(0.0) + .wrap(true) + .wrap_mode(gtk::pango::WrapMode::WordChar) + .selectable(true) + .build(); let summary_group = adw::PreferencesGroup::new(); summary_group.add(&summary); page.add(&summary_group); @@ -173,6 +181,7 @@ impl Component for ImportCsvDialog { bytes: init.bytes, columns: init.columns, sheet: CsvSheet::default(), + read_error: None, options: CsvImportOptions::default(), mapping_rows: Rc::new(RefCell::new(Vec::new())), mapping_group, @@ -193,13 +202,11 @@ impl Component for ImportCsvDialog { ImportCsvDialogInput::MappingChanged => { self.options = self.read_options(); self.show_samples(); + self.show_summary(); } ImportCsvDialogInput::Import => { let mapping = self.mapping(); - if mapping.iter().all(Option::is_none) { - let _ = sender.output(ImportCsvDialogOutput::ShowToast(crate::i18n::gettext( - "Every column is set to Skip, so there is nothing to import.", - ))); + if self.blocker().is_some() { return; } let _ = sender.output(ImportCsvDialogOutput::Import(Box::new(CsvImport { @@ -241,12 +248,11 @@ impl ImportCsvDialog { match read_csv(&self.bytes, &self.options, Some(MAX_PREVIEW_ROWS)) { Ok(sheet) => { self.sheet = sheet; - self.import_button.set_sensitive(true); + self.read_error = None; } Err(error) => { self.sheet = CsvSheet::default(); - self.import_button.set_sensitive(false); - let _ = sender.output(ImportCsvDialogOutput::ShowToast(error.to_string())); + self.read_error = Some(import_error_text(&error)); } } self.rebuild_mapping(sender); @@ -262,12 +268,16 @@ impl ImportCsvDialog { .chain(self.sheet.headers.iter().cloned()) .collect(); let choice_refs: Vec<&str> = choices.iter().map(String::as_str).collect(); + // One list for every row: each row keeps its own selection, and + // a wide file is held once rather than once per column. + let choices = gtk::StringList::new(&choice_refs); let suggested = suggest_mapping(&self.sheet.headers, &self.columns); for (column, field) in self.columns.iter().zip(&suggested) { let row = adw::ComboRow::builder() - .title(glib::markup_escape_text(&column.name)) - .model(>k::StringList::new(&choice_refs)) + .title(glib::markup_escape_text(&column.info.name)) + .model(&choices) + .sensitive(!column.server_filled) .build(); row.set_selected(field.map_or(0, |index| index as u32 + 1)); let sender = sender.clone(); @@ -280,25 +290,50 @@ impl ImportCsvDialog { /// Show what the mapped field actually holds, under each column. fn show_samples(&self) { - for (row, field) in self.mapping_rows.borrow().iter().zip(self.mapping()) { + let rows = self.mapping_rows.borrow(); + for ((row, field), column) in rows.iter().zip(self.mapping()).zip(&self.columns) { row.set_subtitle(&match field { + _ if column.server_filled => crate::i18n::gettext("The database fills this column"), Some(index) => sample_text(&self.sheet, index, &self.options), None => crate::i18n::gettext("Uses the column's own default"), }); } } + /// What stands in the way of importing, if anything. + fn blocker(&self) -> Option { + if let Some(error) = &self.read_error { + return Some(error.clone()); + } + if self.sheet.rows.is_empty() { + return Some(crate::i18n::gettext("The file has no rows to import.")); + } + if self.mapping().iter().all(Option::is_none) { + return Some(crate::i18n::gettext( + "Every column is set to Skip. Pick a field for at least one column to import.", + )); + } + None + } + fn show_summary(&self) { - let text = match self.sheet.headers.is_empty() { - true => crate::i18n::gettext("The file could not be read."), - false if self.sheet.truncated => crate::i18n::gettext_f( + let blocker = self.blocker(); + self.import_button.set_sensitive(blocker.is_none()); + let text = match blocker { + Some(blocker) => { + self.summary.remove_css_class("dim-label"); + self.summary.add_css_class("error"); + self.summary.set_label(&blocker); + return; + } + None if self.sheet.truncated => crate::i18n::gettext_f( "{fields} fields. Showing the first {rows} rows; the whole file is imported.", &[ ("fields", &self.sheet.headers.len().to_string()), ("rows", &self.sheet.rows.len().to_string()), ], ), - false => crate::i18n::ngettext_f( + None => crate::i18n::ngettext_f( "{fields} fields, {rows} row.", "{fields} fields, {rows} rows.", self.sheet.rows.len() as u32, @@ -308,6 +343,8 @@ impl ImportCsvDialog { ], ), }; + self.summary.remove_css_class("error"); + self.summary.add_css_class("dim-label"); self.summary.set_label(&text); } @@ -315,7 +352,9 @@ impl ImportCsvDialog { self.mapping_rows .borrow() .iter() - .map(|row| match row.selected() { + .zip(&self.columns) + .map(|(row, column)| match row.selected() { + _ if column.server_filled => None, 0 => None, index => Some(index as usize - 1), }) @@ -323,6 +362,24 @@ impl ImportCsvDialog { } } +/// What went wrong reading the file, in words the user can act on. +pub(crate) fn import_error_text(error: &ImportError) -> String { + match error { + ImportError::NotUtf8 { offset } => crate::i18n::gettext_f( + "The file isn't UTF-8 text from byte {offset} on. Save it as UTF-8 and import it again.", + &[("offset", &offset.to_string())], + ), + ImportError::Malformed(detail) => { + crate::i18n::gettext_f("The file couldn't be read as CSV: {error}", &[("error", detail)]) + } + ImportError::NoColumns => crate::i18n::gettext("The file has no columns."), + ImportError::TooManyFields { line, limit } => crate::i18n::gettext_f( + "Line {line} has more than {limit} fields, more than any table holds. Check the delimiter.", + &[("line", &line.to_string()), ("limit", &limit.to_string())], + ), + } +} + /// The first few values of one field, as one line. fn sample_text(sheet: &CsvSheet, field: usize, options: &CsvImportOptions) -> String { let values: Vec = sheet @@ -400,20 +457,24 @@ mod tests { #[cfg(test)] mod dialog_tests { use super::*; + use tablepro_core::ColumnInfo; use tablepro_core::column::{ColumnDefault, ColumnType}; /// The dialog only reads a column's name: the type decides what a /// value parses into, which happens after the dialog is gone. - fn column(name: &str) -> ColumnInfo { - ColumnInfo { - name: name.to_owned(), - column_type: ColumnType::unknown(), - nullable: true, - primary_key: false, - is_auto_increment: false, - is_generated: false, - default: ColumnDefault::None, - comment: None, + fn column(name: &str) -> ImportColumn { + ImportColumn { + info: ColumnInfo { + name: name.to_owned(), + column_type: ColumnType::unknown(), + nullable: true, + primary_key: false, + is_auto_increment: false, + is_generated: false, + default: ColumnDefault::None, + comment: None, + }, + server_filled: false, } } @@ -470,16 +531,76 @@ mod dialog_tests { } #[gtk4::test] - fn a_file_that_is_not_csv_leaves_the_import_button_off() { + fn a_column_the_server_fills_cannot_take_a_field() { + let mut id = column("id"); + id.server_filled = true; + let dialog = ImportCsvDialog::builder().launch(ImportCsvDialogInit { + schema: None, + table: "people".to_owned(), + columns: vec![id, column("name")], + bytes: b"id,name\n1,ada\n".to_vec(), + }); + crate::test_support::drain_main_context(); + + let rows = dialog.model().mapping_rows.borrow().clone(); + + assert_eq!(dialog.model().mapping(), vec![None, Some(1)]); + assert!( + !rows[0].is_sensitive(), + "a field could be picked for a column the server fills" + ); + assert_eq!( + rows[0].subtitle().unwrap_or_default().as_str(), + "The database fills this column" + ); + } + + #[gtk4::test] + fn a_file_that_is_not_csv_leaves_the_import_button_off_and_says_why_in_the_dialog() { let dialog = ImportCsvDialog::builder().launch(ImportCsvDialogInit { schema: None, table: "people".to_owned(), columns: vec![column("id")], - bytes: Vec::new(), + bytes: b"id\n\xff\n".to_vec(), }); crate::test_support::drain_main_context(); assert!(!dialog.model().import_button.is_sensitive()); + assert_eq!( + dialog.model().summary.label().as_str(), + "The file isn't UTF-8 text from byte 3 on. Save it as UTF-8 and import it again." + ); + assert!(dialog.model().summary.has_css_class("error")); + } + + #[gtk4::test] + fn every_column_set_to_skip_turns_the_import_button_off() { + let dialog = launch("name,id\nada,1\n"); + crate::test_support::drain_main_context(); + assert!(dialog.model().import_button.is_sensitive()); + + for row in dialog.model().mapping_rows.borrow().iter() { + row.set_selected(0); + } + crate::test_support::drain_main_context(); + + assert!(!dialog.model().import_button.is_sensitive()); + assert_eq!( + dialog.model().summary.label().as_str(), + "Every column is set to Skip. Pick a field for at least one column to import." + ); + } + + #[gtk4::test] + fn a_file_with_only_a_header_has_nothing_to_import() { + let dialog = launch("name,id\n"); + crate::test_support::drain_main_context(); + + assert!(!dialog.model().import_button.is_sensitive()); + assert_eq!( + dialog.model().summary.label().as_str(), + "The file has no rows to import." + ); } } diff --git a/linux/crates/app/src/ui/workspace/import.rs b/linux/crates/app/src/ui/workspace/import.rs index 1c2cf195c9..fae483fca4 100644 --- a/linux/crates/app/src/ui/workspace/import.rs +++ b/linux/crates/app/src/ui/workspace/import.rs @@ -5,15 +5,22 @@ use gtk4::{gio, glib}; use libadwaita::prelude::*; use relm4::{Component, ComponentController}; +use std::ops::Range; +use std::time::Duration; + use tablepro_core::dialect::SqlDialect; use tablepro_core::edit::CellInput; +use tablepro_core::import::{ImportColumn, ImportPlan, plan_import}; use tablepro_core::meta::TableRef; -use tablepro_core::statement::GuardedStatement; -use tablepro_core::{ColumnInfo, ForeignKeyMode, WriteBatch, WriteKind, WriteReport}; +use tablepro_core::{CallOptions, DriverError, WriteKind}; +use tablepro_session::runtime::Tasks; use tablepro_session::{OperationError, OperationKind, Session}; +use tokio_util::sync::CancellationToken; +use crate::services::history_service::HistoryService; +use crate::ui::error_text::{driver_message, operation_error_message}; use crate::ui::file_read::{FileReadError, read_capped_file}; -use crate::ui::import_dialog::{CsvImport, MAX_CSV_FILE_BYTES}; +use crate::ui::import_dialog::{CsvImport, MAX_CSV_FILE_BYTES, import_error_text}; use super::{WorkspaceInput, WorkspaceModel}; use crate::runtime::DeliverInput; @@ -23,6 +30,11 @@ use crate::workspace::RanStatements; /// announces it, so a failed import has something to read back. const IMPORT_LOG_SAMPLE: usize = 3; +/// What each statement of an import adds to the save deadline. A +/// statement carries up to a thousand rows, so this bounds a server that +/// stopped answering rather than one that is merely slow. +const IMPORT_TIME_PER_STATEMENT: Duration = Duration::from_secs(5); + impl WorkspaceModel { /// Ask for a CSV, then read it and the table's columns together so /// the mapping dialog opens with both. @@ -100,7 +112,7 @@ impl WorkspaceModel { &mut self, schema: Option, table: String, - columns: Vec, + columns: Vec, bytes: Vec, ) { use crate::ui::import_dialog::{ImportCsvDialog, ImportCsvDialogInit, ImportCsvDialogOutput}; @@ -117,7 +129,6 @@ impl WorkspaceModel { move |_, out| { input.deliver(match out { ImportCsvDialogOutput::Import(import) => WorkspaceInput::ImportCsvRun(import), - ImportCsvDialogOutput::ShowToast(text) => WorkspaceInput::ShowToast(text), }); } }); @@ -125,58 +136,258 @@ impl WorkspaceModel { self.import_dialog = Some(dialog); } - /// Read the whole file, turn every row into an INSERT and write them - /// as one batch, so a file either lands whole or not at all. + /// Read the whole file, turn its rows into INSERTs and write them as + /// one batch, so a file either lands whole or not at all. The file + /// is read and planned on the blocking pool, never on this thread. pub(super) fn on_import_csv_run(&self, import: CsvImport) { - let Some(session) = self.context.active.get() else { + let Some((session, dialect)) = self + .context + .active + .get() + .and_then(|session| session.current_dialect().map(|dialect| (session, dialect))) + else { self.show_toast(&crate::i18n::gettext("Connect to a database first.")); return; }; - let rows = match import_rows(&import) { - Ok(rows) => rows, - Err(message) => { - self.show_error_alert(&crate::i18n::gettext("Couldn't Import"), &message); - return; - } - }; - if rows.is_empty() { - self.show_toast(&crate::i18n::gettext("That file has no rows to import.")); + let Some(ticket) = self.imports.begin() else { return; - } - - let count = rows.len(); - let table = TableRef::new(import.schema.clone(), import.table.clone()); + }; let history = Some(self.history.clone()); let connection = self.context.active.metadata(); - let dialect = session.current_dialect(); + let tasks = self.context.tasks.clone(); + let work = run_import(ImportRun { + session, + dialect, + import, + cancel: ticket.cancel.clone(), + tasks: tasks.clone(), + history, + connection, + }); + let imports = self.imports.clone(); let input = self.input.clone(); - drop(self.context.tasks.spawn_task(async move { - let written = write_import(&session, table, import.columns, rows).await; - match written { - Ok((sql, report)) => { - // An import changes the database, so it belongs in - // the same history as a statement run from the editor. - let touched = report.total_rows().and_then(|rows| i64::try_from(rows).ok()); - RanStatements::starting(history, connection, dialect, sql) - .finished(touched, tablepro_storage::query_history::Outcome::Success) - .await; + let _delivery = crate::runtime::spawn_and_deliver(&tasks, work, move |finished| { + imports.settle(ticket.id); + match finished { + Ok(Ok(rows)) => { input.deliver(WorkspaceInput::RefreshPage); input.deliver(WorkspaceInput::ShowToast(crate::i18n::ngettext_f( "Imported {n} row.", "Imported {n} rows.", - u32::try_from(count).unwrap_or(u32::MAX), - &[("n", &count.to_string())], + u32::try_from(rows).unwrap_or(u32::MAX), + &[("n", &rows.to_string())], ))); } - Err(error) => { - let message = crate::ui::error_text::operation_error_message(&error); - input.deliver(WorkspaceInput::ShowAlert { - title: crate::i18n::gettext("Couldn't Import"), - body: message, - }); + Ok(Err(ImportEnd::Empty)) => input.deliver(WorkspaceInput::ShowToast(crate::i18n::gettext( + "That file has no rows to import.", + ))), + Ok(Err(ImportEnd::Stopped)) => { + input.deliver(WorkspaceInput::RefreshPage); + input.deliver(WorkspaceInput::ShowToast(crate::i18n::gettext("Import stopped."))); } + Ok(Err(ImportEnd::Failed(message))) => input.deliver(WorkspaceInput::ShowAlert { + title: crate::i18n::gettext("Couldn't Import"), + body: message, + }), + Err(failure) => input.deliver(WorkspaceInput::ShowAlert { + title: crate::i18n::gettext("Couldn't Import"), + body: crate::ui::error_text::task_failure_message(&failure), + }), } - })); + }); + } +} + +/// Everything an import needs once it leaves the GTK thread. +struct ImportRun { + session: Arc, + dialect: Arc, + import: CsvImport, + cancel: CancellationToken, + tasks: Tasks, + history: Option, + connection: Option, +} + +/// Why an import wrote nothing, or not everything. +#[derive(Debug)] +enum ImportEnd { + Empty, + Stopped, + Failed(String), +} + +/// Read and plan the file on the blocking pool, then write the plan. +/// Gives back how many rows landed. +async fn run_import(run: ImportRun) -> Result { + let ImportRun { + session, + dialect, + import, + cancel, + tasks, + history, + connection, + } = run; + let first_line = first_data_line(&import); + let planning_dialect = dialect.clone(); + let planned = tasks + .spawn_blocking_task(move || plan_file(&import, &*planning_dialect)) + .await + .map_err(|failure| ImportEnd::Failed(crate::ui::error_text::task_failure_message(&failure)))??; + let Some((plan, rows)) = planned else { + return Err(ImportEnd::Empty); + }; + if cancel.is_cancelled() { + return Err(ImportEnd::Stopped); + } + let sql: Vec = plan + .batch + .steps + .iter() + .map(|step| step.statement.sql().to_owned()) + .collect(); + tracing::info!( + rows, + statements = sql.len(), + sample = ?sql.iter().take(IMPORT_LOG_SAMPLE).collect::>(), + "importing a CSV" + ); + let batch = Arc::new(plan.batch); + let written = session + .run_cancellable( + OperationKind::Save(WriteKind::RowChanges), + &cancel, + move |lease, options| { + let batch = batch.clone(); + let options = import_call(options, batch.len()); + async move { Ok(lease.write_batch(&batch, options).await?) } + }, + ) + .await; + match written { + Ok(report) => { + // An import changes the database, so it belongs in the same + // history as a statement run from the editor. + let touched = report.total_rows().and_then(|rows| i64::try_from(rows).ok()); + RanStatements::starting(history, connection, Some(dialect), sql) + .finished(touched, tablepro_storage::query_history::Outcome::Success) + .await; + Ok(rows) + } + Err(OperationError::Cancelled) => Err(ImportEnd::Stopped), + Err(error) => Err(ImportEnd::Failed(import_failure_text( + &error, + &plan.step_rows, + first_line, + ))), + } +} + +/// The call the write runs under: the save deadline, and as long again +/// as every statement may take on top of it, so a large file is not cut +/// off by the deadline a single edit gets. +fn import_call(options: CallOptions, statements: usize) -> CallOptions { + let extra = IMPORT_TIME_PER_STATEMENT.saturating_mul(u32::try_from(statements).unwrap_or(u32::MAX)); + CallOptions::new( + options.deadline().map(|deadline| deadline + extra), + options.cancel().clone(), + ) +} + +/// The header takes the first line when there is one, so the first data +/// row is line 2 in the file the user is looking at. +fn first_data_line(import: &CsvImport) -> usize { + match import.options.has_header { + true => 2, + false => 1, + } +} + +/// The plan for the whole file and how many rows it writes, `None` for +/// a file with no rows, or what to tell the user about the file instead. +fn plan_file(import: &CsvImport, dialect: &dyn SqlDialect) -> Result, ImportEnd> { + let rows = import_rows(import).map_err(ImportEnd::Failed)?; + if rows.is_empty() { + return Ok(None); + } + let count = rows.len(); + let table = TableRef::new(import.schema.clone(), import.table.clone()); + let plan = plan_import(dialect, &table, &import.columns, rows).map_err(|error| { + ImportEnd::Failed(crate::ui::error_text::operation_error_message(&OperationError::Build( + error, + ))) + })?; + Ok(Some((plan, count))) +} + +/// Why the write failed, told as the lines of the file the failing +/// statement held rather than as a statement number the user never saw. +fn import_failure_text(error: &OperationError, step_rows: &[Range], first_line: usize) -> String { + let lines = |index: usize| { + step_rows + .get(index) + .map(|rows| (first_line + rows.start, first_line + rows.end.saturating_sub(1))) + }; + match error { + OperationError::Driver(DriverError::RolledBack { + statement_index, + source, + }) => match lines(*statement_index) { + Some((first, last)) if first == last => crate::i18n::gettext_f( + "Line {line} could not be written: {error}. The import was rolled back, so no rows were added.", + &[("line", &first.to_string()), ("error", &driver_message(source))], + ), + Some((first, last)) => crate::i18n::gettext_f( + "Lines {first} to {last} could not be written: {error}. The import was rolled back, so no rows were added.", + &[ + ("first", &first.to_string()), + ("last", &last.to_string()), + ("error", &driver_message(source)), + ], + ), + None => operation_error_message(error), + }, + OperationError::Driver(DriverError::PartiallyApplied { + statement_index, + source, + .. + }) => { + let written: usize = step_rows + .iter() + .take(*statement_index) + .map(ExactSizeIterator::len) + .sum(); + let count = u32::try_from(written).unwrap_or(u32::MAX); + match lines(*statement_index) { + Some((first, last)) if first == last => crate::i18n::ngettext_f( + "Line {line} could not be written: {error}. The {written} row before it stays in the table, which has no transaction to undo it.", + "Line {line} could not be written: {error}. The {written} rows before it stay in the table, which has no transaction to undo them.", + count, + &[ + ("line", &first.to_string()), + ("error", &driver_message(source)), + ("written", &written.to_string()), + ], + ), + // A table without transactions can keep the rows of a + // multi-row statement that came before the one it + // refused. + Some((first, last)) => crate::i18n::ngettext_f( + "Lines {first} to {last} could not be written: {error}. The table has no transaction to undo the import, so the {written} row before them stays, and some of these lines may have been written too.", + "Lines {first} to {last} could not be written: {error}. The table has no transaction to undo the import, so the {written} rows before them stay, and some of these lines may have been written too.", + count, + &[ + ("first", &first.to_string()), + ("last", &last.to_string()), + ("error", &driver_message(source)), + ("written", &written.to_string()), + ], + ), + None => operation_error_message(error), + } + } + _ => operation_error_message(error), } } @@ -194,64 +405,20 @@ async fn read_csv_file(file: &gio::File) -> Result, String> { }) } -async fn read_columns(session: Arc, table: TableRef) -> Result, OperationError> { +/// The table's columns with what the engine lets an insert write, so +/// the mapping never offers a column the server fills. +async fn read_columns(session: Arc, table: TableRef) -> Result, OperationError> { session .run(OperationKind::Metadata, move |lease, options| { let table = table.clone(); - async move { Ok(lease.fetch_columns(&table, options).await?) } - }) - .await -} - -/// Write the rows as one batch on the connection that runs it, giving -/// back the SQL it ran for the history. -async fn write_import( - session: &Session, - table: TableRef, - columns: Vec, - rows: Vec>, -) -> Result<(Vec, WriteReport), OperationError> { - let rows = Arc::new(rows); - session - .run(OperationKind::Save(WriteKind::RowChanges), move |lease, options| { - let table = table.clone(); - let columns = columns.clone(); - let rows = rows.clone(); async move { - let batch = import_batch(&*lease.dialect(), &table, &columns, &rows)?; - let sql: Vec = batch.steps.iter().map(|step| step.statement.sql().to_owned()).collect(); - tracing::info!( - rows = rows.len(), - sample = ?sql.iter().take(IMPORT_LOG_SAMPLE).collect::>(), - "importing a CSV" - ); - let report = lease.write_batch(&batch, options).await?; - Ok((sql, report)) + let definition = lease.fetch_table_definition(&table, options).await?; + Ok(definition.columns.iter().map(ImportColumn::from_definition).collect()) } }) .await } -/// One INSERT per row, as a single batch so the engine runs them in -/// one transaction where it has one. -fn import_batch( - dialect: &dyn SqlDialect, - table: &TableRef, - columns: &[ColumnInfo], - rows: &[Vec], -) -> Result { - let steps = rows - .iter() - .map(|cells| tablepro_core::dml::build_insert(dialect, table, columns, cells).map(GuardedStatement::unguarded)) - .collect::, _>>()?; - Ok(WriteBatch::new( - WriteKind::RowChanges, - table.clone(), - steps, - ForeignKeyMode::Enforced, - )) -} - /// Every row of the file as the cells to insert, or the first row that /// could not be read. /// @@ -260,10 +427,8 @@ fn import_batch( fn import_rows(import: &CsvImport) -> Result>, String> { use tablepro_core::import::{read_csv, row_to_cells}; - let sheet = read_csv(&import.bytes, &import.options, None).map_err(|error| error.to_string())?; - // The header takes the first line when there is one, so the first - // data row is line 2 in the file the user is looking at. - let first_line = if import.options.has_header { 2 } else { 1 }; + let sheet = read_csv(&import.bytes, &import.options, None).map_err(|error| import_error_text(&error))?; + let first_line = first_data_line(import); sheet .rows .iter() @@ -294,7 +459,8 @@ fn import_rows(import: &CsvImport) -> Result>, String> { #[cfg(test)] mod tests { use tablepro_core::column::{ColumnDefault, ColumnKind, ColumnType, TextLength}; - use tablepro_core::{LivenessPolicy, Value}; + use tablepro_core::ddl::{ColumnAttributes, ColumnDefinition, IdentityStyle, TableDefinition}; + use tablepro_core::{ColumnInfo, LivenessPolicy, Value, WriteReport}; use tablepro_session::fakes::{FakeConnector, FakeCredentialSource}; use tablepro_session::operation::DefaultDeadlines; use tablepro_session::{SessionDeps, SessionHealth}; @@ -302,7 +468,7 @@ mod tests { use super::*; - fn column(name: &str) -> ColumnInfo { + fn info(name: &str) -> ColumnInfo { let mut column_type = ColumnType::untyped_text("text"); column_type.kind = ColumnKind::Text { length: TextLength::Unbounded, @@ -319,6 +485,23 @@ mod tests { } } + fn column(name: &str) -> ImportColumn { + ImportColumn { + info: info(name), + server_filled: false, + } + } + + fn definition(name: &str, identity: IdentityStyle) -> ColumnDefinition { + ColumnDefinition::new( + info(name), + ColumnAttributes { + identity, + ..ColumnAttributes::default() + }, + ) + } + fn session(script: ConnectionScript) -> (Arc, Arc) { let (pending, connection) = FakeConnector::pending_session(FakeConnector::request().unwrap(), script); let session = Session::start( @@ -348,9 +531,18 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn csv_import_reads_columns_through_session() { let mut script = ConnectionScript::default(); - script - .fetch_columns - .always(FakeReply::ok(vec![column("name"), column("city")])); + script.fetch_table_definition.always(FakeReply::ok(TableDefinition { + table: TableRef::unqualified("people"), + columns: vec![ + definition("id", IdentityStyle::MssqlIdentity), + definition("name", IdentityStyle::None), + ], + primary_key: None, + indexes: Vec::new(), + foreign_keys: Vec::new(), + inbound_foreign_keys: Vec::new(), + source: None, + })); let (session, connection) = session(script); let columns = crate::test_support::paused_tasks() @@ -359,35 +551,116 @@ mod tests { .unwrap() .unwrap(); - assert_eq!(columns.len(), 2); - assert_eq!(CallCounters::get(&connection.counters().fetch_columns), 1); + assert_eq!( + columns.iter().map(|column| column.server_filled).collect::>(), + vec![true, false], + "an identity that refuses a value was offered to the file" + ); + assert_eq!(CallCounters::get(&connection.counters().fetch_table_definition), 1); assert_eq!(*session.health().borrow(), SessionHealth::Online); } + fn run(session: Arc, import: CsvImport, cancel: CancellationToken) -> ImportRun { + ImportRun { + session, + dialect: Arc::new(tablepro_test_support::FakeDialect), + import, + cancel, + tasks: crate::test_support::paused_tasks(), + history: None, + connection: None, + } + } + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn an_import_is_one_batch_with_an_insert_per_row() { + async fn an_import_writes_its_rows_as_one_multi_row_statement() { let mut script = ConnectionScript::default(); script .write_batch - .always(FakeReply::ok(WriteReport::new(vec![Some(1), Some(1)]))); + .always(FakeReply::ok(WriteReport::new(vec![Some(2)]))); let (session, connection) = session(script); - let import = import("name,city\nAda,London\nGrace,\n"); - let rows = import_rows(&import).unwrap(); + let started = tokio::time::Instant::now(); - let (sql, report) = write_import(&session, TableRef::unqualified("people"), import.columns, rows) - .await - .unwrap(); + let rows = run_import(run( + session, + import("name,city\nAda,London\nGrace,\n"), + CancellationToken::new(), + )) + .await + .unwrap(); - assert_eq!(sql.len(), 2); - assert!(sql.iter().all(|statement| statement.starts_with("INSERT")), "{sql:?}"); - assert_eq!(report.total_rows(), Some(2)); + assert_eq!(rows, 2); assert_eq!(CallCounters::get(&connection.counters().write_batch), 1); + let sql = connection.observed_sql("write_batch"); + assert!( + sql.iter() + .flatten() + .any(|sql| sql.contains("VALUES (") && sql.contains("), (")), + "the rows went one statement each: {sql:?}" + ); + let deadline = connection.observed_deadlines("write_batch")[0].expect("a deadline"); + assert!( + deadline >= started + DefaultDeadlines::ROW_CHANGES + IMPORT_TIME_PER_STATEMENT, + "the deadline did not grow with the import" + ); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn a_stopped_import_says_it_stopped() { + let mut script = ConnectionScript::default(); + script.write_batch.always(FakeReply::UntilStopped); + let (session, connection) = session(script); + let cancel = CancellationToken::new(); + let stop = cancel.clone(); + + let running = crate::test_support::paused_tasks().spawn_task(run_import(run( + session, + import("name,city\nAda,London\n"), + cancel, + ))); + while CallCounters::get(&connection.counters().write_batch) == 0 { + tokio::task::yield_now().await; + } + stop.cancel(); + + assert!(matches!(running.await.unwrap(), Err(ImportEnd::Stopped))); + } + + #[test] + fn a_failed_statement_is_told_as_the_lines_it_held() { + let steps = vec![0..1_000, 1_000..1_500]; + let rolled_back = OperationError::Driver(DriverError::RolledBack { + statement_index: 1, + source: Box::new(DriverError::server("null value in column \"city\"")), + }); + let partial = OperationError::Driver(DriverError::PartiallyApplied { + applied: 1, + statement_index: 1, + failed_outcome: tablepro_core::StatementOutcome::Failed, + source: Box::new(DriverError::server("null value in column \"city\"")), + }); + + let rolled_back = import_failure_text(&rolled_back, &steps, 2); + let partial = import_failure_text(&partial, &steps, 2); + + assert!( + rolled_back.starts_with("Lines 1002 to 1501 could not be written: "), + "{rolled_back}" + ); + assert!( + rolled_back.ends_with("The import was rolled back, so no rows were added."), + "{rolled_back}" + ); + assert!( + partial.contains("so the 1000 rows before them stay, and some of these lines may"), + "{partial}" + ); } #[test] fn a_bad_row_names_its_line_in_the_file() { let mut bad = import("name,city\nAda,London\n"); - bad.columns[1].column_type.kind = ColumnKind::Integer(tablepro_core::column::IntegerKind { + bad.columns[1].info.column_type.kind = ColumnKind::Integer(tablepro_core::column::IntegerKind { width: tablepro_core::column::IntegerWidth::W32, signed: true, }); diff --git a/linux/crates/app/src/ui/workspace/import_ledger.rs b/linux/crates/app/src/ui/workspace/import_ledger.rs new file mode 100644 index 0000000000..0c5fb8d009 --- /dev/null +++ b/linux/crates/app/src/ui/workspace/import_ledger.rs @@ -0,0 +1,263 @@ +use std::cell::RefCell; +use std::collections::BTreeMap; +use std::future::Future; +use std::rc::Rc; + +use futures::channel::oneshot; +use futures::future::LocalBoxFuture; +use tablepro_session::runtime::{HoldToken, ProcessHold}; +use tokio_util::sync::CancellationToken; + +use crate::i18n::{gettext, ngettext_f}; +use crate::ui::format::plural_count; +use crate::workspace::{CloseBlockPrompt, CloseBlocker, InhibitReason, WindowInhibitor}; + +/// The CSV imports a workspace has running. +/// +/// Each one holds the process, so quitting while one writes lets it +/// commit or roll back and report, and closing the window asks before +/// it stops one. While any runs, the session is asked not to log out or +/// suspend under it. +pub(crate) struct ImportLedger { + hold: Rc, + inhibitor: RefCell, + state: RefCell, +} + +#[derive(Default)] +struct LedgerState { + next: u64, + running: BTreeMap, + waiters: Vec>, + /// The workspace shut down: its close already asked about the + /// imports, and a new one would have no window to report to. + shut_down: bool, +} + +struct Running { + cancel: CancellationToken, + _hold: HoldToken, +} + +/// A started import: its id to settle it by, and the token that stops +/// it. +#[derive(Debug, Clone)] +pub(crate) struct ImportTicket { + pub id: u64, + pub cancel: CancellationToken, +} + +impl ImportLedger { + pub(crate) fn new(hold: Rc, inhibitor: WindowInhibitor) -> Rc { + Rc::new(Self { + hold, + inhibitor: RefCell::new(inhibitor), + state: RefCell::default(), + }) + } + + /// Record an import, unless the workspace has shut down. + pub(crate) fn begin(&self) -> Option { + let ticket = { + let mut state = self.state.borrow_mut(); + if state.shut_down { + return None; + } + state.next += 1; + let ticket = ImportTicket { + id: state.next, + cancel: CancellationToken::new(), + }; + state.running.insert( + ticket.id, + Running { + cancel: ticket.cancel.clone(), + _hold: self.hold.acquire(), + }, + ); + ticket + }; + self.refresh_inhibitor(); + Some(ticket) + } + + /// The import ended. A second call for the same one does nothing. + pub(crate) fn settle(&self, id: u64) { + let (finished, woken) = { + let mut state = self.state.borrow_mut(); + let finished = state.running.remove(&id); + let woken = match state.running.is_empty() { + true => std::mem::take(&mut state.waiters), + false => Vec::new(), + }; + (finished, woken) + }; + self.refresh_inhibitor(); + // The hold goes only after the state is consistent, in case + // dropping it ends the main loop. + drop(finished); + for waiter in woken { + let _ = waiter.send(()); + } + } + + pub(crate) fn in_flight(&self) -> usize { + self.state.borrow().running.len() + } + + /// Resolves once no import is running. + pub(crate) fn when_idle(&self) -> impl Future + use<> { + let receiver = match self.in_flight() { + 0 => None, + _ => { + let (sender, receiver) = oneshot::channel(); + self.state.borrow_mut().waiters.push(sender); + Some(receiver) + } + }; + async move { + if let Some(receiver) = receiver { + // A ledger dropped with imports running wakes nobody, + // and a waiter that outlived it has nothing to wait on. + let _ = receiver.await; + } + } + } + + /// Stop every running import. Each settles once its driver has + /// answered the stop. + pub(crate) fn cancel_all(&self) { + for running in self.state.borrow().running.values() { + running.cancel.cancel(); + } + } + + /// Stop every running import and refuse new ones, for a workspace + /// that is going away. + pub(crate) fn shut_down(&self) { + self.state.borrow_mut().shut_down = true; + self.cancel_all(); + } + + fn refresh_inhibitor(&self) { + let reason = (self.in_flight() > 0).then_some(InhibitReason::SavingChanges); + self.inhibitor.borrow_mut().update(reason); + } +} + +impl CloseBlocker for ImportLedger { + fn blocking(&self) -> Option { + let running = self.in_flight(); + (running > 0).then(|| CloseBlockPrompt { + heading: gettext("Stop Imports?"), + body: ngettext_f( + "{n} CSV import is still running. Stopping it rolls back the rows it wrote, unless the table has no transactions.", + "{n} CSV imports are still running. Stopping them rolls back the rows they wrote, unless the table has no transactions.", + plural_count(running as u64), + &[("n", &running.to_string())], + ), + keep_label: gettext("_Keep Importing"), + stop_label: gettext("_Stop Imports"), + }) + } + + fn stop(&self) -> LocalBoxFuture<'static, ()> { + self.cancel_all(); + Box::pin(self.when_idle()) + } +} + +impl std::fmt::Debug for ImportLedger { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ImportLedger") + .field("in_flight", &self.in_flight()) + .finish_non_exhaustive() + } +} + +#[cfg(test)] +mod tests { + use std::cell::Cell; + + use gtk4 as gtk; + use tablepro_session::runtime::CountingHold; + + use super::*; + use crate::workspace::InhibitTarget; + + /// Counts the inhibitions held. + #[derive(Clone, Default)] + struct Session { + held: Rc>, + } + + impl InhibitTarget for Session { + fn inhibit(&self, _flags: gtk::ApplicationInhibitFlags, _reason: &str) -> u32 { + self.held.set(self.held.get() + 1); + self.held.get() + } + + fn uninhibit(&self, _cookie: u32) { + self.held.set(self.held.get() - 1); + } + } + + fn ledger() -> (Rc, Rc, Session) { + let hold = Rc::new(CountingHold::new()); + let session = Session::default(); + let ledger = ImportLedger::new(hold.clone(), WindowInhibitor::new(session.clone())); + (ledger, hold, session) + } + + #[test] + fn a_running_import_holds_the_process_and_asks_before_a_close() { + let (ledger, hold, session) = ledger(); + assert_eq!(ledger.blocking(), None); + + let ticket = ledger.begin().expect("an import starts"); + let prompt = ledger.blocking().expect("a running import blocks the close"); + + assert_eq!(prompt.heading, "Stop Imports?"); + assert!( + prompt.body.starts_with("1 CSV import is still running."), + "{}", + prompt.body + ); + assert_eq!(hold.live(), 1); + assert_eq!(session.held.get(), 1); + + ledger.settle(ticket.id); + + assert_eq!(ledger.blocking(), None); + assert_eq!(hold.live(), 0); + assert_eq!(session.held.get(), 0); + } + + #[tokio::test] + async fn stop_cancels_the_import_and_waits_for_it_to_settle() { + let (ledger, _, _) = ledger(); + let ticket = ledger.begin().expect("an import starts"); + + let mut stopped = ledger.stop(); + assert!(ticket.cancel.is_cancelled(), "the stop did not reach the write"); + assert!( + futures::poll!(&mut stopped).is_pending(), + "the stop ended before the import settled" + ); + ledger.settle(ticket.id); + + stopped.await; + assert_eq!(ledger.in_flight(), 0); + } + + #[test] + fn a_workspace_that_shut_down_starts_no_import() { + let (ledger, _, _) = ledger(); + let running = ledger.begin().expect("an import starts"); + + ledger.shut_down(); + + assert!(running.cancel.is_cancelled()); + assert!(ledger.begin().is_none()); + } +} diff --git a/linux/crates/app/src/ui/workspace/mod.rs b/linux/crates/app/src/ui/workspace/mod.rs index f39644dee9..ff044cec38 100644 --- a/linux/crates/app/src/ui/workspace/mod.rs +++ b/linux/crates/app/src/ui/workspace/mod.rs @@ -10,6 +10,7 @@ mod finished_work; mod health_banner; mod imp; mod import; +mod import_ledger; mod row_ops; mod saves; mod session_watches; @@ -149,6 +150,8 @@ pub(crate) struct WorkspaceModel { close_guards: Rc, /// The exports running from this workspace's grids. exports: Rc, + /// The CSV imports running into this workspace's tables. + imports: Rc, /// Whether the window was last told there is something to lose. reported_dirty: Cell>, history_dialog: Option, @@ -401,7 +404,7 @@ pub(crate) enum WorkspaceInput { ImportCsvReady { schema: Option, table: String, - columns: Vec, + columns: Vec, bytes: Vec, }, ImportCsvRun(Box), @@ -501,6 +504,10 @@ impl Workspace { ); let persistence = workspace_persistence::WorkspacePersistence::new(storage.workspaces().clone()); let exports = workspace.export_ledger(&window, &tasks, &context, &input); + let imports = import_ledger::ImportLedger::new( + context.hold.clone(), + WindowInhibitor::new(ApplicationInhibit::new(&window)), + ); let sidebar = sidebar::build(&workspace, &input, sidebar::SidebarAffordances::for_session(&session)); let WorkspaceTabParts { tab_view, @@ -560,6 +567,7 @@ impl Workspace { workspace_handle, close_guards, exports: exports.clone(), + imports: imports.clone(), reported_dirty: Cell::new(None), history_dialog: None, saved_queries_dialog: None, @@ -573,6 +581,7 @@ impl Workspace { }; imp.model.replace(Some(model)); workspace.register_close_blocker(exports); + workspace.register_close_blocker(imports); workspace } @@ -897,6 +906,7 @@ impl WorkspaceModel { // The close asked about them already; one started since has no // window left to finish in. self.exports.shut_down(); + self.imports.shut_down(); self.context.active.take(); } diff --git a/linux/crates/core/src/ddl/identity_style.rs b/linux/crates/core/src/ddl/identity_style.rs index 9407cc1a6d..50f8d72ae3 100644 --- a/linux/crates/core/src/ddl/identity_style.rs +++ b/linux/crates/core/src/ddl/identity_style.rs @@ -22,3 +22,32 @@ pub enum IdentityStyle { /// SQL Server `IDENTITY(seed, increment)`. MssqlIdentity, } + +impl IdentityStyle { + /// Whether a plain INSERT may give the column a value of its own. + /// GENERATED ALWAYS needs `OVERRIDING SYSTEM VALUE` and SQL Server + /// IDENTITY needs `IDENTITY_INSERT`, so both refuse one. + pub fn accepts_explicit_value(self) -> bool { + !matches!(self, Self::GeneratedAlways | Self::MssqlIdentity) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn only_the_identities_that_need_an_override_refuse_a_value() { + assert!(!IdentityStyle::GeneratedAlways.accepts_explicit_value()); + assert!(!IdentityStyle::MssqlIdentity.accepts_explicit_value()); + for style in [ + IdentityStyle::None, + IdentityStyle::GeneratedByDefault, + IdentityStyle::SequenceDefault, + IdentityStyle::AutoIncrementKeyword, + IdentityStyle::SqliteRowidAlias, + ] { + assert!(style.accepts_explicit_value(), "{style:?}"); + } + } +} diff --git a/linux/crates/core/src/dialect/insert_batch_limits.rs b/linux/crates/core/src/dialect/insert_batch_limits.rs new file mode 100644 index 0000000000..046417e1a2 --- /dev/null +++ b/linux/crates/core/src/dialect/insert_batch_limits.rs @@ -0,0 +1,24 @@ +/// How much one multi-row INSERT may carry on an engine. A batch of rows +/// is cut into statements that each stay within all three. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct InsertBatchLimits { + /// Rows in one VALUES list. + pub rows: usize, + /// Parameters bound to one statement. + pub params: usize, + /// The values one statement carries, measured as their text, so a + /// statement stays well inside the engine's packet or request size. + pub value_bytes: usize, +} + +impl InsertBatchLimits { + /// Within what PostgreSQL (65,535 parameters), MySQL (65,535 + /// placeholders, a 4 MiB packet on older servers) and SQLite (32,766 + /// variables) accept. A thousand rows already make one round trip + /// a small share of the time a statement takes. + pub const DEFAULT: Self = Self { + rows: 1_000, + params: 30_000, + value_bytes: 1 << 20, + }; +} diff --git a/linux/crates/core/src/dialect/mod.rs b/linux/crates/core/src/dialect/mod.rs index e7811dea48..6cb321fe0f 100644 --- a/linux/crates/core/src/dialect/mod.rs +++ b/linux/crates/core/src/dialect/mod.rs @@ -4,6 +4,7 @@ mod bind_error; mod bind_target; mod dialect_capabilities; +mod insert_batch_limits; mod keyset_direction; mod like_form; mod literal_error; @@ -15,6 +16,7 @@ mod test_dialect; pub use bind_error::BindError; pub use bind_target::BindTarget; pub use dialect_capabilities::DialectCapabilities; +pub use insert_batch_limits::InsertBatchLimits; pub use keyset_direction::KeysetDirection; pub use like_form::{LikeCase, LikeForm}; pub use literal_error::LiteralError; diff --git a/linux/crates/core/src/dialect/sql_dialect.rs b/linux/crates/core/src/dialect/sql_dialect.rs index 1bfb37ccdc..61d0d6e4b2 100644 --- a/linux/crates/core/src/dialect/sql_dialect.rs +++ b/linux/crates/core/src/dialect/sql_dialect.rs @@ -5,7 +5,8 @@ use crate::sql_syntax::{ExpressionSyntaxError, HistoryRecording, SqlGrammar, Typ use crate::value::Value; use super::{ - BindError, BindTarget, DialectCapabilities, KeysetDirection, LikeCase, LikeForm, LiteralError, Placeholder, + BindError, BindTarget, DialectCapabilities, InsertBatchLimits, KeysetDirection, LikeCase, LikeForm, LiteralError, + Placeholder, }; /// How one engine spells SQL. @@ -148,6 +149,11 @@ pub trait SqlDialect: Send + Sync + std::fmt::Debug { format!("DELETE FROM {table_sql} WHERE {predicate_sql}") } + /// How much one multi-row INSERT may carry. + fn insert_batch_limits(&self) -> InsertBatchLimits { + InsertBatchLimits::DEFAULT + } + /// Insert a row of nothing but defaults, where the engine has a /// spelling for it. fn insert_default_row(&self, table_sql: &str) -> Option { diff --git a/linux/crates/core/src/dialect/test_dialect.rs b/linux/crates/core/src/dialect/test_dialect.rs index 3044c014e2..348df538cd 100644 --- a/linux/crates/core/src/dialect/test_dialect.rs +++ b/linux/crates/core/src/dialect/test_dialect.rs @@ -9,7 +9,10 @@ use crate::sql_syntax::{ use crate::statement::{BoundParam, ParamType}; use crate::value::Value; -use super::{BindError, BindTarget, DialectCapabilities, KeysetDirection, LiteralError, Placeholder, SqlDialect}; +use super::{ + BindError, BindTarget, DialectCapabilities, InsertBatchLimits, KeysetDirection, LiteralError, Placeholder, + SqlDialect, +}; /// The one ANSI dialect the core builders' own tests share. /// @@ -25,6 +28,7 @@ pub struct TestDialect { default_row: bool, count_all: &'static str, grammar: SqlGrammar, + insert_limits: InsertBatchLimits, binds: Mutex>, } @@ -62,6 +66,7 @@ impl Default for TestDialect { default_row: true, count_all: "COUNT(*)", grammar: SqlGrammar::PostgreSql, + insert_limits: InsertBatchLimits::DEFAULT, binds: Mutex::new(Vec::new()), } } @@ -102,6 +107,10 @@ impl TestDialect { Self { grammar, ..self } } + pub fn with_insert_limits(self, insert_limits: InsertBatchLimits) -> Self { + Self { insert_limits, ..self } + } + pub fn targets(&self) -> Vec { self.recorded().into_iter().map(|bind| bind.target).collect() } @@ -239,6 +248,10 @@ impl SqlDialect for TestDialect { self.count_all } + fn insert_batch_limits(&self) -> InsertBatchLimits { + self.insert_limits + } + fn insert_default_row(&self, table_sql: &str) -> Option { self.default_row .then(|| format!("INSERT INTO {table_sql} DEFAULT VALUES")) diff --git a/linux/crates/core/src/dml/build_insert.rs b/linux/crates/core/src/dml/build_insert.rs index 50ab06c608..a3124de0dc 100644 --- a/linux/crates/core/src/dml/build_insert.rs +++ b/linux/crates/core/src/dml/build_insert.rs @@ -3,14 +3,18 @@ use crate::dialect::{BindTarget, SqlDialect}; use crate::edit::CellInput; use crate::meta::TableRef; use crate::statement::{Statement, StatementBuilder}; +use crate::value::Value; use super::BuildSqlError; -/// The INSERT for a draft row. +/// The INSERT for one row. /// -/// A cell the user left alone is left out of the statement, so the -/// server applies its own default. A generated or auto-increment -/// column is never written, because the server owns its value. +/// A `Default` cell is left out of the statement, so the server applies +/// its own default or next number. A generated column is never written, +/// because the server computes it. Every other value is written, an +/// auto-increment column's included: a value given for one is a key the +/// caller means to keep, and the caller leaves out the identities an +/// engine refuses a value for. pub fn build_insert( dialect: &dyn SqlDialect, table: &TableRef, @@ -32,7 +36,7 @@ pub fn build_insert( let Some(value) = cell.value() else { continue; }; - if column.is_generated || column.is_auto_increment { + if column.is_generated { continue; } markers.push(builder.bind(dialect, value.clone(), BindTarget::Column(column))?); @@ -56,3 +60,45 @@ pub fn build_insert( )); Ok(builder.finish()) } + +/// One INSERT of several rows into the same columns, each row holding +/// one value per column, in column order. +/// +/// One statement takes one round trip for every row in it, and +/// ClickHouse makes one part of it rather than one per row. +pub fn build_multi_row_insert( + dialect: &dyn SqlDialect, + table: &TableRef, + columns: &[&ColumnInfo], + rows: Vec>, +) -> Result { + if columns.is_empty() || rows.is_empty() { + return Err(BuildSqlError::NoInsertValues); + } + let mut builder = StatementBuilder::new(); + let mut tuples = Vec::with_capacity(rows.len()); + for row in rows { + if row.len() != columns.len() { + return Err(BuildSqlError::LengthMismatch { + expected: columns.len(), + found: row.len(), + }); + } + let mut markers = Vec::with_capacity(columns.len()); + for (&column, value) in columns.iter().zip(row) { + markers.push(builder.bind(dialect, value, BindTarget::Column(column))?); + } + tuples.push(format!("({})", markers.join(", "))); + } + let names: Vec = columns + .iter() + .map(|column| dialect.quote_identifier(&column.name)) + .collect(); + builder.push_sql(&format!( + "INSERT INTO {} ({}) VALUES {}", + dialect.qualify(table), + names.join(", "), + tuples.join(", ") + )); + Ok(builder.finish()) +} diff --git a/linux/crates/core/src/dml/mod.rs b/linux/crates/core/src/dml/mod.rs index df340d9bb6..c833ec9cb9 100644 --- a/linux/crates/core/src/dml/mod.rs +++ b/linux/crates/core/src/dml/mod.rs @@ -15,7 +15,7 @@ mod predicate; mod tests; pub use build_delete::build_delete; -pub use build_insert::build_insert; +pub use build_insert::{build_insert, build_multi_row_insert}; pub use build_key_probe::build_key_probe; pub use build_sql_error::BuildSqlError; pub use build_update::build_update; diff --git a/linux/crates/core/src/dml/tests.rs b/linux/crates/core/src/dml/tests.rs index d2d2d9c5d7..93eab785f4 100644 --- a/linux/crates/core/src/dml/tests.rs +++ b/linux/crates/core/src/dml/tests.rs @@ -189,10 +189,9 @@ fn build_key_probe_stops_at_two_rows() { } #[test] -fn build_insert_omits_default_cells_and_generated_columns() { +fn build_insert_omits_generated_columns() { let dialect = TestDialect::exact(); let mut columns = columns(); - columns[0].is_auto_increment = true; columns[2].is_generated = true; let statement = build_insert( @@ -209,10 +208,48 @@ fn build_insert_omits_default_cells_and_generated_columns() { assert_eq!( statement.sql(), - r#"INSERT INTO "public"."users" ("name") VALUES ($1)"#, - "a column the server owns was written" + r#"INSERT INTO "public"."users" ("id", "name") VALUES ($1, $2)"#, + "a generated column was written" ); - assert_eq!(statement.params().len(), 1); + assert_eq!(statement.params().len(), 2); +} + +#[test] +fn build_insert_writes_the_value_given_for_an_auto_increment_column() { + let dialect = TestDialect::exact(); + let mut columns = columns(); + columns[0].is_auto_increment = true; + + let given = build_insert( + &dialect, + &table(), + &columns, + &[ + CellInput::Value(Value::Int(100)), + CellInput::Value(Value::Text("a".to_owned())), + CellInput::Default, + ], + ) + .expect("the insert"); + let left = build_insert( + &dialect, + &table(), + &columns, + &[ + CellInput::Default, + CellInput::Value(Value::Text("a".to_owned())), + CellInput::Default, + ], + ) + .expect("the insert"); + + assert_eq!( + given.sql(), + r#"INSERT INTO "public"."users" ("id", "name") VALUES ($1, $2)"#, + "the id the row was given was dropped" + ); + assert_eq!(given.params()[0].value(), &Value::Int(100)); + assert_eq!(left.sql(), r#"INSERT INTO "public"."users" ("name") VALUES ($1)"#); } #[test] diff --git a/linux/crates/core/src/export/encoder/encode_error.rs b/linux/crates/core/src/export/encoder/encode_error.rs index 4a7c6c2eef..51a08d790f 100644 --- a/linux/crates/core/src/export/encoder/encode_error.rs +++ b/linux/crates/core/src/export/encoder/encode_error.rs @@ -6,4 +6,9 @@ use thiserror::Error; pub enum EncodeError { #[error("column {column} has an undecodable value")] Undecodable { column: String }, + + /// A control character XML 1.0 has no representation for, not even + /// as a character reference. + #[error("column {column} holds a character XML cannot carry")] + NotXmlText { column: String }, } diff --git a/linux/crates/core/src/export/encoder/mod.rs b/linux/crates/core/src/export/encoder/mod.rs index a45a7683cb..830910ea51 100644 --- a/linux/crates/core/src/export/encoder/mod.rs +++ b/linux/crates/core/src/export/encoder/mod.rs @@ -56,7 +56,7 @@ fn readable_cells<'a>( } /// A column's name, or its position for a cell past the header. -fn column_name(columns: &[ResultColumn], index: usize) -> String { +pub(in crate::export) fn column_name(columns: &[ResultColumn], index: usize) -> String { columns .get(index) .map_or_else(|| (index + 1).to_string(), |column| column.name.clone()) diff --git a/linux/crates/core/src/export/encoder/sql_insert_encoder.rs b/linux/crates/core/src/export/encoder/sql_insert_encoder.rs index f0af1dbf1e..dc81b77327 100644 --- a/linux/crates/core/src/export/encoder/sql_insert_encoder.rs +++ b/linux/crates/core/src/export/encoder/sql_insert_encoder.rs @@ -2,26 +2,35 @@ use std::sync::Arc; use crate::column::ResultColumn; use crate::dialect::SqlDialect; +use crate::meta::TableRef; use crate::value::Value; -use super::super::sql::{insert_row, insert_start}; +use super::super::sql::{InsertShape, insert_row, insert_shape}; use super::{EncodeError, RowEncoder, readable_cells}; /// The statements `render_sql_insert` writes, a row at a time. pub struct SqlInsertEncoder { dialect: Arc, columns: Arc<[ResultColumn]>, - /// `None` for a result with no columns, which writes nothing. - start: Option, + /// `None` for a result with no column to write, which writes + /// nothing. + shape: Option, } impl SqlInsertEncoder { - pub fn new(dialect: Arc, table: &str, columns: Arc<[ResultColumn]>) -> Self { - let start = insert_start(&*dialect, table, &columns); + /// `generated` names the columns the server computes, which every + /// statement leaves out. + pub fn new( + dialect: Arc, + table: &TableRef, + columns: Arc<[ResultColumn]>, + generated: &[String], + ) -> Self { + let shape = insert_shape(&*dialect, table, &columns, generated); Self { dialect, columns, - start, + shape, } } } @@ -31,11 +40,11 @@ impl RowEncoder for SqlInsertEncoder { fn row(&mut self, out: &mut Vec, cells: &mut dyn Iterator) -> Result<(), EncodeError> { let cells = readable_cells(&self.columns, cells)?; - let Some(start) = &self.start else { + let Some(shape) = &self.shape else { return Ok(()); }; let mut row = String::new(); - insert_row(&mut row, &*self.dialect, start, &self.columns, cells); + insert_row(&mut row, &*self.dialect, shape, &self.columns, cells); out.extend_from_slice(row.as_bytes()); Ok(()) } diff --git a/linux/crates/core/src/export/encoder/tests.rs b/linux/crates/core/src/export/encoder/tests.rs index c9ae68794b..dfc8aebde4 100644 --- a/linux/crates/core/src/export/encoder/tests.rs +++ b/linux/crates/core/src/export/encoder/tests.rs @@ -1,6 +1,6 @@ use std::sync::Arc; -use super::super::csv::{csv_cell, csv_header, csv_writer_builder, neutralise_formula}; +use super::super::csv::{csv_cell, csv_header, csv_writer_builder}; use super::super::csv_options::{CsvDecimal, CsvDelimiter, CsvLineBreak, CsvOptions, CsvQuote}; use super::super::test_columns::{cols, typed}; use super::super::tsv::tsv_writer_builder; @@ -11,6 +11,7 @@ use super::super::{ use super::*; use crate::column::{ColumnKind, IntegerKind, IntegerWidth}; use crate::dialect::{SqlDialect, TestDialect}; +use crate::meta::TableRef; use crate::value::{JsonText, UndecodableReason, UndecodedValue}; /// Every call's output taken away as soon as it is written, the way a @@ -166,18 +167,13 @@ fn tsv_encoder_matches_render_tsv() { let mut writer = tsv_writer_builder().from_writer(Vec::new()); if with_headers { writer - .write_record( - columns - .iter() - .map(|column| neutralise_formula(&Value::Null, column.name.clone(), true)), - ) + .write_record(columns.iter().map(|column| column.name.clone())) .expect("the header"); } for row in &rows { writer .write_record(row.iter().enumerate().map(|(index, value)| { - value_text(value, column_type(&columns, index)) - .map_or_else(|| "NULL".to_owned(), |text| neutralise_formula(value, text, false)) + value_text(value, column_type(&columns, index)).unwrap_or_else(|| "NULL".to_owned()) })) .expect("the row"); } @@ -229,7 +225,7 @@ fn json_encoder_output_parses_and_matches_render_json() { // rows before the encoders. let expected: Vec = rows .iter() - .map(|row| serde_json::from_str(&row_to_json(&columns, row)).expect("a JSON row")) + .map(|row| serde_json::from_str(&row_to_json(&columns, row).expect("a readable row")).expect("a JSON row")) .collect(); assert_eq!(parsed, expected); assert_eq!(parsed.len(), 10_000); @@ -288,11 +284,14 @@ fn document_encoders_match_their_renderers() { ); assert_eq!( streamed(XmlEncoder::new(columns.clone()), &rows), - render_xml(&columns, &rows) + render_xml(&columns, &rows).expect("the document") ); assert_eq!( - streamed(SqlInsertEncoder::new(dialect.clone(), "t", columns.clone()), &rows), - render_sql_insert(&*dialect, "t", &columns, &rows) + streamed( + SqlInsertEncoder::new(dialect.clone(), &TableRef::unqualified("t"), columns.clone(), &[]), + &rows + ), + render_sql_insert(&*dialect, &TableRef::unqualified("t"), &columns, &[], &rows) ); } @@ -307,11 +306,18 @@ fn document_encoders_write_whole_documents_for_no_rows() { ); assert_eq!( streamed(XmlEncoder::new(columns.clone()), &[]), - render_xml(&columns, &[]) + render_xml(&columns, &[]).expect("the document") + ); + let table = TableRef::unqualified("t"); + assert_eq!( + streamed(SqlInsertEncoder::new(dialect.clone(), &table, columns, &[]), &[]), + "" ); - assert_eq!(streamed(SqlInsertEncoder::new(dialect.clone(), "t", columns), &[]), ""); assert_eq!( - streamed(SqlInsertEncoder::new(dialect, "t", Vec::new().into()), &[Vec::new()]), + streamed( + SqlInsertEncoder::new(dialect, &table, Vec::new().into(), &[]), + &[Vec::new()] + ), "", "a result with no columns has no statement" ); @@ -333,8 +339,9 @@ fn encoder_reports_undecodable_cell() { Box::new(XmlEncoder::new(columns.clone())), Box::new(SqlInsertEncoder::new( Arc::new(TestDialect::default()), - "t", + &TableRef::unqualified("t"), columns.clone(), + &[], )), ]; for mut encoder in encoders { diff --git a/linux/crates/core/src/export/encoder/tsv_encoder.rs b/linux/crates/core/src/export/encoder/tsv_encoder.rs index deb9f9cdd3..f4f2d66b69 100644 --- a/linux/crates/core/src/export/encoder/tsv_encoder.rs +++ b/linux/crates/core/src/export/encoder/tsv_encoder.rs @@ -4,14 +4,14 @@ use crate::column::ResultColumn; use crate::value::Value; use super::super::column_type; -use super::super::csv::neutralise_formula; use super::super::tsv::tsv_writer_builder; use super::super::value_text::value_text; use super::{EncodeError, RecordWriter, RowEncoder, readable_cells}; -/// Tab-separated values, the form a spreadsheet pastes into cells. A -/// paste reads a cell that starts like a formula as one, so free text -/// gets the apostrophe a CSV export gives it. +/// Tab-separated values, the form a spreadsheet pastes into cells. It +/// is what the clipboard carries, so each value goes as the user sees +/// it: a leading apostrophe on `-` or `@alice` would be pasted into +/// every other place the text goes too. pub struct TsvEncoder { columns: Arc<[ResultColumn]>, with_headers: bool, @@ -32,12 +32,8 @@ impl TsvEncoder { impl RowEncoder for TsvEncoder { fn begin(&mut self, out: &mut Vec) { if self.with_headers { - self.writer.write( - out, - self.columns - .iter() - .map(|column| neutralise_formula(&Value::Null, column.name.clone(), true)), - ); + self.writer + .write(out, self.columns.iter().map(|column| column.name.as_str())); } } @@ -45,12 +41,9 @@ impl RowEncoder for TsvEncoder { let cells = readable_cells(&self.columns, cells)?; self.writer.write( out, - cells.iter().enumerate().map( - |(index, value)| match value_text(value, column_type(&self.columns, index)) { - Some(text) => neutralise_formula(value, text, false), - None => "NULL".to_owned(), - }, - ), + cells.iter().enumerate().map(|(index, value)| { + value_text(value, column_type(&self.columns, index)).unwrap_or_else(|| "NULL".to_owned()) + }), ); Ok(()) } diff --git a/linux/crates/core/src/export/encoder/xml_encoder.rs b/linux/crates/core/src/export/encoder/xml_encoder.rs index 866bfa0093..7be654c35c 100644 --- a/linux/crates/core/src/export/encoder/xml_encoder.rs +++ b/linux/crates/core/src/export/encoder/xml_encoder.rs @@ -25,7 +25,7 @@ impl RowEncoder for XmlEncoder { fn row(&mut self, out: &mut Vec, cells: &mut dyn Iterator) -> Result<(), EncodeError> { let cells = readable_cells(&self.columns, cells)?; let mut row = String::new(); - xml_row(&mut row, &self.columns, cells); + xml_row(&mut row, &self.columns, cells)?; out.extend_from_slice(row.as_bytes()); Ok(()) } diff --git a/linux/crates/core/src/export/json.rs b/linux/crates/core/src/export/json.rs index f2bcb8b417..61e89b7faa 100644 --- a/linux/crates/core/src/export/json.rs +++ b/linux/crates/core/src/export/json.rs @@ -103,15 +103,22 @@ fn number_token(serializer: S, digits: &str) -> Result String { +/// single row is rendered on its own. A value the driver could not read +/// is refused, as every export refuses it, rather than shown as a +/// `null` the row does not hold. +pub fn row_to_json(columns: &[ResultColumn], row: &[Value]) -> Result { + if let Some(index) = row.iter().position(|value| matches!(value, Value::Undecodable(_))) { + return Err(EncodeError::Undecodable { + column: super::encoder::column_name(columns, index), + }); + } let names = json_field_names(columns); let value = JsonRow { names: &names, columns, row, }; - serde_json::to_string_pretty(&value).unwrap_or_else(|_| "{}".to_owned()) + Ok(serde_json::to_string_pretty(&value).unwrap_or_else(|_| "{}".to_owned())) } pub fn render_json(columns: &[ResultColumn], rows: &[Vec]) -> Result { @@ -126,7 +133,27 @@ mod tests { use super::*; fn parsed(columns: &[ResultColumn], row: &[Value]) -> serde_json::Value { - serde_json::from_str(&row_to_json(columns, row)).expect("the row is JSON") + serde_json::from_str(&row_to_json(columns, row).expect("a readable row")).expect("the row is JSON") + } + + #[test] + fn an_unreadable_value_is_refused_rather_than_shown_as_null() { + let row = vec![ + Value::Int(1), + Value::Undecodable(Box::new(crate::value::UndecodedValue { + type_name: "text".to_owned(), + reason: crate::value::UndecodableReason::InvalidEncoding, + })), + ]; + + let refused = row_to_json(&cols(&["id", "body"]), &row); + + assert_eq!( + refused, + Err(EncodeError::Undecodable { + column: "body".to_owned() + }) + ); } #[test] @@ -172,7 +199,7 @@ mod tests { Value::Decimal("12345678901234567890.12345678901234567890".parse().expect("a decimal")), ]; - let text = row_to_json(&columns, &row); + let text = row_to_json(&columns, &row).expect("a readable row"); assert!(text.contains("18446744073709551615"), "{text}"); assert!( @@ -197,7 +224,7 @@ mod tests { )]; let row = vec![Value::Decimal("1".parse().expect("a decimal"))]; - let text = row_to_json(&columns, &row); + let text = row_to_json(&columns, &row).expect("a readable row"); assert!(text.contains("1.0000000000"), "{text}"); } @@ -210,7 +237,7 @@ mod tests { crate::value::JsonText::parse(document.to_owned()).expect("valid json"), )]; - let text = row_to_json(&columns, &row); + let text = row_to_json(&columns, &row).expect("a readable row"); assert!( text.contains(&format!("\"payload\": {document}")), @@ -241,7 +268,7 @@ mod tests { let columns = cols(&["r"]); let row = vec![Value::Float32(0.1)]; - let text = row_to_json(&columns, &row); + let text = row_to_json(&columns, &row).expect("a readable row"); assert!( text.contains("\"r\": 0.1\n"), @@ -266,7 +293,7 @@ mod tests { Value::Null, ])]; - let text = row_to_json(&columns, &row); + let text = row_to_json(&columns, &row).expect("a readable row"); assert!(text.contains("1.50,"), "the item lost its column's scale: {text}"); assert_eq!(parsed(&columns, &row)["amounts"], serde_json::json!([1.5, null])); diff --git a/linux/crates/core/src/export/mod.rs b/linux/crates/core/src/export/mod.rs index 3079cde8aa..97091039c2 100644 --- a/linux/crates/core/src/export/mod.rs +++ b/linux/crates/core/src/export/mod.rs @@ -57,6 +57,14 @@ pub enum RenderError { #[error("a sheet holds {limit} columns, and this result has {got}")] TooManyColumns { got: usize, limit: usize }, + + #[error("column {column} holds a value longer than the {limit} characters a sheet cell holds")] + TextTooLong { column: String, limit: usize }, + + /// A value the file cannot carry, refused as every row encoder + /// refuses it. + #[error(transparent)] + Encode(#[from] EncodeError), } /// The text of one cell, at the precision its column declares. diff --git a/linux/crates/core/src/export/sql.rs b/linux/crates/core/src/export/sql.rs index aa631e2602..ce789a1d73 100644 --- a/linux/crates/core/src/export/sql.rs +++ b/linux/crates/core/src/export/sql.rs @@ -12,62 +12,84 @@ use crate::value::Value; /// middle of one long statement takes every row with it, while a /// failure among many takes one. /// +/// The table is written with its schema, and the columns `generated` +/// names are left out, as Copy As INSERT leaves them out: every engine +/// refuses a value for a column it computes. +/// /// A value the engine has no literal for, such as a blob, is written as /// `NULL` behind a comment saying so. Writing a guess would put the /// wrong data in the table, and refusing the whole export would lose /// every other row over one cell. pub fn render_sql_insert( dialect: &dyn SqlDialect, - table: &str, + table: &TableRef, columns: &[ResultColumn], + generated: &[String], rows: &[Vec], ) -> String { - let Some(start) = insert_start(dialect, table, columns) else { + let Some(shape) = insert_shape(dialect, table, columns, generated) else { return String::new(); }; let mut out = String::with_capacity(rows.len() * columns.len() * 12); for row in rows { - insert_row(&mut out, dialect, &start, columns, row); + insert_row(&mut out, dialect, &shape, columns, row); } out } -/// `INSERT INTO table (columns) VALUES `, which every row's statement -/// starts with. `None` for a result with no columns, which has no -/// statement to write. -pub(super) fn insert_start(dialect: &dyn SqlDialect, table: &str, columns: &[ResultColumn]) -> Option { - if columns.is_empty() { - return None; - } +/// What every row's statement shares: `INSERT INTO table (columns) +/// VALUES `, and which of the result's columns it writes. +pub(super) struct InsertShape { + start: String, + written: Vec, +} + +/// The shape of every row's statement, or `None` for a result with no +/// column to write, which has no statement. +pub(super) fn insert_shape( + dialect: &dyn SqlDialect, + table: &TableRef, + columns: &[ResultColumn], + generated: &[String], +) -> Option { + let written: Vec = columns.iter().map(|column| !generated.contains(&column.name)).collect(); let names: Vec = columns .iter() - .map(|column| dialect.quote_identifier(&column.name)) + .zip(&written) + .filter(|(_, written)| **written) + .map(|(column, _)| dialect.quote_identifier(&column.name)) .collect(); - Some(format!( - "INSERT INTO {} ({}) VALUES ", - dialect.quote_identifier(table), - names.join(", ") - )) + if names.is_empty() { + return None; + } + Some(InsertShape { + start: format!("INSERT INTO {} ({}) VALUES ", dialect.qualify(table), names.join(", ")), + written, + }) } pub(super) fn insert_row<'a>( out: &mut String, dialect: &dyn SqlDialect, - start: &str, + shape: &InsertShape, columns: &[ResultColumn], row: impl IntoIterator, ) { let mut cells = row.into_iter(); let values: Vec = columns .iter() - .map(|column| match cells.next() { - Some(value) => literal(dialect, value, column), - // A row shorter than its header: the cell is absent, which - // is as close to unknown as SQL has. - None => "NULL".to_owned(), + .zip(&shape.written) + .filter_map(|(column, written)| { + let literal = match cells.next() { + Some(value) => literal(dialect, value, column), + // A row shorter than its header: the cell is absent, + // which is as close to unknown as SQL has. + None => "NULL".to_owned(), + }; + written.then_some(literal) }) .collect(); - out.push_str(start); + out.push_str(&shape.start); out.push('('); out.push_str(&values.join(", ")); out.push_str(");\n"); @@ -130,7 +152,39 @@ mod tests { use crate::dialect::{LiteralError, TestDialect}; fn render(table: &str, columns: &[ResultColumn], rows: &[Vec]) -> String { - render_sql_insert(&TestDialect::default(), table, columns, rows) + render_sql_insert( + &TestDialect::default(), + &TableRef::unqualified(table), + columns, + &[], + rows, + ) + } + + #[test] + fn the_table_is_written_with_its_schema() { + let sql = render_sql_insert( + &TestDialect::default(), + &TableRef::new(Some("sales".to_owned()), "orders"), + &cols(&["id"]), + &[], + &[vec![Value::Int(1)]], + ); + + assert_eq!(sql, "INSERT INTO \"sales\".\"orders\" (\"id\") VALUES (1);\n"); + } + + #[test] + fn a_generated_column_is_left_to_the_server() { + let sql = render_sql_insert( + &TestDialect::default(), + &TableRef::unqualified("orders"), + &cols(&["id", "total", "name"]), + &["total".to_owned()], + &[vec![Value::Int(1), Value::Int(9), Value::Text("a".into())]], + ); + + assert_eq!(sql, "INSERT INTO \"orders\" (\"id\", \"name\") VALUES (1, 'a');\n"); } #[test] diff --git a/linux/crates/core/src/export/tsv.rs b/linux/crates/core/src/export/tsv.rs index d20c685163..ef92f68b7c 100644 --- a/linux/crates/core/src/export/tsv.rs +++ b/linux/crates/core/src/export/tsv.rs @@ -51,19 +51,20 @@ mod tests { assert_eq!(render_tsv(&columns, &rows, false).unwrap(), "\"say \"\"hi\"\"\"\n"); } - /// A spreadsheet runs a pasted cell that starts like a formula. #[test] - fn tsv_neutralises_a_formula_in_free_text() { - let columns = cols(&["=sum", "b", "c"]); + fn tsv_keeps_text_that_starts_like_a_formula_as_it_is() { + let columns = cols(&["=sum", "b", "c", "d", "e"]); let rows = vec![vec![ - Value::Text("=cmd|' /C calc'!A0".into()), + Value::Text("-".into()), + Value::Text("+1 555 0100".into()), + Value::Text("@alice".into()), + Value::Text("=1+1".into()), Value::Int(-5), - Value::Text("-5".into()), ]]; assert_eq!( render_tsv(&columns, &rows, true).unwrap(), - "'=sum\tb\tc\n'=cmd|' /C calc'!A0\t-5\t-5\n" + "=sum\tb\tc\td\te\n-\t+1 555 0100\t@alice\t=1+1\t-5\n" ); } diff --git a/linux/crates/core/src/export/value_text.rs b/linux/crates/core/src/export/value_text.rs index d9684088e1..412ceb8677 100644 --- a/linux/crates/core/src/export/value_text.rs +++ b/linux/crates/core/src/export/value_text.rs @@ -1,4 +1,4 @@ -use chrono::Timelike; +use chrono::{Datelike, Timelike}; use crate::column::{ColumnKind, ColumnType}; use crate::value::{SqlDecimal, Value}; @@ -24,19 +24,21 @@ pub(super) fn kind_text(value: &Value, kind: &ColumnKind) -> Option { offset_text(time.offset) )), Value::Timestamp(stamp) => temporal_text(stamp, |stamp| { + let (date, era) = calendar_date(stamp.date()); format!( - "{}{}", - stamp.format("%Y-%m-%d %H:%M:%S"), + "{date} {}{}{era}", + stamp.format("%H:%M:%S"), fraction_text(stamp.nanosecond(), kind.fractional_digits()) ) }), Value::TimestampTz(stamp) => temporal_text(stamp, |stamp| { let stamp = stamp.to_datetime(); + let (date, era) = calendar_date(stamp.date_naive()); format!( - "{}{}{}", - stamp.format("%Y-%m-%d %H:%M:%S"), + "{date} {}{}{}{era}", + stamp.format("%H:%M:%S"), fraction_text(stamp.nanosecond(), kind.fractional_digits()), - stamp.format("%:z") + offset_text(*stamp.offset()) ) }), Value::Decimal(decimal) => Some(decimal_text(decimal, kind.decimal_scale())), @@ -128,15 +130,27 @@ pub fn value_to_text(v: &Value) -> Option { Value::Bytes(b) => Some(format!("0x{}", crate::hex::encode_lower(b))), Value::Uuid(u) => Some(u.to_string()), Value::Json(j) => Some(j.as_str().to_owned()), - Value::Date(d) => temporal_text(d, |date| date.format("%Y-%m-%d").to_string()), + Value::Date(d) => temporal_text(d, |date| { + let (date, era) = calendar_date(*date); + format!("{date}{era}") + }), // The fraction is kept as it came, never rounded to seconds. Value::Time(t) => Some(t.format(None)), Value::TimeTz(t) => Some(format!("{}{}", t.time.format(None), offset_text(t.offset))), - Value::Timestamp(t) => temporal_text(t, |stamp| stamp.format("%Y-%m-%d %H:%M:%S%.f").to_string()), + Value::Timestamp(t) => temporal_text(t, |stamp| { + let (date, era) = calendar_date(stamp.date()); + format!("{date} {}{era}", stamp.format("%H:%M:%S%.f")) + }), // The original offset, not UTC: the row said what zone it was // written in and the export says the same. Value::TimestampTz(t) => temporal_text(t, |stamp| { - stamp.to_datetime().format("%Y-%m-%d %H:%M:%S%.f%:z").to_string() + let stamp = stamp.to_datetime(); + let (date, era) = calendar_date(stamp.date_naive()); + format!( + "{date} {}{}{era}", + stamp.format("%H:%M:%S%.f"), + offset_text(*stamp.offset()) + ) }), Value::Interval(i) => Some(i.to_string()), Value::Bits(b) => Some(b.to_string()), @@ -166,11 +180,28 @@ fn temporal_text(value: &crate::value::Temporal, finite: impl Fn(&T) -> St } } +/// An offset as PostgreSQL prints it, with the seconds a local mean +/// time offset such as `+00:19:32` carries. fn offset_text(offset: chrono::FixedOffset) -> String { let total = offset.local_minus_utc(); let sign = if total < 0 { '-' } else { '+' }; - let minutes = total.abs() / 60; - format!("{sign}{:02}:{:02}", minutes / 60, minutes % 60) + let seconds = total.abs(); + let (hours, minutes, seconds) = (seconds / 3600, seconds / 60 % 60, seconds % 60); + match seconds { + 0 => format!("{sign}{hours:02}:{minutes:02}"), + _ => format!("{sign}{hours:02}:{minutes:02}:{seconds:02}"), + } +} + +/// The date as PostgreSQL prints it, and the era that goes at the very +/// end of the value. ISO year 0 is 1 BC, so it is written as such, and +/// a year past 9999 keeps its digits with no sign. +fn calendar_date(date: chrono::NaiveDate) -> (String, &'static str) { + let (year, era) = match date.year() { + year if year <= 0 => (1 - year, " BC"), + year => (year, ""), + }; + (format!("{year:04}-{:02}-{:02}", date.month(), date.day()), era) } /// An array as JSON array text, which is the one spelling every engine @@ -303,6 +334,68 @@ mod tests { ); } + #[test] + fn a_date_before_year_one_is_written_as_postgresql_writes_it() { + assert_eq!(value_to_text(&date(0, 1, 1)).as_deref(), Some("0001-01-01 BC")); + assert_eq!(value_to_text(&date(-43, 3, 15)).as_deref(), Some("0044-03-15 BC")); + assert_eq!(value_to_text(&date(1, 1, 1)).as_deref(), Some("0001-01-01")); + assert_eq!(value_to_text(&date(10_000, 1, 1)).as_deref(), Some("10000-01-01")); + + let column = typed("timestamp(0)", stamp(Some(0), TemporalZone::Naive)); + let stamp = Value::Timestamp(Temporal::Finite( + NaiveDate::from_ymd_opt(0, 1, 1) + .expect("a date") + .and_hms_opt(10, 0, 0) + .expect("a time"), + )); + assert_eq!(value_text(&stamp, &column).as_deref(), Some("0001-01-01 10:00:00 BC")); + assert_eq!(value_to_text(&stamp).as_deref(), Some("0001-01-01 10:00:00 BC")); + } + + #[test] + fn an_offset_with_seconds_keeps_them() { + let offset = chrono::FixedOffset::east_opt(5 * 3600 + 30 * 60 + 15).expect("an offset"); + let time = crate::value::TimeWithOffset { + time: SqlTime::new(false, 12, 0, 0, 0).expect("a time"), + offset, + }; + let stamp = Value::TimestampTz(Temporal::Finite(crate::value::OffsetTimestamp::from_datetime( + NaiveDate::from_ymd_opt(1890, 1, 1) + .expect("a date") + .and_hms_opt(12, 0, 0) + .expect("a time") + .and_local_timezone(offset) + .single() + .expect("one instant"), + ))); + + assert_eq!( + value_to_text(&Value::TimeTz(time)).as_deref(), + Some("12:00:00+05:30:15") + ); + assert_eq!( + value_text(&Value::TimeTz(time), &typed("timetz", time_tz())).as_deref(), + Some("12:00:00+05:30:15") + ); + assert_eq!(value_to_text(&stamp).as_deref(), Some("1890-01-01 12:00:00+05:30:15")); + assert_eq!( + value_text(&stamp, &typed("timestamptz", stamp_kind_offset())).as_deref(), + Some("1890-01-01 12:00:00+05:30:15") + ); + } + + fn time_tz() -> ColumnKind { + ColumnKind::Time { + fractional_digits: None, + with_offset: true, + range: TimeRange::TimeOfDay, + } + } + + fn stamp_kind_offset() -> ColumnKind { + stamp(None, TemporalZone::Offset) + } + #[test] fn value_to_text_names_the_non_finite_values() { assert_eq!(value_to_text(&Value::Float64(f64::NAN)), Some("NaN".to_owned())); diff --git a/linux/crates/core/src/export/xlsx.rs b/linux/crates/core/src/export/xlsx.rs index 6b4a3a1ce4..6f551a9cdd 100644 --- a/linux/crates/core/src/export/xlsx.rs +++ b/linux/crates/core/src/export/xlsx.rs @@ -1,8 +1,10 @@ use std::sync::Arc; +use chrono::{Datelike, Timelike}; use rust_xlsxwriter::{Format, Workbook, Worksheet}; -use super::{RenderError, cell_text}; +use super::encoder::column_name; +use super::{EncodeError, RenderError, cell_text}; use crate::column::ResultColumn; use crate::value::{Temporal, Value}; @@ -11,6 +13,14 @@ use crate::value::{Temporal, Value}; const MAX_ROWS: usize = 1_048_576; const MAX_COLUMNS: usize = 16_384; +/// The characters one cell holds. +const MAX_CELL_CHARS: usize = 32_767; + +/// The significant digits Excel keeps of a number. A number with more +/// comes back changed even when an f64 holds it exactly: +/// `4111111111111111` reads back as `4111111111111110`. +const EXCEL_DIGITS: usize = 15; + /// The rows a sheet holds under its header row. pub const XLSX_MAX_DATA_ROWS: usize = MAX_ROWS - 1; @@ -45,8 +55,16 @@ pub struct XlsxSheetWriter { workbook: Workbook, columns: Arc<[ResultColumn]>, rows: usize, + formats: CellFormats, +} + +/// How the date and time cells are shown. +struct CellFormats { date: Format, timestamp: Format, + /// For a timestamp with a fraction of a second, which the plain + /// format would hide. A sheet shows at most milliseconds. + timestamp_fraction: Format, } impl XlsxSheetWriter { @@ -72,8 +90,11 @@ impl XlsxSheetWriter { workbook, columns, rows: 0, - date: Format::new().set_num_format("yyyy-mm-dd"), - timestamp: Format::new().set_num_format("yyyy-mm-dd hh:mm:ss"), + formats: CellFormats { + date: Format::new().set_num_format("yyyy-mm-dd"), + timestamp: Format::new().set_num_format("yyyy-mm-dd hh:mm:ss"), + timestamp_fraction: Format::new().set_num_format("yyyy-mm-dd hh:mm:ss.000"), + }, }) } @@ -97,8 +118,7 @@ impl XlsxSheetWriter { workbook, columns, rows: written, - date, - timestamp, + formats, } = self; let sheet = workbook.worksheet_from_index(0).map_err(xlsx_error)?; for row in rows { @@ -108,7 +128,7 @@ impl XlsxSheetWriter { if column_index >= MAX_COLUMNS { break; } - write_cell(sheet, sheet_row, column_index as u16, columns, value, date, timestamp)?; + write_cell(sheet, sheet_row, column_index as u16, columns, value, formats)?; } *written += 1; } @@ -126,29 +146,58 @@ fn write_cell( column: u16, columns: &[ResultColumn], value: &Value, - date: &Format, - timestamp: &Format, + formats: &CellFormats, ) -> Result<(), RenderError> { - match value { + let written = match value { // A blank cell, not an empty string: Excel's COUNT and AVERAGE // read the two differently, and NULL is the absent one. Value::Null => Ok(()), + // Left blank, it would read as a NULL the table does not hold. + Value::Undecodable(_) => { + return Err(RenderError::Encode(EncodeError::Undecodable { + column: column_name(columns, usize::from(column)), + })); + } Value::Bool(flag) => sheet.write_boolean(row, column, *flag).map(drop), - Value::Date(Temporal::Finite(day)) => sheet.write_datetime_with_format(row, column, day, date).map(drop), - Value::Timestamp(Temporal::Finite(at)) => { - sheet.write_datetime_with_format(row, column, at, timestamp).map(drop) + Value::Date(Temporal::Finite(day)) if on_excel_calendar(*day) => sheet + .write_datetime_with_format(row, column, day, &formats.date) + .map(drop), + Value::Timestamp(Temporal::Finite(at)) if on_excel_calendar(at.date()) => { + let format = timestamp_format(columns, column, at.nanosecond(), formats); + sheet.write_datetime_with_format(row, column, at, format).map(drop) } // A sheet cell carries no zone, so an offset timestamp goes in // at UTC. The text formats keep the offset; this one cannot. - Value::TimestampTz(Temporal::Finite(at)) => sheet - .write_datetime_with_format(row, column, at.utc(), timestamp) - .map(drop), + Value::TimestampTz(Temporal::Finite(at)) if on_excel_calendar(at.utc().date()) => { + let at = at.utc(); + let format = timestamp_format(columns, column, at.nanosecond(), formats); + sheet.write_datetime_with_format(row, column, at, format).map(drop) + } _ => match sheet_number(columns, column, value) { Some(number) => sheet.write_number(row, column, number).map(drop), - None => write_text(sheet, row, column, columns, value), + None => return write_text(sheet, row, column, columns, value), }, + }; + written.map_err(xlsx_error) +} + +/// Whether a sheet can hold the date as one: Excel's calendar runs from +/// 1900 to 9999, and a date outside it would show as `####` or a wrong +/// day, so it goes in as text instead. +fn on_excel_calendar(day: chrono::NaiveDate) -> bool { + (1900..=9999).contains(&day.year()) +} + +/// The timestamp format that shows the fraction the column declares, or +/// the one the value carries where the column declares none. +fn timestamp_format<'a>(columns: &[ResultColumn], column: u16, nanos: u32, formats: &'a CellFormats) -> &'a Format { + let declared = columns + .get(usize::from(column)) + .and_then(|column| column.column_type.kind.fractional_digits()); + match declared.map_or(nanos != 0, |digits| digits > 0) { + true => &formats.timestamp_fraction, + false => &formats.timestamp, } - .map_err(xlsx_error) } /// The number a cell goes in as, or `None` for one that goes in as @@ -168,24 +217,35 @@ fn sheet_number(columns: &[ResultColumn], column: u16, value: &Value) -> Option< } /// Everything else goes in as the same text the other exports write, -/// so a cell reads the same whichever format the user picked. +/// so a cell reads the same whichever format the user picked. A value +/// longer than a cell holds is refused by name rather than cut. fn write_text( sheet: &mut Worksheet, row: u32, column: u16, columns: &[ResultColumn], value: &Value, -) -> Result<(), rust_xlsxwriter::XlsxError> { - match cell_text(columns, column as usize, value) { - Some(text) => sheet.write_string(row, column, text).map(drop), - None => Ok(()), +) -> Result<(), RenderError> { + let Some(text) = cell_text(columns, column as usize, value) else { + return Ok(()); + }; + if text.chars().count() > MAX_CELL_CHARS { + return Err(RenderError::TextTooLong { + column: column_name(columns, usize::from(column)), + limit: MAX_CELL_CHARS, + }); } + sheet.write_string(row, column, text).map(drop).map_err(xlsx_error) } -/// The value as an f64, when that f64 prints back as the same number. -/// `None` when it does not, which is the case this exists to catch. +/// The value as an f64, when that f64 prints back as the same number +/// and Excel keeps all its digits. `None` when it does not, which is the +/// case this exists to catch. fn exact_number(columns: &[ResultColumn], column: u16, value: &Value) -> Option { let text = cell_text(columns, column as usize, value)?; + if significant_digits(&text) > EXCEL_DIGITS { + return None; + } let parsed: f64 = text.parse().ok()?; // `{}` on an f64 prints the shortest form that round-trips, so // comparing against it is comparing the value, not the spelling. @@ -196,6 +256,13 @@ fn exact_number(columns: &[ResultColumn], column: u16, value: &Value) -> Option< } } +/// The digits between the first and last nonzero ones: `-0.0120` has +/// two, and `1000` has one. +fn significant_digits(text: &str) -> usize { + let digits: String = text.chars().filter(char::is_ascii_digit).collect(); + digits.trim_start_matches('0').trim_end_matches('0').len() +} + fn without_scale_padding(text: &str) -> &str { match text.contains('.') { true => text.trim_end_matches('0').trim_end_matches('.'), @@ -210,7 +277,8 @@ fn real_number(number: f32) -> f64 { } /// A sheet name Excel accepts: at most 31 characters, none of -/// `[]:*?/\`, not empty, and not wrapped in apostrophes. +/// `[]:*?/\`, not empty, not wrapped in apostrophes, and not +/// `History`, which English Excel keeps for itself. fn sheet_title(name: &str) -> String { let cleaned: String = name .chars() @@ -221,9 +289,10 @@ fn sheet_title(name: &str) -> String { .take(31) .collect(); let trimmed = cleaned.trim().trim_matches('\''); - match trimmed.is_empty() { - true => "Sheet1".to_owned(), - false => trimmed.to_owned(), + match trimmed { + "" => "Sheet1".to_owned(), + reserved if reserved.eq_ignore_ascii_case("history") => format!("{reserved}_"), + name => name.to_owned(), } } @@ -315,8 +384,8 @@ mod tests { assert_eq!(sheet_number(&columns, 0, &Value::Int(42)), Some(42.0)); assert_eq!( - sheet_number(&columns, 0, &Value::Int(-(1 << 53))), - Some(-9_007_199_254_740_992.0) + sheet_number(&columns, 0, &Value::Int(-999_999_999_999_999)), + Some(-999_999_999_999_999.0) ); assert_eq!(sheet_number(&columns, 0, &Value::Int((1 << 53) + 1)), None); assert_eq!(sheet_number(&columns, 0, &Value::UInt(u64::MAX)), None); @@ -342,12 +411,16 @@ mod tests { } fn sheet_xml(bytes: &[u8]) -> String { + part_xml(bytes, "xl/worksheets/sheet1.xml") + } + + fn part_xml(bytes: &[u8], name: &str) -> String { use std::io::Read; let mut archive = zip::ZipArchive::new(std::io::Cursor::new(bytes)).expect("a zip archive"); - let mut sheet = archive.by_name("xl/worksheets/sheet1.xml").expect("the sheet"); + let mut part = archive.by_name(name).expect("the part"); let mut xml = String::new(); - sheet.read_to_string(&mut xml).expect("the sheet's XML"); + part.read_to_string(&mut xml).expect("the part's XML"); xml } @@ -388,6 +461,109 @@ mod tests { ); } + fn undecodable() -> Value { + Value::Undecodable(Box::new(crate::value::UndecodedValue { + type_name: "text".to_owned(), + reason: crate::value::UndecodableReason::InvalidEncoding, + })) + } + + #[test] + fn an_unreadable_value_is_refused_rather_than_written_as_a_blank() { + let refused = render_xlsx(&cols(&["id", "body"]), &[vec![Value::Int(1), undecodable()]], "Results"); + + assert!( + matches!(&refused, Err(RenderError::Encode(EncodeError::Undecodable { column })) if column == "body"), + "{refused:?}" + ); + } + + #[test] + fn a_number_past_fifteen_digits_stays_text() { + let columns = cols(&["card"]); + + assert_eq!( + sheet_number(&columns, 0, &Value::Int(4_111_111_111_111_111)), + None, + "Excel shows 4111111111111110" + ); + assert_eq!( + sheet_number( + &columns, + 0, + &Value::Decimal("12345678901234.56".parse().expect("decimal")) + ), + None + ); + assert_eq!( + sheet_number(&columns, 0, &Value::Int(411_111_111_111_111)), + Some(411_111_111_111_111.0) + ); + assert_eq!( + sheet_number(&columns, 0, &Value::Int(1_000_000_000_000_000_000)), + Some(1e18), + "trailing zeros are not digits Excel loses" + ); + assert_eq!( + sheet_number( + &columns, + 0, + &Value::Decimal("0.000000000000000123".parse().expect("decimal")) + ), + Some(1.23e-16), + "leading zeros are not digits Excel loses" + ); + } + + #[test] + fn a_value_longer_than_a_cell_holds_names_its_column() { + let rows = vec![vec![Value::Int(1), Value::Text("x".repeat(32_768))]]; + + let refused = render_xlsx(&cols(&["id", "body"]), &rows, "Results"); + + assert!( + matches!(&refused, Err(RenderError::TextTooLong { column, .. }) if column == "body"), + "{refused:?}" + ); + assert!( + render_xlsx(&cols(&["body"]), &[vec![Value::Text("x".repeat(32_767))]], "Results").is_ok(), + "a value at the limit was refused" + ); + } + + #[test] + fn a_date_before_1900_is_written_as_text() { + let day = chrono::NaiveDate::from_ymd_opt(1899, 12, 31).expect("a date"); + let rows = vec![vec![Value::Date(Temporal::Finite(day))]]; + + let xml = sheet_xml(&render_xlsx(&cols(&["born"]), &rows, "Results").expect("render")); + + assert!(xml.contains("1899-12-31<"), "{xml}"); + } + + #[test] + fn a_timestamp_with_a_fraction_shows_it() { + let at = chrono::NaiveDate::from_ymd_opt(2024, 6, 15) + .expect("a date") + .and_hms_milli_opt(12, 0, 0, 250) + .expect("a time"); + let rows = vec![vec![Value::Timestamp(Temporal::Finite(at))]]; + + let styles = part_xml( + &render_xlsx(&cols(&["at"]), &rows, "Results").expect("render"), + "xl/styles.xml", + ); + + assert!(styles.contains("hh:mm:ss.000"), "{styles}"); + } + + #[test] + fn the_name_excel_reserves_is_not_used_for_the_sheet() { + assert!(!sheet_title("History").eq_ignore_ascii_case("history")); + assert!(!sheet_title("history").eq_ignore_ascii_case("history")); + assert_eq!(sheet_title("History of orders"), "History of orders"); + } + #[test] fn every_value_kind_writes_without_failing() { let columns = cols(&["a", "b", "c", "d", "e", "f", "g"]); diff --git a/linux/crates/core/src/export/xml.rs b/linux/crates/core/src/export/xml.rs index 31a45af683..e19e1cd8db 100644 --- a/linux/crates/core/src/export/xml.rs +++ b/linux/crates/core/src/export/xml.rs @@ -2,6 +2,7 @@ use crate::column::ResultColumn; use crate::value::Value; use super::column_type; +use super::encoder::{EncodeError, XmlEncoder, encode_rows}; use super::value_text::value_text; /// A result as XML. @@ -10,33 +11,35 @@ use super::value_text::value_text; /// because a SQL column may be called `order total` or `1`, and /// neither is a name XML allows. Putting it in an attribute keeps /// every result representable without renaming the user's columns. -pub fn render_xml(columns: &[ResultColumn], rows: &[Vec]) -> String { - let mut out = String::with_capacity(rows.len() * columns.len() * 24); - out.push_str(XML_HEAD); - for row in rows { - xml_row(&mut out, columns, row); - } - out.push_str(XML_TAIL); - out +pub fn render_xml(columns: &[ResultColumn], rows: &[Vec]) -> Result { + encode_rows(XmlEncoder::new(columns.into()), rows) } pub(super) const XML_HEAD: &str = "\n\n"; pub(super) const XML_TAIL: &str = "\n"; -pub(super) fn xml_row<'a>(out: &mut String, columns: &[ResultColumn], row: impl IntoIterator) { +/// One row, or the column holding a character XML 1.0 cannot carry. +pub(super) fn xml_row<'a>( + out: &mut String, + columns: &[ResultColumn], + row: impl IntoIterator, +) -> Result<(), EncodeError> { out.push_str(" \n"); for (index, value) in row.into_iter().enumerate() { let name = columns .get(index) .map(|column| column.name.as_str()) .unwrap_or_default(); + let refused = || EncodeError::NotXmlText { + column: super::encoder::column_name(columns, index), + }; out.push_str(" { out.push_str("\">"); - escape_into(out, &text); + escape_into(out, &text, Position::Text).map_err(|_| refused())?; out.push_str("\n"); } // An empty element with the flag, so a NULL reads back as @@ -45,17 +48,27 @@ pub(super) fn xml_row<'a>(out: &mut String, columns: &[ResultColumn], row: impl } } out.push_str(" \n"); + Ok(()) +} + +#[derive(Clone, Copy, PartialEq, Eq)] +enum Position { + Text, + Attribute, } -/// Escape for both text and attribute positions, and drop what XML 1.0 -/// cannot carry at all. +/// A character XML 1.0 has no representation for. +struct NotXmlText; + +/// Escape for a text or attribute position. /// -/// The control characters below `space`, apart from tab, newline and -/// carriage return, have no representation in XML 1.0: a numeric -/// reference to them is as invalid as the character itself. A document -/// containing one would fail to parse, so they are dropped and the -/// rest of the value is kept. -fn escape_into(out: &mut String, text: &str) { +/// A parser reads a raw carriage return back as a line feed, and a raw +/// tab or line feed in an attribute as a space, so those go in as +/// character references. The control characters below `space` other +/// than tab, line feed and carriage return have no representation in +/// XML 1.0, not even as a reference, so a value holding one is refused +/// rather than written without it. +fn escape_into(out: &mut String, text: &str, position: Position) -> Result<(), NotXmlText> { for ch in text.chars() { match ch { '&' => out.push_str("&"), @@ -63,14 +76,17 @@ fn escape_into(out: &mut String, text: &str) { '>' => out.push_str(">"), '"' => out.push_str("""), '\'' => out.push_str("'"), - '\t' | '\n' | '\r' => out.push(ch), - other if (other as u32) < 0x20 => {} + '\r' => out.push_str(" "), + '\t' if position == Position::Attribute => out.push_str(" "), + '\n' if position == Position::Attribute => out.push_str(" "), + '\t' | '\n' => out.push(ch), // The surrogate range cannot appear in a Rust `char`, and // these two are the remaining non-characters XML refuses. - '\u{FFFE}' | '\u{FFFF}' => {} + other if (other as u32) < 0x20 || matches!(other, '\u{FFFE}' | '\u{FFFF}') => return Err(NotXmlText), other => out.push(other), } } + Ok(()) } #[cfg(test)] @@ -83,7 +99,7 @@ mod tests { let columns = cols(&["order total", "1"]); let rows = vec![vec![Value::Int(2), Value::Int(3)]]; - let xml = render_xml(&columns, &rows); + let xml = render_xml(&columns, &rows).expect("the document"); assert!(xml.contains("2"), "{xml}"); assert!(xml.contains("3"), "{xml}"); @@ -93,7 +109,7 @@ mod tests { fn markup_in_a_value_is_escaped() { let rows = vec![vec![Value::Text("a < b & \"c\"".into())]]; - let xml = render_xml(&cols(&["a"]), &rows); + let xml = render_xml(&cols(&["a"]), &rows).expect("the document"); assert!(xml.contains("a < b & "c""), "{xml}"); } @@ -102,25 +118,49 @@ mod tests { fn a_null_is_told_apart_from_the_empty_string() { let rows = vec![vec![Value::Null, Value::Text(String::new())]]; - let xml = render_xml(&cols(&["a", "b"]), &rows); + let xml = render_xml(&cols(&["a", "b"]), &rows).expect("the document"); assert!(xml.contains(""), "{xml}"); assert!(xml.contains(""), "{xml}"); } + fn encoded_row(columns: &[ResultColumn], row: &[Value]) -> Result { + use super::super::RowEncoder; + + let mut out = Vec::new(); + XmlEncoder::new(columns.into()).row(&mut out, &mut row.iter())?; + Ok(String::from_utf8(out).expect("utf-8")) + } + #[test] - fn a_character_xml_cannot_carry_is_dropped_rather_than_breaking_the_document() { - let rows = vec![vec![Value::Text("a\u{1}b\tc".into())]]; + fn a_carriage_return_survives_a_parser_reading_it_back() { + let xml = encoded_row(&cols(&["a\rb"]), &[Value::Text("one\r\ntwo\rthree\tfour".into())]).expect("the row"); - let xml = render_xml(&cols(&["a"]), &rows); + assert!( + xml.contains("one \ntwo three\tfour"), + "{xml}" + ); + } + + #[test] + fn a_break_in_a_column_name_is_kept_in_its_attribute() { + let xml = encoded_row(&cols(&["a\tb\nc"]), &[Value::Int(1)]).expect("the row"); + + assert!(xml.contains("1"), "{xml}"); + } + + #[test] + fn a_character_xml_cannot_carry_is_refused_rather_than_dropped() { + let refused = encoded_row(&cols(&["a"]), &[Value::Text("a\u{1}b\tc".into())]); + let refused_name = encoded_row(&cols(&["a\u{1}"]), &[Value::Int(1)]); - assert!(xml.contains("ab\tc"), "{xml}"); - assert!(!xml.contains('\u{1}'), "an unrepresentable character survived"); + assert_eq!(refused, Err(EncodeError::NotXmlText { column: "a".to_owned() })); + assert!(refused_name.is_err(), "{refused_name:?}"); } #[test] fn an_empty_result_is_still_a_document() { - let xml = render_xml(&cols(&["a"]), &[]); + let xml = render_xml(&cols(&["a"]), &[]).expect("the document"); assert!(xml.starts_with(""), "{xml}"); assert!(xml.contains("\n"), "{xml}"); diff --git a/linux/crates/core/src/import/csv_import.rs b/linux/crates/core/src/import/csv_import.rs index e1dd3f3352..1ae1fa7101 100644 --- a/linux/crates/core/src/import/csv_import.rs +++ b/linux/crates/core/src/import/csv_import.rs @@ -4,11 +4,19 @@ use crate::column::ColumnInfo; use crate::edit::{CellInput, parse_literal_text}; use crate::export::CsvDelimiter; +use super::ImportColumn; + /// How many rows the mapping dialog shows. Enough to tell whether the /// delimiter and the header switch are right, few enough that a /// gigabyte file still opens the dialog at once. pub const MAX_PREVIEW_ROWS: usize = 50; +/// The most fields one record may have. No engine takes a table this +/// wide (MySQL stops at 4,096 columns), and every row of the preview is +/// padded to the widest, so a line of nothing but commas must not decide +/// how much is allocated. +pub const MAX_FIELDS: usize = 4_096; + #[derive(Debug, Error, PartialEq, Eq)] pub enum ImportError { #[error("the file is not valid UTF-8 at byte {offset}")] @@ -19,6 +27,9 @@ pub enum ImportError { #[error("the file has no columns in it")] NoColumns, + + #[error("line {line} has more than {limit} fields")] + TooManyFields { line: u64, limit: usize }, } /// What a row could not be turned into, named well enough for the user @@ -86,12 +97,13 @@ pub fn read_csv(bytes: &[u8], options: &CsvImportOptions, limit: Option) .from_reader(bytes); let headers: Vec = match options.has_header { - true => reader - .headers() - .map_err(|error| ImportError::Malformed(error.to_string()))? - .iter() - .map(str::to_owned) - .collect(), + true => { + let header = reader + .headers() + .map_err(|error| ImportError::Malformed(error.to_string()))?; + within_width(header)?; + header.iter().map(str::to_owned).collect() + } false => Vec::new(), }; @@ -105,6 +117,7 @@ pub fn read_csv(bytes: &[u8], options: &CsvImportOptions, limit: Option) truncated = true; break; } + within_width(&record)?; rows.push(record.iter().map(str::to_owned).collect()); } @@ -129,17 +142,31 @@ pub fn read_csv(bytes: &[u8], options: &CsvImportOptions, limit: Option) }) } +/// Refuse a record wider than `MAX_FIELDS` before its fields are copied. +fn within_width(record: &csv::StringRecord) -> Result<(), ImportError> { + match record.len() > MAX_FIELDS { + true => Err(ImportError::TooManyFields { + line: record.position().map_or(1, csv::Position::line), + limit: MAX_FIELDS, + }), + false => Ok(()), + } +} + /// A starting mapping: each table column takes the CSV field whose /// header matches its name, ignoring case and surrounding space. /// /// Columns the file has no match for are left unmapped, which the /// dialog shows as "Skip" and the insert turns into the column's own -/// default. -pub fn suggest_mapping(headers: &[String], columns: &[ColumnInfo]) -> Vec> { +/// default. A column the server fills is never mapped. +pub fn suggest_mapping(headers: &[String], columns: &[ImportColumn]) -> Vec> { columns .iter() .map(|column| { - let wanted = column.name.trim().to_lowercase(); + if column.server_filled { + return None; + } + let wanted = column.info.name.trim().to_lowercase(); headers.iter().position(|header| header.trim().to_lowercase() == wanted) }) .collect() @@ -150,11 +177,12 @@ pub fn suggest_mapping(headers: &[String], columns: &[ColumnInfo]) -> Vec