diff --git a/linux/crates/app/src/ui/editor/tests.rs b/linux/crates/app/src/ui/editor/tests.rs index 2c8b62ac58..2ecfe3921d 100644 --- a/linux/crates/app/src/ui/editor/tests.rs +++ b/linux/crates/app/src/ui/editor/tests.rs @@ -515,7 +515,10 @@ fn truncated_result_reveals_banner_and_complete_result_hides_it() { let complete = shown_banner(&harness); assert!(capped.is_revealed(), "the capped result has no banner"); - assert_eq!(capped.title(), "Showing the first 3 rows"); + assert_eq!( + capped.title(), + "Stopped after the first 3 rows: the statement did not run past them" + ); assert_eq!(capped.button_label().as_deref(), Some("Export All…")); assert!(!complete.is_revealed(), "a complete result shows a banner"); } diff --git a/linux/crates/app/src/ui/result_pane/result_summary.rs b/linux/crates/app/src/ui/result_pane/result_summary.rs index af01ec7ab8..92b79cea66 100644 --- a/linux/crates/app/src/ui/result_pane/result_summary.rs +++ b/linux/crates/app/src/ui/result_pane/result_summary.rs @@ -63,12 +63,20 @@ impl ResultSummary { plural_count(total), &[("shown", &shown), ("total", &format(total))], )), - (Truncation::ReadPastLimit | Truncation::ServerStopped, _) => Some(ngettext_f( + (Truncation::ReadPastLimit, None) => Some(ngettext_f( "Showing the first {shown} row", "Showing the first {shown} rows", plural_count(self.retained), &[("shown", &shown)], )), + // A SELECT that calls a function per row did that for these + // rows only, which "showing" alone would hide. + (Truncation::ServerStopped, _) => Some(ngettext_f( + "Stopped after the first {shown} row: the statement did not run past it", + "Stopped after the first {shown} rows: the statement did not run past them", + plural_count(self.retained), + &[("shown", &shown)], + )), } } @@ -127,7 +135,7 @@ mod tests { assert_eq!( stopped.limit_title_with(english).as_deref(), - Some("Showing the first 1,000 rows") + Some("Stopped after the first 1,000 rows: the statement did not run past them") ); assert_eq!( read_past.limit_title_with(english).as_deref(), diff --git a/linux/crates/app/src/ui/structure_tab/review_dialog.rs b/linux/crates/app/src/ui/structure_tab/review_dialog.rs index 48e5898ba8..8e2aa6f66f 100644 --- a/linux/crates/app/src/ui/structure_tab/review_dialog.rs +++ b/linux/crates/app/src/ui/structure_tab/review_dialog.rs @@ -84,6 +84,10 @@ fn destructive_text(change: &DestructiveChange) -> String { "Changes column “{column}” from {from} to {to}. A value the new type can't hold is converted or refused.", &[("column", column), ("from", from), ("to", to)], ), + DestructiveChange::SetNotNull { column } => crate::i18n::gettext_f( + "Makes column “{column}” NOT NULL. A NULL already in it is converted or refused.", + &[("column", column)], + ), DestructiveChange::DropIndex { index } => crate::i18n::gettext_f("Drops index “{index}”.", &[("index", index)]), DestructiveChange::DropForeignKey { key } => { crate::i18n::gettext_f("Drops foreign key “{key}”.", &[("key", key)]) @@ -235,6 +239,16 @@ mod tests { assert_eq!(atomic.body(), ""); } + #[test] + fn a_new_not_null_says_what_happens_to_nulls() { + assert_eq!( + destructive_text(&DestructiveChange::SetNotNull { + column: "email".to_owned() + }), + "Makes column “email” NOT NULL. A NULL already in it is converted or refused." + ); + } + #[test] fn a_rebuild_names_what_it_makes_again() { let text = rebuild_text(&TableRebuild { diff --git a/linux/crates/core/src/ddl/change_review.rs b/linux/crates/core/src/ddl/change_review.rs index 403dbfa1c5..57e1e7252e 100644 --- a/linux/crates/core/src/ddl/change_review.rs +++ b/linux/crates/core/src/ddl/change_review.rs @@ -8,7 +8,7 @@ pub struct ChangeReview { /// The edits the user made, counted as the action bar counts them. pub operations: usize, pub statements: Vec, - /// The rebuild first, then the drops and type changes. + /// The rebuild first, then the drops, type changes and new NOT NULLs. pub destructive: Vec, } @@ -48,6 +48,15 @@ impl ChangeReview { } }), ); + destructive.extend( + changes + .altered_columns + .iter() + .filter(|alteration| alteration.changed.nullability && !alteration.draft.nullable) + .map(|alteration| DestructiveChange::SetNotNull { + column: alteration.original.name().to_owned(), + }), + ); destructive.extend( changes .dropped_indexes diff --git a/linux/crates/core/src/ddl/destructive_change.rs b/linux/crates/core/src/ddl/destructive_change.rs index 0008af881c..8a9d4ad6d5 100644 --- a/linux/crates/core/src/ddl/destructive_change.rs +++ b/linux/crates/core/src/ddl/destructive_change.rs @@ -16,6 +16,11 @@ pub enum DestructiveChange { from: String, to: String, }, + /// A NULL already in the column has to go: the server refuses the + /// change, or on some engines puts a default value in its place. + SetNotNull { + column: String, + }, DropIndex { index: String, }, diff --git a/linux/crates/core/src/ddl/tests.rs b/linux/crates/core/src/ddl/tests.rs index f7203e0e31..2f3354a227 100644 --- a/linux/crates/core/src/ddl/tests.rs +++ b/linux/crates/core/src/ddl/tests.rs @@ -751,8 +751,37 @@ fn unchanged_catalog_type_is_not_type_change() { email.nullable = false; }); - assert!(catalog.destructive.is_empty(), "{:?}", catalog.destructive); - assert!(retyped.destructive.is_empty(), "{:?}", retyped.destructive); + let not_null = [DestructiveChange::SetNotNull { + column: "email".to_owned(), + }]; + assert_eq!(catalog.destructive, not_null); + assert_eq!(retyped.destructive, not_null); +} + +/// MySQL without strict mode and ClickHouse turn the NULLs in a column +/// made NOT NULL into 0 or an empty string, so the change is reviewed. +#[test] +fn set_not_null_is_destructive_and_drop_not_null_is_not() { + let tightened = review(&["ALTER TABLE users ALTER COLUMN legacy SET NOT NULL"], |draft| { + draft_column(draft, "legacy").nullable = false; + }); + assert_eq!( + tightened.destructive, + [DestructiveChange::SetNotNull { + column: "legacy".to_owned() + }] + ); + assert!(tightened.requires_confirmation(crate::WriteAtomicity::Transactional)); + + let mut original = users(); + if let Some(legacy) = original.columns.iter_mut().find(|column| column.name() == "legacy") { + legacy.info.nullable = false; + } + let mut draft = TableDraft::from_definition(&original); + draft_column(&mut draft, "legacy").nullable = true; + let plan = DdlPlan::from_sql(["ALTER TABLE users ALTER COLUMN legacy DROP NOT NULL".to_owned()]); + let loosened = ChangeReview::review(&diff_table(&original, &draft), &plan); + assert!(loosened.destructive.is_empty(), "{:?}", loosened.destructive); } #[test] diff --git a/linux/crates/core/src/edit/parse_literal_text.rs b/linux/crates/core/src/edit/parse_literal_text.rs index 743374528d..40b56f3d7d 100644 --- a/linux/crates/core/src/edit/parse_literal_text.rs +++ b/linux/crates/core/src/edit/parse_literal_text.rs @@ -1,4 +1,4 @@ -use chrono::{DateTime, NaiveDate, NaiveDateTime, NaiveTime}; +use chrono::{DateTime, NaiveDate, NaiveDateTime, NaiveTime, Timelike}; use crate::column::{BoolStorage, ColumnKind, ColumnType, FloatWidth, IntegerKind, TemporalZone, TimeRange}; use crate::value::{ @@ -56,6 +56,14 @@ fn keeps_text_on_failure(kind: &ColumnKind) -> bool { } fn strict(input: &str, column: &ColumnType, purpose: Purpose) -> Result { + let value = strict_value(input, column, purpose)?; + if purpose == Purpose::Store { + check_declared_fraction(&value, column.kind.fractional_digits())?; + } + Ok(value) +} + +fn strict_value(input: &str, column: &ColumnType, purpose: Purpose) -> Result { let trimmed = input.trim(); match &column.kind { ColumnKind::Bool(BoolStorage::Native) => parse_bool(trimmed).map(Value::Bool), @@ -185,6 +193,28 @@ fn check_declared_digits( } } +/// Refuse a fraction of a second finer than the column keeps. The +/// server rounds it, and `23:59:59.5` in a `timestamp(0)` becomes the +/// next day. +fn check_declared_fraction(value: &Value, digits: Option) -> Result<(), EditParseError> { + let Some(digits) = digits.filter(|digits| *digits < 9) else { + return Ok(()); + }; + let nanos = match value { + Value::Time(time) => time.total_nanos().rem_euclid(1_000_000_000), + Value::TimeTz(time) => time.time.total_nanos().rem_euclid(1_000_000_000), + Value::Timestamp(Temporal::Finite(stamp)) => i128::from(stamp.nanosecond()), + Value::TimestampTz(Temporal::Finite(stamp)) => i128::from(stamp.utc().nanosecond()), + _ => return Ok(()), + }; + match nanos % 10_i128.pow(9 - u32::from(digits)) { + 0 => Ok(()), + _ => Err(EditParseError::ScaleExceeded { + scale: u32::from(digits), + }), + } +} + fn parse_float(input: &str, width: FloatWidth) -> Result { if let Some(value) = non_finite(input) { return Ok(match width { @@ -231,9 +261,16 @@ fn parse_time(input: &str) -> Result { if let Ok(time) = NaiveTime::parse_from_str(input, "%H:%M") { return Ok(Value::Time(SqlTime::from_time_of_day(time))); } - Err(EditParseError::InvalidTime) + // PostgreSQL's time runs to 24:00:00 inclusive, which no NaiveTime + // holds. + match input.parse::() { + Ok(time) if time.total_nanos() == END_OF_DAY_NANOS => Ok(Value::Time(time)), + _ => Err(EditParseError::InvalidTime), + } } +const END_OF_DAY_NANOS: i128 = 86_400 * 1_000_000_000; + /// MySQL TIME, which measures an amount of time and so runs past 24 /// hours and below zero. fn parse_duration(input: &str) -> Result { @@ -735,6 +772,53 @@ mod tests { ); } + #[test] + fn a_fraction_finer_than_the_column_keeps_is_refused() { + let declared = |digits| ColumnKind::Timestamp { + fractional_digits: Some(digits), + zone: TemporalZone::Naive, + }; + + assert_eq!( + parse(declared(0), "2024-12-31 23:59:59.5"), + Err(EditParseError::ScaleExceeded { scale: 0 }), + "the server would have rounded into the next day" + ); + assert!(parse(declared(3), "2024-12-31 23:59:59.120").is_ok()); + assert_eq!( + parse(declared(3), "2024-12-31 23:59:59.1234"), + Err(EditParseError::ScaleExceeded { scale: 3 }) + ); + assert_eq!( + parse( + ColumnKind::Time { + fractional_digits: Some(0), + with_offset: false, + range: TimeRange::Duration, + }, + "-01:00:00.5" + ), + Err(EditParseError::ScaleExceeded { scale: 0 }) + ); + assert!( + parse_filter_text("2024-12-31 23:59:59.5", &column(declared(0), false)).is_ok(), + "a comparison was held to the column's precision" + ); + assert!(parse(stamp(TemporalZone::Naive), "2024-12-31 23:59:59.123456789").is_ok()); + } + + #[test] + fn the_end_of_the_day_is_a_time_of_day() { + let Ok(Value::Time(end)) = parse(time(false, TimeRange::TimeOfDay), "24:00:00") else { + panic!("24:00:00 did not parse"); + }; + assert_eq!(end.format(None), "24:00:00"); + assert_eq!( + parse(time(false, TimeRange::TimeOfDay), "24:00:01"), + Err(EditParseError::InvalidTime) + ); + } + #[test] fn a_network_column_keeps_the_servers_own_text() { let Ok(Value::Other(other)) = parse(ColumnKind::Network(crate::column::NetworkFamily::Cidr), "10.0.0.0/8") diff --git a/linux/crates/core/src/result_event.rs b/linux/crates/core/src/result_event.rs index fb5a752e30..576b3a54eb 100644 --- a/linux/crates/core/src/result_event.rs +++ b/linux/crates/core/src/result_event.rs @@ -21,8 +21,9 @@ pub struct ServerNotice { pub enum Truncation { /// Every row the statement produced is here. Complete, - /// The server stopped sending, because the query had a LIMIT of its - /// own or the run was stopped. + /// The statement itself was stopped at the row limit, so it never + /// ran for the rows past it: a function it calls per row ran for the + /// rows kept and no others. ServerStopped, /// The run hit its row limit and there were more rows behind it. ReadPastLimit, diff --git a/linux/crates/drivers/mysql/src/catalog.rs b/linux/crates/drivers/mysql/src/catalog.rs index 5f15bdc3b2..8ebd6cfe82 100644 --- a/linux/crates/drivers/mysql/src/catalog.rs +++ b/linux/crates/drivers/mysql/src/catalog.rs @@ -15,7 +15,7 @@ pub(crate) use mysql_extra::{ExtraGenerated, MySqlExtra}; pub(crate) use table_definition::{ COLUMN_ENGINE_ATTRIBUTES_SQL, ColumnCheck, INBOUND_FOREIGN_KEYS_SQL, ListedSrid, MARIADB_COLUMN_CHECKS_SQL, MARIADB_GEOMETRY_SRIDS_SQL, column_checks, geometry_srids, inbound_foreign_keys, place_column_checks, - printed_names, + printed_names, quoted_default, }; use sqlx::mysql::{MySql, MySqlColumn}; diff --git a/linux/crates/drivers/mysql/src/catalog/table_definition.rs b/linux/crates/drivers/mysql/src/catalog/table_definition.rs index 1d5c0e8dce..b1ce32ddc0 100644 --- a/linux/crates/drivers/mysql/src/catalog/table_definition.rs +++ b/linux/crates/drivers/mysql/src/catalog/table_definition.rs @@ -124,6 +124,18 @@ fn column_line<'a>(create_statement: &'a str, column: &str) -> Option<&'a str> { }) } +/// The quoted DEFAULT on the column's line of SHOW CREATE TABLE, without +/// its quotes. The server prints a TIMESTAMP default there as wall time +/// in the session's zone, where MySQL's `COLUMN_DEFAULT` keeps the text +/// of whichever session last altered the table, in that session's zone. +pub(crate) fn quoted_default(create_statement: &str, column: &str) -> Option { + const MARKER: &str = " DEFAULT '"; + let line = column_line(create_statement, column)?; + let rest = line.get(line.find(MARKER)? + MARKER.len()..)?; + let end = rest.find('\'')?; + rest.get(..end).map(str::to_owned) +} + fn ends_with_check(line: &str, clause: &str) -> bool { line.strip_suffix(')') .and_then(|rest| rest.strip_suffix(clause)) @@ -171,6 +183,16 @@ mod tests { ) } + #[test] + fn a_quoted_default_is_read_from_its_own_columns_line() { + let create = "CREATE TABLE `t` (\n `id` int NOT NULL,\n `at` timestamp NULL DEFAULT '1999-12-31 17:00:00' \ + COMMENT 'x',\n `made` timestamp NULL DEFAULT CURRENT_TIMESTAMP,\n PRIMARY KEY (`id`)\n)"; + + assert_eq!(quoted_default(create, "at").as_deref(), Some("1999-12-31 17:00:00")); + assert_eq!(quoted_default(create, "made"), None); + assert_eq!(quoted_default(create, "missing"), None); + } + #[test] fn only_a_geometry_column_that_sets_an_srid_keeps_one() { let rows = [ diff --git a/linux/crates/drivers/mysql/src/dialect.rs b/linux/crates/drivers/mysql/src/dialect.rs index 262c11741f..f85e04bd77 100644 --- a/linux/crates/drivers/mysql/src/dialect.rs +++ b/linux/crates/drivers/mysql/src/dialect.rs @@ -87,7 +87,11 @@ impl SqlDialect for MySqlDialect { BindTarget::Column(column) => column_param_type(column), }; let (value, param_type) = adapt(value, param_type)?; - Ok(Placeholder::bound("?", BoundParam::new(value, param_type)?)) + let marker = match param_type { + ParamType::TimestampTz => crate::server_zone::TIMESTAMP_PLACEHOLDER, + _ => "?", + }; + Ok(Placeholder::bound(marker, BoundParam::new(value, param_type)?)) } fn literal(&self, value: &Value, _column: &ColumnType) -> Result { @@ -241,8 +245,8 @@ fn column_param_type(column: &ColumnInfo) -> ParamType { ColumnKind::Date => ParamType::Date, ColumnKind::Time { with_offset: false, .. } => ParamType::Time, ColumnKind::Time { with_offset: true, .. } => ParamType::TimeTz, - // A workspace session's zone is UTC, so the instant binds as - // its UTC wall time. + // The instant binds as its UTC wall time, which the placeholder + // converts to whatever zone the session is in. ColumnKind::Timestamp { zone: TemporalZone::Offset, .. @@ -604,10 +608,23 @@ mod tests { }, ) .expect("a typed null"); + assert_eq!( + stamp.sql, "CONVERT_TZ(?, '+00:00', @@session.time_zone)", + "a write runs in the server's zone, so the UTC wall time is converted there" + ); assert_eq!( stamp.param.map(|param| param.param_type().clone()), Some(ParamType::TimestampTz) ); + let naive = bind( + Value::Null, + ColumnKind::Timestamp { + fractional_digits: None, + zone: TemporalZone::Naive, + }, + ) + .expect("a DATETIME null"); + assert_eq!(naive.sql, "?", "a DATETIME is wall time in any zone"); assert!(matches!( bind(Value::Text("x".to_owned()), ColumnKind::Date), Err(BindError::ValueTypeMismatch { .. }) diff --git a/linux/crates/drivers/mysql/src/encode.rs b/linux/crates/drivers/mysql/src/encode.rs index af4c4faf21..de7219ba1a 100644 --- a/linux/crates/drivers/mysql/src/encode.rs +++ b/linux/crates/drivers/mysql/src/encode.rs @@ -40,8 +40,8 @@ fn bind_one<'q>(query: MySqlQuery<'q>, param: &BoundParam) -> Result query.bind(*date), (_, Value::Timestamp(Temporal::Finite(stamp))) => query.bind(*stamp), - // The workspace session reads and writes TIMESTAMP in UTC, so the - // instant goes as its UTC wall time. + // The instant goes as its UTC wall time, and the dialect's + // placeholder converts it to the session's zone. (_, Value::TimestampTz(Temporal::Finite(stamp))) => query.bind(stamp.utc()), (_, Value::Date(_) | Value::Timestamp(_) | Value::TimestampTz(_)) => return Err(unrepresentable(value)), (_, Value::Decimal(decimal)) if !decimal.is_finite() => return Err(unrepresentable(value)), diff --git a/linux/crates/drivers/mysql/src/lib.rs b/linux/crates/drivers/mysql/src/lib.rs index b047be4799..710f1c8b26 100644 --- a/linux/crates/drivers/mysql/src/lib.rs +++ b/linux/crates/drivers/mysql/src/lib.rs @@ -26,6 +26,7 @@ mod mysql_temporal; mod mysql_timestamp_mode; mod mysql_wire_format; mod physical; +mod server_zone; mod session_statements; mod standalone; mod stream_read; diff --git a/linux/crates/drivers/mysql/src/metadata.rs b/linux/crates/drivers/mysql/src/metadata.rs index e4e37b884a..20915a4f6c 100644 --- a/linux/crates/drivers/mysql/src/metadata.rs +++ b/linux/crates/drivers/mysql/src/metadata.rs @@ -5,21 +5,22 @@ use futures_util::TryStreamExt; use sqlx::mysql::MySqlConnection; use sqlx::{AssertSqlSafe, Executor as _, Row as _}; -use tablepro_core::column::{ColumnInfo, SqlExpression}; +use tablepro_core::column::{ColumnDefault, ColumnInfo, ColumnKind, SqlExpression, TemporalZone}; use tablepro_core::ddl::{ - InboundForeignKey, PrimaryKeyConstraint, ReferentialAction, TableDefinition, TableSource, UnmodeledAttribute, + ColumnDefinition, InboundForeignKey, PrimaryKeyConstraint, ReferentialAction, TableDefinition, TableSource, + UnmodeledAttribute, }; use tablepro_core::dialect::SqlDialect; use tablepro_core::meta::{ IndexInfo, IndexKey, ObjectInfo, ObjectKind, ObjectScope, RowCountEstimate, RowIdentity, SchemaInfo, TableEstimate, TableRef, }; -use tablepro_core::{DriverError, ForeignKeyInfo}; +use tablepro_core::{DriverError, ForeignKeyInfo, Value}; use crate::catalog::{ COLUMN_ENGINE_ATTRIBUTES_SQL, CatalogCells, ColumnCheck, INBOUND_FOREIGN_KEYS_SQL, ListedSrid, MARIADB_COLUMN_CHECKS_SQL, MARIADB_GEOMETRY_SRIDS_SQL, MySqlColumnRow, column_checks, column_definition, - column_info, columns_sql, geometry_srids, inbound_foreign_keys, place_column_checks, + column_info, columns_sql, geometry_srids, inbound_foreign_keys, place_column_checks, quoted_default, }; use crate::decode::decode_row; use crate::dialect::MySqlDialect; @@ -356,7 +357,7 @@ pub(crate) async fn fetch_table_definition( } MySqlFlavour::MySql => Vec::new(), }; - let columns = rows + let mut columns: Vec = rows .iter() .enumerate() .map(|(index, row)| { @@ -379,6 +380,11 @@ pub(crate) async fn fetch_table_definition( definition }) .collect(); + for definition in &mut columns { + if let Some(default) = timestamp_default(connection, flavour, &create_statement, definition).await? { + definition.info.default = default; + } + } let indexes = fetch_indexes(connection, dialect, table).await?; let primary_key = indexes .iter() @@ -411,6 +417,61 @@ pub(crate) async fn fetch_table_definition( }) } +/// A TIMESTAMP's constant default as wall time in the server's zone, +/// which is where a schema change runs (`server_zone`), so restating it +/// keeps the instant. This session reads in UTC, so SHOW CREATE TABLE +/// prints it in UTC. A zero default names no instant and stays as is. +async fn timestamp_default( + connection: &mut MySqlConnection, + flavour: MySqlFlavour, + create_statement: &str, + definition: &ColumnDefinition, +) -> Result, DriverError> { + let info = &definition.info; + let is_instant = matches!( + info.column_type.kind, + ColumnKind::Timestamp { + zone: TemporalZone::Offset, + .. + } + ); + if !is_instant || info.default.is_none() { + return Ok(None); + } + let Some(utc) = quoted_default(create_statement, &info.name) else { + return Ok(None); + }; + if utc.starts_with("0000-00-00") { + return Ok(Some(ColumnDefault::Literal(Value::Text(utc)))); + } + let digits = utc.split_once('.').map_or(0, |(_, fraction)| fraction.len()); + let row = sqlx::query("SELECT CAST(CONVERT_TZ(?, '+00:00', @@GLOBAL.time_zone) AS CHAR)") + .persistent(false) + .bind(utc) + .fetch_one(&mut *connection) + .await + .map_err(|error| map_error(&error, flavour))?; + let wall: Option<&[u8]> = row.try_get_unchecked(0).map_err(|error| DriverError::Decode { + column: info.name.clone(), + detail: error.to_string(), + })?; + Ok(wall.map(|wall| { + let wall = String::from_utf8_lossy(wall); + ColumnDefault::Literal(Value::Text(with_fraction_digits(&wall, digits))) + })) +} + +/// CONVERT_TZ answers with six fractional digits whatever it was given, +/// and the default reads back as the server printed it only with the +/// digits the column declares. +fn with_fraction_digits(text: &str, digits: usize) -> String { + let (whole, fraction) = text.split_once('.').unwrap_or((text, "")); + match fraction.get(..digits).filter(|kept| !kept.is_empty()) { + Some(kept) => format!("{whole}.{kept}"), + None => whole.to_owned(), + } +} + /// Engine attributes on columns, from MySQL 8.0.21 on. An older server /// has no such table, which is the same as having no attributes. async fn engine_attributes( @@ -479,6 +540,19 @@ mod tests { use super::*; + #[test] + fn a_converted_default_keeps_the_digits_the_column_declares() { + assert_eq!( + with_fraction_digits("2000-01-01 07:00:00.000000", 0), + "2000-01-01 07:00:00" + ); + assert_eq!( + with_fraction_digits("2000-01-01 07:00:00.250000", 3), + "2000-01-01 07:00:00.250" + ); + assert_eq!(with_fraction_digits("2000-01-01 07:00:00", 0), "2000-01-01 07:00:00"); + } + fn column(name: &str, nullable: bool) -> ColumnInfo { ColumnInfo { name: name.to_owned(), diff --git a/linux/crates/drivers/mysql/src/mysql_call.rs b/linux/crates/drivers/mysql/src/mysql_call.rs index 6fcd9fa9ab..40945f73d0 100644 --- a/linux/crates/drivers/mysql/src/mysql_call.rs +++ b/linux/crates/drivers/mysql/src/mysql_call.rs @@ -109,4 +109,8 @@ impl Tainter { pub(crate) fn taint(&self) { self.0.store(true, Ordering::SeqCst); } + + pub(crate) fn is_tainted(&self) -> bool { + self.0.load(Ordering::SeqCst) + } } diff --git a/linux/crates/drivers/mysql/src/physical.rs b/linux/crates/drivers/mysql/src/physical.rs index b1ce6fabe0..579bd21a05 100644 --- a/linux/crates/drivers/mysql/src/physical.rs +++ b/linux/crates/drivers/mysql/src/physical.rs @@ -19,7 +19,7 @@ use crate::workspace_session::{WORKSPACE_SQL_MODE_STATEMENT, WORKSPACE_TIME_ZONE #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum SessionKind { /// The pool, a streamed read: TIMESTAMP in UTC, backslashes - /// pinned to be escapes. + /// pinned to be escapes, strict mode on. Workspace, /// An editor tab: the server's own zone and sql_mode, as the mysql /// client would have them. diff --git a/linux/crates/drivers/mysql/src/server_zone.rs b/linux/crates/drivers/mysql/src/server_zone.rs new file mode 100644 index 0000000000..be4240ed97 --- /dev/null +++ b/linux/crates/drivers/mysql/src/server_zone.rs @@ -0,0 +1,239 @@ +//! Which time zone a workspace connection is in, and when. +//! +//! A TIMESTAMP is an instant, but the server reads and writes it as wall +//! time in the session's `time_zone`, and evaluates `CURRENT_TIMESTAMP` +//! and `NOW()` in that zone too. One zone cannot serve both: +//! +//! - Reads run in `+00:00`, the zone a workspace session opens in. UTC +//! has no repeated hour, so every TIMESTAMP reads as the exact instant +//! it holds. +//! - Saves and schema changes run in the server's own zone, +//! `@@GLOBAL.time_zone`, the one the mysql client and every other +//! client that sets none gets. A `DATETIME DEFAULT CURRENT_TIMESTAMP` +//! or `ON UPDATE CURRENT_TIMESTAMP` then stores the wall time other +//! clients store, and an ALTER between DATETIME and TIMESTAMP reads +//! and writes the wall times they mean. A write batch switches the +//! zone before its first statement and puts `+00:00` back after its +//! last; a connection where that fails is closed, never pooled. +//! - A TIMESTAMP value is bound as its UTC wall time inside +//! [`TIMESTAMP_PLACEHOLDER`], which converts it to the session's zone. +//! On a read the conversion does nothing; on a write it gives the wall +//! time the server turns back into the same instant. +//! +//! That last step has one gap. In the hour a zone with daylight saving +//! repeats in autumn, one wall time names two instants and the server +//! picks one: MySQL 8.4 and MariaDB 11.4 both store 05:30Z for 06:30Z +//! in America/New_York, and a key compared there matches the other row. +//! So before a write sends anything, each instant it binds is converted +//! to the server's zone and back; one that does not come back the same +//! refuses the whole batch. A zone written as a fixed offset has no such +//! hour and skips the check. +//! +//! A TIMESTAMP column's constant default is restated by every MODIFY of +//! the column, so the structure tab reads it in the server's zone too +//! (`metadata::timestamp_default`). + +use chrono::NaiveDateTime; +use sqlx::mysql::MySqlConnection; +use sqlx::{AssertSqlSafe, Executor as _, Row as _}; +use tablepro_core::DriverError; +use tablepro_core::statement::{BoundParam, GuardedStatement}; +use tablepro_core::value::{Temporal, Value}; + +use crate::decode::decode_row; +use crate::errors::map_error; +use crate::flavour::MySqlFlavour; +use crate::mysql_timestamp_mode::MySqlTimestampMode; +use crate::mysql_wire_format::MySqlWireFormat; + +/// Where a write batch runs. +pub(crate) const SERVER_ZONE_STATEMENT: &str = "SET SESSION time_zone = @@GLOBAL.time_zone"; + +/// Where a workspace connection reads, put back after a write. +pub(crate) const READ_ZONE_STATEMENT: &str = "SET SESSION time_zone = '+00:00'"; + +/// A TIMESTAMP parameter, bound as UTC wall time. +pub(crate) const TIMESTAMP_PLACEHOLDER: &str = "CONVERT_TZ(?, '+00:00', @@session.time_zone)"; + +/// How the refusal reads: "This database has no {feature}." +const REPEATED_HOUR: &str = "single instant for a TIMESTAMP in the hour its time zone repeats"; + +/// Instants checked per round trip. Each is bound twice, far inside +/// the server's 65,535 placeholders. +const CHECK_CHUNK: usize = 100; + +/// The step that binds an instant the session's zone cannot hold apart +/// from another, if any. Run after [`SERVER_ZONE_STATEMENT`] and before +/// anything is written. +pub(crate) async fn repeated_instant( + connection: &mut MySqlConnection, + flavour: MySqlFlavour, + steps: &[GuardedStatement], +) -> Result, DriverError> { + let mut instants: Vec<(NaiveDateTime, usize)> = Vec::new(); + for (index, step) in steps.iter().enumerate() { + for instant in step_instants(step) { + if !instants.iter().any(|(seen, _)| *seen == instant) { + instants.push((instant, index)); + } + } + } + if instants.is_empty() || has_fixed_offset(&session_zone(connection, flavour).await?) { + return Ok(None); + } + for chunk in instants.chunks(CHECK_CHUNK) { + let mut query = sqlx::query(AssertSqlSafe(round_trip_sql(chunk.len()))).persistent(false); + for (instant, _) in chunk { + query = query.bind(*instant).bind(*instant); + } + let row = query + .fetch_one(&mut *connection) + .await + .map_err(|error| map_error(&error, flavour))?; + let kept = decode_row(&row, MySqlWireFormat::Binary, MySqlTimestampMode::WorkspaceUtc); + let lost = chunk + .iter() + .zip(kept) + .find(|(_, kept)| *kept != Value::Int(1)) + .map(|((_, step), _)| *step); + if lost.is_some() { + return Ok(lost); + } + } + Ok(None) +} + +/// The refusal for a batch [`repeated_instant`] found a step in. +pub(crate) fn repeated_hour() -> DriverError { + DriverError::Unsupported { feature: REPEATED_HOUR } +} + +/// Every finite instant a step binds, its probe's included. +fn step_instants(step: &GuardedStatement) -> impl Iterator + '_ { + let probe: &[BoundParam] = step.guard.probe().map_or(&[], |(probe, _)| probe.params()); + step.statement + .params() + .iter() + .chain(probe) + .filter_map(|param| match param.value() { + Value::TimestampTz(Temporal::Finite(stamp)) => Some(stamp.utc()), + _ => None, + }) +} + +/// One column per instant, 1 where it comes back from the session's +/// zone as itself. +fn round_trip_sql(instants: usize) -> String { + let column = "CONVERT_TZ(CONVERT_TZ(?, '+00:00', @@session.time_zone), @@session.time_zone, '+00:00') <=> ?"; + format!("SELECT {}", vec![column; instants].join(", ")) +} + +async fn session_zone(connection: &mut MySqlConnection, flavour: MySqlFlavour) -> Result { + let row = connection + .fetch_one(sqlx::raw_sql("SELECT @@session.time_zone")) + .await + .map_err(|error| map_error(&error, flavour))?; + let bytes: &[u8] = row.try_get_unchecked(0).map_err(|error| DriverError::Decode { + column: "@@session.time_zone".to_owned(), + detail: error.to_string(), + })?; + Ok(String::from_utf8_lossy(bytes).into_owned()) +} + +/// `+07:00` and `-03:30` never repeat an hour; `SYSTEM` and a named zone +/// may. +fn has_fixed_offset(zone: &str) -> bool { + zone.starts_with(['+', '-']) +} + +#[cfg(test)] +mod tests { + use chrono::NaiveDate; + use tablepro_core::ServerInfo; + use tablepro_core::column::{ColumnDefault, ColumnInfo, ColumnKind, ColumnType, ReadForm, TemporalZone}; + use tablepro_core::dialect::BindTarget; + use tablepro_core::statement::{RowGuard, StatementBuilder}; + use tablepro_core::value::OffsetTimestamp; + + use super::*; + use crate::dialect::MySqlDialect; + use crate::workspace_session::WORKSPACE_TIME_ZONE; + + #[test] + fn the_read_zone_is_the_one_the_session_opens_in() { + assert!(READ_ZONE_STATEMENT.ends_with(&format!("'{WORKSPACE_TIME_ZONE}'"))); + assert!(TIMESTAMP_PLACEHOLDER.contains(&format!("'{WORKSPACE_TIME_ZONE}'"))); + } + + #[test] + fn only_a_fixed_offset_skips_the_check() { + assert!(has_fixed_offset("+07:00")); + assert!(has_fixed_offset("-03:30")); + assert!(!has_fixed_offset("SYSTEM")); + assert!(!has_fixed_offset("Europe/Berlin")); + assert!(!has_fixed_offset("UTC")); + } + + fn stamp_column() -> ColumnInfo { + ColumnInfo { + name: "at".to_owned(), + column_type: ColumnType { + name: "timestamp".to_owned(), + kind: ColumnKind::Timestamp { + fractional_digits: None, + zone: TemporalZone::Offset, + }, + catalog: None, + dynamic_storage: false, + read_form: ReadForm::Native, + }, + nullable: true, + primary_key: false, + is_auto_increment: false, + is_generated: false, + default: ColumnDefault::None, + comment: None, + } + } + + fn statement(values: Vec) -> tablepro_core::statement::Statement { + let dialect = MySqlDialect::new(MySqlFlavour::MySql, &ServerInfo::new("MySQL", "8.4.3", 8, 4, 3)); + let column = stamp_column(); + let mut builder = StatementBuilder::new(); + builder.push_sql("UPDATE t SET a = 1 WHERE at IN ("); + for value in values { + builder + .push_value(&dialect, value, BindTarget::Column(&column)) + .expect("a TIMESTAMP binds"); + } + builder.push_sql(")"); + builder.finish() + } + + fn at(hour: u32) -> Value { + let stamp = NaiveDate::from_ymd_opt(2024, 11, 3) + .and_then(|date| date.and_hms_opt(hour, 30, 0)) + .expect("a time"); + Value::TimestampTz(Temporal::Finite(OffsetTimestamp::from_datetime( + stamp.and_utc().fixed_offset(), + ))) + } + + #[test] + fn every_instant_a_step_binds_is_checked() { + let step = GuardedStatement { + statement: statement(vec![at(5), Value::Null]), + guard: RowGuard::ProbeExactly { + probe: statement(vec![at(6)]), + rows: 1, + }, + }; + + let hours: Vec = step_instants(&step) + .map(|stamp| stamp.format("%H").to_string()) + .collect(); + + assert_eq!(hours, ["05", "06"]); + assert_eq!(round_trip_sql(2).matches("<=> ?").count(), 2, "one column per instant"); + } +} diff --git a/linux/crates/drivers/mysql/src/workspace_session.rs b/linux/crates/drivers/mysql/src/workspace_session.rs index 48815d0c63..8234bd747d 100644 --- a/linux/crates/drivers/mysql/src/workspace_session.rs +++ b/linux/crates/drivers/mysql/src/workspace_session.rs @@ -5,11 +5,19 @@ /// literal would mean itself on some servers and an escape on others. /// The DDL the app generates writes some literals that have no hex form /// (`COMMENT '...'`), so the workspace pins the one reading those -/// literals were written for. sqlx sends no sql_mode of its own when +/// literals were written for. +/// +/// STRICT_ALL_TABLES is added because a server without strict mode +/// stores what it can and only warns: a VARCHAR(5) keeps `abcde` of +/// `abcdefgh`, an unknown ENUM label becomes `''`, a TINYINT clamps 300 +/// to 127, and a column made NOT NULL turns its NULLs into 0 or `''`. +/// Strict mode refuses each of these with an error, on every engine, +/// so a save, an import or a schema change either stores what it was +/// given or fails. sqlx sends no sql_mode of its own when /// `pipes_as_concat` and `no_engine_substitution` are off, so this and /// the time zone are the only session changes on top of the user's. pub(crate) const WORKSPACE_SQL_MODE_STATEMENT: &str = "SET SESSION sql_mode = TRIM(BOTH ',' FROM \ - REPLACE(CONCAT(',', @@SESSION.sql_mode, ','), ',NO_BACKSLASH_ESCAPES,', ','))"; + CONCAT(REPLACE(CONCAT(',', @@SESSION.sql_mode, ','), ',NO_BACKSLASH_ESCAPES,', ','), 'STRICT_ALL_TABLES'))"; /// The zone workspace connections read TIMESTAMP in, so a value is the /// instant it names whatever zone the server or the user is in. @@ -20,9 +28,10 @@ mod tests { use super::*; #[test] - fn the_statement_removes_only_the_backslash_mode() { + fn the_statement_removes_the_backslash_mode_and_adds_strict_mode() { assert!(WORKSPACE_SQL_MODE_STATEMENT.starts_with("SET SESSION sql_mode = ")); assert!(WORKSPACE_SQL_MODE_STATEMENT.contains("',NO_BACKSLASH_ESCAPES,'")); + assert!(WORKSPACE_SQL_MODE_STATEMENT.contains("'STRICT_ALL_TABLES'")); assert!(!WORKSPACE_SQL_MODE_STATEMENT.contains("PIPES_AS_CONCAT")); } } diff --git a/linux/crates/drivers/mysql/src/write_batch.rs b/linux/crates/drivers/mysql/src/write_batch.rs index 44c7c4e5a3..d13e98b66e 100644 --- a/linux/crates/drivers/mysql/src/write_batch.rs +++ b/linux/crates/drivers/mysql/src/write_batch.rs @@ -22,6 +22,7 @@ use crate::flavour::MySqlFlavour; use crate::mysql_call::Tainter; use crate::mysql_timestamp_mode::MySqlTimestampMode; use crate::mysql_wire_format::MySqlWireFormat; +use crate::server_zone::{READ_ZONE_STATEMENT, SERVER_ZONE_STATEMENT, repeated_hour, repeated_instant}; use crate::session_statements::statement_timeout; use crate::workspace_call::{count, prepared}; @@ -52,7 +53,28 @@ pub(crate) struct WriteContext<'a> { pub stop: &'a MySqlStop, } +/// Run the batch in the server's own time zone and put the read zone +/// back after, as `server_zone` explains. pub(crate) async fn perform(context: &mut WriteContext<'_>, plan: &WritePlan) -> Result { + if let Err(error) = run(context, SERVER_ZONE_STATEMENT).await { + return Err(plan.ledger.failed(0, error, SessionFate::RolledBack)); + } + let outcome = in_server_zone(context, plan).await; + if !context.tainter.is_tainted() + && let Err(error) = run(context, READ_ZONE_STATEMENT).await + { + tracing::warn!(%error, "the read time zone could not be put back; closing the connection"); + context.tainter.taint(); + } + outcome +} + +async fn in_server_zone(context: &mut WriteContext<'_>, plan: &WritePlan) -> Result { + match repeated_instant(context.connection, context.flavour, &plan.batch.steps).await { + Ok(None) => {} + Ok(Some(index)) => return Err(plan.ledger.failed(index, repeated_hour(), SessionFate::RolledBack)), + Err(error) => return Err(plan.ledger.failed(0, error, SessionFate::RolledBack)), + } match (plan.batch.kind, plan.atomicity) { (WriteKind::RowChanges, WriteAtomicity::Transactional) => transactional(context, plan).await, (WriteKind::RowChanges, WriteAtomicity::StatementByStatement) => stepwise(context, plan).await, diff --git a/linux/crates/drivers/mysql/tests/integration/ddl.rs b/linux/crates/drivers/mysql/tests/integration/ddl.rs index 5f06c01022..cba77d91b0 100644 --- a/linux/crates/drivers/mysql/tests/integration/ddl.rs +++ b/linux/crates/drivers/mysql/tests/integration/ddl.rs @@ -58,6 +58,56 @@ fn assert_kept(before: &TableDefinition, after: &TableDefinition, flavour: Mysql } } +/// A schema change runs in the server's zone, so a TIMESTAMP's constant +/// default is restated as wall time there and keeps its instant. MySQL +/// used to restate it unquoted, which is a syntax error. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn timestamp_constant_default_keeps_its_instant_off_utc() { + for flavour in FLAVOURS { + let fixture = start_with( + MysqlOptions::new(flavour) + .allowing_plaintext() + .with_server_argument("--default-time-zone=+07:00"), + ) + .await; + let connection = connect(&fixture, false).await; + setup( + connection.as_ref(), + &["CREATE TABLE kept (id int PRIMARY KEY, at timestamp NULL DEFAULT '2000-01-01 00:00:00')"], + ) + .await; + + let (before, _) = change(connection.as_ref(), |column| { + if column.name == "at" { + column.comment = Some("restated".to_owned()); + } + }) + .await; + apply( + connection.as_ref(), + WriteKind::RowChanges, + &["INSERT INTO kept (id) VALUES (1)"], + ) + .await + .expect("a row that takes the default"); + + assert_eq!( + before.column("at").map(|column| column.info.default.clone()), + Some(ColumnDefault::Literal(tablepro_core::Value::Text( + "2000-01-01 00:00:00".to_owned() + ))), + "{flavour:?}" + ); + assert_eq!( + text(&scalar(connection.as_ref(), "SELECT UNIX_TIMESTAMP(at) FROM kept").await), + "946659600", + "{flavour:?}: the default moved to another instant" + ); + connection.close().await.expect("a close"); + } +} + fn collation(definition: &TableDefinition, column: &str) -> Option { definition .column(column) diff --git a/linux/crates/drivers/mysql/tests/integration/lenient_server.rs b/linux/crates/drivers/mysql/tests/integration/lenient_server.rs new file mode 100644 index 0000000000..904eb24b89 --- /dev/null +++ b/linux/crates/drivers/mysql/tests/integration/lenient_server.rs @@ -0,0 +1,84 @@ +//! A server started with an empty sql_mode stores what it can of a bad +//! value and only warns. The workspace session is strict, so a save or a +//! schema change on such a server fails instead of changing the value. + +use tablepro_core::{Value, WriteKind}; +use tablepro_test_fixtures::MysqlOptions; + +use crate::support::{FLAVOURS, apply_on, connect, rows, session, session_scalar, setup, start_with, table}; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn a_value_the_column_cannot_hold_is_refused_not_altered() { + for flavour in FLAVOURS { + let fixture = start_with( + MysqlOptions::new(flavour) + .allowing_plaintext() + .with_server_argument("--sql-mode="), + ) + .await; + let connection = connect(&fixture, false).await; + setup( + connection.as_ref(), + &[ + "CREATE TABLE lenient (id int PRIMARY KEY, label varchar(5) NOT NULL, \ + state enum('draft','done') NOT NULL, flag tinyint(1) NOT NULL, note int NULL) ENGINE=InnoDB", + "INSERT INTO lenient VALUES (1, 'ok', 'draft', 1, NULL)", + "CREATE TABLE lenient_myisam (id int PRIMARY KEY, label varchar(5) NOT NULL) ENGINE=MyISAM", + ], + ) + .await; + + let bad_rows = [ + "UPDATE lenient SET label = 'abcdefgh' WHERE id = 1", + "UPDATE lenient SET state = 'unknown' WHERE id = 1", + "UPDATE lenient SET flag = 300 WHERE id = 1", + "INSERT INTO lenient VALUES (2, 'abcdefgh', 'draft', 0, NULL)", + ]; + for sql in bad_rows { + let outcome = apply_on(connection.as_ref(), WriteKind::RowChanges, &table("lenient"), &[sql]).await; + assert!(outcome.is_err(), "{flavour:?} `{sql}` stored: {outcome:?}"); + } + let myisam = apply_on( + connection.as_ref(), + WriteKind::RowChanges, + &table("lenient_myisam"), + &["INSERT INTO lenient_myisam VALUES (1, 'abcdefgh')"], + ) + .await; + assert!(myisam.is_err(), "{flavour:?} MyISAM stored a cut value: {myisam:?}"); + let not_null = apply_on( + connection.as_ref(), + WriteKind::SchemaChanges, + &table("lenient"), + &["ALTER TABLE lenient MODIFY note int NOT NULL"], + ) + .await; + assert!(not_null.is_err(), "{flavour:?} a NULL became 0: {not_null:?}"); + + assert_eq!( + rows(connection.as_ref(), "SELECT label, state, flag, note FROM lenient").await, + [vec![ + Value::Text("ok".to_owned()), + Value::Text("draft".to_owned()), + Value::Int(1), + Value::Null, + ]], + "{flavour:?}" + ); + assert_eq!( + rows(connection.as_ref(), "SELECT COUNT(*) FROM lenient_myisam").await, + [vec![Value::Int(0)]], + "{flavour:?}" + ); + + let editor = session(connection.as_ref()).await; + let editor_mode = session_scalar(editor.as_ref(), "SELECT @@SESSION.sql_mode").await; + assert!( + !editor_mode.contains("STRICT"), + "{flavour:?}: the editor took the workspace's mode: {editor_mode}" + ); + editor.close().await; + connection.close().await.expect("a close"); + } +} diff --git a/linux/crates/drivers/mysql/tests/integration/main.rs b/linux/crates/drivers/mysql/tests/integration/main.rs index 51a48296eb..f884a5b1ba 100644 --- a/linux/crates/drivers/mysql/tests/integration/main.rs +++ b/linux/crates/drivers/mysql/tests/integration/main.rs @@ -11,9 +11,11 @@ mod ddl; mod decode; mod editor_session; mod flavour_suite; +mod lenient_server; mod metadata; mod non_transactional; mod row_limit; +mod server_time_zone; mod support; mod tls; mod write_batch; diff --git a/linux/crates/drivers/mysql/tests/integration/server_time_zone.rs b/linux/crates/drivers/mysql/tests/integration/server_time_zone.rs new file mode 100644 index 0000000000..c8dc873e40 --- /dev/null +++ b/linux/crates/drivers/mysql/tests/integration/server_time_zone.rs @@ -0,0 +1,232 @@ +//! A server whose own zone is not UTC. The workspace reads TIMESTAMP in +//! UTC, but a save or a schema change has to mean what the same SQL +//! means from any other client, which runs in the server's zone. + +use std::sync::Arc; + +use chrono::NaiveDate; +use tablepro_core::dialect::{BindTarget, SqlDialect}; +use tablepro_core::statement::{GuardedStatement, RowGuard, StatementBuilder}; +use tablepro_core::value::{OffsetTimestamp, Temporal}; +use tablepro_core::{Connection, DriverError, ForeignKeyMode, Value, WriteBatch, WriteKind, WriteReport}; +use tablepro_test_fixtures::{MysqlFixture, MysqlFlavour, MysqlOptions}; + +use crate::support::{ + CALL, FLAVOURS, apply_on, connect, connect_root, rows, run_ok, scalar, session, session_scalar, setup, start_with, + table, text, within, +}; + +fn instant(year: i32, month: u32, day: u32, hour: u32, minute: u32) -> Value { + let stamp = NaiveDate::from_ymd_opt(year, month, day) + .and_then(|date| date.and_hms_opt(hour, minute, 0)) + .expect("a valid time"); + Value::TimestampTz(Temporal::Finite(OffsetTimestamp::from_datetime( + stamp.and_utc().fixed_offset(), + ))) +} + +/// `UPDATE SET v = WHERE at = `, with `at` bound the way +/// the grid binds a TIMESTAMP key. +async fn update_by_instant( + connection: &dyn Connection, + name: &str, + v: i64, + at: Value, +) -> Result { + let dialect = connection.dialect(); + let columns = connection + .fetch_columns(&table(name), within(CALL)) + .await + .expect("the columns"); + let column = |wanted: &str| { + columns + .iter() + .find(|column| column.name == wanted) + .cloned() + .expect("the column") + }; + let mut builder = StatementBuilder::new(); + builder.push_sql(&format!("UPDATE {} SET v = ", dialect.quote_identifier(name))); + builder + .push_value(dialect.as_ref(), Value::Int(v), BindTarget::Column(&column("v"))) + .expect("v binds"); + builder.push_sql(" WHERE at = "); + builder + .push_value(dialect.as_ref(), at, BindTarget::Column(&column("at"))) + .expect("at binds"); + let step = GuardedStatement { + statement: builder.finish(), + guard: RowGuard::AffectedExactly(1), + }; + let batch = WriteBatch::new(WriteKind::RowChanges, table(name), vec![step], ForeignKeyMode::Enforced); + connection.write_batch(&batch, within(CALL)).await +} + +/// An INSERT of `at` as the grid writes it, leaving `created` to its +/// default. +async fn insert_instant(connection: &dyn Connection, at: Value) { + let dialect: Arc = connection.dialect(); + let columns = connection + .fetch_columns(&table("stamped"), within(CALL)) + .await + .expect("the columns"); + let at_column = columns.iter().find(|column| column.name == "at").expect("at"); + let mut builder = StatementBuilder::new(); + builder.push_sql("INSERT INTO stamped (id, at) VALUES (1, "); + builder + .push_value(dialect.as_ref(), at, BindTarget::Column(at_column)) + .expect("at binds"); + builder.push_sql(")"); + let step = GuardedStatement { + statement: builder.finish(), + guard: RowGuard::AffectedExactly(1), + }; + let batch = WriteBatch::new( + WriteKind::RowChanges, + table("stamped"), + vec![step], + ForeignKeyMode::Enforced, + ); + connection.write_batch(&batch, within(CALL)).await.expect("the insert"); +} + +/// The server's zone is +07:00. CURRENT_TIMESTAMP into a DATETIME and an +/// ALTER from DATETIME to TIMESTAMP have to use it, while a TIMESTAMP +/// still reads as its instant. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn writes_and_ddl_use_the_servers_zone() { + for flavour in FLAVOURS { + let fixture = start_with( + MysqlOptions::new(flavour) + .allowing_plaintext() + .with_server_argument("--default-time-zone=+07:00"), + ) + .await; + let connection = connect(&fixture, false).await; + setup( + connection.as_ref(), + &[ + "CREATE TABLE stamped (id int PRIMARY KEY, created datetime DEFAULT CURRENT_TIMESTAMP, \ + at timestamp NULL)", + "CREATE TABLE converted (id int PRIMARY KEY, d datetime NULL)", + "INSERT INTO converted VALUES (1, '2025-01-01 10:00:00')", + ], + ) + .await; + + insert_instant(connection.as_ref(), instant(2025, 1, 1, 5, 0)).await; + apply_on( + connection.as_ref(), + WriteKind::SchemaChanges, + &table("converted"), + &["ALTER TABLE converted MODIFY d timestamp NULL"], + ) + .await + .expect("the ALTER"); + + let editor = session(connection.as_ref()).await; + assert_eq!( + session_scalar(editor.as_ref(), "SELECT @@session.time_zone").await, + "+07:00", + "{flavour:?}" + ); + let drift = session_scalar( + editor.as_ref(), + "SELECT ABS(TIMESTAMPDIFF(MINUTE, created, NOW())) FROM stamped", + ) + .await; + assert!( + drift.parse::().is_ok_and(|minutes| minutes < 5), + "{flavour:?}: CURRENT_TIMESTAMP stored {drift} minutes away from the server's wall time" + ); + assert_eq!( + session_scalar(editor.as_ref(), "SELECT DATE_FORMAT(at, '%Y-%m-%d %H:%i') FROM stamped").await, + "2025-01-01 12:00", + "{flavour:?}: the TIMESTAMP holds another instant" + ); + assert_eq!( + session_scalar( + editor.as_ref(), + "SELECT DATE_FORMAT(d, '%Y-%m-%d %H:%i') FROM converted" + ) + .await, + "2025-01-01 10:00", + "{flavour:?}: the ALTER moved the wall time other clients see" + ); + editor.close().await; + + assert_eq!( + scalar(connection.as_ref(), "SELECT at FROM stamped").await, + instant(2025, 1, 1, 5, 0), + "{flavour:?}: a TIMESTAMP reads as its instant" + ); + for _ in 0..drivers_mysql::WORKSPACE_POOL_MAX_SIZE { + assert_eq!( + text(&scalar(connection.as_ref(), "SELECT @@session.time_zone").await), + "+00:00", + "{flavour:?}: a write left a pool connection in the server's zone" + ); + } + connection.close().await.expect("a close"); + } +} + +/// Load the named zones and make America/New_York the server's zone. +async fn new_york(fixture: &MysqlFixture) { + let root = fixture.root_credentials(); + let load = match fixture.flavour() { + MysqlFlavour::Mysql => "mysql_tzinfo_to_sql /usr/share/zoneinfo 2>/dev/null | mysql -uroot mysql", + MysqlFlavour::Mariadb => "mariadb-tzinfo-to-sql /usr/share/zoneinfo 2>/dev/null | mariadb -uroot mysql", + }; + let output = fixture + .exec(&["sh", "-c", &format!("export MYSQL_PWD='{}'; {load}", root.password)]) + .await + .expect("the zone tables load"); + assert!(output.succeeded(), "{}", output.stderr_text()); + let root = connect_root(fixture).await; + setup(root.as_ref(), &["SET GLOBAL time_zone = 'America/New_York'"]).await; + root.close().await.expect("a close"); +} + +/// 2024-11-03 01:30 happens twice in New York, at 05:30Z and at 06:30Z. +/// A save in that zone cannot tell them apart, so it refuses rather than +/// write to the other row. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn an_instant_in_a_repeated_hour_is_refused() { + for flavour in FLAVOURS { + let fixture = start_with(MysqlOptions::new(flavour).allowing_plaintext()).await; + new_york(&fixture).await; + let connection = connect(&fixture, false).await; + setup( + connection.as_ref(), + &["CREATE TABLE fallback (at timestamp NOT NULL PRIMARY KEY, v int NOT NULL)"], + ) + .await; + let editor = session(connection.as_ref()).await; + run_ok( + editor.as_ref(), + "SET time_zone = '+00:00'; INSERT INTO fallback VALUES \ + ('2024-11-03 05:30:00', 1), ('2024-11-03 06:30:00', 2), ('2024-11-03 08:30:00', 3)", + ) + .await; + editor.close().await; + + let second = update_by_instant(connection.as_ref(), "fallback", 20, instant(2024, 11, 3, 6, 30)).await; + assert!( + matches!(&second, Err(DriverError::RolledBack { source, .. }) if matches!(**source, DriverError::Unsupported { .. })), + "{flavour:?}: {second:?}" + ); + update_by_instant(connection.as_ref(), "fallback", 30, instant(2024, 11, 3, 8, 30)) + .await + .expect("an hour that does not repeat"); + + assert_eq!( + rows(connection.as_ref(), "SELECT v FROM fallback ORDER BY at").await, + [vec![Value::Int(1)], vec![Value::Int(2)], vec![Value::Int(30)]], + "{flavour:?}" + ); + connection.close().await.expect("a close"); + } +} diff --git a/linux/crates/drivers/postgres/src/ddl.rs b/linux/crates/drivers/postgres/src/ddl.rs index fee93d94c4..0e178bddb5 100644 --- a/linux/crates/drivers/postgres/src/ddl.rs +++ b/linux/crates/drivers/postgres/src/ddl.rs @@ -1,4 +1,4 @@ -use tablepro_core::column::{ColumnKind, ColumnType, IntegerKind, IntegerWidth, NetworkFamily}; +use tablepro_core::column::{ColumnKind, ColumnType, IntegerKind, IntegerWidth, NetworkFamily, TextLength}; use tablepro_core::ddl::ddl_sql::{column_list, create_index_sql, default_sql, foreign_key_sql}; use tablepro_core::ddl::{ BuildDdlError, CollationName, ColumnAlteration, ColumnAttributes, ColumnCapabilities, ColumnCompression, @@ -205,6 +205,37 @@ impl DdlDialect for PgDialect { } } +/// The type the USING clause casts to when the new type has a length. +/// +/// An explicit cast to a length cuts what does not fit: +/// `'abcdefghijkl'::varchar(10)` is `abcdefghij`, and the same goes for +/// `char(n)` and `bit(n)`. So the cast stops at the type without one, +/// and the server's own assignment to the column, which ALTER applies to +/// the USING result, refuses a value that is too long, as an INSERT +/// would. A bit string takes the cast to `varbit` only from another bit +/// string: an integer has a cast to `bit(n)` and none to `varbit`. +fn unlimited_cast(original: &ColumnKind, target: &ColumnKind) -> Option { + match (original, target) { + ( + _, + ColumnKind::Text { + length: TextLength::Fixed(_), + }, + ) => Some("bpchar".to_owned()), + ( + _, + ColumnKind::Text { + length: TextLength::Varying(_), + }, + ) => Some("varchar".to_owned()), + (ColumnKind::BitString { .. }, ColumnKind::BitString { .. }) => Some("varbit".to_owned()), + (ColumnKind::Array { element: original }, ColumnKind::Array { element: target }) => { + unlimited_cast(original, target).map(|element| format!("{element}[]")) + } + _ => None, + } +} + impl PgDialect { /// The ALTER TABLE actions that change one column, and the sequence /// restart an identity it gains needs after them. @@ -231,9 +262,11 @@ impl PgDialect { .filter(|_| collatable(&resolved.column_type.kind)) .map(|collation| format!(" COLLATE {}", self.collation(collation))) .unwrap_or_default(); + let using = unlimited_cast(&alteration.original.info.column_type.kind, &resolved.column_type.kind) + .unwrap_or_else(|| resolved.sql.clone()); actions.push(format!( - "ALTER COLUMN {name} TYPE {}{collation} USING {name}::{}", - resolved.sql, resolved.sql + "ALTER COLUMN {name} TYPE {}{collation} USING {name}::{using}", + resolved.sql )); actions.extend(restated_storage(&name, &alteration.original.attributes, &resolved)); } @@ -767,7 +800,7 @@ mod tests { sql(plan), [ "ALTER TABLE \"app\".\"users\" ALTER COLUMN \"email\" TYPE varchar(320) COLLATE \ - \"pg_catalog\".\"C\" USING \"email\"::varchar(320), ALTER COLUMN \"code\" TYPE integer USING \ + \"pg_catalog\".\"C\" USING \"email\"::varchar, ALTER COLUMN \"code\" TYPE integer USING \ \"code\"::integer" ] ); @@ -775,6 +808,20 @@ mod tests { assert!(!collatable(&dialect().parse_type("uuid").expect("a type").kind)); } + #[test] + fn a_length_cast_stops_at_the_type_without_one() { + let kind = |declaration: &str| dialect().parse_type(declaration).expect("a type").kind; + let using = |from: &str, to: &str| unlimited_cast(&kind(from), &kind(to)); + + assert_eq!(using("varchar(255)", "varchar(10)").as_deref(), Some("varchar")); + assert_eq!(using("text", "character(5)").as_deref(), Some("bpchar")); + assert_eq!(using("bit(8)", "bit(4)").as_deref(), Some("varbit")); + assert_eq!(using("varchar(20)[]", "varchar(5)[]").as_deref(), Some("varchar[]")); + assert_eq!(using("integer", "bit(4)"), None, "an integer has no cast to varbit"); + assert_eq!(using("text", "integer"), None); + assert_eq!(using("numeric(10,2)", "numeric(5,2)"), None); + } + #[test] fn a_type_change_restates_storage_and_compression_the_new_type_takes() { let mut original = users(); @@ -793,7 +840,7 @@ mod tests { sql(plan), [ "ALTER TABLE \"app\".\"users\" ALTER COLUMN \"email\" TYPE varchar(320) USING \ - \"email\"::varchar(320), ALTER COLUMN \"email\" SET STORAGE EXTERNAL, ALTER COLUMN \"email\" SET \ + \"email\"::varchar, ALTER COLUMN \"email\" SET STORAGE EXTERNAL, ALTER COLUMN \"email\" SET \ COMPRESSION lz4, ALTER COLUMN \"status\" TYPE bigint USING \"status\"::bigint" ] ); diff --git a/linux/crates/drivers/postgres/src/dialect.rs b/linux/crates/drivers/postgres/src/dialect.rs index 4c3de9db06..66ee5f3123 100644 --- a/linux/crates/drivers/postgres/src/dialect.rs +++ b/linux/crates/drivers/postgres/src/dialect.rs @@ -325,6 +325,11 @@ fn literal(value: &Value) -> Result { reason: format!("{:?}", undecodable.reason), }); } + // The export's JSON array text is no array input PostgreSQL + // reads, and ARRAY[...] would type text elements as text[], + // which an int[] column refuses. A quoted array literal takes + // the column's own type. + Value::Array(elements) => quoted(&array_literal(elements)?), other => match value_to_text(other) { Some(text) => quoted(&text), None => { @@ -336,6 +341,40 @@ fn literal(value: &Value) -> Result { }) } +/// PostgreSQL's array input: `{...}` with each element in double +/// quotes, so a comma, a brace, a space or the text `NULL` stays the +/// element's own, and an unquoted NULL for a missing one. A nested array +/// is a dimension. +fn array_literal(elements: &[Value]) -> Result { + let mut parts = Vec::with_capacity(elements.len()); + for element in elements { + parts.push(match element { + Value::Null => "NULL".to_owned(), + Value::Array(inner) => array_literal(inner)?, + Value::Undecodable(undecodable) => { + return Err(LiteralError::Undecodable { + reason: format!("{:?}", undecodable.reason), + }); + } + // bytea's own input, where the export text would be 0x... + Value::Bytes(bytes) => array_element(&format!("\\x{}", tablepro_core::hex::encode_lower(bytes))), + other => match value_to_text(other) { + Some(text) => array_element(&text), + None => { + return Err(LiteralError::Unrepresentable { + found: other.variant_name(), + }); + } + }, + }); + } + Ok(format!("{{{}}}", parts.join(","))) +} + +fn array_element(text: &str) -> String { + format!("\"{}\"", text.replace('\\', "\\\\").replace('"', "\\\"")) +} + /// A string literal that reads the same whatever /// `standard_conforming_strings` is. fn quoted(text: &str) -> String { @@ -599,6 +638,35 @@ mod tests { assert_eq!(literal(&Value::Date(Temporal::Infinity)), Ok("'infinity'".to_owned())); } + #[test] + fn an_array_is_an_array_literal_postgres_reads() { + assert_eq!( + literal(&Value::Array(vec![Value::Int(1), Value::Int(2), Value::Null])), + Ok("'{\"1\",\"2\",NULL}'".to_owned()) + ); + assert_eq!( + literal(&Value::Array(vec![ + Value::Text("a,b".to_owned()), + Value::Text("NULL".to_owned()), + Value::Text("say \"hi\"".to_owned()), + Value::Text("it's".to_owned()), + ])), + Ok("E'{\"a,b\",\"NULL\",\"say \\\\\"hi\\\\\"\",\"it''s\"}'".to_owned()) + ); + assert_eq!( + literal(&Value::Array(vec![ + Value::Array(vec![Value::Int(1), Value::Int(2)]), + Value::Array(vec![Value::Int(3), Value::Int(4)]), + ])), + Ok("'{{\"1\",\"2\"},{\"3\",\"4\"}}'".to_owned()) + ); + assert_eq!(literal(&Value::Array(Vec::new())), Ok("'{}'".to_owned())); + assert_eq!( + literal(&Value::Array(vec![Value::Bytes(vec![0xab])])), + Ok("E'{\"\\\\\\\\xab\"}'".to_owned()) + ); + } + #[test] fn like_and_keyset_spelling() { let dialect = dialect(); diff --git a/linux/crates/drivers/postgres/src/editor/run.rs b/linux/crates/drivers/postgres/src/editor/run.rs index 86c1bd2050..cca1a904cb 100644 --- a/linux/crates/drivers/postgres/src/editor/run.rs +++ b/linux/crates/drivers/postgres/src/editor/run.rs @@ -336,11 +336,7 @@ impl<'a> Run<'a> { // A cursor stops one row past the limit, so how // many rows there were in all is not known. total_rows: (!(cursor && truncated)).then_some(set.total), - truncation: if truncated { - Truncation::ReadPastLimit - } else { - Truncation::Complete - }, + truncation: truncation(truncated, cursor), }) .await; *set = ResultSet::default(); @@ -391,6 +387,18 @@ impl<'a> Run<'a> { } } +/// Why a set ended where it did. A cursor that stopped at the limit +/// stopped the statement with it: what the statement does per row, a +/// function it calls in its select list included, never happened for the +/// rows past the limit, and that is what the result has to say. +fn truncation(truncated: bool, cursor: bool) -> Truncation { + match (truncated, cursor) { + (false, _) => Truncation::Complete, + (true, true) => Truncation::ServerStopped, + (true, false) => Truncation::ReadPastLimit, + } +} + /// How a run that was stopped ends. The server's 57014 is the /// acknowledgement; any other error was the statement's own. fn end_after_stop(interrupt: Interrupt, error: &DriverError) -> RunEnd { @@ -414,6 +422,13 @@ mod tests { use super::*; + #[test] + fn a_cursor_cut_at_the_limit_says_the_statement_stopped() { + assert_eq!(truncation(true, true), Truncation::ServerStopped); + assert_eq!(truncation(true, false), Truncation::ReadPastLimit); + assert_eq!(truncation(false, true), Truncation::Complete); + } + #[test] fn a_stopped_run_ends_by_what_stopped_it() { let acknowledged = DriverError::Timeout { diff --git a/linux/crates/drivers/postgres/tests/integration/ddl.rs b/linux/crates/drivers/postgres/tests/integration/ddl.rs index db06c3dcc2..efe090bc5c 100644 --- a/linux/crates/drivers/postgres/tests/integration/ddl.rs +++ b/linux/crates/drivers/postgres/tests/integration/ddl.rs @@ -44,6 +44,80 @@ async fn change(connection: &dyn Connection, table: &TableRef, edit: impl Fn(&mu read_table(connection, table).await } +/// The plan for one column's new type, run as the structure tab saves. +async fn retype( + connection: &dyn Connection, + column: &str, + declaration: &str, +) -> Result<(), tablepro_core::DriverError> { + let before = read(connection).await; + let mut draft = TableDraft::from_definition(&before); + for draft_column in &mut draft.columns { + if draft_column.name == column { + draft_column.column_type = DraftType::Typed(declaration.to_owned()); + } + } + let plan = connection + .dialect() + .ddl() + .expect("PostgreSQL plans its DDL") + .plan_alter_table(&before, &diff_table(&before, &draft)) + .expect("the change plans"); + connection + .write_batch(&plan.into_write_batch(kept()), within(CALL)) + .await + .map(|_| ()) +} + +/// An explicit cast to `varchar(10)`, `char(5)` or `bit(4)` cuts the +/// value to fit, so a narrowing has to fail on a value that does not, +/// as an INSERT of it would. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn narrowing_refuses_a_value_that_does_not_fit() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + setup( + connection.as_ref(), + &[ + "CREATE TABLE kept (id int PRIMARY KEY, label varchar(255), code char(10), flags bit(8), \ + tags varchar(20)[], n int)", + "INSERT INTO kept VALUES (1, repeat('x', 20), 'abcdefghij', B'10101010', ARRAY['abcdefghijkl'], 12345)", + ], + ) + .await; + + for (column, narrower) in [ + ("label", "varchar(10)"), + ("code", "char(5)"), + ("flags", "bit(4)"), + ("tags", "varchar(5)[]"), + ] { + let outcome = retype(connection.as_ref(), column, narrower).await; + assert!(outcome.is_err(), "{column} to {narrower} cut the value: {outcome:?}"); + } + assert_eq!( + value( + connection.as_ref(), + "SELECT length(label) || '|' || code || '|' || flags::text || '|' || tags[1] FROM kept" + ) + .await, + "20|abcdefghij|10101010|abcdefghijkl" + ); + + retype(connection.as_ref(), "label", "varchar(30)") + .await + .expect("a value that fits"); + retype(connection.as_ref(), "n", "bit(16)") + .await + .expect("an integer still converts to a bit string"); + assert_eq!( + value(connection.as_ref(), "SELECT length(label) || '|' || n::text FROM kept").await, + "20|0011000000111001" + ); + connection.close().await.expect("a close"); +} + fn type_of(definition: &TableDefinition, column: &str) -> String { definition .column(column) diff --git a/linux/crates/drivers/postgres/tests/integration/decode.rs b/linux/crates/drivers/postgres/tests/integration/decode.rs index 59dd97d33e..00c388f41f 100644 --- a/linux/crates/drivers/postgres/tests/integration/decode.rs +++ b/linux/crates/drivers/postgres/tests/integration/decode.rs @@ -148,6 +148,50 @@ async fn f006_round_trip_matrix() { } } +/// SQL export and Copy As INSERT write an array through the dialect's +/// literal, which PostgreSQL has to read back as the same array. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn an_exported_array_inserts_back_as_it_was() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + let shape = "(id int PRIMARY KEY, i int[], t text[], m int[][], b bytea[], e int[])"; + setup( + connection.as_ref(), + &[ + &format!("CREATE TABLE arrays {shape}"), + &format!("CREATE TABLE copied {shape}"), + r#"INSERT INTO arrays VALUES (1, '{1,2,NULL}', ARRAY['a,b', 'NULL', NULL, 'say "hi"', 'back\slash', 'it''s', ''], + '{{1,2},{3,4}}', ARRAY['\xab00'::bytea], '{}')"#, + ], + ) + .await; + let select = |table: &str| format!("SELECT id, i, t, m, b, e FROM {table}"); + let read = connection + .query_bound( + &crate::support::statement(&select("arrays")), + RowLimit::EDITOR_DEFAULT, + within(CALL), + ) + .await + .expect("the read"); + + let insert = + tablepro_core::export::render_sql_insert(connection.dialect().as_ref(), "copied", &read.columns, &read.rows); + crate::support::apply( + connection.as_ref(), + tablepro_core::WriteKind::RowChanges, + &[insert.trim_end().trim_end_matches(';')], + ) + .await + .expect("the exported INSERT runs"); + + assert_eq!( + crate::support::rows(connection.as_ref(), &select("copied")).await, + read.rows + ); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[ignore = "requires docker"] async fn an_empty_page_still_has_its_columns() { diff --git a/linux/crates/drivers/postgres/tests/integration/editor_session.rs b/linux/crates/drivers/postgres/tests/integration/editor_session.rs index faaf76396b..058eca484d 100644 --- a/linux/crates/drivers/postgres/tests/integration/editor_session.rs +++ b/linux/crates/drivers/postgres/tests/integration/editor_session.rs @@ -131,7 +131,7 @@ async fn select_inside_begin_uses_savepoint_cursor_row_limit() { events.finished().as_slice(), [ResultEvent::ResultSetFinished { retained: 100, - truncation: Truncation::ReadPastLimit, + truncation: Truncation::ServerStopped, .. }] )); @@ -141,6 +141,51 @@ async fn select_inside_begin_uses_savepoint_cursor_row_limit() { assert_eq!(committed.transaction_state, TransactionState::Idle); } +/// The cursor stops the statement at the limit, so a function it calls +/// per row ran for the rows fetched and no others, and the driver's +/// COMMIT keeps that. The result has to say the statement stopped, not +/// only that the grid shows part of it. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn select_with_side_effects_says_it_stopped_at_the_limit() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + setup( + connection.as_ref(), + &[ + "CREATE TABLE orders AS SELECT generate_series(1, 5000) AS id", + "CREATE TABLE archived (id int)", + "CREATE FUNCTION archive_order(order_id int) RETURNS int LANGUAGE sql VOLATILE AS \ + 'INSERT INTO archived VALUES (order_id) RETURNING order_id'", + ], + ) + .await; + let session = session(connection.as_ref()).await; + + let (summary, events) = run_with(session.as_ref(), "SELECT archive_order(id) FROM orders", limited(100)).await; + + assert_eq!(summary.end, RunEnd::Completed); + assert!( + matches!( + events.finished().as_slice(), + [ResultEvent::ResultSetFinished { + retained: 100, + total_rows: None, + truncation: Truncation::ServerStopped, + }] + ), + "{:?}", + events.finished() + ); + let Value::Int(archived) = scalar(connection.as_ref(), "SELECT count(*) FROM archived").await else { + panic!("a count"); + }; + assert!( + (100..5000).contains(&archived), + "the statement ran for {archived} rows, not only up to the limit" + ); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[ignore = "requires docker"] async fn delete_returning_50000_limit_100_deletes_all() { diff --git a/linux/docs/drivers/mysql.md b/linux/docs/drivers/mysql.md index 446613d2e2..3b84577ceb 100644 --- a/linux/docs/drivers/mysql.md +++ b/linux/docs/drivers/mysql.md @@ -42,10 +42,26 @@ SET SESSION innodb_lock_wait_timeout = 10 - Lock waits get the workspace's 10 s, in whole seconds rounded up. - Numbers are sent as integers, never floats a server could read in another locale. -A workspace session also pins two things the app's generated SQL depends on: `time_zone = '+00:00'`, so a `TIMESTAMP` reads as the instant it names, and `sql_mode` without `NO_BACKSLASH_ESCAPES`, because generated DDL writes literals such as `COMMENT '...'` that have no hex form. An editor session keeps the server's own zone and `sql_mode`, as the mysql client would. +A workspace session also pins what the app's generated SQL depends on: + +- `time_zone = '+00:00'` for reads, so a `TIMESTAMP` reads as the instant it names. UTC repeats no hour, so the read is exact. +- `sql_mode` without `NO_BACKSLASH_ESCAPES`, because generated DDL writes literals such as `COMMENT '...'` that have no hex form. +- `sql_mode` with `STRICT_ALL_TABLES`. Without strict mode the server stores what it can and only warns: a too-long string is cut, an unknown `ENUM` label becomes `''`, a number out of range is clamped, and a column made `NOT NULL` turns its NULLs into `0` or `''`. Strict mode makes each of these an error, so a save, an import or a schema change stores what it was given or fails. + +An editor session keeps the server's own zone and `sql_mode`, as the mysql client would. sqlx's own session statements (`PIPES_AS_CONCAT`, `NO_ENGINE_SUBSTITUTION`, its `SET NAMES`) are turned off. Its time zone is off too, except on a workspace session, where sqlx's `time_zone` option sends the `+00:00` above. Its statement cache is 0 so no prepared statement outlives its call, and its statement logging is off. +### Time zone of a write + +A save or a schema change runs in the server's own zone, `@@GLOBAL.time_zone`, and the batch puts `+00:00` back when it ends; a connection where that fails is closed. In UTC, `DATETIME DEFAULT CURRENT_TIMESTAMP` would store UTC wall time, 7 hours off on a `+07:00` server, and an `ALTER` from `DATETIME` to `TIMESTAMP` would shift every value. + +- A `TIMESTAMP` value binds as its UTC wall time inside `CONVERT_TZ(?, '+00:00', @@session.time_zone)`, which does nothing on a read and gives the server's wall time on a write. +- In the hour a daylight-saving zone repeats, one wall time names two instants and the server picks one, so a key compared there can match the other row. Before a write sends anything, each instant it binds goes to the server's zone and back; one that changes refuses the batch. A zone written as an offset (`+07:00`) skips the check. +- A `TIMESTAMP` column's constant default is read from `SHOW CREATE TABLE` and converted to the server's zone, so a `MODIFY` restates the same instant. MySQL's `COLUMN_DEFAULT` keeps the text in the zone of whoever last altered the table, so it is not used for this. + +`crates/drivers/mysql/src/server_zone.rs` has the reasoning. + ## TLS - `Disable` connects without TLS. A server with `require_secure_transport` answers with error 3159, reported as `ServerRequiresTls`. diff --git a/linux/docs/drivers/postgresql.md b/linux/docs/drivers/postgresql.md index 435adeaf79..2c41388cee 100644 --- a/linux/docs/drivers/postgresql.md +++ b/linux/docs/drivers/postgresql.md @@ -117,7 +117,7 @@ CLOSE tablepro_cursor COMMIT -- or RELEASE SAVEPOINT tablepro_cursor_savepoint ``` -- One row past the limit tells a full result from a cut one. A cut result has `total_rows: None`. +- One row past the limit tells a full result from a cut one. A cut result has `total_rows: None` and `Truncation::ServerStopped`: the statement never ran past the rows fetched, so a function it calls per row, such as `SELECT archive_order(id) FROM orders`, ran for those rows only, and the driver's `COMMIT` keeps what they did. The result says so rather than read the rest: `MOVE FORWARD ALL` would run the whole statement for every plain `SELECT` too, which is what the limit is there to avoid. - Each step goes as its own message, so a trailing `--` comment in the statement cannot swallow the `FETCH`. - A `DECLARE` the server refuses because the statement cannot be a cursor (a data-modifying `WITH`: 0A000; `SELECT INTO`: 42601) is rolled back to the savepoint or out of the driver's transaction, and the statement runs again as a plain statement. Inside the user's transaction any other failure aborts their transaction, as it would without the limit. - Error positions are corrected for the `DECLARE` prefix, so they count from the start of the user's statement. diff --git a/linux/po/tablepro.pot b/linux/po/tablepro.pot index 09abf6971b..4b3e40d007 100644 --- a/linux/po/tablepro.pot +++ b/linux/po/tablepro.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: tablepro 0.1.0\n" "Report-Msgid-Bugs-To: https://github.com/TableProApp/TablePro/issues\n" -"POT-Creation-Date: 2026-10-17 11:27+0000\n" +"POT-Creation-Date: 2026-10-17 11:45+0000\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -2909,6 +2909,14 @@ msgid_plural "Showing the first {shown} rows" msgstr[0] "" msgstr[1] "" +#: crates/app/src/ui/result_pane/result_summary.rs +#, rust-format +msgid "Stopped after the first {shown} row: the statement did not run past it" +msgid_plural "" +"Stopped after the first {shown} rows: the statement did not run past them" +msgstr[0] "" +msgstr[1] "" + #: crates/app/src/ui/result_pane/result_summary.rs #, rust-format msgid "{rows}+" @@ -3530,6 +3538,13 @@ msgid "" "hold is converted or refused." msgstr "" +#: crates/app/src/ui/structure_tab/review_dialog.rs +#, rust-format +msgid "" +"Makes column “{column}” NOT NULL. A NULL already in it is converted or " +"refused." +msgstr "" + #: crates/app/src/ui/structure_tab/review_dialog.rs #, rust-format msgid "Drops index “{index}”."