From 2a6306270417419f74c0c3da6fd5af6a08b4b9eb Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 00:44:26 +0700 Subject: [PATCH 1/9] feat(linux): open referenced and referencing rows from a foreign key --- linux/ROADMAP.md | 4 +- .../app/src/application/accelerators.rs | 1 + .../app/src/ui/browse_tab/browse_command.rs | 2 + .../src/ui/browse_tab/browse_request_kind.rs | 3 + .../app/src/ui/browse_tab/browse_tab_input.rs | 1 + .../src/ui/browse_tab/clipboard_handlers.rs | 1 + .../app/src/ui/browse_tab/clipboard_input.rs | 3 + .../app/src/ui/browse_tab/command_handlers.rs | 21 + linux/crates/app/src/ui/browse_tab/fetch.rs | 28 ++ linux/crates/app/src/ui/browse_tab/imp.rs | 3 + linux/crates/app/src/ui/browse_tab/mod.rs | 52 +- .../app/src/ui/browse_tab/table_filter.rs | 25 + .../app/src/ui/browse_tab/tests/load.rs | 8 +- .../crates/app/src/ui/browse_tab/tests/mod.rs | 80 ++- .../app/src/ui/browse_tab/tests/references.rs | 199 ++++++++ linux/crates/app/src/ui/error_text/edit.rs | 2 + linux/crates/app/src/ui/error_text/mod.rs | 2 + .../crates/app/src/ui/error_text/reference.rs | 54 ++ linux/crates/app/src/ui/filter_strip/mod.rs | 4 +- .../app/src/ui/filter_strip/rule_operators.rs | 20 - linux/crates/app/src/ui/filter_strip/tests.rs | 8 +- linux/crates/app/src/ui/grid/context_menu.rs | 12 +- linux/crates/app/src/ui/grid/grid_keyboard.rs | 84 +++- linux/crates/app/src/ui/grid/grid_menus.rs | 97 +++- linux/crates/app/src/ui/grid/grid_msg.rs | 4 + linux/crates/app/src/ui/grid/mod.rs | 3 + .../crates/app/src/ui/grid/reference_menu.rs | 474 ++++++++++++++++++ .../app/src/ui/grid/tab_grid_context.rs | 10 +- linux/crates/app/src/ui/grid/table_keys.rs | 46 ++ linux/crates/app/src/ui/grid/test_grid.rs | 34 +- linux/crates/app/src/ui/menu_label.rs | 18 + linux/crates/app/src/ui/mod.rs | 2 + .../app/src/ui/picker/connection_menu.rs | 8 +- .../app/src/ui/session_window/tests/mod.rs | 6 +- .../app/src/ui/structure_tab/form_dialog.rs | 11 - .../app/src/ui/structure_tab/index_dialog.rs | 3 +- linux/crates/app/src/ui/workspace/mod.rs | 18 + .../crates/app/src/ui/workspace/references.rs | 109 ++++ linux/crates/app/src/ui/workspace/shell.rs | 1 + .../app/src/ui/workspace/sidebar/tests.rs | 1 + .../crates/app/src/ui/workspace/structure.rs | 1 + .../app/src/ui/workspace/tab_factory.rs | 65 ++- linux/crates/app/src/ui/workspace/tab_spec.rs | 2 + .../crates/app/src/ui/workspace/tests/mod.rs | 1 + .../app/src/ui/workspace/tests/references.rs | 229 +++++++++ .../app/src/ui/workspace/tests/tab_title.rs | 1 + .../src/ui/workspace/workspace_persistence.rs | 77 ++- .../app/src/ui/workspace/workspace_tabs.rs | 9 +- linux/crates/app/src/workspace/tab_record.rs | 9 + .../app/src/workspace/workspace_record.rs | 4 + .../src/workspace/workspace_record_builder.rs | 1 + .../app/src/workspace/workspace_store.rs | 1 + linux/crates/app/tests/accessibility/main.rs | 1 + linux/crates/core/src/browse/mod.rs | 4 + .../crates/core/src/browse/reference_error.rs | 30 ++ linux/crates/core/src/browse/row_reference.rs | 321 ++++++++++++ linux/crates/core/src/connection.rs | 11 +- .../crates/core/src/edit/edit_parse_error.rs | 2 + .../core/src/edit/parse_literal_text.rs | 34 +- linux/crates/core/src/filter/build_filter.rs | 10 + linux/crates/core/src/filter/operators_for.rs | 24 +- linux/crates/core/src/hex.rs | 32 ++ .../drivers/clickhouse/src/connection.rs | 11 +- linux/crates/drivers/mssql/src/connection.rs | 17 +- linux/crates/drivers/mssql/src/metadata.rs | 15 +- .../drivers/mssql/tests/integration/filter.rs | 38 ++ .../mssql/tests/integration/metadata.rs | 5 + linux/crates/drivers/mysql/src/connection.rs | 17 +- linux/crates/drivers/mysql/src/metadata.rs | 28 +- .../mysql/tests/integration/metadata.rs | 6 + .../postgres/src/catalog/table_definition.rs | 7 +- .../crates/drivers/postgres/src/connection.rs | 17 +- .../postgres/src/metadata/foreign_keys.rs | 5 + .../drivers/postgres/src/metadata/mod.rs | 19 +- .../postgres/tests/integration/filter.rs | 25 + .../postgres/tests/integration/metadata.rs | 84 ++++ .../crates/drivers/sqlite/src/catalog/mod.rs | 4 + linux/crates/drivers/sqlite/src/connection.rs | 14 +- linux/crates/drivers/sqlite/src/metadata.rs | 158 ++++-- .../sqlite/tests/integration/catalog.rs | 108 ++++ .../sqlite/tests/integration/dialect.rs | 21 + .../crates/test-support/src/call_counters.rs | 1 + .../test-support/src/connection_script.rs | 4 +- .../test-support/src/fake_connection.rs | 20 +- linux/crates/test-support/src/replies.rs | 5 + linux/data/resources/shortcuts-dialog.ui | 6 + linux/docs/adding-drivers.md | 2 +- linux/po/POTFILES.in | 8 + 88 files changed, 2766 insertions(+), 210 deletions(-) create mode 100644 linux/crates/app/src/ui/browse_tab/table_filter.rs create mode 100644 linux/crates/app/src/ui/browse_tab/tests/references.rs create mode 100644 linux/crates/app/src/ui/error_text/reference.rs create mode 100644 linux/crates/app/src/ui/grid/reference_menu.rs create mode 100644 linux/crates/app/src/ui/grid/table_keys.rs create mode 100644 linux/crates/app/src/ui/menu_label.rs create mode 100644 linux/crates/app/src/ui/workspace/references.rs create mode 100644 linux/crates/app/src/ui/workspace/tests/references.rs create mode 100644 linux/crates/core/src/browse/reference_error.rs create mode 100644 linux/crates/core/src/browse/row_reference.rs diff --git a/linux/ROADMAP.md b/linux/ROADMAP.md index 4252135c22..340d2fc403 100644 --- a/linux/ROADMAP.md +++ b/linux/ROADMAP.md @@ -43,12 +43,13 @@ The app runs on GNOME 50 ([ADR 0015](docs/decisions/0015-gnome-50-platform-floor | Capability | Detail | |---|---| | Data grid | A virtualized `GtkColumnView` with keyboard-focusable cells, pages of 100 to 10,000 rows, and a pager with an exact or estimated row count; a table of about 100,000 rows or fewer is counted on its own, a larger one offers Count Rows | -| Filters | Per-column operators, a raw WHERE fragment checked as one expression, match any or all, kept per table | +| Filters | Per-column operators, a raw WHERE fragment checked as one expression, match any or all; binary columns compare to `0x` hex. Kept per table, and per tab across a restart or a reopen | | Sorting | A header click sorts tables on the server and query results in the grid, one column at a time | | Cell editing | Editors by column kind: inline text, a multi-line editor, a calendar for dates, a toggle for booleans, JSON checked before saving; values are edited from their exact text, never from what the cell displays. Table tabs only, on tables with a primary key; key, generated and auto-increment columns, and binary, spatial, array and composite values, are read-only | | Rows | Insert, duplicate and delete; Set NULL and Empty, and DEFAULT on a new row; undo and redo; save or discard pending changes, and stop a save. On ClickHouse a save is not atomic, and Stop takes effect between statements | | Bulk delete | Deleting 10 rows or more asks first, unless Confirm Before Deleting Rows is off in Preferences | | Copy | Copy; Copy As rows, with headers, JSON, CSV with or without headers, Markdown or IN clause, and INSERT in an editable table tab; Copy Column Name; Show Row as JSON | +| Foreign keys | Open Referenced Row (Ctrl+J) on a key cell opens the table it points at in a new tab, filtered on every column of the key; Show Referencing Rows on a saved row opens a table whose key points at it. A tab already showing those rows comes to the front | | Column widths | Kept per table; Widen Column and Narrow Column in the cell menu (Shift+Alt+Right and Shift+Alt+Left) change them from the keyboard | | CSV import | Into an existing table, with a column mapping, in one write batch (one transaction where the engine has them), up to 64 MiB | | Sidebar | Tables (partitioned ones included), views, materialized views and foreign tables by schema, with search and Refresh, read again after the editor runs DDL, Open in New Tab and Show CREATE TABLE; Edit Structure, Import CSV and Drop Table for tables | @@ -134,7 +135,6 @@ Not built yet, in no order. - Save the column order per table. - Functions, procedures, sequences and triggers in the sidebar. The drivers already list the first three. - The CREATE INDEX statement for one index. Show CREATE TABLE lists them after the table on PostgreSQL, SQL Server and SQLite (with the triggers on SQLite). -- Jump from a foreign key to the table it references. - Reorder columns in the structure tab by dragging, with the ALTER previewed. - An ER diagram. diff --git a/linux/crates/app/src/application/accelerators.rs b/linux/crates/app/src/application/accelerators.rs index 619ac096de..d8b794f0c0 100644 --- a/linux/crates/app/src/application/accelerators.rs +++ b/linux/crates/app/src/application/accelerators.rs @@ -159,6 +159,7 @@ pub const ACCELERATORS: &[Accelerator] = &[ component(GridCell, "cell.toggle", &["space"], Listed(DataGrid)), component(GridCell, "cell.menu", &["Menu", "F10"], Listed(DataGrid)), component(GridCell, "cell.set-null", &["BackSpace"], Listed(DataGrid)), + component(GridCell, "cell.open-referenced-row", &["j"], Listed(DataGrid)), // GtkTreeView's keys for its focused header. A column view's header // takes no focus, so they act on the focused cell's column. component(GridCell, "cell.widen-column", &["Right"], Listed(DataGrid)), diff --git a/linux/crates/app/src/ui/browse_tab/browse_command.rs b/linux/crates/app/src/ui/browse_tab/browse_command.rs index 869b266c03..3f1a0dbcd2 100644 --- a/linux/crates/app/src/ui/browse_tab/browse_command.rs +++ b/linux/crates/app/src/ui/browse_tab/browse_command.rs @@ -6,6 +6,7 @@ use tablepro_session::OperationError; use super::page_reply::PageReply; use crate::runtime::Stamped; +use crate::ui::grid::TableKeys; use crate::ui::tab_load::LoadFailure; /// The table's columns from the session's catalog, and how its rows are @@ -24,4 +25,5 @@ pub(crate) enum BrowseCommand { Page(Stamped>), Estimate(Stamped>), Count(Stamped>), + Keys(Stamped>), } diff --git a/linux/crates/app/src/ui/browse_tab/browse_request_kind.rs b/linux/crates/app/src/ui/browse_tab/browse_request_kind.rs index a7aeb39eb5..0421fc0ef8 100644 --- a/linux/crates/app/src/ui/browse_tab/browse_request_kind.rs +++ b/linux/crates/app/src/ui/browse_tab/browse_request_kind.rs @@ -10,4 +10,7 @@ pub(crate) enum BrowseRequestKind { Estimate, /// The exact total, which can scan the whole table. Count, + /// The table's foreign keys both ways, which the grid follows. Read + /// with each read of the columns, and never in the way of the rows. + Keys, } diff --git a/linux/crates/app/src/ui/browse_tab/browse_tab_input.rs b/linux/crates/app/src/ui/browse_tab/browse_tab_input.rs index 23cd1c4b17..870dd93490 100644 --- a/linux/crates/app/src/ui/browse_tab/browse_tab_input.rs +++ b/linux/crates/app/src/ui/browse_tab/browse_tab_input.rs @@ -70,6 +70,7 @@ impl From for BrowseTabInput { GridMsg::DeleteRows { rows } => EditInput::DeleteRows(rows).into(), GridMsg::InsertRow => EditInput::InsertRow.into(), GridMsg::DuplicateRows { rows } => EditInput::DuplicateRows(rows).into(), + GridMsg::FollowReference(reference) => ClipboardInput::GridFollowReference(reference).into(), } } } 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 cbd22f6ede..56fc4b9c72 100644 --- a/linux/crates/app/src/ui/browse_tab/clipboard_handlers.rs +++ b/linux/crates/app/src/ui/browse_tab/clipboard_handlers.rs @@ -29,6 +29,7 @@ impl BrowseModel { } ClipboardInput::GridShowRowJson(request) => fx.output(BrowseTabOutput::ShowRowJson(request)), ClipboardInput::GridShowToast(message) => fx.output(BrowseTabOutput::ShowToast(message)), + ClipboardInput::GridFollowReference(reference) => fx.output(BrowseTabOutput::FollowReference(reference)), } } diff --git a/linux/crates/app/src/ui/browse_tab/clipboard_input.rs b/linux/crates/app/src/ui/browse_tab/clipboard_input.rs index 209743dfdc..a6093bc3b9 100644 --- a/linux/crates/app/src/ui/browse_tab/clipboard_input.rs +++ b/linux/crates/app/src/ui/browse_tab/clipboard_input.rs @@ -1,3 +1,5 @@ +use tablepro_core::browse::RowReference; + use crate::ui::clipboard::{CopyRequest, InClauseRows}; use crate::ui::export::ExportSource; use crate::ui::grid::{RowJsonRequest, RowObject}; @@ -16,4 +18,5 @@ pub(crate) enum ClipboardInput { PasteNotSupported, GridShowRowJson(RowJsonRequest), GridShowToast(String), + GridFollowReference(RowReference), } diff --git a/linux/crates/app/src/ui/browse_tab/command_handlers.rs b/linux/crates/app/src/ui/browse_tab/command_handlers.rs index 6e2d0adfc3..b9f6e740aa 100644 --- a/linux/crates/app/src/ui/browse_tab/command_handlers.rs +++ b/linux/crates/app/src/ui/browse_tab/command_handlers.rs @@ -24,6 +24,9 @@ impl BrowseModel { } self.identity = Some(reply.identity); self.on_columns_loaded(reply.columns.to_vec(), fx); + // A table whose columns changed may have other + // keys too. + self.after_columns.insert(BrowseRequestKind::Keys); for kind in std::mem::take(&mut self.after_columns) { self.fetch(kind, fx); } @@ -65,6 +68,24 @@ impl BrowseModel { self.retry_if_link_moved(BrowseRequestKind::Estimate, fx); } } + // The keys only add to the menus, so a failure leaves the tab + // as it is, with the keys it had, and is asked again on the + // next connection. + BrowseCommand::Keys(Stamped { stamp, value }) => { + if !self.sequencer.accept(BrowseRequestKind::Keys, stamp) { + return; + } + self.failed.record(BrowseRequestKind::Keys, &value); + match value { + Ok(keys) => self.keys.set(keys), + Err(error) => { + if error != OperationError::Cancelled { + tracing::warn!(category = ?error.category(), "browse foreign keys failed"); + } + self.retry_if_link_moved(BrowseRequestKind::Keys, fx); + } + } + } // A failed count is not asked again on a reconnect: Try Again // runs it, so a count that ran out of time does not run for // another deadline, or under a filter applied since. diff --git a/linux/crates/app/src/ui/browse_tab/fetch.rs b/linux/crates/app/src/ui/browse_tab/fetch.rs index 3d471ca4f0..968af1a99e 100644 --- a/linux/crates/app/src/ui/browse_tab/fetch.rs +++ b/linux/crates/app/src/ui/browse_tab/fetch.rs @@ -11,6 +11,7 @@ use super::columns_fetch::{ColumnFreshness, load_columns}; use super::{BrowseCommand, BrowseModel, BrowseRequestKind, ColumnsReply, DataInput, Effects, PageReply}; use crate::runtime::{RequestTicket, Stamped}; use crate::ui::export::TableExport; +use crate::ui::grid::TableKeys; use crate::ui::tab_load::LoadFailure; /// What a page or a count reads, copied off the tab so the work owns @@ -43,9 +44,36 @@ impl BrowseModel { BrowseRequestKind::Page => self.fetch_page(fx), BrowseRequestKind::Estimate => self.fetch_estimate(), BrowseRequestKind::Count => self.start_count(self.total.estimate(), fx), + BrowseRequestKind::Keys => self.fetch_keys(), } } + /// Read the table's keys both ways in one operation, so a reconnect + /// between them cannot pair keys of two connections. + fn fetch_keys(&mut self) { + let ticket = self.issue(BrowseRequestKind::Keys); + let session = self.session.clone(); + let table = self.table_ref(); + let cancel = ticket.cancel.clone(); + let work = async move { + session + .run_cancellable(OperationKind::Metadata, &cancel, move |lease, options| { + let table = table.clone(); + async move { + let outbound = lease.fetch_foreign_keys(&table, options.clone()).await?; + let inbound = lease.fetch_inbound_foreign_keys(&table, options).await?; + Ok(TableKeys { + table, + outbound, + inbound, + }) + } + }) + .await + }; + self.deliver(ticket, work, BrowseCommand::Keys, OperationError::from); + } + /// Read the columns, then the page built from them and the table's /// estimate. No page is asked for before the columns land, since a /// page is built from them, and a failure to read them fails the read diff --git a/linux/crates/app/src/ui/browse_tab/imp.rs b/linux/crates/app/src/ui/browse_tab/imp.rs index 4346df5147..0644841016 100644 --- a/linux/crates/app/src/ui/browse_tab/imp.rs +++ b/linux/crates/app/src/ui/browse_tab/imp.rs @@ -108,6 +108,9 @@ impl ObjectImpl for BrowseTab { .build(), Signal::builder("stop-save-requested").build(), Signal::builder("refresh-requested").build(), + Signal::builder("follow-reference") + .param_types([glib::BoxedAnyObject::static_type()]) + .build(), ] }) } diff --git a/linux/crates/app/src/ui/browse_tab/mod.rs b/linux/crates/app/src/ui/browse_tab/mod.rs index d9bd2b9688..70843497ad 100644 --- a/linux/crates/app/src/ui/browse_tab/mod.rs +++ b/linux/crates/app/src/ui/browse_tab/mod.rs @@ -45,6 +45,7 @@ mod save_input; mod selection_handlers; mod selection_input; mod selection_snapshot; +mod table_filter; #[cfg(test)] mod tests; @@ -58,9 +59,10 @@ use gtk4::glib; use gtk4::prelude::*; use gtk4::subclass::prelude::*; use libadwaita as adw; -use tablepro_core::ColumnInfo; +use tablepro_core::browse::RowReference; use tablepro_core::dialect::SqlDialect; use tablepro_core::meta::{RowIdentity, TableRef}; +use tablepro_core::{ColumnInfo, FilterSet}; use tablepro_session::operation::DeadlinePolicy; use tablepro_session::runtime::{Tasks, WatchForwarder}; use tablepro_session::{LinkGeneration, Session}; @@ -88,9 +90,10 @@ pub(crate) use self::row_total::RowTotal; pub(crate) use self::save_input::SaveInput; pub(crate) use self::selection_input::SelectionInput; use self::selection_snapshot::SelectionSnapshot; +pub(crate) use self::table_filter::TableFilter; use super::filter_strip::FilterStrip; use super::grid::{ - ColumnWidthBinding, GridRowIndex, GridSink, RowJsonRequest, RowObject, TabGridContext, present_row_json, + ColumnWidthBinding, GridRowIndex, GridSink, KeysHandle, RowJsonRequest, RowObject, TabGridContext, present_row_json, }; use super::tab_load::{ContentLoad, FirstLoad, start_first_load}; use crate::runtime::{FailedRequests, RequestSequencer}; @@ -120,6 +123,7 @@ pub struct BrowseTabInit { pub page_size: u64, pub initial_offset: u64, pub initial_sort: Option<(usize, bool)>, + pub filter: TableFilter, pub column_widths: crate::persistence::ColumnWidthStore, pub filter_settings: crate::persistence::FilterSettingsStore, /// The session the tab reads through. The tab belongs to it and is @@ -137,13 +141,12 @@ impl BrowseTab { let tab: Self = glib::Object::new(); let imp = tab.imp(); - // Restore the saved filter for this (connection, schema, table) - // up front so both the model and the inline strip start with the - // same FilterSet. - let initial_filter = init - .connection_id - .map(|id| init.filter_settings.filters(id, init.schema.as_deref(), &init.table)) - .unwrap_or_default(); + // The model and the inline strip start with the same FilterSet. + let (initial_filter, reveal_filter) = init.filter.opening(|| { + init.connection_id + .map(|id| init.filter_settings.filters(id, init.schema.as_deref(), &init.table)) + .unwrap_or_default() + }); let widgets = layout::build( &tab, &layout::LayoutInit { @@ -153,10 +156,14 @@ impl BrowseTab { }, ); let filter_strip = layout::filter_strip(&tab, &initial_filter); + if reveal_filter { + filter_strip.set_revealed(true); + } let actions = browse_actions::install_browse_actions(&tab, init.page_size); let link_generation = init.session.link_generation(); let dialect = init.session.current_dialect(); let link_changes = init.session.link_changes(); + let keys = KeysHandle::new(TableRef::new(init.schema.clone(), init.table.clone())); let model = BrowseModel { this: tab.downgrade(), session: init.session, @@ -169,6 +176,7 @@ impl BrowseTab { link_generation, identity: None, dialect, + keys, tracker: init.tracker, schema: init.schema, table: init.table, @@ -312,6 +320,12 @@ impl BrowseTab { self.read(|model| model.current_sort).flatten() } + /// The filter the rows on screen were read under, not what the strip + /// holds before Apply. + pub fn applied_filter(&self) -> FilterSet { + self.read(|model| model.current_filter.clone()).unwrap_or_default() + } + /// A read of the tab's rows is running. pub fn is_loading(&self) -> bool { self.read(|model| model.tab_loading).unwrap_or(false) @@ -331,6 +345,17 @@ impl BrowseTab { ) } + /// The grid asked to open the rows a foreign key leads to. + pub(crate) fn connect_follow_reference(&self, f: F) -> glib::SignalHandlerId { + self.connect_closure( + "follow-reference", + false, + glib::closure_local!(move |tab: &Self, payload: glib::BoxedAnyObject| { + f(tab, payload.borrow::().clone()); + }), + ) + } + pub fn connect_alert(&self, f: F) -> glib::SignalHandlerId { self.connect_closure( "alert", @@ -412,6 +437,9 @@ impl BrowseTab { BrowseTabOutput::StopSaveRequested => self.emit_by_name::<()>("stop-save-requested", &[]), BrowseTabOutput::LoadingChanged(loading) => self.emit_by_name::<()>("loading-changed", &[&loading]), BrowseTabOutput::RefreshRequested => self.emit_by_name::<()>("refresh-requested", &[]), + BrowseTabOutput::FollowReference(reference) => { + self.emit_by_name::<()>("follow-reference", &[&glib::BoxedAnyObject::new(reference)]); + } } } @@ -480,6 +508,9 @@ pub(crate) struct BrowseModel { /// The current connection's dialect, which says whether a filter can /// be written for it. Read again after every reconnect. dialect: Option>, + /// The table's foreign keys, which every grid the tab builds reads + /// as its menu opens. + keys: KeysHandle, tracker: TabTrackerHandle, schema: Option, @@ -564,6 +595,8 @@ pub(crate) enum BrowseTabOutput { LoadingChanged(bool), /// F5 asked the window to read the tab again. RefreshRequested, + /// The grid asked to open the rows a foreign key leads to. + FollowReference(RowReference), } impl BrowseModel { @@ -596,6 +629,7 @@ impl BrowseModel { schema: self.schema.clone(), table: self.table.clone(), }), + keys: Some(self.keys.clone()), } } diff --git a/linux/crates/app/src/ui/browse_tab/table_filter.rs b/linux/crates/app/src/ui/browse_tab/table_filter.rs new file mode 100644 index 0000000000..2cb9e67e8d --- /dev/null +++ b/linux/crates/app/src/ui/browse_tab/table_filter.rs @@ -0,0 +1,25 @@ +use tablepro_core::FilterSet; + +/// The filter a table tab opens under. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum TableFilter { + /// The one last applied to the table, as the sidebar opens it. + Remembered, + /// The tab's own, as it was when the tab was saved or closed. + Kept(FilterSet), + /// A foreign key's, which the tab opens with its filter strip shown. + Followed(FilterSet), +} + +impl TableFilter { + /// The set the tab opens under and whether its strip shows it. Only + /// a tab on the table's remembered filter reads it, so one with its + /// own leaves the table's alone until the user applies another. + pub(crate) fn opening(self, remembered: impl FnOnce() -> FilterSet) -> (FilterSet, bool) { + match self { + Self::Remembered => (remembered(), false), + Self::Kept(set) => (set, false), + Self::Followed(set) => (set, true), + } + } +} diff --git a/linux/crates/app/src/ui/browse_tab/tests/load.rs b/linux/crates/app/src/ui/browse_tab/tests/load.rs index 316b567e6d..a476cb0f9d 100644 --- a/linux/crates/app/src/ui/browse_tab/tests/load.rs +++ b/linux/crates/app/src/ui/browse_tab/tests/load.rs @@ -559,10 +559,10 @@ fn the_loading_page_names_a_table_with_an_ampersand() { &fake.session, &runtime, &stores, - "R&D", - Default::default(), - Default::default(), - false, + super::Opening { + table: "R&D", + ..super::Opening::users() + }, ); let shown = || crate::test_support::status_page_description(&tab.imp().loading_page); diff --git a/linux/crates/app/src/ui/browse_tab/tests/mod.rs b/linux/crates/app/src/ui/browse_tab/tests/mod.rs index f2eb09c7e3..c75f1305b6 100644 --- a/linux/crates/app/src/ui/browse_tab/tests/mod.rs +++ b/linux/crates/app/src/ui/browse_tab/tests/mod.rs @@ -1,4 +1,5 @@ mod load; +mod references; mod total; use std::sync::Arc; @@ -18,7 +19,7 @@ use tablepro_test_support::{CallCounters, ConnectionScript, FakeConnection, Fake use super::row_deletion::RowDeletion; use super::{ BrowseTab, BrowseTabInit, ClipboardInput, DataInput, EditInput, Effects, PagingInput, RowTotal, SaveInput, - apply_draft_changes, + TableFilter, apply_draft_changes, }; use crate::services::change_tracker::{RowKey, StatementSource}; use crate::services::tab_tracker_handle::TabTrackerHandle; @@ -138,6 +139,8 @@ pub(super) fn serving_table() -> ConnectionScript { .table_estimate .always(FakeReply::ok(estimate(RowCountEstimate::Approximate(2)))); script.probe.always(FakeReply::ok(())); + script.fetch_foreign_keys.always(FakeReply::ok(Vec::new())); + script.fetch_inbound_foreign_keys.always(FakeReply::ok(Vec::new())); script } @@ -170,17 +173,24 @@ impl Harness { reconnects: impl IntoIterator, saved: FilterSet, ) -> Self { - Self::open_with(script, reconnects, saved, false) + Self::open_with(script, reconnects, saved, TableFilter::Remembered, false) + } + + /// The tab opens under `filter`, the table having been left with + /// `saved`. + pub(super) fn open_on(script: ConnectionScript, saved: FilterSet, filter: TableFilter) -> Self { + Self::open_with(script, [], saved, filter, false) } fn open_read_only(script: ConnectionScript) -> Self { - Self::open_with(script, [], FilterSet::default(), true) + Self::open_with(script, [], FilterSet::default(), TableFilter::Remembered, true) } fn open_with( script: ConnectionScript, reconnects: impl IntoIterator, saved: FilterSet, + filter: TableFilter, read_only: bool, ) -> Self { libadwaita::init().expect("libadwaita initialises under the test backend"); @@ -189,7 +199,18 @@ impl Harness { let fake = FakeSession::start(script, reconnects, runtime.tasks()); let stores = TabStores::new(); let tracker = TabTrackerHandle::default(); - let tab = filtered_users_tab(&fake.session, &runtime, &stores, saved, tracker.clone(), read_only); + let tab = table_tab( + &fake.session, + &runtime, + &stores, + Opening { + saved, + filter, + tracker: tracker.clone(), + read_only, + ..Opening::users() + }, + ); Self { tab: Some(tab), tracker, @@ -317,36 +338,44 @@ impl TabStores { } pub(super) fn users_tab(session: &Arc, runtime: &crate::runtime::AppRuntime, stores: &TabStores) -> BrowseTab { - filtered_users_tab( - session, - runtime, - stores, - FilterSet::default(), - TabTrackerHandle::default(), - false, - ) + table_tab(session, runtime, stores, Opening::users()) } -pub(super) fn filtered_users_tab( - session: &Arc, - runtime: &crate::runtime::AppRuntime, - stores: &TabStores, - saved: FilterSet, - tracker: TabTrackerHandle, - read_only: bool, -) -> BrowseTab { - table_tab(session, runtime, stores, "users", saved, tracker, read_only) +/// What a test tab opens on. +pub(super) struct Opening { + pub(super) table: &'static str, + /// The filter the table was left with. + pub(super) saved: FilterSet, + pub(super) filter: TableFilter, + pub(super) tracker: TabTrackerHandle, + pub(super) read_only: bool, +} + +impl Opening { + pub(super) fn users() -> Self { + Self { + table: "users", + saved: FilterSet::default(), + filter: TableFilter::Remembered, + tracker: TabTrackerHandle::default(), + read_only: false, + } + } } pub(super) fn table_tab( session: &Arc, runtime: &crate::runtime::AppRuntime, stores: &TabStores, - table: &str, - saved: FilterSet, - tracker: TabTrackerHandle, - read_only: bool, + opening: Opening, ) -> BrowseTab { + let Opening { + table, + saved, + filter, + tracker, + read_only, + } = opening; let tasks = runtime.tasks(); let connection_id = uuid::Uuid::new_v4(); let root = stores.root.path(); @@ -361,6 +390,7 @@ pub(super) fn table_tab( page_size: 100, initial_offset: 0, initial_sort: None, + filter, column_widths: crate::persistence::ColumnWidthStore::load(root.join("column-widths.json"), &tasks), filter_settings, session: session.clone(), diff --git a/linux/crates/app/src/ui/browse_tab/tests/references.rs b/linux/crates/app/src/ui/browse_tab/tests/references.rs new file mode 100644 index 0000000000..b8f7b80983 --- /dev/null +++ b/linux/crates/app/src/ui/browse_tab/tests/references.rs @@ -0,0 +1,199 @@ +use std::cell::RefCell; +use std::rc::Rc; + +use gtk4::subclass::prelude::ObjectSubclassIsExt; +use tablepro_core::browse::RowReference; +use tablepro_core::column::{ColumnKind, TextLength}; +use tablepro_core::ddl::InboundForeignKey; +use tablepro_core::filter::{Combinator, FilterOp, FilterRule, FilterSet, FilterValue}; +use tablepro_core::meta::TableRef; +use tablepro_core::{ColumnInfo, DriverError, ForeignKeyInfo}; +use tablepro_session::fakes::FakeEstablish; +use tablepro_test_support::{CallCounters, FakeReply}; + +use super::{Harness, TableFilter, WAIT, column, rows, serving_table, text}; +use crate::test_support::{drain_main_context, wait_until}; +use crate::ui::browse_tab::BrowseTabInput; +use crate::ui::grid::{GridMsg, TableKeys}; + +fn name_key() -> ForeignKeyInfo { + ForeignKeyInfo { + name: "users_name_fkey".to_owned(), + columns: vec!["name".to_owned()], + ref_schema: None, + ref_table: "people".to_owned(), + ref_columns: vec!["name".to_owned()], + on_delete: None, + on_update: None, + } +} + +fn orders_key() -> InboundForeignKey { + InboundForeignKey { + name: "orders_user_id_fkey".to_owned(), + table: TableRef::unqualified("orders"), + columns: vec!["user_id".to_owned()], + referenced_columns: vec!["id".to_owned()], + } +} + +fn keys_of(harness: &Harness) -> TableKeys { + let model = harness.tab().imp().model.borrow(); + let keys = model.as_ref().expect("the tab is open").keys.get(); + (*keys).clone() +} + +fn name_is(name: &str) -> FilterSet { + FilterSet { + combinator: Combinator::And, + rules: vec![FilterRule { + column: "name".to_owned(), + op: FilterOp::Eq, + value: Some(FilterValue::Single(name.to_owned())), + }], + extra_sql: None, + } +} + +/// The table's columns with `name` typed as text, which a filter binds +/// as text. +fn text_columns() -> Vec { + let mut name = column("name", false); + name.column_type.kind = ColumnKind::Text { + length: TextLength::Unbounded, + }; + vec![column("id", true), name] +} + +/// The filter the tab's table keeps for the next tab opened on it. +fn remembered(harness: &Harness) -> FilterSet { + let model = harness.tab().imp().model.borrow(); + let model = model.as_ref().expect("the tab is open"); + let connection_id = model.connection_id.expect("a saved connection"); + model.filter_settings.filters(connection_id, None, &model.table) +} + +#[gtk4::test] +fn the_tab_reads_its_tables_keys_both_ways_with_its_columns() { + let mut script = serving_table(); + script.query_bound.always(FakeReply::ok(rows(&["Ada"]))); + script.fetch_foreign_keys.always(FakeReply::ok(vec![name_key()])); + script + .fetch_inbound_foreign_keys + .always(FakeReply::ok(vec![orders_key()])); + let harness = Harness::open(script, []); + let counters = harness.connection.counters(); + + wait_until(WAIT, || !keys_of(&harness).outbound.is_empty()).expect("the keys were read"); + + let keys = keys_of(&harness); + assert_eq!(keys.table, TableRef::unqualified("users")); + assert_eq!(keys.outbound, [name_key()]); + assert_eq!(keys.inbound, [orders_key()]); + assert_eq!(CallCounters::get(&counters.fetch_inbound_foreign_keys), 1); +} + +/// The keys only add to the cell menu, so a read of them that fails +/// leaves the rows on screen and is asked again on the next connection. +#[gtk4::test] +fn a_failed_key_read_leaves_the_rows_and_is_asked_again_on_the_next_link() { + let mut first = serving_table(); + first.query_bound.always(FakeReply::ok(rows(&["kept"]))); + first + .fetch_foreign_keys + .always(FakeReply::err(DriverError::server("permission denied"))); + let mut second = serving_table(); + second.fetch_foreign_keys.always(FakeReply::ok(vec![name_key()])); + let harness = Harness::open(first, [FakeEstablish::Connect(second)]); + let counters = harness.connection.counters(); + wait_until(WAIT, || harness.shown() == text(&["kept"])).expect("the first page"); + wait_until(WAIT, || CallCounters::get(&counters.fetch_foreign_keys) == 1).expect("the keys were asked for"); + drain_main_context(); + assert_eq!(harness.visible_page().as_deref(), Some("grid")); + assert!(keys_of(&harness).outbound.is_empty()); + + harness.connection.mark_lost(); + + wait_until(WAIT, || keys_of(&harness).outbound == [name_key()]).expect("the keys were asked for again"); + let replacement = harness.fake.replacement(); + assert_eq!( + CallCounters::get(&replacement.counters().query_bound), + 0, + "the rows were read again for a failed key read" + ); + assert_eq!(harness.shown(), text(&["kept"])); +} + +#[gtk4::test] +fn a_reference_the_grid_follows_goes_to_the_window() { + let harness = Harness::open(serving_table(), []); + let asked = Rc::new(RefCell::new(Vec::new())); + harness.tab().connect_follow_reference({ + let asked = asked.clone(); + move |_, reference| asked.borrow_mut().push(reference) + }); + let reference = RowReference { + table: TableRef::unqualified("people"), + filter: name_is("Ada"), + }; + + harness + .tab() + .send(BrowseTabInput::from(GridMsg::FollowReference(reference.clone()))); + + assert_eq!(*asked.borrow(), [reference]); +} + +/// A tab opened from a key shows the key's rows without changing the +/// filter the table keeps for the sidebar's next open, until the user +/// applies one of their own. +#[gtk4::test] +fn a_tab_opened_on_a_key_reads_under_its_filter_and_leaves_the_tables_alone() { + let mut script = serving_table(); + script.fetch_columns.always(FakeReply::ok(text_columns())); + script + .query_bound + .when_sql_contains("\"name\" = ", FakeReply::ok(rows(&["Ada"]))) + .always(FakeReply::ok(rows(&["everyone"]))); + let harness = Harness::open_on(script, name_is("Grace"), TableFilter::Followed(name_is("Ada"))); + + wait_until(WAIT, || harness.shown() == text(&["Ada"])).expect("the key's rows"); + + assert_eq!(harness.tab().applied_filter(), name_is("Ada")); + assert_eq!( + remembered(&harness), + name_is("Grace"), + "the key's filter replaced the table's" + ); + let model = harness.tab().imp().model.borrow(); + let strip = &model.as_ref().expect("the tab is open").filter_strip; + assert!(strip.revealed(), "the strip that says why one row shows is hidden"); + assert_eq!(strip.filter(), name_is("Ada")); + drop(model); + + harness + .tab() + .send(super::PagingInput::FilterApplied(FilterSet::default())); + + wait_until(WAIT, || harness.shown() == text(&["everyone"])).expect("the cleared filter's rows"); + assert_eq!( + remembered(&harness), + FilterSet::default(), + "an applied filter was not kept for the table" + ); +} + +#[gtk4::test] +fn a_restored_tab_keeps_its_own_filter_with_the_strip_closed() { + let mut script = serving_table(); + script.fetch_columns.always(FakeReply::ok(text_columns())); + script.query_bound.always(FakeReply::ok(rows(&["Ada"]))); + let harness = Harness::open_on(script, name_is("Grace"), TableFilter::Kept(name_is("Ada"))); + + wait_until(WAIT, || harness.shown() == text(&["Ada"])).expect("the first page"); + + assert_eq!(harness.tab().applied_filter(), name_is("Ada")); + assert_eq!(remembered(&harness), name_is("Grace")); + let model = harness.tab().imp().model.borrow(); + assert!(!model.as_ref().expect("the tab is open").filter_strip.revealed()); +} diff --git a/linux/crates/app/src/ui/error_text/edit.rs b/linux/crates/app/src/ui/error_text/edit.rs index 9437e0f216..040e6de896 100644 --- a/linux/crates/app/src/ui/error_text/edit.rs +++ b/linux/crates/app/src/ui/error_text/edit.rs @@ -46,6 +46,7 @@ pub fn edit_parse_message(error: &EditParseError) -> String { EditParseError::OffsetRequired => gettext("Add a time zone offset, such as +02:00"), EditParseError::InvalidInterval => gettext("Enter an interval, such as 1 day 02:00:00"), EditParseError::InvalidBits => gettext("Enter bits as 0 and 1"), + EditParseError::InvalidBytes => gettext("Enter bytes as 0x and hex digits, such as 0x1f2e"), EditParseError::BitLengthMismatch { expected } => ngettext_f( "Enter exactly {n} bit", "Enter exactly {n} bits", @@ -118,6 +119,7 @@ mod tests { EditParseError::OffsetRequired, EditParseError::InvalidInterval, EditParseError::InvalidBits, + EditParseError::InvalidBytes, EditParseError::BitLengthMismatch { expected: 8 }, EditParseError::UnknownEnumLabel, EditParseError::NotEditable(ReadOnlyReason::Bytes), diff --git a/linux/crates/app/src/ui/error_text/mod.rs b/linux/crates/app/src/ui/error_text/mod.rs index 141cbb27c4..7f43fa9b7f 100644 --- a/linux/crates/app/src/ui/error_text/mod.rs +++ b/linux/crates/app/src/ui/error_text/mod.rs @@ -10,6 +10,7 @@ mod driver; mod edit; mod files; mod operation; +mod reference; mod saved_connection; mod script; mod secret; @@ -27,6 +28,7 @@ pub use driver::{config_message, driver_message}; pub use edit::edit_parse_message; pub use files::storage_io_message; pub use operation::{connect_error_message, operation_error_message, task_failure_message}; +pub use reference::reference_message; pub use saved_connection::saved_connection_error_message; pub use script::script_diagnostic_message; pub use ssh::ssh_destination_problem_text; diff --git a/linux/crates/app/src/ui/error_text/reference.rs b/linux/crates/app/src/ui/error_text/reference.rs new file mode 100644 index 0000000000..6857cbbff9 --- /dev/null +++ b/linux/crates/app/src/ui/error_text/reference.rs @@ -0,0 +1,54 @@ +use tablepro_core::browse::ReferenceError; + +use crate::i18n::{gettext, gettext_f}; + +/// Why a foreign key could not be followed from a row, as a toast says +/// it. +pub fn reference_message(error: &ReferenceError) -> String { + match error { + ReferenceError::NullKey => gettext("The key is empty in this row"), + ReferenceError::MissingColumn { column } => gettext_f("This row has no column {column}", &[("column", column)]), + ReferenceError::UnevenKey { key } => gettext_f("Can't read the columns of the key {key}", &[("key", key)]), + ReferenceError::UnknownSchema { table } => { + gettext_f("Can't tell which schema {table} is in", &[("table", table)]) + } + ReferenceError::NoFilterText { column } => { + gettext_f("Can't filter {column} on this value", &[("column", column)]) + } + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashSet; + + use super::*; + + #[test] + fn each_reason_reads_differently_and_names_what_it_is_about() { + let errors = [ + ReferenceError::NullKey, + ReferenceError::MissingColumn { + column: "owner_id".to_owned(), + }, + ReferenceError::UnevenKey { + key: "fk_notes_0".to_owned(), + }, + ReferenceError::UnknownSchema { + table: "customers".to_owned(), + }, + ReferenceError::NoFilterText { + column: "outline".to_owned(), + }, + ]; + + let messages: Vec = errors.iter().map(reference_message).collect(); + + let distinct: HashSet<&String> = messages.iter().collect(); + assert_eq!(distinct.len(), messages.len(), "{messages:?}"); + assert!(messages[1].contains("owner_id"), "{messages:?}"); + assert!(messages[2].contains("fk_notes_0"), "{messages:?}"); + assert!(messages[3].contains("customers"), "{messages:?}"); + assert!(messages[4].contains("outline"), "{messages:?}"); + } +} diff --git a/linux/crates/app/src/ui/filter_strip/mod.rs b/linux/crates/app/src/ui/filter_strip/mod.rs index a3a6ab1f09..a99ebcb2e6 100644 --- a/linux/crates/app/src/ui/filter_strip/mod.rs +++ b/linux/crates/app/src/ui/filter_strip/mod.rs @@ -23,7 +23,6 @@ use libadwaita as adw; use tablepro_core::{ColumnInfo, Combinator, FilterOp, FilterRule, FilterSet, FilterValue}; use self::filter_rule_object::FilterRuleObject; -use self::rule_operators::is_filterable; pub(super) use self::rule_row::FilterRuleRow; glib::wrapper! { @@ -82,8 +81,7 @@ impl FilterStrip { /// Rebuild the rows against new columns: each row's column and /// operator choices come from them. pub fn set_columns(&self, columns: Vec) { - let filterable: Rc<[ColumnInfo]> = columns.into_iter().filter(is_filterable).collect(); - self.imp().columns.replace(filterable); + self.imp().columns.replace(Rc::from(columns)); let rules = self.rules(); let items: Vec = rules.iter::().filter_map(Result::ok).collect(); rules.splice(0, rules.n_items(), &items); diff --git a/linux/crates/app/src/ui/filter_strip/rule_operators.rs b/linux/crates/app/src/ui/filter_strip/rule_operators.rs index 206ac90fd0..71537b68fd 100644 --- a/linux/crates/app/src/ui/filter_strip/rule_operators.rs +++ b/linux/crates/app/src/ui/filter_strip/rule_operators.rs @@ -77,15 +77,6 @@ fn shape_for(op: FilterOp) -> ValueShape { } } -/// A column the user can filter on at all. -/// -/// A blob is left out of the dropdown: nothing they could type would -/// compare against one, so offering it would only lead to a filter -/// that never matches. -pub(super) fn is_filterable(column: &ColumnInfo) -> bool { - !matches!(column.column_type.kind, tablepro_core::column::ColumnKind::Bytes { .. }) -} - /// A column the strip can fall back on when the schema has not /// loaded, so a saved rule still renders with its own name. pub(super) fn placeholder_column(name: &str) -> ColumnInfo { @@ -119,17 +110,6 @@ mod tests { assert_eq!(ValueShape::List.empty_value(), Some(FilterValue::List(Vec::new()))); } - #[test] - fn a_blob_is_not_filterable() { - let mut blob = placeholder_column("payload"); - blob.column_type.kind = tablepro_core::column::ColumnKind::Bytes { - length: tablepro_core::column::TextLength::Unbounded, - }; - - assert!(!is_filterable(&blob)); - assert!(is_filterable(&placeholder_column("name"))); - } - #[test] fn the_null_tests_take_no_value_and_between_takes_two() { assert_eq!(shape_for(FilterOp::IsNull), ValueShape::None); diff --git a/linux/crates/app/src/ui/filter_strip/tests.rs b/linux/crates/app/src/ui/filter_strip/tests.rs index dc7d507c0c..9adce9b690 100644 --- a/linux/crates/app/src/ui/filter_strip/tests.rs +++ b/linux/crates/app/src/ui/filter_strip/tests.rs @@ -301,8 +301,10 @@ fn a_saved_filter_opens_the_rule_editor_and_an_empty_one_does_not() { assert!(!strip_with(Vec::new()).imp().rules_expander.is_expanded()); } +/// A blob compares for equality on its hex form, so it is a column like +/// any other. #[gtk4::test] -fn add_rule_appends_a_row_for_the_first_filterable_column() { +fn add_rule_appends_a_row_for_the_first_column() { let strip = Hosted::new(new_strip( vec![blob("payload"), placeholder_column("name")], &FilterSet::default(), @@ -319,7 +321,7 @@ fn add_rule_appends_a_row_for_the_first_filterable_column() { assert_eq!( strip.filter().rules, [FilterRule { - column: "name".to_owned(), + column: "payload".to_owned(), op: FilterOp::Eq, value: Some(FilterValue::Single(String::new())), }] @@ -397,7 +399,7 @@ fn set_columns_rebuilds_the_column_choices() { strip.set_columns(vec![placeholder_column("a"), blob("payload"), integer("total")]); let [row] = rows(&strip).try_into().expect("one rule row"); - assert_eq!(column_choices(&row), ["a", "total"]); + assert_eq!(column_choices(&row), ["a", "payload", "total"]); assert_eq!(strip.filter().rules, [rule("a")], "new columns changed the rule"); } diff --git a/linux/crates/app/src/ui/grid/context_menu.rs b/linux/crates/app/src/ui/grid/context_menu.rs index e0a9634832..a72b712bf9 100644 --- a/linux/crates/app/src/ui/grid/context_menu.rs +++ b/linux/crates/app/src/ui/grid/context_menu.rs @@ -74,6 +74,7 @@ pub(super) fn prepare_menu(menus: &GridMenus, cell: &GridCell, column_view: >k let rows = menu_rows_of(column_view, cell); menus.context.set(cell, &rows); menus.actions.arm(cell, &rows); + menus.references.arm(cell); match cell.lock() { None => menus.editable_popover.clone(), Some(_) => menus.readonly_popover.clone(), @@ -114,19 +115,10 @@ mod tests { use crate::services::change_tracker::RowKey; use crate::test_support::{FocusThief, after_next_frame, descendants, drain_main_context, wait_until}; use crate::ui::grid::test_grid::{ - Grid, WAIT, focus_in_grid, key, page, two_text_columns, two_text_rows, two_text_values, + Grid, WAIT, focus_in_grid, key, open_menu, page, two_text_columns, two_text_rows, two_text_values, }; use crate::ui::grid::{ColumnWidthBinding, GridCell, GridMsg, RowObject}; - fn open_menu(cell: &GridCell) { - let controllers = cell.observe_controllers(); - let gesture = (0..controllers.n_items()) - .filter_map(|index| controllers.item(index).and_downcast::()) - .find(|gesture| gesture.button() == 3) - .expect("a cell answers a right-click"); - gesture.emit_by_name::<()>("pressed", &[&1i32, &1.0f64, &1.0f64]); - } - fn choose(grid: &Grid, actions: &[&str]) { for action in actions { grid.view diff --git a/linux/crates/app/src/ui/grid/grid_keyboard.rs b/linux/crates/app/src/ui/grid/grid_keyboard.rs index 87a5000208..b9b321a28d 100644 --- a/linux/crates/app/src/ui/grid/grid_keyboard.rs +++ b/linux/crates/app/src/ui/grid/grid_keyboard.rs @@ -7,6 +7,7 @@ use super::cell_preset::cell_presets; use super::column_resize::{ColumnResize, resize_column}; use super::context_menu::prepare_menu; use super::grid_menus::GridMenus; +use super::reference_menu::OPEN_ONE; use super::{CellPreset, GridCell, GridMsg, GridSink}; use crate::application::{ShortcutOwner, component_trigger, controller_name}; @@ -32,11 +33,16 @@ impl GridKeyboard { }; add("cell.edit", on_focused_cell(|_, cell| edit_or_toggle(cell))); add("cell.toggle", on_focused_cell(|_, cell| toggle(cell))); + let reference_menus = menus.clone(); add( "cell.menu", on_focused_cell(move |widget, cell| show_menu(widget, cell, &menus)), ); add("cell.set-null", on_focused_cell(move |_, cell| set_null(cell, &sink))); + add( + "cell.open-referenced-row", + on_focused_cell(move |widget, cell| open_referenced_row(widget, cell, &reference_menus)), + ); add( "cell.widen-column", on_focused_cell(|widget, cell| resize(widget, cell, ColumnResize::Wider)), @@ -83,8 +89,14 @@ fn show_menu(widget: >k::Widget, cell: &GridCell, menus: &GridMenus) -> glib:: return glib::Propagation::Proceed; }; let popover = prepare_menu(menus, cell, column_view); - // A key has no pointer position, so the menu points at the whole - // cell, in the coordinates of the view the menu is parented to. + point_at_cell(&popover, cell, column_view); + popover.popup(); + glib::Propagation::Stop +} + +/// A key has no pointer position, so a menu points at the whole cell, +/// in the coordinates of the view the menu is parented to. +fn point_at_cell(popover: >k::PopoverMenu, cell: &GridCell, column_view: >k::ColumnView) { let bounds = cell.compute_bounds(column_view).map(|bounds| { gdk::Rectangle::new( bounds.x() as i32, @@ -94,7 +106,30 @@ fn show_menu(widget: >k::Widget, cell: &GridCell, menus: &GridMenus) -> glib:: ) }); popover.set_pointing_to(bounds.as_ref()); - popover.popup(); +} + +/// Follow the one key the cell's column is in, or offer the keys to +/// choose from when it is in several. A column in none lets the key go +/// on. +fn open_referenced_row(widget: >k::Widget, cell: &GridCell, menus: &GridMenus) -> glib::Propagation { + let Some(column_view) = widget.downcast_ref::() else { + return glib::Propagation::Proceed; + }; + if menus.references.offers_for(cell).open.is_empty() { + return glib::Propagation::Proceed; + } + prepare_menu(menus, cell, column_view); + match menus.references.only_choice() { + Some(_) => { + if let Err(error) = column_view.activate_action(&format!("cell.{OPEN_ONE}"), None) { + tracing::debug!(%error, "the grid has no action to open a referenced row"); + } + } + None => { + point_at_cell(&menus.reference_chooser, cell, column_view); + menus.reference_chooser.popup(); + } + } glib::Propagation::Stop } @@ -138,6 +173,7 @@ mod tests { trigger_keys, }; use crate::test_support::{descendants, wait_until}; + use crate::ui::grid::reference_menu::tests::{filter, followed, key as foreign_key, orders_grid, public}; use crate::ui::grid::test_grid::{Grid, WAIT, content, edited_rows, key, named, recorder, text, two_text_rows}; use crate::ui::grid::{ CellPreset, GridCell, GridMsg, GridSorting, RowObject, TabGridContext, begin_edit, build_column_view, @@ -503,6 +539,48 @@ mod tests { window.destroy(); } + #[gtk4::test] + fn ctrl_j_follows_the_one_key_of_the_focused_cell_and_lets_the_key_go_on_elsewhere() { + let grid = orders_grid(vec![foreign_key(&["customer_id"], public("customers"), &["id"])]); + let ctrl = gdk::ModifierType::CONTROL_MASK; + + grid.focus(0, 1); + assert!(press(&grid.view, gdk::Key::j, ctrl)); + grid.focus(0, 2); + assert!(!press(&grid.view, gdk::Key::j, ctrl), "a column in no key took Ctrl+J"); + grid.focus(1, 1); + assert!(!press(&grid.view, gdk::Key::j, ctrl), "a NULL key took Ctrl+J"); + + assert_eq!(followed(&grid), [(public("customers"), filter(&[("id", "42")]))]); + } + + #[gtk4::test] + fn ctrl_j_on_a_column_in_two_keys_offers_them_at_the_cell() { + let crm = tablepro_core::meta::TableRef::new(Some("crm".to_owned()), "customers"); + let grid = orders_grid(vec![ + foreign_key(&["tenant_id"], public("tenants"), &["id"]), + foreign_key(&["tenant_id", "customer_id"], crm, &["tenant_id", "id"]), + ]); + grid.focus(0, 2); + + assert!(press(&grid.view, gdk::Key::j, gdk::ModifierType::CONTROL_MASK)); + + let mut chooser = None; + wait_until(WAIT, || { + chooser = descendants(&grid.view) + .into_iter() + .filter_map(|widget| widget.downcast::().ok()) + .find(|popover| popover.is_visible()); + chooser.is_some() + }) + .expect("the keys are offered"); + let chooser = chooser.expect("an open chooser"); + let model = chooser.menu_model().expect("the chooser's keys"); + assert_eq!(model.n_items(), 2); + assert!(followed(&grid).is_empty(), "a key was followed before one was chosen"); + chooser.popdown(); + } + #[gtk4::test] fn grid_keys_proceed_with_no_focused_cell() { let grid = two_text_rows(); diff --git a/linux/crates/app/src/ui/grid/grid_menus.rs b/linux/crates/app/src/ui/grid/grid_menus.rs index c1aaeaa71a..3c93c3214c 100644 --- a/linux/crates/app/src/ui/grid/grid_menus.rs +++ b/linux/crates/app/src/ui/grid/grid_menus.rs @@ -12,6 +12,7 @@ use super::cell_preset::cell_presets; use super::column_resize::{ColumnResize, resize_column}; use super::context_menu::{CellContext, MenuTarget}; use super::grid_columns::ShownColumn; +use super::reference_menu::{OPEN_CHOICE, OPEN_ONE, REFERENCING, ReferenceMenu, chosen, follow}; use super::row_json_dialog::RowJsonRequest; use super::selection::export_snapshot; use super::{ @@ -83,6 +84,10 @@ pub(super) struct GridMenus { pub(super) editable_popover: gtk::PopoverMenu, pub(super) readonly_popover: gtk::PopoverMenu, pub(super) actions: CellActions, + pub(super) references: ReferenceMenu, + /// The keys a column is in, when it is in more than one, for the + /// keyboard to choose from. + pub(super) reference_chooser: gtk::PopoverMenu, } #[derive(Clone, Copy)] @@ -110,7 +115,7 @@ impl MenuShape { }; } -fn build_cell_menu(shape: MenuShape) -> gio::Menu { +fn build_cell_menu(shape: MenuShape, references: &gio::Menu) -> gio::Menu { let menu = gio::Menu::new(); let edit_section = gio::Menu::new(); let hidden_while_disabled = |label: String, action: &str| { @@ -169,6 +174,7 @@ fn build_cell_menu(shape: MenuShape) -> gio::Menu { Some("cell.show-row-json"), ); menu.append_section(None, &json_section); + menu.append_section(None, references); // A header's click and drag, which no key reaches. let column_section = gio::Menu::new(); @@ -221,12 +227,14 @@ pub(super) fn install_grid_context_menus(init: GridMenuInit<'_>) -> GridMenus { tab_ctx, } = init; let editable = tab_ctx.editable; + let tab = Rc::new(tab_ctx); let kit = CellActionKit { context: Rc::new(CellContext::default()), view: column_view.downgrade(), sink, + references: ReferenceMenu::new(tab.clone(), columns.clone()), columns, - tab: Rc::new(tab_ctx), + tab, }; let group = gio::SimpleActionGroup::new(); @@ -249,8 +257,9 @@ pub(super) fn install_grid_context_menus(init: GridMenuInit<'_>) -> GridMenus { clear_sort: simple_action(&group, "clear-sort"), }; - let (editable_popover, readonly_popover) = cell_popovers(column_view, editable); - let mut popovers_for_destroy = vec![editable_popover.clone()]; + let (editable_popover, readonly_popover) = cell_popovers(column_view, editable, kit.references.section()); + let reference_chooser = eager_popover(column_view, kit.references.choices()); + let mut popovers_for_destroy = vec![editable_popover.clone(), reference_chooser.clone()]; if readonly_popover != editable_popover { popovers_for_destroy.push(readonly_popover.clone()); } @@ -268,6 +277,8 @@ pub(super) fn install_grid_context_menus(init: GridMenuInit<'_>) -> GridMenus { editable_popover, readonly_popover, actions, + references: kit.references, + reference_chooser, } } @@ -283,6 +294,7 @@ struct CellActionKit { sink: GridSink, columns: Arc<[ResultColumn]>, tab: Rc, + references: ReferenceMenu, } impl CellActionKit { @@ -317,6 +329,23 @@ impl CellActionKit { }) } + /// An action on the row the menu opened on that names one of the + /// items it offered. + fn on_choice(&self, name: &str, act: impl Fn(RowObject, usize) + 'static) -> CellEntry { + let ctx = self.context.clone(); + let view = self.view.clone(); + gio::ActionEntry::builder(name) + .parameter_type(Some(&u32::static_variant_type())) + .activate(move |_, _, parameter| { + let Some(cv) = view.upgrade() else { return }; + let (Some(MenuTarget { row, .. }), Some(index)) = (ctx.target(&cv), chosen(parameter)) else { + return; + }; + act(row, index); + }) + .build() + } + fn send(&self, name: &str, msg: impl Fn(&GridCell, RowObject, Vec) -> GridMsg + 'static) -> CellEntry { let s = self.sink.clone(); self.on_target(name, move |cell, row, rows| s(msg(cell, row, rows))) @@ -326,10 +355,41 @@ impl CellActionKit { fn reading_entries(&self) -> Vec { let mut entries = self.copy_entries(); entries.extend([self.copy_column_name(), self.show_row_json(), self.export()]); + entries.extend(self.reference_entries()); entries.extend(self.column_entries()); entries } + /// Follow a foreign key from the row, either way. Reading another + /// table changes nothing, so a read-only grid offers them too. + fn reference_entries(&self) -> Vec { + let open_one = { + let (menu, sink) = (self.references.clone(), self.sink.clone()); + self.on_target(OPEN_ONE, move |_cell, row, _rows| { + if let Some(reference) = menu.only_choice().and_then(|index| menu.referenced(index, &row)) { + follow(&sink, reference); + } + }) + }; + let open_choice = { + let (menu, sink) = (self.references.clone(), self.sink.clone()); + self.on_choice(OPEN_CHOICE, move |row, index| { + if let Some(reference) = menu.referenced(index, &row) { + follow(&sink, reference); + } + }) + }; + let referencing = { + let (menu, sink) = (self.references.clone(), self.sink.clone()); + self.on_choice(REFERENCING, move |row, index| { + if let Some(reference) = menu.referencing(index, &row) { + follow(&sink, reference); + } + }) + }; + vec![open_one, open_choice, referencing] + } + /// Sort by the cell's column, as a header click does, or change the /// width a header drag sets. fn column_entries(&self) -> Vec { @@ -497,17 +557,12 @@ impl CellActionKit { /// The menus a right click on a cell opens: one for a row that can be /// edited and one for a row that can't. A read-only grid has a single /// shape and both name it. -fn cell_popovers(column_view: >k::ColumnView, editable: bool) -> (gtk::PopoverMenu, gtk::PopoverMenu) { - // Popovers are parented eagerly so each PopoverMenu's action muxer - // snapshots the ColumnView's `cell` group at set_parent() time. - // Lazy parenting in the gesture handler drops every activation - // silently. - let make_popover = |shape: MenuShape| { - let popover = gtk::PopoverMenu::from_model_full(&build_cell_menu(shape), gtk::PopoverMenuFlags::NESTED); - popover.set_has_arrow(true); - popover.set_parent(column_view); - popover - }; +fn cell_popovers( + column_view: >k::ColumnView, + editable: bool, + references: &gio::Menu, +) -> (gtk::PopoverMenu, gtk::PopoverMenu) { + let make_popover = |shape: MenuShape| eager_popover(column_view, &build_cell_menu(shape, references)); if editable { (make_popover(MenuShape::FULL), make_popover(MenuShape::ROW_OPS)) } else { @@ -516,6 +571,16 @@ fn cell_popovers(column_view: >k::ColumnView, editable: bool) -> (gtk::Popover } } +/// Popovers are parented eagerly so each PopoverMenu's action muxer +/// snapshots the ColumnView's `cell` group at set_parent() time. Lazy +/// parenting in the gesture handler drops every activation silently. +fn eager_popover(column_view: >k::ColumnView, model: &gio::Menu) -> gtk::PopoverMenu { + let popover = gtk::PopoverMenu::from_model_full(model, gtk::PopoverMenuFlags::NESTED); + popover.set_has_arrow(true); + popover.set_parent(column_view); + popover +} + /// Insert Row on a right click below the last row. fn install_empty_space_menu(column_view: >k::ColumnView) -> gtk::PopoverMenu { let empty_menu = gio::Menu::new(); @@ -591,7 +656,7 @@ mod tests { #[gtk4::test] fn the_cell_menu_shows_the_cell_keys() { - let menu = build_cell_menu(MenuShape::FULL).upcast::(); + let menu = build_cell_menu(MenuShape::FULL, &gio::Menu::new()).upcast::(); assert_eq!( accel_of(&menu, "cell.set-null"), diff --git a/linux/crates/app/src/ui/grid/grid_msg.rs b/linux/crates/app/src/ui/grid/grid_msg.rs index 62eb484e8a..184327ee74 100644 --- a/linux/crates/app/src/ui/grid/grid_msg.rs +++ b/linux/crates/app/src/ui/grid/grid_msg.rs @@ -1,5 +1,7 @@ use std::rc::Rc; +use tablepro_core::browse::RowReference; + use super::{CellPreset, RowJsonRequest, RowObject}; use crate::services::tracker_edit::TrackerEdit; use crate::ui::clipboard::{CopyRequest, InClauseRows}; @@ -46,4 +48,6 @@ pub enum GridMsg { DuplicateRows { rows: Vec, }, + /// Open the rows a foreign key leads to from the row. + FollowReference(RowReference), } diff --git a/linux/crates/app/src/ui/grid/mod.rs b/linux/crates/app/src/ui/grid/mod.rs index 221e7c4931..8698d35c7c 100644 --- a/linux/crates/app/src/ui/grid/mod.rs +++ b/linux/crates/app/src/ui/grid/mod.rs @@ -31,6 +31,7 @@ mod grid_row_index; mod inline_editor; mod menu_rows; mod multiline_editor; +mod reference_menu; mod result_grid; mod result_sorters; mod row_factory; @@ -43,6 +44,7 @@ mod row_state_icon; mod row_watches; mod selection; mod tab_grid_context; +mod table_keys; #[cfg(test)] mod test_grid; mod toggle_cell; @@ -72,3 +74,4 @@ pub(crate) use self::row_snapshot::RowSnapshot; pub(crate) use self::row_state_column::data_columns; pub(crate) use self::selection::{export_snapshot, selected_positions}; pub(crate) use self::tab_grid_context::TabGridContext; +pub(crate) use self::table_keys::{KeysHandle, TableKeys}; diff --git a/linux/crates/app/src/ui/grid/reference_menu.rs b/linux/crates/app/src/ui/grid/reference_menu.rs new file mode 100644 index 0000000000..71ffd75c50 --- /dev/null +++ b/linux/crates/app/src/ui/grid/reference_menu.rs @@ -0,0 +1,474 @@ +use std::cell::RefCell; +use std::rc::Rc; +use std::sync::Arc; + +use gtk4::prelude::*; +use gtk4::{gio, glib}; +use tablepro_core::browse::{ReferenceError, RowReference}; +use tablepro_core::column::ResultColumn; +use tablepro_core::meta::TableRef; + +use super::{GridCell, GridMsg, GridSink, KeysHandle, RowObject, TabGridContext, TableKeys}; +use crate::ui::menu_label; + +pub(super) const OPEN_ONE: &str = "open-referenced-row"; +pub(super) const OPEN_CHOICE: &str = "open-reference"; +pub(super) const REFERENCING: &str = "show-referencing-rows"; + +/// The keys a row leads somewhere by, as a menu opened on one of its +/// cells offers them. +#[derive(Debug, Clone, Default)] +pub(super) struct Offers { + keys: Option>, + /// The table's keys that hold the cell's column, by position. + pub(super) open: Vec, + /// The other tables' keys that point at the row, by position. + pub(super) referencing: Vec, +} + +/// The cell menu's foreign key items: one section both cell menus show, +/// filled as a menu opens, since a submenu cannot hide itself. +#[derive(Clone)] +pub(super) struct ReferenceMenu { + tab: Rc, + columns: Arc<[ResultColumn]>, + section: gio::Menu, + /// The choices a column in more than one key offers, which the + /// keyboard shows on its own. + choices: gio::Menu, + armed: Rc>, +} + +impl ReferenceMenu { + pub(super) fn new(tab: Rc, columns: Arc<[ResultColumn]>) -> Self { + Self { + tab, + columns, + section: gio::Menu::new(), + choices: gio::Menu::new(), + armed: Rc::default(), + } + } + + pub(super) fn section(&self) -> &gio::Menu { + &self.section + } + + pub(super) fn choices(&self) -> &gio::Menu { + &self.choices + } + + /// What a menu opened on `cell` would offer, under the keys read so + /// far. A grid of query results has no keys and offers nothing. + pub(super) fn offers_for(&self, cell: &GridCell) -> Offers { + let (Some(keys), Some(row)) = (self.tab.keys.as_ref().map(KeysHandle::get), cell.row()) else { + return Offers::default(); + }; + let Some(column) = self.columns.get(cell.column()) else { + return Offers::default(); + }; + let shown = self.tab.snapshot(&row).to_values(); + let open = keys + .outbound + .iter() + .enumerate() + .filter(|(_, key)| key.columns.contains(&column.name)) + .filter(|(_, key)| leads_somewhere(RowReference::referenced(&keys.table, key, &self.columns, &shown))) + .map(|(index, _)| index) + .collect(); + // Other rows point at the row as it was saved; a draft is not + // saved, so nothing points at it yet. + let referencing = match row.cells() { + Some(saved) => keys + .inbound + .iter() + .enumerate() + .filter(|(_, key)| leads_somewhere(RowReference::referencing(&keys.table, key, &self.columns, &saved))) + .map(|(index, _)| index) + .collect(), + None => Vec::new(), + }; + Offers { + keys: Some(keys), + open, + referencing, + } + } + + /// Fill the section for `cell` as its menu opens. + pub(super) fn arm(&self, cell: &GridCell) { + let offers = self.offers_for(cell); + self.section.remove_all(); + self.choices.remove_all(); + if let Some(keys) = offers.keys.as_deref() { + for &index in &offers.open { + if let Some(key) = keys.outbound.get(index) { + let target = TableRef::new(key.ref_schema.clone(), key.ref_table.clone()); + self.choices.append_item(&choice( + &key_label(&keys.table, &target, &key.columns), + OPEN_CHOICE, + index, + )); + } + } + match offers.open.len() { + 0 => {} + 1 => self.section.append_item(&crate::application::menu_item( + &crate::i18n::gettext("Open Referenced Row"), + &format!("cell.{OPEN_ONE}"), + )), + _ => self + .section + .append_submenu(Some(&crate::i18n::gettext("Open Referenced Row")), &self.choices), + } + let referencing = gio::Menu::new(); + for &index in &offers.referencing { + if let Some(key) = keys.inbound.get(index) { + referencing.append_item(&choice( + &key_label(&keys.table, &key.table, &key.columns), + REFERENCING, + index, + )); + } + } + if referencing.n_items() > 0 { + self.section + .append_submenu(Some(&crate::i18n::gettext("Show Referencing Rows")), &referencing); + } + } + self.armed.replace(offers); + } + + /// The row the outbound key at `index` points at from `row`, under + /// the keys the menu opened with. + pub(super) fn referenced(&self, index: usize, row: &RowObject) -> Option> { + let armed = self.armed.borrow(); + let keys = armed.keys.as_ref()?; + let key = keys.outbound.get(index)?; + let shown = self.tab.snapshot(row).to_values(); + Some(RowReference::referenced(&keys.table, key, &self.columns, &shown)) + } + + /// The rows the inbound key at `index` holds that point at `row`. + pub(super) fn referencing(&self, index: usize, row: &RowObject) -> Option> { + let armed = self.armed.borrow(); + let keys = armed.keys.as_ref()?; + let key = keys.inbound.get(index)?; + let saved = row.cells()?; + Some(RowReference::referencing(&keys.table, key, &self.columns, &saved)) + } + + /// The one outbound key the armed menu offers, for its plain item. + pub(super) fn only_choice(&self) -> Option { + match self.armed.borrow().open.as_slice() { + [index] => Some(*index), + _ => None, + } + } +} + +/// Open what a key leads to, or say why it cannot. +pub(super) fn follow(sink: &GridSink, reference: Result) { + match reference { + Ok(reference) => sink(GridMsg::FollowReference(reference)), + Err(error) => sink(GridMsg::ShowToast(crate::ui::error_text::reference_message(&error))), + } +} + +/// A key that cannot be followed is still offered, so the toast it +/// raises says why; one with nothing to follow is not. +fn leads_somewhere(reference: Result) -> bool { + match reference { + Ok(_) => true, + Err(error) => !error.leaves_nothing_to_follow(), + } +} + +fn choice(label: &str, action: &str, index: usize) -> gio::MenuItem { + let item = gio::MenuItem::new(Some(label), None); + let target = u32::try_from(index).unwrap_or(u32::MAX); + item.set_action_and_target_value(Some(&format!("cell.{action}")), Some(&target.to_variant())); + item +} + +/// The other table, named with its schema where that is not the +/// source's, and the key's columns on the side that holds the key. +fn key_label(source: &TableRef, table: &TableRef, columns: &[String]) -> String { + let name = match (&table.schema, table.schema == source.schema) { + (Some(schema), false) => format!("{schema}.{}", table.name), + _ => table.name.clone(), + }; + menu_label(&format!("{name} ({})", columns.join(", "))) +} + +/// The index a parameterized item carries. +pub(super) fn chosen(parameter: Option<&glib::Variant>) -> Option { + parameter + .and_then(glib::Variant::get::) + .and_then(|index| usize::try_from(index).ok()) +} + +#[cfg(test)] +pub(super) mod tests { + use gtk4 as gtk; + use gtk4::gio; + use gtk4::prelude::*; + use tablepro_core::ddl::InboundForeignKey; + use tablepro_core::filter::{Combinator, FilterOp, FilterRule, FilterSet, FilterValue}; + use tablepro_core::meta::TableRef; + use tablepro_core::{ForeignKeyInfo, Value}; + + use crate::test_support::descendants; + use crate::ui::grid::test_grid::{Grid, named, open_menu}; + use crate::ui::grid::{GridMsg, TableKeys}; + + pub(in crate::ui::grid) fn public(name: &str) -> TableRef { + TableRef::new(Some("public".to_owned()), name) + } + + pub(in crate::ui::grid) fn key(columns: &[&str], to: TableRef, ref_columns: &[&str]) -> ForeignKeyInfo { + ForeignKeyInfo { + name: format!("{}_fkey", columns.join("_")), + columns: columns.iter().map(|name| (*name).to_owned()).collect(), + ref_schema: to.schema, + ref_table: to.name, + ref_columns: ref_columns.iter().map(|name| (*name).to_owned()).collect(), + on_delete: None, + on_update: None, + } + } + + pub(in crate::ui::grid) fn filter(rules: &[(&str, &str)]) -> FilterSet { + FilterSet { + combinator: Combinator::And, + rules: rules + .iter() + .map(|(column, text)| FilterRule { + column: (*column).to_owned(), + op: FilterOp::Eq, + value: Some(FilterValue::Single((*text).to_owned())), + }) + .collect(), + extra_sql: None, + } + } + + /// `orders` keyed by `id`, whose `customer_id` points at customers. + pub(in crate::ui::grid) fn orders_grid(outbound: Vec) -> Grid { + Grid::open_with_keys( + vec![ + named("id", "integer", true), + named("customer_id", "integer", false), + named("tenant_id", "integer", false), + ], + vec![ + vec![Value::Int(1), Value::Int(42), Value::Int(7)], + vec![Value::Int(2), Value::Null, Value::Int(7)], + ], + TableKeys { + table: public("orders"), + outbound, + inbound: Vec::new(), + }, + ) + } + + /// The foreign key items the open cell menu holds, submenus opened, + /// as each one's label, action and the index it carries. + pub(in crate::ui::grid) fn reference_items(grid: &Grid) -> Vec<(String, Option, Option)> { + let menu = descendants(&grid.view) + .into_iter() + .filter_map(|widget| widget.downcast::().ok()) + .filter_map(|popover| popover.menu_model()) + .find(|model| { + items(model) + .iter() + .any(|(_, action, _)| action.as_deref() == Some("cell.show-row-json")) + }) + .expect("the cell menu"); + items(&menu) + .into_iter() + .filter(|(label, action, _)| { + label.contains("Referenc") || action.as_ref().is_some_and(|a| a.contains("referenc")) + }) + .collect() + } + + fn items(menu: &gio::MenuModel) -> Vec<(String, Option, Option)> { + let mut found = Vec::new(); + for position in 0..menu.n_items() { + let attribute = |name: &str| menu.item_attribute_value(position, name, None); + if let Some(label) = attribute("label").and_then(|value| value.get::()) { + found.push(( + label, + attribute("action").and_then(|value| value.get::()), + attribute("target").and_then(|value| value.get::()), + )); + } + for link in ["section", "submenu"] { + if let Some(inner) = menu.item_link(position, link) { + found.extend(items(&inner)); + } + } + } + found + } + + pub(in crate::ui::grid) fn followed(grid: &Grid) -> Vec<(TableRef, FilterSet)> { + grid.take_messages() + .into_iter() + .filter_map(|msg| match msg { + GridMsg::FollowReference(reference) => Some((reference.table, reference.filter)), + _ => None, + }) + .collect() + } + + fn item(label: &str, action: Option<&str>, target: Option) -> (String, Option, Option) { + (label.to_owned(), action.map(str::to_owned), target) + } + + #[gtk4::test] + fn a_key_cell_offers_open_referenced_row_and_sends_the_rows_it_names() { + let grid = orders_grid(vec![key(&["customer_id"], public("customers"), &["id"])]); + + open_menu(&grid.cell_of(1, 1)); + let offered = reference_items(&grid); + grid.view + .activate_action("cell.open-referenced-row", None) + .expect("the menu's Open Referenced Row"); + + assert_eq!( + offered, + [item("Open Referenced Row", Some("cell.open-referenced-row"), None)] + ); + assert_eq!(followed(&grid), [(public("customers"), filter(&[("id", "42")]))]); + } + + #[gtk4::test] + fn a_null_key_and_a_column_in_no_key_offer_nothing() { + let grid = orders_grid(vec![key(&["customer_id"], public("customers"), &["id"])]); + + open_menu(&grid.cell_of(2, 1)); + let on_null = reference_items(&grid); + open_menu(&grid.cell_of(1, 2)); + let on_other = reference_items(&grid); + + assert_eq!(on_null, []); + assert_eq!(on_other, []); + } + + #[gtk4::test] + fn a_column_in_two_keys_offers_each_and_follows_the_one_chosen() { + let crm = TableRef::new(Some("crm".to_owned()), "customers"); + let grid = orders_grid(vec![ + key(&["tenant_id"], public("tenants"), &["id"]), + key(&["tenant_id", "customer_id"], crm.clone(), &["tenant_id", "id"]), + ]); + + open_menu(&grid.cell_of(1, 2)); + let offered = reference_items(&grid); + grid.view + .activate_action("cell.open-reference", Some(&1u32.to_variant())) + .expect("the second key"); + + assert_eq!( + offered, + [ + item("Open Referenced Row", None, None), + item("tenants (tenant__id)", Some("cell.open-reference"), Some(0)), + item( + "crm.customers (tenant__id, customer__id)", + Some("cell.open-reference"), + Some(1) + ), + ] + ); + assert_eq!(followed(&grid), [(crm, filter(&[("tenant_id", "7"), ("id", "42")]))]); + } + + #[gtk4::test] + fn a_key_the_catalogue_could_not_pair_is_offered_and_says_why_it_cannot_open() { + let grid = orders_grid(vec![key(&["customer_id"], public("ghost"), &[])]); + + open_menu(&grid.cell_of(1, 1)); + let offered = reference_items(&grid).len(); + grid.view + .activate_action("cell.open-referenced-row", None) + .expect("the menu's Open Referenced Row"); + + assert_eq!(offered, 1); + let toasts: Vec = grid + .take_messages() + .into_iter() + .filter_map(|msg| match msg { + GridMsg::ShowToast(text) => Some(text), + GridMsg::FollowReference(reference) => panic!("opened {reference:?}"), + _ => None, + }) + .collect(); + assert_eq!(toasts.len(), 1); + assert!(toasts[0].contains("customer_id_fkey"), "{toasts:?}"); + } + + #[gtk4::test] + fn referencing_rows_are_offered_for_a_saved_row_and_not_for_a_draft() { + let orders = TableRef::new(Some("sales".to_owned()), "orders"); + let grid = Grid::open_with_keys( + vec![named("id", "integer", true), named("name", "text", false)], + vec![vec![Value::Int(1), Value::Text("Ada".into())]], + TableKeys { + table: public("customers"), + outbound: Vec::new(), + inbound: vec![InboundForeignKey { + name: "orders_customer_id_fkey".to_owned(), + table: orders.clone(), + columns: vec!["customer_id".to_owned()], + referenced_columns: vec!["id".to_owned()], + }], + }, + ); + + open_menu(&grid.cell_of(1, 1)); + let offered = reference_items(&grid); + grid.view + .activate_action("cell.show-referencing-rows", Some(&0u32.to_variant())) + .expect("the menu's Show Referencing Rows"); + let opened = followed(&grid); + grid.insert_draft(); + let draft = grid + .cells() + .into_iter() + .find(|cell| cell.row().is_some_and(|row| row.draft_id().is_some())) + .expect("the draft's cell"); + open_menu(&draft); + + assert_eq!( + offered, + [ + item("Show Referencing Rows", None, None), + item( + "sales.orders (customer__id)", + Some("cell.show-referencing-rows"), + Some(0) + ), + ] + ); + assert_eq!(opened, [(orders, filter(&[("customer_id", "1")]))]); + assert_eq!(reference_items(&grid), [], "a draft is offered rows that point at it"); + } + + #[gtk4::test] + fn a_result_grid_offers_no_reference_items() { + let grid = Grid::open( + vec![named("id", "integer", true), named("customer_id", "integer", false)], + vec![vec![Value::Int(1), Value::Int(42)]], + ); + + open_menu(&grid.cell_of(1, 1)); + + assert_eq!(reference_items(&grid), []); + assert!(grid.view.activate_action("cell.open-referenced-row", None).is_ok()); + assert_eq!(followed(&grid), []); + } +} diff --git a/linux/crates/app/src/ui/grid/tab_grid_context.rs b/linux/crates/app/src/ui/grid/tab_grid_context.rs index 1843d93f89..a4241ec761 100644 --- a/linux/crates/app/src/ui/grid/tab_grid_context.rs +++ b/linux/crates/app/src/ui/grid/tab_grid_context.rs @@ -5,13 +5,13 @@ use tablepro_core::edit::CellInput; use tablepro_core::meta::RowIdentity; use super::row_state_icon::RowPendingState; -use super::{ColumnWidthBinding, GridRowId, RowObject, RowSnapshot}; +use super::{ColumnWidthBinding, GridRowId, KeysHandle, RowObject, RowSnapshot}; use crate::services::cell_overlay::CellMarks; use crate::services::change_tracker::{CellState, RowKey}; use crate::services::tab_tracker_handle::TabTrackerHandle; -/// `tracker` and `column_widths` are `None` for a grid nothing tracks, -/// such as editor results. +/// `tracker`, `column_widths` and `keys` are `None` for a grid nothing +/// tracks, such as editor results. #[derive(Debug, Clone)] pub struct TabGridContext { pub tracker: Option, @@ -22,6 +22,9 @@ pub struct TabGridContext { /// row cannot change. pub identity: RowIdentity, pub column_widths: Option, + /// The foreign keys of the table the rows come from, which the cell + /// menu follows. + pub keys: Option, } impl Default for TabGridContext { @@ -31,6 +34,7 @@ impl Default for TabGridContext { editable: false, identity: RowIdentity::Unordered, column_widths: None, + keys: None, } } } diff --git a/linux/crates/app/src/ui/grid/table_keys.rs b/linux/crates/app/src/ui/grid/table_keys.rs new file mode 100644 index 0000000000..7a4bd75719 --- /dev/null +++ b/linux/crates/app/src/ui/grid/table_keys.rs @@ -0,0 +1,46 @@ +use std::cell::RefCell; +use std::rc::Rc; + +use tablepro_core::ForeignKeyInfo; +use tablepro_core::ddl::InboundForeignKey; +use tablepro_core::meta::TableRef; + +/// A table's foreign keys both ways, as its tab last read them. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct TableKeys { + pub(crate) table: TableRef, + /// The table's own keys, which point at other rows. + pub(crate) outbound: Vec, + /// Other tables' keys that point at this one. + pub(crate) inbound: Vec, +} + +impl TableKeys { + pub(crate) fn none(table: TableRef) -> Self { + Self { + table, + outbound: Vec::new(), + inbound: Vec::new(), + } + } +} + +/// Shared by a table tab and every grid it builds, so keys read after a +/// grid was built reach its menus. A menu takes the keys as it opens, +/// so a read that lands while it is open changes nothing it shows. +#[derive(Debug, Clone)] +pub struct KeysHandle(Rc>>); + +impl KeysHandle { + pub(crate) fn new(table: TableRef) -> Self { + Self(Rc::new(RefCell::new(Rc::new(TableKeys::none(table))))) + } + + pub(crate) fn get(&self) -> Rc { + self.0.borrow().clone() + } + + pub(crate) fn set(&self, keys: TableKeys) { + self.0.replace(Rc::new(keys)); + } +} diff --git a/linux/crates/app/src/ui/grid/test_grid.rs b/linux/crates/app/src/ui/grid/test_grid.rs index ee98a3b8b8..29580719cd 100644 --- a/linux/crates/app/src/ui/grid/test_grid.rs +++ b/linux/crates/app/src/ui/grid/test_grid.rs @@ -10,8 +10,8 @@ use tablepro_core::meta::RowIdentity; use tablepro_core::{ColumnInfo, Value}; use super::{ - ColumnWidthBinding, GridCell, GridContent, GridMsg, GridSink, GridSorting, RowObject, TabGridContext, begin_edit, - build_column_view, data_columns, + ColumnWidthBinding, GridCell, GridContent, GridMsg, GridSink, GridSorting, KeysHandle, RowObject, TabGridContext, + TableKeys, begin_edit, build_column_view, data_columns, }; use crate::services::change_tracker::RowKey; use crate::services::tab_tracker_handle::TabTrackerHandle; @@ -70,6 +70,7 @@ pub(super) fn tracked(tracker: &TabTrackerHandle) -> TabGridContext { editable: true, identity: keyed_by_id(), column_widths: None, + keys: None, } } @@ -109,7 +110,7 @@ pub(super) struct Grid { impl Grid { /// `rows` keyed by their first cell, in a grid that edits them. pub(super) fn open(columns: Vec, rows: Vec>) -> Self { - Self::open_in(columns, rows, None) + Self::open_in(columns, rows, None, None) } /// As `open`, with the column widths saved through `widths`. @@ -118,10 +119,22 @@ impl Grid { rows: Vec>, widths: ColumnWidthBinding, ) -> Self { - Self::open_in(columns, rows, Some(widths)) + Self::open_in(columns, rows, Some(widths), None) } - fn open_in(columns: Vec, rows: Vec>, widths: Option) -> Self { + /// As `open`, for a table with `keys`. + pub(super) fn open_with_keys(columns: Vec, rows: Vec>, keys: TableKeys) -> Self { + let handle = KeysHandle::new(keys.table.clone()); + handle.set(keys); + Self::open_in(columns, rows, None, Some(handle)) + } + + fn open_in( + columns: Vec, + rows: Vec>, + widths: Option, + keys: Option, + ) -> Self { let tracker = TabTrackerHandle::default(); let (sink, messages) = recorder(); let (view, selection) = build_column_view( @@ -131,6 +144,7 @@ impl Grid { GridSorting::Server { initial: None }, TabGridContext { column_widths: widths, + keys, ..tracked(&tracker) }, ); @@ -242,6 +256,16 @@ pub(super) fn page(ids: std::ops::Range) -> Vec { persisted(&rows) } +/// Open the cell's menu as a right click does. +pub(super) fn open_menu(cell: &GridCell) { + let controllers = cell.observe_controllers(); + let gesture = (0..controllers.n_items()) + .filter_map(|index| controllers.item(index).and_downcast::()) + .find(|gesture| gesture.button() == 3) + .expect("a cell answers a right-click"); + gesture.emit_by_name::<()>("pressed", &[&1i32, &1.0f64, &1.0f64]); +} + /// Whether the window's focus is the grid or inside it. pub(super) fn focus_in_grid(grid: &Grid) -> bool { gtk::prelude::GtkWindowExt::focus(&grid.window) diff --git a/linux/crates/app/src/ui/menu_label.rs b/linux/crates/app/src/ui/menu_label.rs new file mode 100644 index 0000000000..f3862c1307 --- /dev/null +++ b/linux/crates/app/src/ui/menu_label.rs @@ -0,0 +1,18 @@ +/// A GtkPopoverMenu reads an underscore in a label as a mnemonic, so a +/// name the user or the database chose keeps its underscores by +/// doubling them. +pub(crate) fn menu_label(text: &str) -> String { + text.replace('_', "__") +} + +#[cfg(test)] +mod tests { + use super::menu_label; + + #[test] + fn underscores_are_doubled_and_nothing_else_changes() { + assert_eq!(menu_label("account_id"), "account__id"); + assert_eq!(menu_label("__x"), "____x"); + assert_eq!(menu_label("Sales Q1"), "Sales Q1"); + } +} diff --git a/linux/crates/app/src/ui/mod.rs b/linux/crates/app/src/ui/mod.rs index 481565c97a..41cf12a3ee 100644 --- a/linux/crates/app/src/ui/mod.rs +++ b/linux/crates/app/src/ui/mod.rs @@ -23,6 +23,7 @@ mod host_key_failure; mod host_key_review; pub(crate) mod icons; mod import_dialog; +mod menu_label; mod name_entry; mod picker; mod plain_toast; @@ -43,6 +44,7 @@ mod window_geometry; mod workspace; pub(crate) mod write_confirmation; +pub(crate) use menu_label::menu_label; pub(crate) use plain_toast::plain_toast; pub(crate) use preferences_dialog::PreferencesDialog; pub(crate) use session_window::{SessionWindow, SessionWindowInit, SessionWindowInput}; diff --git a/linux/crates/app/src/ui/picker/connection_menu.rs b/linux/crates/app/src/ui/picker/connection_menu.rs index 1f5cdbfeff..750aafc88e 100644 --- a/linux/crates/app/src/ui/picker/connection_menu.rs +++ b/linux/crates/app/src/ui/picker/connection_menu.rs @@ -3,6 +3,8 @@ use gtk4::{gio, glib}; use tablepro_storage::{ConnectionColor, ConnectionGroupName}; use uuid::Uuid; +use crate::ui::menu_label; + pub(super) const PICKER: &str = "picker"; pub(super) const CONNECT: &str = "connect"; pub(super) const EDIT: &str = "edit"; @@ -104,12 +106,6 @@ fn group_menu(target: &glib::Variant, groups: &[ConnectionGroupName]) -> gio::Me menu } -/// A GtkPopoverMenu reads an underscore in a label as a mnemonic, so a -/// group the user named keeps its underscores by doubling them. -fn menu_label(text: &str) -> String { - text.replace('_', "__") -} - pub(super) fn color_name(color: ConnectionColor) -> String { match color { ConnectionColor::Blue => crate::i18n::gettext("Blue"), diff --git a/linux/crates/app/src/ui/session_window/tests/mod.rs b/linux/crates/app/src/ui/session_window/tests/mod.rs index 2cfccd433f..7e41b58cc9 100644 --- a/linux/crates/app/src/ui/session_window/tests/mod.rs +++ b/linux/crates/app/src/ui/session_window/tests/mod.rs @@ -119,7 +119,11 @@ impl TestWindow { } connection.probe.always(FakeReply::ok(())); connection.list_schemas.always(FakeReply::ok(Vec::new())); - connection.list_objects.always(FakeReply::ok(Vec::new())); + // A test that lists the database's objects for every read keeps + // its list. + if !connection.list_objects.answers_always() { + connection.list_objects.always(FakeReply::ok(Vec::new())); + } let mut drivers = DriverRegistry::new(); let capabilities = if network { &FakeDriver::NETWORK_CAPABILITIES diff --git a/linux/crates/app/src/ui/structure_tab/form_dialog.rs b/linux/crates/app/src/ui/structure_tab/form_dialog.rs index 7d53c77244..5ea8e43f43 100644 --- a/linux/crates/app/src/ui/structure_tab/form_dialog.rs +++ b/linux/crates/app/src/ui/structure_tab/form_dialog.rs @@ -141,12 +141,6 @@ pub(super) fn mark_invalid(widget: &impl IsA, problem: Option<&str> crate::ui::a11y::show_invalid(widget, problem); } -/// A GtkPopoverMenu reads an underscore in a label as a mnemonic, so a -/// column name keeps its underscores by doubling them. -pub(super) fn menu_label(text: &str) -> String { - text.replace('_', "__") -} - #[cfg(test)] pub(super) mod tests { use std::time::Duration; @@ -173,11 +167,6 @@ pub(super) mod tests { OwnedWindow::presented(adw::Window::new()) } - #[test] - fn a_column_name_keeps_its_underscores_in_a_menu() { - assert_eq!(menu_label("account_id"), "account__id"); - } - /// A form handed to its dialog as the add dialogs hand theirs: the /// state holds the form, and the form its dialog. fn open_form(window: &adw::Window) -> (glib::WeakRef, std::rc::Weak) { diff --git a/linux/crates/app/src/ui/structure_tab/index_dialog.rs b/linux/crates/app/src/ui/structure_tab/index_dialog.rs index 54df15c48b..93a42414c7 100644 --- a/linux/crates/app/src/ui/structure_tab/index_dialog.rs +++ b/linux/crates/app/src/ui/structure_tab/index_dialog.rs @@ -11,8 +11,9 @@ use libadwaita as adw; use libadwaita::prelude::*; use tablepro_core::IndexInfo; -use super::form_dialog::{FormDialog, give_state, mark_invalid, menu_label}; +use super::form_dialog::{FormDialog, give_state, mark_invalid}; use super::index_draft::{DraftKey, IndexDraft, IndexDraftError}; +use crate::ui::menu_label; /// The dialog, which owns the form's state (`give_state`). pub(super) struct IndexDialog { diff --git a/linux/crates/app/src/ui/workspace/mod.rs b/linux/crates/app/src/ui/workspace/mod.rs index 51ba750807..dae5017189 100644 --- a/linux/crates/app/src/ui/workspace/mod.rs +++ b/linux/crates/app/src/ui/workspace/mod.rs @@ -9,6 +9,7 @@ mod health_banner; mod imp; mod import; mod import_ledger; +mod references; mod row_ops; mod saves; mod session_watches; @@ -242,6 +243,7 @@ pub enum ClosedTabDescriptor { offset: u64, page_size: u64, sort: Option<(usize, bool)>, + filter: tablepro_core::FilterSet, }, Structure { schema: Option, @@ -528,6 +530,18 @@ pub(crate) enum WorkspaceInput { }, /// The table is gone, so its tabs close without asking. DropTableSucceeded(tablepro_core::TableRef), + /// A table tab's grid asked to open the rows a foreign key leads to. + FollowReference { + tab: uuid::Uuid, + reference: Box, + }, + /// Whether the table a key names is in its schema, read before a + /// tab opens on it. `None` when the list could not be read. + ReferenceChecked { + tab: uuid::Uuid, + reference: Box, + found: Option, + }, SchemaChanged { schema: Option, table: Option, @@ -1019,6 +1033,10 @@ impl WorkspaceModel { WorkspaceInput::DropTableChecked { table, referencing } => self.on_drop_table_checked(table, &referencing), WorkspaceInput::DropTableConfirmed { schema, table } => self.on_drop_table_confirmed(schema, table), WorkspaceInput::DropTableSucceeded(table) => self.on_drop_table_succeeded(table), + WorkspaceInput::FollowReference { tab, reference } => self.on_follow_reference(tab, *reference), + WorkspaceInput::ReferenceChecked { tab, reference, found } => { + self.on_reference_checked(tab, *reference, found); + } WorkspaceInput::SchemaChanged { schema, table } => self.on_schema_changed(schema, table), WorkspaceInput::NewEditorTab => self.open_editor_tab(None), WorkspaceInput::EditorTabRunStateChanged(id, running) => self.on_editor_tab_run_state_changed(id, running), diff --git a/linux/crates/app/src/ui/workspace/references.rs b/linux/crates/app/src/ui/workspace/references.rs new file mode 100644 index 0000000000..6e555df276 --- /dev/null +++ b/linux/crates/app/src/ui/workspace/references.rs @@ -0,0 +1,109 @@ +use libadwaita as adw; +use tablepro_core::browse::RowReference; +use tablepro_core::meta::{ObjectInfo, ObjectScope}; +use uuid::Uuid; + +use super::tab_spec::{TabSpec, TablePage}; +use super::{WorkspaceInput, WorkspaceModel, WorkspaceTab}; +use crate::runtime::DeliverInput; +use crate::ui::browse_tab::TableFilter; + +impl WorkspaceModel { + /// A tab already on the table under the same filter is shown again. + /// Otherwise the table is looked for in its schema first, so a key + /// whose table is gone says so rather than opening an error page. + pub(super) fn on_follow_reference(&mut self, source: Uuid, reference: RowReference) { + if let Some(page) = self.tab_showing(&reference) { + self.tab_view.set_selected_page(&page); + return; + } + let Some(session) = self.context.active.get() else { + return; + }; + let scope = match &reference.table.schema { + Some(schema) => ObjectScope::Schema(schema.clone()), + None => ObjectScope::DefaultSchema, + }; + let input = self.input.clone(); + let name = reference.table.name.clone(); + let work = async move { + let lists = |objects: &[ObjectInfo]| { + objects + .iter() + .any(|object| object.name == name && object.kind.is_browsable()) + }; + // The catalog's copy can be older than the key, so a table + // it does not list is looked for once more on the server. + match session.objects(scope.clone()).await { + Ok(objects) if lists(&objects) => Ok(true), + _ => session.refresh_objects(scope).await.map(|objects| lists(&objects)), + } + }; + crate::runtime::spawn_and_deliver(&self.context.tasks, work, move |listed| { + let found = match listed { + Ok(Ok(found)) => Some(found), + Ok(Err(error)) => { + tracing::debug!(category = ?error.category(), "the objects of a key's schema could not be read"); + None + } + Err(failure) => { + tracing::debug!(%failure, "the read of a key's schema did not finish"); + None + } + }; + input.deliver(WorkspaceInput::ReferenceChecked { + tab: source, + reference: Box::new(reference), + found, + }); + }); + } + + /// A list that could not be read leaves the answer to the tab, whose + /// own read says what went wrong. + pub(super) fn on_reference_checked(&mut self, source: Uuid, reference: RowReference, found: Option) { + if found == Some(false) { + let table = match &reference.table.schema { + Some(schema) => format!("{schema}.{}", reference.table.name), + None => reference.table.name.clone(), + }; + self.show_toast(&crate::i18n::gettext_f( + "Can't find the table {table}", + &[("table", &table)], + )); + return; + } + if let Some(page) = self.tab_showing(&reference) { + self.tab_view.set_selected_page(&page); + return; + } + let spec = TabSpec::Table { + table: reference.table, + page: TablePage::first(u64::from(self.settings.default_page_size())), + filter: TableFilter::Followed(reference.filter), + }; + let source_page = self.workspace_tabs.borrow().get(&source).map(|tab| tab.page().clone()); + let page = match source_page { + Some(parent) => self.factory.create_beside(spec, &parent), + None => self.factory.create(spec), + }; + if let Some(page) = page { + self.tab_view.set_selected_page(&page); + } + } + + /// The table tab on the reference's table whose rows were read under + /// exactly its filter. + fn tab_showing(&self, reference: &RowReference) -> Option { + self.workspace_tabs.borrow().values().find_map(|tab| match tab { + WorkspaceTab::Table(slot) + if slot.schema == reference.table.schema + && slot.table == reference.table.name + && slot.browse.applied_filter() == reference.filter => + { + Some(slot.page.clone()) + } + _ => None, + }) + } +} diff --git a/linux/crates/app/src/ui/workspace/shell.rs b/linux/crates/app/src/ui/workspace/shell.rs index 509ecd67ab..f8ebe662b8 100644 --- a/linux/crates/app/src/ui/workspace/shell.rs +++ b/linux/crates/app/src/ui/workspace/shell.rs @@ -436,6 +436,7 @@ pub(super) mod tests { model.open_tab(super::super::tab_spec::TabSpec::Table { table: tablepro_core::meta::TableRef::new(Some("public".to_owned()), "customers"), page: super::super::tab_spec::TablePage::first(1_000), + filter: crate::ui::browse_tab::TableFilter::Remembered, }) }) .flatten() diff --git a/linux/crates/app/src/ui/workspace/sidebar/tests.rs b/linux/crates/app/src/ui/workspace/sidebar/tests.rs index a99de37cfe..76365c567e 100644 --- a/linux/crates/app/src/ui/workspace/sidebar/tests.rs +++ b/linux/crates/app/src/ui/workspace/sidebar/tests.rs @@ -574,6 +574,7 @@ fn sync_selection_selects_matching_row_and_clears_for_editor() { model.open_tab(TabSpec::Table { table: table("users"), page: TablePage::first(100), + filter: crate::ui::browse_tab::TableFilter::Remembered, }) }); wait_until(WAIT, || sidebar_selected(&workspace) == Some(table("users"))).expect("the table's row selected"); diff --git a/linux/crates/app/src/ui/workspace/structure.rs b/linux/crates/app/src/ui/workspace/structure.rs index 5565b75231..942b2abb13 100644 --- a/linux/crates/app/src/ui/workspace/structure.rs +++ b/linux/crates/app/src/ui/workspace/structure.rs @@ -285,6 +285,7 @@ impl WorkspaceModel { self.open_tab(TabSpec::Table { table: TableRef::new(schema.clone(), name.clone()), page: TablePage::first(page_size), + filter: crate::ui::browse_tab::TableFilter::Remembered, }); self.input.deliver(WorkspaceInput::SchemaChanged { schema, diff --git a/linux/crates/app/src/ui/workspace/tab_factory.rs b/linux/crates/app/src/ui/workspace/tab_factory.rs index ede0a9f16a..8807f66c9f 100644 --- a/linux/crates/app/src/ui/workspace/tab_factory.rs +++ b/linux/crates/app/src/ui/workspace/tab_factory.rs @@ -25,7 +25,7 @@ use crate::services::change_tracker_registry::ChangeTrackerRegistry; use crate::services::history_service::HistoryService; use crate::services::session_context::SessionContext; use crate::services::structure_tracker_registry::StructureTrackerRegistry; -use crate::ui::browse_tab::{BrowseTab, BrowseTabInit}; +use crate::ui::browse_tab::{BrowseTab, BrowseTabInit, TableFilter}; use crate::ui::editor::{SqlEditor, SqlEditorInit}; use crate::ui::structure_tab::{StructureMode, StructureTab, StructureTabInit}; use crate::ui::tab_load::FirstLoad; @@ -67,15 +67,21 @@ impl TabFactory { /// Build the tab and append its page, unselected. `None` when the /// spec needs a session and the workspace has none any more. pub(super) fn create(&self, spec: TabSpec) -> Option { - self.build(spec, FirstLoad::Now) + self.build(spec, FirstLoad::Now, None) + } + + /// As `create`, with the page placed after `parent` and its other + /// children, so closing it goes back there. + pub(super) fn create_beside(&self, spec: TabSpec, parent: &adw::TabPage) -> Option { + self.build(spec, FirstLoad::Now, Some(parent)) } /// A saved tab, which reads its table the first time it is shown. pub(super) fn restore(&self, spec: TabSpec) -> Option { - self.build(spec, FirstLoad::WhenShown) + self.build(spec, FirstLoad::WhenShown, None) } - fn build(&self, spec: TabSpec, first_load: FirstLoad) -> Option { + fn build(&self, spec: TabSpec, first_load: FirstLoad, parent: Option<&adw::TabPage>) -> Option { let id = Uuid::new_v4(); let slot = match spec { TabSpec::Editor { @@ -83,7 +89,16 @@ impl TabFactory { draft, file, } => self.editor(id, initial_query, draft, file), - TabSpec::Table { table, page } => self.table(id, table, page, first_load)?, + TabSpec::Table { table, page, filter } => self.table( + id, + TableOpening { + table, + page, + filter, + first_load, + parent, + }, + )?, TabSpec::Structure { target } => self.structure(id, target, first_load)?, }; Some(self.insert(id, slot)) @@ -104,8 +119,11 @@ impl TabFactory { page } - fn append(&self, child: &impl IsA, id: Uuid) -> adw::TabPage { - let page = self.parts.view.append(child); + fn append(&self, child: &impl IsA, id: Uuid, parent: Option<&adw::TabPage>) -> adw::TabPage { + let page = match parent { + Some(parent) => self.parts.view.add_page(child, Some(parent)), + None => self.parts.view.append(child), + }; self.parts.index.insert(&page, id); page } @@ -136,7 +154,7 @@ impl TabFactory { editor.restore_file(gio::File::for_uri(&file.uri), file.modified); } let ordinal = self.free_ordinal(); - let page = self.append(&editor, id); + let page = self.append(&editor, id, None); let label = match (opened_empty, editor.file()) { (_, Some(file)) => EditorLabel::file(&file), (true, None) => EditorLabel::Untitled(ordinal), @@ -158,10 +176,16 @@ impl TabFactory { }) } - fn table(&self, id: Uuid, table: TableRef, page: TablePage, first_load: FirstLoad) -> Option { + fn table(&self, id: Uuid, opening: TableOpening<'_>) -> Option { let parts = &self.parts; let session = parts.context.active.get()?; - let TableRef { schema, name } = table; + let TableOpening { + table: TableRef { schema, name }, + page, + filter, + first_load, + parent, + } = opening; let browse = BrowseTab::new(BrowseTabInit { tracker: parts.change_trackers.register(id), schema: schema.clone(), @@ -171,6 +195,7 @@ impl TabFactory { page_size: page.page_size, initial_offset: page.offset, initial_sort: page.sort, + filter, column_widths: parts.storage.column_widths().clone(), filter_settings: parts.storage.filter_settings().clone(), session, @@ -180,7 +205,7 @@ impl TabFactory { first_load, }); connect_browse(&browse, id, &parts.input); - let tab_page = self.append(&browse, id); + let tab_page = self.append(&browse, id, parent); let follow = follow_loading(&tab_page, browse.is_loading()); browse.connect_loading_changed(move |_, loading| follow(loading)); Some(WorkspaceTab::Table(TableTabSlot { @@ -209,7 +234,7 @@ impl TabFactory { first_load, }); connect_structure(&structure, id, &parts.input); - let page = self.append(&structure, id); + let page = self.append(&structure, id, None); let follow = follow_loading(&page, structure.is_loading()); structure.connect_loading_changed(move |_, loading| follow(loading)); Some(WorkspaceTab::Structure(StructureTabSlot { @@ -240,6 +265,15 @@ impl TabFactory { } } +/// What a table tab opens on, and where its page goes. +struct TableOpening<'a> { + table: TableRef, + page: TablePage, + filter: TableFilter, + first_load: FirstLoad, + parent: Option<&'a adw::TabPage>, +} + pub(super) const PREVIEW_CHARS: i32 = 200; /// The start of an editor's script, read without copying the rest. @@ -318,6 +352,13 @@ fn connect_browse(browse: &BrowseTab, tab_id: Uuid, input: &WorkspaceSender) { browse.connect_stop_save_requested(move |_| stop.deliver(WorkspaceInput::StopSaving(tab_id))); let refresh = input.clone(); browse.connect_refresh_requested(move |_| refresh.deliver(WorkspaceInput::RefreshTab(tab_id))); + let follow = input.clone(); + browse.connect_follow_reference(move |_, reference| { + follow.deliver(WorkspaceInput::FollowReference { + tab: tab_id, + reference: Box::new(reference), + }); + }); } /// Forward an editor tab's signals into the workspace. The toast diff --git a/linux/crates/app/src/ui/workspace/tab_spec.rs b/linux/crates/app/src/ui/workspace/tab_spec.rs index 4b4423f25c..2041c5b33e 100644 --- a/linux/crates/app/src/ui/workspace/tab_spec.rs +++ b/linux/crates/app/src/ui/workspace/tab_spec.rs @@ -2,6 +2,7 @@ use tablepro_core::meta::TableRef; use tablepro_storage::DraftId; use super::structure_target::StructureTarget; +use crate::ui::browse_tab::TableFilter; use crate::workspace::EditorFileRecord; /// A tab to open. Every way a tab opens builds one of these: the sidebar, @@ -19,6 +20,7 @@ pub(crate) enum TabSpec { Table { table: TableRef, page: TablePage, + filter: TableFilter, }, Structure { target: StructureTarget, diff --git a/linux/crates/app/src/ui/workspace/tests/mod.rs b/linux/crates/app/src/ui/workspace/tests/mod.rs index d376e2e7dc..1115ec7a62 100644 --- a/linux/crates/app/src/ui/workspace/tests/mod.rs +++ b/linux/crates/app/src/ui/workspace/tests/mod.rs @@ -1,3 +1,4 @@ mod close_tab; mod large_catalog; +mod references; mod tab_title; diff --git a/linux/crates/app/src/ui/workspace/tests/references.rs b/linux/crates/app/src/ui/workspace/tests/references.rs new file mode 100644 index 0000000000..afb15ed5e1 --- /dev/null +++ b/linux/crates/app/src/ui/workspace/tests/references.rs @@ -0,0 +1,229 @@ +use gtk4::prelude::*; +use gtk4::subclass::prelude::*; +use tablepro_core::browse::RowReference; +use tablepro_core::column::{ColumnDefault, ColumnType}; +use tablepro_core::filter::{Combinator, FilterOp, FilterRule, FilterSet, FilterValue}; +use tablepro_core::meta::{ObjectInfo, ObjectKind, RowCountEstimate, RowIdentity, TableEstimate, TableRef}; +use tablepro_core::{ColumnInfo, QueryResult, Value}; +use tablepro_test_support::{ConnectionScript, FakeReply}; +use uuid::Uuid; + +use super::super::workspace_persistence::workspace_record; +use super::super::{OpenMode, WorkspaceInput}; +use crate::runtime::DeliverInput; +use crate::test_support::{descendants, drain_main_context, wait_until}; +use crate::ui::browse_tab::BrowseTab; +use crate::ui::session_window::tests::{TestWindow, WAIT}; +use crate::workspace::TabRecord; + +fn column(name: &str, primary_key: bool) -> ColumnInfo { + ColumnInfo { + name: name.to_owned(), + column_type: ColumnType::untyped_text("text"), + nullable: !primary_key, + primary_key, + is_auto_increment: false, + is_generated: false, + default: ColumnDefault::None, + comment: None, + } +} + +/// `orders` and `customers`, each with one keyed row. +fn serving_tables(script: &mut ConnectionScript) { + let columns = [column("id", true), column("name", false)]; + script.fetch_columns.always(FakeReply::ok(columns.to_vec())); + script.row_identity.always(FakeReply::ok(RowIdentity::UniqueKey { + columns: vec!["id".to_owned()], + })); + script.count_rows.always(FakeReply::ok(1)); + script.table_estimate.always(FakeReply::ok(TableEstimate { + rows: RowCountEstimate::Approximate(1), + heap_blocks: None, + })); + script.query_bound.always(FakeReply::ok(QueryResult::new( + columns.iter().map(ColumnInfo::result_column).collect::>(), + vec![vec![Value::Int(42), Value::Text("Ada".to_owned())]], + ))); + script.list_objects.always(FakeReply::ok( + ["orders", "customers"] + .into_iter() + .map(|name| ObjectInfo { + schema: None, + name: name.to_owned(), + kind: ObjectKind::Table, + }) + .collect(), + )); + script.fetch_foreign_keys.always(FakeReply::ok(Vec::new())); + script.fetch_inbound_foreign_keys.always(FakeReply::ok(Vec::new())); +} + +fn customer(id: &str) -> RowReference { + RowReference { + table: TableRef::unqualified("customers"), + filter: FilterSet { + combinator: Combinator::And, + rules: vec![FilterRule { + column: "id".to_owned(), + op: FilterOp::Eq, + value: Some(FilterValue::Single(id.to_owned())), + }], + extra_sql: None, + }, + } +} + +fn open_orders(test: &TestWindow) -> (Uuid, libadwaita::TabPage) { + let workspace = test.workspace(); + let before = workspace.selected_tab(); + workspace + .input() + .expect("the workspace's input") + .deliver(WorkspaceInput::SelectTable { + schema: None, + name: "orders".to_owned(), + open_mode: OpenMode::NewTab, + }); + let mut id = None; + wait_until(WAIT, || { + id = workspace.selected_tab().filter(|tab| Some(*tab) != before); + id.is_some() + }) + .expect("the orders tab"); + let page = workspace.imp().tab_view.selected_page().expect("the orders page"); + (id.expect("the tab's id"), page) +} + +fn follow(test: &TestWindow, source: Uuid, reference: RowReference) { + test.workspace() + .input() + .expect("the workspace's input") + .deliver(WorkspaceInput::FollowReference { + tab: source, + reference: Box::new(reference), + }); +} + +/// The table tab in front once it is not `source`. +fn front_tab_other_than(test: &TestWindow, source: &libadwaita::TabPage) -> (libadwaita::TabPage, BrowseTab) { + let view = test.workspace().imp().tab_view.clone(); + let mut front = None; + wait_until(WAIT, || { + front = view.selected_page().filter(|page| page != source); + front.is_some() + }) + .expect("a new tab in front"); + let page = front.expect("the page in front"); + let tab = page.child().downcast::().expect("a table tab"); + (page, tab) +} + +#[gtk4::test] +fn a_followed_key_opens_its_table_beside_the_tab_under_the_keys_filter() { + let test = TestWindow::connected_with("FollowOpensBeside", serving_tables); + let (source, source_page) = open_orders(&test); + let view = test.workspace().imp().tab_view.clone(); + // Another tab at the end, so beside the source is not simply last. + let pages = view.n_pages(); + test.workspace() + .input() + .expect("the workspace's input") + .deliver(WorkspaceInput::NewEditorTab); + wait_until(WAIT, || view.n_pages() == pages + 1).expect("the editor tab"); + view.set_selected_page(&source_page); + + follow(&test, source, customer("42")); + + let (page, tab) = front_tab_other_than(&test, &source_page); + assert_eq!(tab.table(), "customers"); + assert_eq!(tab.applied_filter(), customer("42").filter); + assert_eq!(view.page_position(&page), view.page_position(&source_page) + 1); + assert_eq!(page.parent().as_ref(), Some(&source_page)); +} + +#[gtk4::test] +fn the_same_rows_followed_again_show_the_tab_already_open() { + let test = TestWindow::connected_with("FollowReveals", serving_tables); + let (source, source_page) = open_orders(&test); + let view = test.workspace().imp().tab_view.clone(); + follow(&test, source, customer("42")); + let (opened, _) = front_tab_other_than(&test, &source_page); + let pages = view.n_pages(); + + view.set_selected_page(&source_page); + follow(&test, source, customer("42")); + wait_until(WAIT, || view.selected_page().as_ref() == Some(&opened)).expect("the open tab comes back"); + view.set_selected_page(&source_page); + follow(&test, source, customer("7")); + let (other, tab) = front_tab_other_than(&test, &source_page); + + assert_eq!(view.n_pages(), pages + 1, "only the other customer opened a tab"); + assert_ne!(other, opened); + assert_eq!(tab.applied_filter(), customer("7").filter); +} + +#[gtk4::test] +fn a_key_to_a_table_that_is_gone_says_so_and_opens_nothing() { + let test = TestWindow::connected_with("FollowMissing", serving_tables); + let (source, _) = open_orders(&test); + let view = test.workspace().imp().tab_view.clone(); + let pages = view.n_pages(); + let overlay = test.workspace().toast_overlay(); + + follow( + &test, + source, + RowReference { + table: TableRef::unqualified("ghost"), + ..customer("42") + }, + ); + + wait_until(WAIT, || { + descendants(&overlay) + .iter() + .filter_map(|widget| widget.downcast_ref::()) + .any(|label| label.label() == "Can't find the table ghost") + }) + .expect("the toast says the table is gone"); + drain_main_context(); + assert_eq!(view.n_pages(), pages); +} + +/// Two tabs on one table keep their own filters through a restart and a +/// reopen, rather than the table's last one. +#[gtk4::test] +fn a_followed_tab_keeps_its_filter_in_the_saved_tabs_and_through_a_reopen() { + let test = TestWindow::connected_with("FollowKeepsFilter", serving_tables); + let (source, source_page) = open_orders(&test); + follow(&test, source, customer("42")); + let (page, _) = front_tab_other_than(&test, &source_page); + let workspace = test.workspace(); + + let record = workspace + .with_model(|model| workspace_record(&model.tab_view, &model.workspace_tabs.borrow(), &model.index)) + .expect("the saved tabs"); + let kept: Vec> = record + .tabs + .iter() + .filter_map(|tab| match tab { + TabRecord::Table { table, filter, .. } if table == "customers" => Some(filter.clone()), + _ => None, + }) + .collect(); + assert_eq!(kept, [Some(customer("42").filter)]); + + workspace.imp().tab_view.set_selected_page(&page); + let input = workspace.input().expect("the workspace's input"); + input.deliver(WorkspaceInput::CloseActiveWorkspaceTab); + wait_until(WAIT, || { + workspace.imp().tab_view.selected_page().as_ref() == Some(&source_page) + }) + .expect("the tab closes back to its source"); + input.deliver(WorkspaceInput::ReopenClosedTab); + let (_, reopened) = front_tab_other_than(&test, &source_page); + + assert_eq!(reopened.table(), "customers"); + assert_eq!(reopened.applied_filter(), customer("42").filter); +} diff --git a/linux/crates/app/src/ui/workspace/tests/tab_title.rs b/linux/crates/app/src/ui/workspace/tests/tab_title.rs index 97aaaa559f..02dfa12713 100644 --- a/linux/crates/app/src/ui/workspace/tests/tab_title.rs +++ b/linux/crates/app/src/ui/workspace/tests/tab_title.rs @@ -276,6 +276,7 @@ fn tabs_name_the_schema_once_the_session_has_two() { model.open_tab(TabSpec::Table { table: TableRef::new(Some("public".to_owned()), "orders"), page: super::super::tab_spec::TablePage::first(100), + filter: crate::ui::browse_tab::TableFilter::Remembered, }) }) .flatten() diff --git a/linux/crates/app/src/ui/workspace/workspace_persistence.rs b/linux/crates/app/src/ui/workspace/workspace_persistence.rs index aaca79771b..61903b84d7 100644 --- a/linux/crates/app/src/ui/workspace/workspace_persistence.rs +++ b/linux/crates/app/src/ui/workspace/workspace_persistence.rs @@ -121,7 +121,7 @@ pub(super) fn workspace_record( index: &TabIndex, ) -> WorkspaceRecord { let pages = view.pages(); - let snapshots: Vec = (0..pages.n_items()) + let mut snapshots: Vec = (0..pages.n_items()) .filter_map(|position| pages.item(position).and_downcast::()) .filter_map(|page| { let id = index.id_of(&page)?; @@ -132,10 +132,34 @@ pub(super) fn workspace_record( }) }) .collect(); + keep_filters_within(&mut snapshots, KEPT_FILTER_BUDGET); let selected = view.selected_page().and_then(|page| index.id_of(&page)); crate::workspace::workspace_record(&snapshots, selected) } +/// How many bytes of filters the saved tabs keep in all. The workspace +/// file is refused past its size cap, and a filter has no bound of its +/// own: one followed from a large binary key can be most of a megabyte. +const KEPT_FILTER_BUDGET: usize = 256 * 1024; + +/// Keep each table tab's filter while they fit `budget`, in tab order. +/// A tab past it is saved without one and opens on the table's +/// remembered filter, rather than the whole file becoming unreadable. +fn keep_filters_within(snapshots: &mut [TabSnapshot], mut budget: usize) { + for snapshot in snapshots { + let Some(TabRecord::Table { filter, .. }) = &mut snapshot.record else { + continue; + }; + let size = filter.as_ref().map_or(0, |filter| { + serde_json::to_vec(filter).map_or(usize::MAX, |json| json.len()) + }); + match budget.checked_sub(size) { + Some(left) => budget = left, + None => *filter = None, + } + } +} + fn tab_record(tab: &WorkspaceTab) -> Option { match tab { WorkspaceTab::Editor(slot) => Some(TabRecord::Editor { @@ -160,6 +184,7 @@ fn tab_record(tab: &WorkspaceTab) -> Option { .browse .current_sort() .map(|(column, ascending)| SortRecord { column, ascending }), + filter: Some(slot.browse.applied_filter()), }), } } @@ -197,6 +222,55 @@ mod tests { const WAIT: Duration = Duration::from_secs(5); + fn filtered_table(name: &str, value: &str) -> TabSnapshot { + let filter = tablepro_core::FilterSet { + combinator: tablepro_core::filter::Combinator::And, + rules: vec![tablepro_core::filter::FilterRule { + column: "id".to_owned(), + op: tablepro_core::filter::FilterOp::Eq, + value: Some(tablepro_core::filter::FilterValue::Single(value.to_owned())), + }], + extra_sql: None, + }; + TabSnapshot { + id: Uuid::new_v4(), + record: Some(TabRecord::Table { + schema: None, + table: name.to_owned(), + offset: 0, + page_size: 100, + sort: None, + filter: Some(filter), + }), + } + } + + fn kept(snapshots: &[TabSnapshot]) -> Vec { + snapshots + .iter() + .map(|snapshot| matches!(&snapshot.record, Some(TabRecord::Table { filter: Some(_), .. }))) + .collect() + } + + /// A filter followed from a large binary key would make the file too + /// large to read back, and with it every saved tab. + #[test] + fn filters_past_the_budget_are_saved_without_their_tab_filter() { + let large = format!("0x{}", "ab".repeat(KEPT_FILTER_BUDGET / 2)); + let mut snapshots = vec![ + filtered_table("users", "1"), + filtered_table("blobs", &large), + filtered_table("orders", "2"), + ]; + + keep_filters_within(&mut snapshots, KEPT_FILTER_BUDGET); + + assert_eq!(kept(&snapshots), [true, false, true]); + let mut tight = vec![filtered_table("users", "1"), filtered_table("orders", "2")]; + keep_filters_within(&mut tight, 1); + assert_eq!(kept(&tight), [false, false]); + } + struct Fixture { runtime: crate::runtime::AppRuntime, settings: crate::test_support::MemorySettings, @@ -356,6 +430,7 @@ mod tests { table: "orders".to_owned(), offset: 0, page_size: 1_000, + filter: None, sort: None, }, TabRecord::Structure { diff --git a/linux/crates/app/src/ui/workspace/workspace_tabs.rs b/linux/crates/app/src/ui/workspace/workspace_tabs.rs index d0333dec9e..7c2ce6e1bb 100644 --- a/linux/crates/app/src/ui/workspace/workspace_tabs.rs +++ b/linux/crates/app/src/ui/workspace/workspace_tabs.rs @@ -12,7 +12,7 @@ use tablepro_session::SessionKey; use tablepro_storage::{DraftId, DraftScope}; use uuid::Uuid; -use crate::ui::browse_tab::{BrowseTabInput, PagingInput, SaveInput}; +use crate::ui::browse_tab::{BrowseTabInput, PagingInput, SaveInput, TableFilter}; use crate::ui::structure_tab::StructureMode; use crate::workspace::{ EditorFileRecord, RestoredDrafts, TabRecord, WorkspaceFileName, WorkspaceLoad, WorkspaceRecord, WorkspaceSaveError, @@ -108,6 +108,7 @@ impl WorkspaceModel { offset, page_size, sort, + filter, } => TabSpec::Table { table: TableRef::new(schema, table), page: TablePage { @@ -115,6 +116,7 @@ impl WorkspaceModel { page_size, sort: sort.map(|sort| (sort.column, sort.ascending)), }, + filter: filter.map_or(TableFilter::Remembered, TableFilter::Kept), }, TabRecord::Structure { schema, table } => TabSpec::Structure { target: StructureTarget::Existing(TableRef::new(schema, table)), @@ -331,6 +333,7 @@ impl WorkspaceModel { self.open_tab(TabSpec::Table { table: TableRef::new(schema, name), page: TablePage::first(page_size), + filter: TableFilter::Remembered, }); } @@ -581,6 +584,7 @@ fn describe_closed_tab(slot: &WorkspaceTab) -> Option { offset: s.browse.current_offset(), page_size: s.browse.page_size(), sort: s.browse.current_sort(), + filter: s.browse.applied_filter(), }), WorkspaceTab::Structure(s) => match s.mode { // A draft names no table to reopen. @@ -627,6 +631,7 @@ impl WorkspaceModel { offset, page_size, sort, + filter, } => { self.open_tab(TabSpec::Table { table: TableRef::new(schema, table), @@ -635,6 +640,7 @@ impl WorkspaceModel { page_size, sort, }, + filter: TableFilter::Kept(filter), }); } ClosedTabDescriptor::Structure { schema, table } => { @@ -786,6 +792,7 @@ mod tests { table: name.to_owned(), offset: 0, page_size: 100, + filter: None, sort: None, }; store diff --git a/linux/crates/app/src/workspace/tab_record.rs b/linux/crates/app/src/workspace/tab_record.rs index d8d1e7fc56..5d6200d361 100644 --- a/linux/crates/app/src/workspace/tab_record.rs +++ b/linux/crates/app/src/workspace/tab_record.rs @@ -1,6 +1,7 @@ use std::ops::RangeInclusive; use serde::{Deserialize, Serialize}; +use tablepro_core::FilterSet; use tablepro_storage::DraftId; use super::{EditorFileRecord, SortRecord}; @@ -18,6 +19,11 @@ pub enum TabRecord { offset: u64, page_size: u64, sort: Option, + /// The tab's own filter, which two tabs on one table can hold + /// apart. `None` in a record written before tabs kept one, + /// which opens on the table's remembered filter as it did. + #[serde(default, skip_serializing_if = "Option::is_none")] + filter: Option, }, Structure { schema: Option, @@ -43,6 +49,7 @@ impl TabRecord { offset, page_size, sort, + filter, } => { if table.is_empty() { return None; @@ -58,6 +65,7 @@ impl TabRecord { offset, page_size, sort, + filter, }) } Self::Structure { schema, table } => (!table.is_empty()).then_some(Self::Structure { schema, table }), @@ -77,6 +85,7 @@ mod tests { offset: 0, page_size, sort: None, + filter: None, } } diff --git a/linux/crates/app/src/workspace/workspace_record.rs b/linux/crates/app/src/workspace/workspace_record.rs index 1d58bdd802..91436fb19a 100644 --- a/linux/crates/app/src/workspace/workspace_record.rs +++ b/linux/crates/app/src/workspace/workspace_record.rs @@ -60,6 +60,7 @@ mod tests { table: name.to_owned(), offset: 0, page_size: 1_000, + filter: None, sort: None, } } @@ -74,6 +75,7 @@ mod tests { table: "orders".to_owned(), offset: 5_000, page_size: 5_000, + filter: None, sort: Some(SortRecord { column: 2, ascending: false, @@ -119,6 +121,7 @@ mod tests { table: "orders".to_owned(), offset: 5_000, page_size: 5_000, + filter: None, sort: Some(SortRecord { column: 2, ascending: false, @@ -129,6 +132,7 @@ mod tests { table: "events".to_owned(), offset: 0, page_size: 1_000, + filter: None, sort: None, }, TabRecord::Editor { diff --git a/linux/crates/app/src/workspace/workspace_record_builder.rs b/linux/crates/app/src/workspace/workspace_record_builder.rs index 3617e7bb14..7e81155c91 100644 --- a/linux/crates/app/src/workspace/workspace_record_builder.rs +++ b/linux/crates/app/src/workspace/workspace_record_builder.rs @@ -39,6 +39,7 @@ mod tests { table: name.to_owned(), offset: 0, page_size: 1_000, + filter: None, sort: None, } } diff --git a/linux/crates/app/src/workspace/workspace_store.rs b/linux/crates/app/src/workspace/workspace_store.rs index 91c86534e2..4bfbf6a293 100644 --- a/linux/crates/app/src/workspace/workspace_store.rs +++ b/linux/crates/app/src/workspace/workspace_store.rs @@ -463,6 +463,7 @@ mod tests { table: format!("t{index}"), offset: 0, page_size: 1_000, + filter: None, sort: None, }) .collect(), diff --git a/linux/crates/app/tests/accessibility/main.rs b/linux/crates/app/tests/accessibility/main.rs index d92b805e28..76c4b852bf 100644 --- a/linux/crates/app/tests/accessibility/main.rs +++ b/linux/crates/app/tests/accessibility/main.rs @@ -412,6 +412,7 @@ fn one_table() -> ConnectionScript { script.fetch_columns.always(FakeReply::ok(columns.clone())); script.fetch_indexes.always(FakeReply::ok(Vec::new())); script.fetch_foreign_keys.always(FakeReply::ok(Vec::new())); + script.fetch_inbound_foreign_keys.always(FakeReply::ok(Vec::new())); script.row_identity.always(FakeReply::ok(RowIdentity::UniqueKey { columns: vec!["id".to_owned()], })); diff --git a/linux/crates/core/src/browse/mod.rs b/linux/crates/core/src/browse/mod.rs index 56f49b134c..fef4eeaf7f 100644 --- a/linux/crates/core/src/browse/mod.rs +++ b/linux/crates/core/src/browse/mod.rs @@ -13,7 +13,9 @@ mod page_request; mod page_spec; mod paging_mode; mod read_page; +mod reference_error; mod row_key_layout; +mod row_reference; mod select_parts; mod sort_key; mod split_page; @@ -32,6 +34,8 @@ pub use page_request::PageRequest; pub use page_spec::PageSpec; pub use paging_mode::{PagingMode, paging_mode}; pub use read_page::{PageRead, read_page}; +pub use reference_error::ReferenceError; pub use row_key_layout::RowKeyLayout; +pub use row_reference::RowReference; pub use sort_key::SortKey; pub use split_page::split_page; diff --git a/linux/crates/core/src/browse/reference_error.rs b/linux/crates/core/src/browse/reference_error.rs new file mode 100644 index 0000000000..36047b4f59 --- /dev/null +++ b/linux/crates/core/src/browse/reference_error.rs @@ -0,0 +1,30 @@ +use thiserror::Error; + +/// Why a foreign key leads nowhere from one row. +#[derive(Debug, Clone, PartialEq, Eq, Error)] +pub enum ReferenceError { + /// A key with any NULL column is not checked against the other + /// table under MATCH SIMPLE, so there is no row to point at. + #[error("the key holds NULL in this row")] + NullKey, + #[error("the row has no column {column}")] + MissingColumn { column: String }, + /// The catalogue gave the key's two sides different column counts, + /// or none, as SQLite does for a key whose parent is gone. + #[error("the columns of key {key} could not be read")] + UnevenKey { key: String }, + /// The source table sits in a schema and the key names none, so the + /// table it names could be any table of that name. + #[error("the schema of {table} is not known")] + UnknownSchema { table: String }, + #[error("the value of {column} has no text a filter can compare")] + NoFilterText { column: String }, +} + +impl ReferenceError { + /// The row has nothing to follow, rather than something that cannot + /// be followed, so a menu offers nothing for it. + pub fn leaves_nothing_to_follow(&self) -> bool { + matches!(self, Self::NullKey | Self::MissingColumn { .. }) + } +} diff --git a/linux/crates/core/src/browse/row_reference.rs b/linux/crates/core/src/browse/row_reference.rs new file mode 100644 index 0000000000..42e9b5078b --- /dev/null +++ b/linux/crates/core/src/browse/row_reference.rs @@ -0,0 +1,321 @@ +use crate::column::{ColumnKind, ColumnType, ResultColumn}; +use crate::ddl::InboundForeignKey; +use crate::edit::{EditText, edit_text}; +use crate::filter::{Combinator, FilterOp, FilterRule, FilterSet, FilterValue}; +use crate::foreign_key_info::ForeignKeyInfo; +use crate::meta::TableRef; +use crate::value::Value; + +use super::ReferenceError; + +/// The rows a foreign key leads to from one row: the table at the other +/// end, filtered on every column of the key to the row's values. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RowReference { + pub table: TableRef, + pub filter: FilterSet, +} + +impl RowReference { + /// The row that `key`, a key of `source`, points at from the row + /// whose cells are `values` under `columns`. + pub fn referenced( + source: &TableRef, + key: &ForeignKeyInfo, + columns: &[ResultColumn], + values: &[Value], + ) -> Result { + let cells = key_cells(&key.columns, columns, values)?; + let filter = key_filter(&key.name, cells, &key.ref_columns)?; + let table = target(source, key.ref_schema.as_deref(), &key.ref_table)?; + Ok(Self { table, filter }) + } + + /// The rows of another table whose `key` points at the row of + /// `source` whose cells are `values` under `columns`. + pub fn referencing( + source: &TableRef, + key: &InboundForeignKey, + columns: &[ResultColumn], + values: &[Value], + ) -> Result { + let cells = key_cells(&key.referenced_columns, columns, values)?; + let filter = key_filter(&key.name, cells, &key.columns)?; + let table = target(source, key.table.schema.as_deref(), &key.table.name)?; + Ok(Self { table, filter }) + } +} + +/// The row's cell under each of `names`, in key order. +fn key_cells<'a>( + names: &[String], + columns: &'a [ResultColumn], + values: &'a [Value], +) -> Result, ReferenceError> { + let mut cells = Vec::with_capacity(names.len()); + for name in names { + let cell = columns + .iter() + .position(|column| column.name == *name) + .and_then(|index| Some((&columns.get(index)?.column_type, values.get(index)?))); + let Some((column_type, value)) = cell else { + return Err(ReferenceError::MissingColumn { column: name.clone() }); + }; + if matches!(value, Value::Null) { + return Err(ReferenceError::NullKey); + } + cells.push((column_type, value)); + } + Ok(cells) +} + +fn key_filter(key: &str, cells: Vec<(&ColumnType, &Value)>, targets: &[String]) -> Result { + if cells.is_empty() || cells.len() != targets.len() { + return Err(ReferenceError::UnevenKey { key: key.to_owned() }); + } + let rules = cells + .into_iter() + .zip(targets) + .map(|((column_type, value), target)| { + let text = filter_text(value, column_type) + .ok_or_else(|| ReferenceError::NoFilterText { column: target.clone() })?; + Ok(FilterRule { + column: target.clone(), + op: FilterOp::Eq, + value: Some(FilterValue::Single(text)), + }) + }) + .collect::>()?; + Ok(FilterSet { + combinator: Combinator::And, + rules, + extra_sql: None, + }) +} + +/// The text the filter parses back into `value`: a blob in the `0x` +/// form an export writes, anything else as an editor would start from. +/// A blob in a column declared otherwise, which SQLite allows, has no +/// such text: the filter would read it as the column's type and compare +/// a string. +fn filter_text(value: &Value, column_type: &ColumnType) -> Option { + match value { + Value::Bytes(bytes) if matches!(column_type.kind, ColumnKind::Bytes { .. }) => { + Some(format!("0x{}", crate::hex::encode_lower(bytes))) + } + Value::Bytes(_) => None, + other => match edit_text(other, column_type) { + EditText::Editable(text) => Some(text), + EditText::ReadOnly(_) => None, + }, + } +} + +fn target(source: &TableRef, schema: Option<&str>, name: &str) -> Result { + if source.schema.is_some() && schema.is_none() { + return Err(ReferenceError::UnknownSchema { table: name.to_owned() }); + } + Ok(TableRef::new(schema.map(str::to_owned), name.to_owned())) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn text_column(name: &str) -> ResultColumn { + ResultColumn::new(name, ColumnType::untyped_text("text")) + } + + fn typed(name: &str, kind: ColumnKind) -> ResultColumn { + ResultColumn::new( + name, + ColumnType { + name: "t".to_owned(), + kind, + catalog: None, + dynamic_storage: false, + read_form: crate::column::ReadForm::Native, + }, + ) + } + + fn orders() -> TableRef { + TableRef::new(Some("sales".to_owned()), "orders".to_owned()) + } + + fn key(columns: &[&str], ref_schema: Option<&str>, ref_columns: &[&str]) -> ForeignKeyInfo { + ForeignKeyInfo { + name: "orders_customer_fkey".to_owned(), + columns: columns.iter().map(|name| (*name).to_owned()).collect(), + ref_schema: ref_schema.map(str::to_owned), + ref_table: "customers".to_owned(), + ref_columns: ref_columns.iter().map(|name| (*name).to_owned()).collect(), + on_delete: None, + on_update: None, + } + } + + fn eq(column: &str, text: &str) -> FilterRule { + FilterRule { + column: column.to_owned(), + op: FilterOp::Eq, + value: Some(FilterValue::Single(text.to_owned())), + } + } + + #[test] + fn a_composite_key_filters_on_every_column_in_key_order() { + let columns = [text_column("id"), text_column("region"), text_column("customer_id")]; + let values = [ + Value::Int(9), + Value::Text("eu ".to_owned()), + Value::Text("42".to_owned()), + ]; + let key = key(&["customer_id", "region"], Some("crm"), &["id", "region"]); + + let reference = RowReference::referenced(&orders(), &key, &columns, &values).expect("a reference"); + + assert_eq!( + reference.table, + TableRef::new(Some("crm".to_owned()), "customers".to_owned()) + ); + assert_eq!(reference.filter.combinator, Combinator::And); + assert_eq!(reference.filter.rules, vec![eq("id", "42"), eq("region", "eu ")]); + assert_eq!(reference.filter.extra_sql, None); + } + + #[test] + fn a_null_in_any_column_leaves_nothing_to_follow() { + let columns = [text_column("customer_id"), text_column("region")]; + let values = [Value::Text("42".to_owned()), Value::Null]; + let key = key(&["customer_id", "region"], Some("sales"), &["id", "region"]); + + let error = RowReference::referenced(&orders(), &key, &columns, &values).expect_err("a NULL part"); + + assert_eq!(error, ReferenceError::NullKey); + assert!(error.leaves_nothing_to_follow()); + } + + #[test] + fn an_empty_text_is_a_value_not_a_null() { + let columns = [text_column("code")]; + let key = key(&["code"], Some("sales"), &["code"]); + + let reference = RowReference::referenced(&orders(), &key, &columns, &[Value::Text(String::new())]) + .expect("an empty string is a key value"); + + assert_eq!(reference.filter.rules, vec![eq("code", "")]); + } + + #[test] + fn a_blob_is_written_in_its_hex_form() { + let columns = [typed( + "customer_uuid", + ColumnKind::Bytes { + length: crate::column::TextLength::Fixed(16), + }, + )]; + let key = key(&["customer_uuid"], Some("sales"), &["uuid"]); + + let reference = RowReference::referenced(&orders(), &key, &columns, &[Value::Bytes(vec![0x0a, 0xff])]) + .expect("a binary key"); + + assert_eq!(reference.filter.rules, vec![eq("uuid", "0x0aff")]); + } + + #[test] + fn a_blob_in_a_column_declared_as_text_is_refused_rather_than_compared_as_text() { + let columns = [text_column("customer_code")]; + let key = key(&["customer_code"], Some("sales"), &["code"]); + + let error = RowReference::referenced(&orders(), &key, &columns, &[Value::Bytes(vec![0xab])]) + .expect_err("a blob under a text declaration"); + + assert_eq!( + error, + ReferenceError::NoFilterText { + column: "code".to_owned() + } + ); + } + + #[test] + fn a_key_the_catalogue_could_not_pair_up_is_refused_and_shown() { + let columns = [text_column("owner")]; + let dangling = key(&["owner"], Some("sales"), &[]); + + let error = RowReference::referenced(&orders(), &dangling, &columns, &[Value::Int(1)]).expect_err("uneven"); + + assert_eq!( + error, + ReferenceError::UnevenKey { + key: "orders_customer_fkey".to_owned() + } + ); + assert!(!error.leaves_nothing_to_follow()); + } + + #[test] + fn a_key_with_no_schema_from_a_table_in_one_is_refused_rather_than_guessed() { + let columns = [text_column("customer_id")]; + let unqualified = key(&["customer_id"], None, &["id"]); + + let error = + RowReference::referenced(&orders(), &unqualified, &columns, &[Value::Int(1)]).expect_err("no schema"); + + assert_eq!( + error, + ReferenceError::UnknownSchema { + table: "customers".to_owned() + } + ); + let main = TableRef::new(None, "orders".to_owned()); + let reference = RowReference::referenced(&main, &unqualified, &columns, &[Value::Int(1)]) + .expect("an engine without schemas names none on either side"); + assert_eq!(reference.table, TableRef::new(None, "customers".to_owned())); + } + + #[test] + fn a_value_with_no_text_form_names_its_column() { + let columns = [typed("shape", ColumnKind::Spatial)]; + let key = key(&["shape"], Some("sales"), &["outline"]); + + let error = RowReference::referenced(&orders(), &key, &columns, &[Value::Int(1)]).expect_err("no text"); + + assert_eq!( + error, + ReferenceError::NoFilterText { + column: "outline".to_owned() + } + ); + } + + #[test] + fn a_missing_column_leaves_nothing_to_follow() { + let error = RowReference::referenced(&orders(), &key(&["gone"], Some("sales"), &["id"]), &[], &[]) + .expect_err("no such column"); + + assert!(error.leaves_nothing_to_follow(), "{error:?}"); + } + + #[test] + fn referencing_rows_filter_the_other_table_on_its_own_columns() { + let customers = TableRef::new(Some("crm".to_owned()), "customers".to_owned()); + let columns = [text_column("id"), text_column("region")]; + let values = [Value::Int(42), Value::Text("eu".to_owned())]; + let inbound = InboundForeignKey { + name: "orders_customer_fkey".to_owned(), + table: orders(), + columns: vec!["customer_id".to_owned(), "customer_region".to_owned()], + referenced_columns: vec!["id".to_owned(), "region".to_owned()], + }; + + let reference = RowReference::referencing(&customers, &inbound, &columns, &values).expect("a reference"); + + assert_eq!(reference.table, orders()); + assert_eq!( + reference.filter.rules, + vec![eq("customer_id", "42"), eq("customer_region", "eu")] + ); + } +} diff --git a/linux/crates/core/src/connection.rs b/linux/crates/core/src/connection.rs index b04d34e2c2..7c4dbc60e8 100644 --- a/linux/crates/core/src/connection.rs +++ b/linux/crates/core/src/connection.rs @@ -6,7 +6,7 @@ use tokio_util::sync::{CancellationToken, WaitForCancellationFutureOwned}; use crate::call_options::CallOptions; use crate::column::ColumnInfo; use crate::commit::{CommitResolution, CommitToken}; -use crate::ddl::TableDefinition; +use crate::ddl::{InboundForeignKey, TableDefinition}; use crate::dialect::SqlDialect; use crate::editor_session::{EditorSession, EditorSessionOptions}; use crate::error::DriverError; @@ -65,6 +65,15 @@ pub trait Connection: Send + Sync { options: CallOptions, ) -> Result, DriverError>; + /// The foreign keys of other tables that point at this one, as + /// `fetch_table_definition` lists them, without the rest of the + /// definition. + async fn fetch_inbound_foreign_keys( + &self, + table: &TableRef, + options: CallOptions, + ) -> Result, DriverError>; + /// The table as structure changes plan against it, read in one call /// so its columns, keys and dependents come from the same moment. async fn fetch_table_definition( diff --git a/linux/crates/core/src/edit/edit_parse_error.rs b/linux/crates/core/src/edit/edit_parse_error.rs index ea1c45a6b3..3db143ecb8 100644 --- a/linux/crates/core/src/edit/edit_parse_error.rs +++ b/linux/crates/core/src/edit/edit_parse_error.rs @@ -45,6 +45,8 @@ pub enum EditParseError { InvalidInterval, #[error("enter a bit string of 0 and 1")] InvalidBits, + #[error("enter bytes as 0x and hex digits")] + InvalidBytes, #[error("this column holds {expected} bits")] BitLengthMismatch { expected: u32 }, #[error("this is not one of the values this column allows")] diff --git a/linux/crates/core/src/edit/parse_literal_text.rs b/linux/crates/core/src/edit/parse_literal_text.rs index 68c8b49803..3e3ba424ea 100644 --- a/linux/crates/core/src/edit/parse_literal_text.rs +++ b/linux/crates/core/src/edit/parse_literal_text.rs @@ -427,17 +427,26 @@ fn parse_bits(input: &str) -> Result { /// takes one here. pub fn parse_filter_text(input: &str, column: &ColumnType) -> Result { match column.kind { + // The form an export and a copy write a blob in. + ColumnKind::Bytes { .. } => parse_hex_bytes(input.trim()), // A filter on a column with no editable text form compares text // the user typed; the dialect decides what that means. - ColumnKind::Bytes { .. } - | ColumnKind::Spatial - | ColumnKind::Array { .. } - | ColumnKind::Variant - | ColumnKind::Composite => Ok(Value::Text(input.to_owned())), + ColumnKind::Spatial | ColumnKind::Array { .. } | ColumnKind::Variant | ColumnKind::Composite => { + Ok(Value::Text(input.to_owned())) + } _ => parse(input, column, Purpose::Compare), } } +fn parse_hex_bytes(input: &str) -> Result { + input + .strip_prefix("0x") + .or_else(|| input.strip_prefix("0X")) + .and_then(crate::hex::decode) + .map(Value::Bytes) + .ok_or(EditParseError::InvalidBytes) +} + /// A `TimeTz` from text, for the engines that have one. pub fn parse_time_with_offset(input: &str) -> Result { let trimmed = input.trim(); @@ -810,14 +819,21 @@ mod tests { } #[test] - fn a_filter_on_a_binary_column_compares_text() { + fn a_filter_on_a_binary_column_reads_the_hex_form_an_export_writes() { let binary = column(bytes(), false); assert_eq!( - parse_filter_text("0xff", &binary), - Ok(Value::Text("0xff".to_owned())), - "a filter could not be typed against a binary column" + parse_filter_text(" 0x0aFF ", &binary), + Ok(Value::Bytes(vec![0x0a, 0xff])) ); + assert_eq!(parse_filter_text("0X", &binary), Ok(Value::Bytes(Vec::new()))); + for text in ["ff", "0xf", "0xzz", ""] { + assert_eq!( + parse_filter_text(text, &binary), + Err(EditParseError::InvalidBytes), + "{text:?}" + ); + } } #[test] diff --git a/linux/crates/core/src/filter/build_filter.rs b/linux/crates/core/src/filter/build_filter.rs index 7953786f29..5ee11e3f7e 100644 --- a/linux/crates/core/src/filter/build_filter.rs +++ b/linux/crates/core/src/filter/build_filter.rs @@ -366,6 +366,16 @@ mod tests { assert!(build(&rule("photo", FilterOp::IsNull, None)).is_ok()); } + #[test] + fn a_blob_compares_equal_to_its_hex_form_as_bound_bytes() { + let statement = built(&single("photo", FilterOp::Eq, "0x00Ff")); + + assert_eq!(statement.sql(), r#""photo" = $1"#); + assert_eq!(statement.params()[0].value(), &Value::Bytes(vec![0x00, 0xff])); + let error = build(&single("photo", FilterOp::Eq, "00ff")).expect_err("hex without 0x"); + assert!(matches!(error, BuildFilterError::Parse { .. }), "{error:?}"); + } + #[test] fn a_value_that_is_not_of_the_columns_type_is_reported_before_the_server_sees_it() { let error = build(&single("age", FilterOp::Eq, "seven")).expect_err("text in a number column"); diff --git a/linux/crates/core/src/filter/operators_for.rs b/linux/crates/core/src/filter/operators_for.rs index c2ecddd6ad..f73db2c194 100644 --- a/linux/crates/core/src/filter/operators_for.rs +++ b/linux/crates/core/src/filter/operators_for.rs @@ -43,15 +43,12 @@ const EQUALITY: &[FilterOp] = &[ /// lists them. /// /// Driven by the column's kind rather than its type name, so a new -/// engine spelling never needs a new branch here. A binary column gets -/// only the null tests: there is no text form of a blob to compare. +/// engine spelling never needs a new branch here. A binary column +/// compares for equality on its `0x` hex form, which is how a key such +/// as a `BINARY(16)` id is followed. pub fn operators_for(kind: &ColumnKind) -> &'static [FilterOp] { match kind { - ColumnKind::Bytes { .. } - | ColumnKind::Spatial - | ColumnKind::Array { .. } - | ColumnKind::Variant - | ColumnKind::Composite => NULL_ONLY, + ColumnKind::Spatial | ColumnKind::Array { .. } | ColumnKind::Variant | ColumnKind::Composite => NULL_ONLY, ColumnKind::Text { .. } | ColumnKind::Json { .. } | ColumnKind::Enum { .. } @@ -67,7 +64,11 @@ pub fn operators_for(kind: &ColumnKind) -> &'static [FilterOp] { | ColumnKind::Time { .. } | ColumnKind::Timestamp { .. } | ColumnKind::Interval => COMPARISON, - ColumnKind::Bool(_) | ColumnKind::Uuid | ColumnKind::BitString { .. } | ColumnKind::Network(_) => EQUALITY, + ColumnKind::Bool(_) + | ColumnKind::Uuid + | ColumnKind::BitString { .. } + | ColumnKind::Network(_) + | ColumnKind::Bytes { .. } => EQUALITY, } } @@ -85,7 +86,7 @@ mod tests { } #[test] - fn a_kind_with_no_comparable_text_gets_only_the_null_tests() { + fn a_kind_with_no_comparable_text_gets_only_the_null_tests_and_a_blob_compares_equal() { let array = ColumnKind::Array { element: Box::new(ColumnKind::Uuid), }; @@ -96,6 +97,11 @@ mod tests { "{kind:?}" ); } + let blob = operators_for(&ColumnKind::Bytes { + length: crate::column::TextLength::Unbounded, + }); + assert!(blob.contains(&FilterOp::Eq), "{blob:?}"); + assert!(!blob.contains(&FilterOp::Contains), "{blob:?}"); assert!(operators_for(&ColumnKind::Money { scale: Some(2) }).contains(&FilterOp::Between)); assert!(operators_for(&ColumnKind::Xml).contains(&FilterOp::Contains)); } diff --git a/linux/crates/core/src/hex.rs b/linux/crates/core/src/hex.rs index 1cfaab5dcc..6ba0ed8a21 100644 --- a/linux/crates/core/src/hex.rs +++ b/linux/crates/core/src/hex.rs @@ -9,6 +9,22 @@ pub fn encode_lower(bytes: &[u8]) -> String { encoded } +/// Hex digits of either case back into bytes. `None` for an odd count +/// or anything that is not a hex digit. +pub fn decode(text: &str) -> Option> { + if !text.len().is_multiple_of(2) { + return None; + } + text.as_bytes() + .chunks(2) + .map(|pair| { + let high = char::from(*pair.first()?).to_digit(16)?; + let low = char::from(*pair.get(1)?).to_digit(16)?; + u8::try_from((high << 4) | low).ok() + }) + .collect() +} + #[cfg(test)] mod tests { use super::*; @@ -19,4 +35,20 @@ mod tests { assert_eq!(encode_lower(&[0x00, 0x0f, 0xa5, 0xff]), "000fa5ff"); assert_eq!(encode_lower(b"TablePro"), "5461626c6550726f"); } + + #[test] + fn decode_reads_back_what_encode_wrote_in_either_case() { + let bytes = [0x00, 0x0f, 0xa5, 0xff]; + assert_eq!(decode(&encode_lower(&bytes)), Some(bytes.to_vec())); + assert_eq!(decode("A5fF"), Some(vec![0xa5, 0xff])); + assert_eq!(decode(""), Some(Vec::new())); + } + + #[test] + fn decode_refuses_an_odd_count_and_other_characters() { + assert_eq!(decode("abc"), None); + assert_eq!(decode("zz"), None); + assert_eq!(decode("+1"), None); + assert_eq!(decode("é1"), None); + } } diff --git a/linux/crates/drivers/clickhouse/src/connection.rs b/linux/crates/drivers/clickhouse/src/connection.rs index a45a4dd5c1..9201a12a08 100644 --- a/linux/crates/drivers/clickhouse/src/connection.rs +++ b/linux/crates/drivers/clickhouse/src/connection.rs @@ -5,7 +5,7 @@ use std::time::Duration; use async_trait::async_trait; use tablepro_core::column::ColumnInfo; -use tablepro_core::ddl::TableDefinition; +use tablepro_core::ddl::{InboundForeignKey, TableDefinition}; use tablepro_core::dialect::SqlDialect; use tablepro_core::meta::{IndexInfo, ObjectInfo, ObjectScope, RowIdentity, SchemaInfo, TableEstimate, TableRef}; use tablepro_core::statement::Statement; @@ -301,6 +301,15 @@ impl Connection for ClickHouseConnection { Ok(Vec::new()) } + async fn fetch_inbound_foreign_keys( + &self, + _table: &TableRef, + _options: CallOptions, + ) -> Result, DriverError> { + self.ensure_open()?; + Ok(Vec::new()) + } + async fn fetch_table_definition( &self, table: &TableRef, diff --git a/linux/crates/drivers/mssql/src/connection.rs b/linux/crates/drivers/mssql/src/connection.rs index 99c8f5f714..ef8d608120 100644 --- a/linux/crates/drivers/mssql/src/connection.rs +++ b/linux/crates/drivers/mssql/src/connection.rs @@ -5,7 +5,7 @@ use std::time::Duration; use async_trait::async_trait; use deadpool::managed::{PoolError, TimeoutType}; use tablepro_core::column::ColumnInfo; -use tablepro_core::ddl::TableDefinition; +use tablepro_core::ddl::{InboundForeignKey, TableDefinition}; use tablepro_core::dialect::SqlDialect; use tablepro_core::meta::{IndexInfo, ObjectInfo, ObjectScope, RowIdentity, SchemaInfo, TableEstimate, TableRef}; use tablepro_core::statement::Statement; @@ -299,6 +299,21 @@ impl Connection for MssqlConnection { .await } + async fn fetch_inbound_foreign_keys( + &self, + table: &TableRef, + options: CallOptions, + ) -> Result, DriverError> { + let relation = self.relation(table); + self.read( + options, + read_op(move |client, watch| { + Box::pin(async move { metadata::fetch_inbound_foreign_keys(client, watch, &relation).await }) + }), + ) + .await + } + async fn fetch_table_definition( &self, table: &TableRef, diff --git a/linux/crates/drivers/mssql/src/metadata.rs b/linux/crates/drivers/mssql/src/metadata.rs index 989b10c3da..3f9ed74dec 100644 --- a/linux/crates/drivers/mssql/src/metadata.rs +++ b/linux/crates/drivers/mssql/src/metadata.rs @@ -3,7 +3,7 @@ //! which resolves an unqualified name in the login's default schema the //! way the server does everywhere else. -use tablepro_core::ddl::{ReferentialAction, TableDefinition}; +use tablepro_core::ddl::{InboundForeignKey, ReferentialAction, TableDefinition}; use tablepro_core::meta::{ IndexInfo, IndexKey, ObjectInfo, ObjectKind, ObjectScope, RowCountEstimate, RowIdentity, SchemaInfo, TableEstimate, TableRef, @@ -281,18 +281,27 @@ pub(crate) async fn fetch_table_definition( let primary = catalogue_rows(client, watch, PRIMARY_KEY_SQL, &[text_param(relation)]).await?; let indexes = fetch_indexes(client, watch, relation).await?; let foreign_keys = fetch_foreign_keys(client, watch, relation).await?; - let inbound = catalogue_rows(client, watch, INBOUND_FOREIGN_KEYS_SQL, &[text_param(relation)]).await?; + let inbound_foreign_keys = fetch_inbound_foreign_keys(client, watch, relation).await?; Ok(TableDefinition { table: table.clone(), columns: columns.iter().map(column_definition).collect(), primary_key: primary_key(&primary)?, indexes, foreign_keys, - inbound_foreign_keys: inbound_foreign_keys(&inbound)?, + inbound_foreign_keys, source: None, }) } +pub(crate) async fn fetch_inbound_foreign_keys( + client: &mut MssqlClient, + watch: &Watch, + relation: &str, +) -> Result, Interrupt> { + let rows = catalogue_rows(client, watch, INBOUND_FOREIGN_KEYS_SQL, &[text_param(relation)]).await?; + Ok(inbound_foreign_keys(&rows)?) +} + #[cfg(test)] mod tests { use super::*; diff --git a/linux/crates/drivers/mssql/tests/integration/filter.rs b/linux/crates/drivers/mssql/tests/integration/filter.rs index 0394fc0259..e678f8dad1 100644 --- a/linux/crates/drivers/mssql/tests/integration/filter.rs +++ b/linux/crates/drivers/mssql/tests/integration/filter.rs @@ -94,3 +94,41 @@ async fn a_decimal_filter_keeps_the_digits_it_was_given() { assert_eq!(rows, expected, "{column} {op:?} {text}"); } } + +/// A binary key followed from another table arrives as its hex text. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn equals_on_varbinary_takes_the_hex_form() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + setup( + connection.as_ref(), + &[ + "CREATE TABLE dbo.keys (id int PRIMARY KEY, k varbinary(16))", + "INSERT INTO dbo.keys VALUES (1, 0x0AFF), (2, 0x0A)", + ], + ) + .await; + let table = dbo("keys"); + let columns = columns(connection.as_ref(), &table).await; + let dialect = connection.dialect(); + let mut builder = StatementBuilder::new(); + builder.push_sql(&format!("SELECT id FROM {} WHERE ", dialect.qualify(&table))); + let filter = FilterSet { + rules: vec![FilterRule { + column: "k".to_owned(), + op: FilterOp::Eq, + value: Some(FilterValue::Single("0x0aff".to_owned())), + }], + ..FilterSet::default() + }; + build_filter(&mut builder, dialect.as_ref(), &columns, &filter).expect("a filter"); + + let rows = connection + .query_bound(&builder.finish(), RowLimit::EDITOR_DEFAULT, within(CALL)) + .await + .expect("the filtered rows") + .rows; + + assert_eq!(rows, vec![vec![Value::Int(1)]]); +} diff --git a/linux/crates/drivers/mssql/tests/integration/metadata.rs b/linux/crates/drivers/mssql/tests/integration/metadata.rs index d80817776a..c1f3baf099 100644 --- a/linux/crates/drivers/mssql/tests/integration/metadata.rs +++ b/linux/crates/drivers/mssql/tests/integration/metadata.rs @@ -186,7 +186,12 @@ async fn fetch_table_definition_lists_inbound_foreign_keys() { .fetch_foreign_keys(&dbo("lines"), within(CALL)) .await .expect("the foreign keys"); + let alone = connection + .fetch_inbound_foreign_keys(&dbo("orders"), within(CALL)) + .await + .expect("the keys pointing at orders"); + assert_eq!(alone, orders.inbound_foreign_keys); assert_eq!(orders.inbound_foreign_keys.len(), 1); let inbound = &orders.inbound_foreign_keys[0]; assert_eq!(inbound.name, "FK_lines_orders"); diff --git a/linux/crates/drivers/mysql/src/connection.rs b/linux/crates/drivers/mysql/src/connection.rs index 77c974647d..0a4e3e4ad9 100644 --- a/linux/crates/drivers/mysql/src/connection.rs +++ b/linux/crates/drivers/mysql/src/connection.rs @@ -5,7 +5,7 @@ use async_trait::async_trait; use deadpool::managed::{PoolError, TimeoutType}; use sqlx::Executor as _; use tablepro_core::column::ColumnInfo; -use tablepro_core::ddl::TableDefinition; +use tablepro_core::ddl::{InboundForeignKey, TableDefinition}; use tablepro_core::dialect::SqlDialect; use tablepro_core::meta::{IndexInfo, ObjectInfo, ObjectScope, RowIdentity, SchemaInfo, TableEstimate, TableRef}; use tablepro_core::statement::Statement; @@ -319,6 +319,21 @@ impl Connection for MysqlConnection { .await } + async fn fetch_inbound_foreign_keys( + &self, + table: &TableRef, + options: CallOptions, + ) -> Result, DriverError> { + let table = table.clone(); + self.read( + options, + read_op(move |connection, flavour| { + Box::pin(async move { metadata::fetch_inbound_foreign_keys(connection, flavour, &table).await }) + }), + ) + .await + } + async fn fetch_table_definition( &self, table: &TableRef, diff --git a/linux/crates/drivers/mysql/src/metadata.rs b/linux/crates/drivers/mysql/src/metadata.rs index 20915a4f6c..e1ad61b008 100644 --- a/linux/crates/drivers/mysql/src/metadata.rs +++ b/linux/crates/drivers/mysql/src/metadata.rs @@ -260,6 +260,24 @@ pub(crate) async fn fetch_foreign_keys( Ok(keys) } +/// The keys of every database the account can see that point at the +/// table, since a key can cross databases. +pub(crate) async fn fetch_inbound_foreign_keys( + connection: &mut MySqlConnection, + flavour: MySqlFlavour, + table: &TableRef, +) -> Result, DriverError> { + let rows = catalog_rows( + connection, + flavour, + INBOUND_FOREIGN_KEYS_SQL, + "KEY_COLUMN_USAGE", + &table_binds(table), + ) + .await?; + inbound_foreign_keys(&rows) +} + fn referential_action(rule: &str) -> Option { ReferentialAction::ALL .into_iter() @@ -394,15 +412,7 @@ pub(crate) async fn fetch_table_definition( columns: index.key_columns(), }); let foreign_keys = fetch_foreign_keys(connection, flavour, table).await?; - let inbound = catalog_rows( - connection, - flavour, - INBOUND_FOREIGN_KEYS_SQL, - "KEY_COLUMN_USAGE", - &table_binds(table), - ) - .await?; - let inbound_foreign_keys: Vec = inbound_foreign_keys(&inbound)?; + let inbound_foreign_keys = fetch_inbound_foreign_keys(connection, flavour, table).await?; Ok(TableDefinition { table: table.clone(), columns, diff --git a/linux/crates/drivers/mysql/tests/integration/metadata.rs b/linux/crates/drivers/mysql/tests/integration/metadata.rs index 009348f756..8e6e0ec261 100644 --- a/linux/crates/drivers/mysql/tests/integration/metadata.rs +++ b/linux/crates/drivers/mysql/tests/integration/metadata.rs @@ -175,7 +175,13 @@ async fn fetch_table_definition_lists_inbound_foreign_keys() { .await .expect("the child"); + let alone = connection + .fetch_inbound_foreign_keys(&table("parent"), within(CALL)) + .await + .expect("the keys pointing at the parent"); + let inbound = &parent.inbound_foreign_keys; + assert_eq!(&alone, inbound, "{flavour:?}"); assert_eq!(inbound.len(), 1, "{flavour:?}"); assert_eq!(inbound[0].name, "child_parent"); assert_eq!(inbound[0].table.name, "child"); diff --git a/linux/crates/drivers/postgres/src/catalog/table_definition.rs b/linux/crates/drivers/postgres/src/catalog/table_definition.rs index bd36787fc8..f4e203a793 100644 --- a/linux/crates/drivers/postgres/src/catalog/table_definition.rs +++ b/linux/crates/drivers/postgres/src/catalog/table_definition.rs @@ -13,7 +13,10 @@ SELECT c.conname::text, FROM pg_catalog.pg_constraint c WHERE c.conrelid = $1::text::regclass AND c.contype = 'p'"; -/// Foreign keys on other tables that point at this one. +/// Foreign keys on other tables that point at this one. A partition of +/// a partitioned table that points here holds a clone of its parent's +/// key, and its rows are the parent's rows, so only the parent's key is +/// listed. pub(crate) const INBOUND_FOREIGN_KEYS_SQL: &str = "\ SELECT c.conname::text, n.nspname::text, @@ -30,6 +33,8 @@ FROM pg_catalog.pg_constraint c JOIN pg_catalog.pg_class t ON t.oid = c.conrelid JOIN pg_catalog.pg_namespace n ON n.oid = t.relnamespace WHERE c.confrelid = $1::text::regclass AND c.contype = 'f' + AND NOT EXISTS (SELECT 1 FROM pg_catalog.pg_constraint p + WHERE p.oid = c.conparentid AND p.confrelid = c.confrelid) ORDER BY n.nspname, t.relname, c.conname"; pub(crate) fn inbound_foreign_key(row: &Row) -> Result { diff --git a/linux/crates/drivers/postgres/src/connection.rs b/linux/crates/drivers/postgres/src/connection.rs index ac3e4e08c0..c9953dc1b3 100644 --- a/linux/crates/drivers/postgres/src/connection.rs +++ b/linux/crates/drivers/postgres/src/connection.rs @@ -4,7 +4,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; use async_trait::async_trait; use deadpool_postgres::{Pool, PoolError, TimeoutType, Timeouts}; use tablepro_core::column::ColumnInfo; -use tablepro_core::ddl::TableDefinition; +use tablepro_core::ddl::{InboundForeignKey, TableDefinition}; use tablepro_core::dialect::SqlDialect; use tablepro_core::meta::{IndexInfo, ObjectInfo, ObjectScope, RowIdentity, SchemaInfo, TableEstimate, TableRef}; use tablepro_core::statement::Statement; @@ -301,6 +301,21 @@ impl Connection for PgConnection { .await } + async fn fetch_inbound_foreign_keys( + &self, + table: &TableRef, + options: CallOptions, + ) -> Result, DriverError> { + let relation = self.relation(table); + self.read( + options, + read_op(move |client| { + Box::pin(async move { metadata::fetch_inbound_foreign_keys(client, &relation).await }) + }), + ) + .await + } + async fn fetch_table_definition( &self, table: &TableRef, diff --git a/linux/crates/drivers/postgres/src/metadata/foreign_keys.rs b/linux/crates/drivers/postgres/src/metadata/foreign_keys.rs index 3b6816f6cd..2a57ad9702 100644 --- a/linux/crates/drivers/postgres/src/metadata/foreign_keys.rs +++ b/linux/crates/drivers/postgres/src/metadata/foreign_keys.rs @@ -4,6 +4,9 @@ use tokio_postgres::Client; use super::{decode_error, text_rows}; +/// A key to a partitioned table has a clone on the same table for each +/// partition it reaches, made and dropped with it, so only the key the +/// user declared is listed. const FOREIGN_KEYS_SQL: &str = "\ SELECT c.conname::text, ARRAY(SELECT a.attname::text @@ -22,6 +25,8 @@ FROM pg_catalog.pg_constraint c JOIN pg_catalog.pg_class t ON t.oid = c.confrelid JOIN pg_catalog.pg_namespace n ON n.oid = t.relnamespace WHERE c.conrelid = $1::text::regclass AND c.contype = 'f' + AND NOT EXISTS (SELECT 1 FROM pg_catalog.pg_constraint p + WHERE p.oid = c.conparentid AND p.conrelid = c.conrelid) ORDER BY c.conname"; pub(crate) async fn fetch_foreign_keys(client: &Client, relation: &str) -> Result, DriverError> { diff --git a/linux/crates/drivers/postgres/src/metadata/mod.rs b/linux/crates/drivers/postgres/src/metadata/mod.rs index d55b53f0fe..6346646441 100644 --- a/linux/crates/drivers/postgres/src/metadata/mod.rs +++ b/linux/crates/drivers/postgres/src/metadata/mod.rs @@ -18,7 +18,7 @@ use futures_util::TryStreamExt; use postgres_types::{ToSql, Type}; use tablepro_core::DriverError; use tablepro_core::column::ColumnInfo; -use tablepro_core::ddl::{PrimaryKeyConstraint, TableDefinition}; +use tablepro_core::ddl::{InboundForeignKey, PrimaryKeyConstraint, TableDefinition}; use tablepro_core::meta::TableRef; use tokio_postgres::{Client, Row}; @@ -68,6 +68,17 @@ async fn column_rows(client: &Client, columns_sql: &str, relation: &str) -> Resu .collect() } +pub(crate) async fn fetch_inbound_foreign_keys( + client: &Client, + relation: &str, +) -> Result, DriverError> { + text_rows(client, INBOUND_FOREIGN_KEYS_SQL, &[relation]) + .await? + .iter() + .map(inbound_foreign_key) + .collect() +} + pub(crate) async fn fetch_table_definition( client: &Client, columns_sql: &str, @@ -82,7 +93,7 @@ pub(crate) async fn fetch_table_definition( text_rows(client, PRIMARY_KEY_NAME_SQL, ¶ms), fetch_indexes(client, relation), fetch_foreign_keys(client, relation), - text_rows(client, INBOUND_FOREIGN_KEYS_SQL, ¶ms), + fetch_inbound_foreign_keys(client, relation), )?; let primary_key = primary_key .first() @@ -94,10 +105,6 @@ pub(crate) async fn fetch_table_definition( }) }) .transpose()?; - let inbound_foreign_keys = inbound_foreign_keys - .iter() - .map(inbound_foreign_key) - .collect::>()?; Ok(TableDefinition { table: table.clone(), columns: columns.iter().map(column_definition).collect(), diff --git a/linux/crates/drivers/postgres/tests/integration/filter.rs b/linux/crates/drivers/postgres/tests/integration/filter.rs index 6443945357..79fde25f50 100644 --- a/linux/crates/drivers/postgres/tests/integration/filter.rs +++ b/linux/crates/drivers/postgres/tests/integration/filter.rs @@ -75,3 +75,28 @@ async fn contains_and_equals_on_json_jsonb_xml_and_enum() { ); assert_eq!(ids_where(connection.as_ref(), "m", FilterOp::Eq, "calm").await, first()); } + +/// A binary key followed from another table arrives as its hex text. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn equals_on_bytea_takes_the_hex_form() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + setup( + connection.as_ref(), + &[ + "CREATE TABLE docs (id int PRIMARY KEY, b bytea)", + "INSERT INTO docs VALUES (1, '\\x0aff'), (2, '\\x0a')", + ], + ) + .await; + + assert_eq!( + ids_where(connection.as_ref(), "b", FilterOp::Eq, "0x0AFF").await, + vec![Value::Int(1)] + ); + assert_eq!( + ids_where(connection.as_ref(), "b", FilterOp::NotEq, "0x0aff").await, + vec![Value::Int(2)] + ); +} diff --git a/linux/crates/drivers/postgres/tests/integration/metadata.rs b/linux/crates/drivers/postgres/tests/integration/metadata.rs index 693c269e88..982e8617f7 100644 --- a/linux/crates/drivers/postgres/tests/integration/metadata.rs +++ b/linux/crates/drivers/postgres/tests/integration/metadata.rs @@ -321,6 +321,90 @@ async fn fetch_table_definition_reports_pk_name_collation_comment_and_inbound_ke assert!(definition.source.is_none()); } +/// A key to or from a partitioned table has a clone per partition, +/// which the server makes and drops with it and refuses to drop alone. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn partition_clones_of_a_key_are_not_listed_as_keys() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + setup( + connection.as_ref(), + &[ + "CREATE TABLE customers (id int, region text, PRIMARY KEY (id, region)) PARTITION BY LIST (region)", + "CREATE TABLE customers_eu PARTITION OF customers FOR VALUES IN ('eu')", + "CREATE TABLE customers_us PARTITION OF customers FOR VALUES IN ('us')", + "CREATE TABLE orders (id int PRIMARY KEY, customer_id int, region text, \ + CONSTRAINT orders_to_customers FOREIGN KEY (customer_id, region) REFERENCES customers (id, region))", + "CREATE TABLE accounts (id int PRIMARY KEY)", + "CREATE TABLE events (id int, at int, account_id int REFERENCES accounts (id), PRIMARY KEY (id, at)) \ + PARTITION BY RANGE (at)", + "CREATE TABLE events_2025 PARTITION OF events FOR VALUES FROM (2025) TO (2026)", + "CREATE TABLE events_2026 PARTITION OF events FOR VALUES FROM (2026) TO (2027)", + ], + ) + .await; + + let outbound = connection + .fetch_foreign_keys(&table("orders"), within(CALL)) + .await + .expect("the keys of orders"); + let inbound = connection + .fetch_inbound_foreign_keys(&table("accounts"), within(CALL)) + .await + .expect("the keys pointing at accounts"); + let partition = connection + .fetch_foreign_keys(&table("events_2025"), within(CALL)) + .await + .expect("the keys of a partition"); + let definition = connection + .fetch_table_definition(&table("accounts"), within(CALL)) + .await + .expect("the definition of accounts"); + + let referenced: Vec<(&str, &str)> = outbound + .iter() + .map(|key| (key.name.as_str(), key.ref_table.as_str())) + .collect(); + assert_eq!(referenced, [("orders_to_customers", "customers")]); + assert_eq!(outbound[0].ref_columns, ["id", "region"]); + let referencing: Vec<&TableRef> = inbound.iter().map(|key| &key.table).collect(); + assert_eq!(referencing, [&table("events")]); + assert_eq!(definition.inbound_foreign_keys, inbound); + assert_eq!( + partition.iter().map(|key| key.ref_table.as_str()).collect::>(), + ["accounts"], + "a partition still has the key it inherited" + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn inbound_keys_name_the_referencing_schema_and_pair_their_columns() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + setup( + connection.as_ref(), + &[ + "CREATE SCHEMA sales", + "CREATE TABLE customers (id int, region text, PRIMARY KEY (id, region))", + "CREATE TABLE sales.orders (id int PRIMARY KEY, buyer int, buyer_region text, \ + FOREIGN KEY (buyer_region, buyer) REFERENCES customers (region, id))", + ], + ) + .await; + + let inbound = connection + .fetch_inbound_foreign_keys(&table("customers"), within(CALL)) + .await + .expect("the keys pointing at customers"); + + assert_eq!(inbound.len(), 1); + assert_eq!(inbound[0].table, TableRef::new(Some("sales".to_owned()), "orders")); + assert_eq!(inbound[0].columns, ["buyer_region", "buyer"]); + assert_eq!(inbound[0].referenced_columns, ["region", "id"]); +} + const CATALOG_TABLE: &[&str] = &[ "CREATE TYPE mood AS ENUM ('sad', 'ok', 'happy')", "CREATE DOMAIN positive AS int4 CHECK (VALUE > 0)", diff --git a/linux/crates/drivers/sqlite/src/catalog/mod.rs b/linux/crates/drivers/sqlite/src/catalog/mod.rs index 4283069c7f..5aea310a45 100644 --- a/linux/crates/drivers/sqlite/src/catalog/mod.rs +++ b/linux/crates/drivers/sqlite/src/catalog/mod.rs @@ -35,6 +35,10 @@ pub(crate) const TABLE_XINFO_SQL: &str = "SELECT cid, name, type, \"notnull\", d /// `table_list` takes no schema argument, so the schema is a filter. pub(crate) const TABLE_LIST_SQL: &str = "SELECT type, wr, strict FROM pragma_table_list(?1) WHERE schema = ?2"; +/// The table's name as declared. `table_list` matches its argument +/// without regard to case, as every SQLite name lookup does. +pub(crate) const DECLARED_TABLE_NAME_SQL: &str = "SELECT name FROM pragma_table_list(?1) WHERE schema = ?2"; + pub(crate) const INDEX_LIST_SQL: &str = "SELECT name, \"unique\", origin, partial FROM pragma_index_list(?1, ?2) ORDER BY name"; diff --git a/linux/crates/drivers/sqlite/src/connection.rs b/linux/crates/drivers/sqlite/src/connection.rs index dacd00a17f..12ebd2c200 100644 --- a/linux/crates/drivers/sqlite/src/connection.rs +++ b/linux/crates/drivers/sqlite/src/connection.rs @@ -6,7 +6,7 @@ use async_trait::async_trait; use futures::future::BoxFuture; use sqlx::sqlite::{SqliteConnectOptions, SqliteConnection as SqlxConnection, SqlitePool, SqlitePoolOptions}; use sqlx::{Connection as _, Executor as _}; -use tablepro_core::ddl::TableDefinition; +use tablepro_core::ddl::{InboundForeignKey, TableDefinition}; use tablepro_core::dialect::SqlDialect; use tablepro_core::meta::{IndexInfo, ObjectInfo, ObjectScope, RowIdentity, SchemaInfo, TableEstimate, TableRef}; use tablepro_core::statement::Statement; @@ -315,6 +315,18 @@ impl Connection for SqliteConnection { .await } + async fn fetch_inbound_foreign_keys( + &self, + table: &TableRef, + options: CallOptions, + ) -> Result, DriverError> { + let table = table.clone(); + self.read(options, move |connection| { + Box::pin(async move { metadata::fetch_inbound_foreign_keys(connection, &table).await }) + }) + .await + } + async fn fetch_table_definition( &self, table: &TableRef, diff --git a/linux/crates/drivers/sqlite/src/metadata.rs b/linux/crates/drivers/sqlite/src/metadata.rs index f4f1ef472a..ffa826a40f 100644 --- a/linux/crates/drivers/sqlite/src/metadata.rs +++ b/linux/crates/drivers/sqlite/src/metadata.rs @@ -9,8 +9,8 @@ use sqlx::sqlite::{SqliteConnection as SqlxConnection, SqliteRow}; use sqlx::{AssertSqlSafe, Row}; use tablepro_core::column::ColumnInfo; use tablepro_core::ddl::{ - ColumnDefinition, GeneratedColumn, PrimaryKeyConstraint, ReferentialAction, SchemaObjectKind, TableDefinition, - TableSource, + ColumnDefinition, GeneratedColumn, InboundForeignKey, PrimaryKeyConstraint, ReferentialAction, SchemaObjectKind, + TableDefinition, TableSource, }; use tablepro_core::meta::{ EngineRowId, IndexInfo, IndexKey, ObjectInfo, ObjectKind, ObjectScope, RowCountEstimate, RowIdentity, SchemaInfo, @@ -161,8 +161,14 @@ async fn foreign_key_rows( Ok(keys) } -/// Name the keys from the table's CREATE TABLE, and give a key that -/// names no parent columns the parent's primary key. +/// Name the keys from the table's CREATE TABLE, and spell each parent +/// and its columns as the parent declares them: the pragma repeats the +/// REFERENCES clause as written, and every other read here, the grid's +/// filter among them, names them as declared. A key that names no +/// parent columns takes the parent's primary key. A parent that is gone, +/// as one dropped while foreign keys were off, keeps the clause's +/// spelling, and a key of it naming no columns is left with none rather +/// than failing the read of every key of the table. async fn resolve_foreign_keys( connection: &mut SqlxConnection, table: &TableRef, @@ -171,18 +177,132 @@ async fn resolve_foreign_keys( ) -> Result, DriverError> { let declared = create_statement.and_then(declared_foreign_key_names); let mut names = foreign_key_names(&table.name, declared, keys.len()).into_iter(); + let mut parents: Vec<(String, Option)> = Vec::new(); let mut resolved = Vec::with_capacity(keys.len()); for (mut key, to_primary) in keys.into_values() { key.name = names.next().unwrap_or_default(); - if to_primary { - let parent = TableRef::new(table.schema.clone(), key.ref_table.clone()); - key.ref_columns = table_shape(connection, &parent).await?.primary_key(); + let known = parents + .iter() + .position(|(written, _)| written.eq_ignore_ascii_case(&key.ref_table)); + let index = match known { + Some(index) => index, + None => { + let parent = TableRef::new(table.schema.clone(), key.ref_table.clone()); + let found = declared_table(connection, &parent).await?; + parents.push((key.ref_table.clone(), found)); + parents.len() - 1 + } + }; + if let Some((_, Some(parent))) = parents.get(index) { + key.ref_table = parent.name.clone(); + key.ref_columns = match to_primary { + true => parent.primary_key(), + false => key.ref_columns.iter().map(|column| parent.spelling(column)).collect(), + }; } resolved.push(key); } Ok(resolved) } +/// A table's declared name and columns. +struct DeclaredTable { + name: String, + columns: Vec, +} + +impl DeclaredTable { + fn primary_key(&self) -> Vec { + let mut keyed: Vec<&SqliteColumnRow> = self.columns.iter().filter(|column| column.pk > 0).collect(); + keyed.sort_by_key(|column| column.pk); + keyed.into_iter().map(|column| column.name.clone()).collect() + } + + /// `written` as the table declares it. SQLite folds ASCII letters + /// alone when it compares names, so this does too. + fn spelling(&self, written: &str) -> String { + declared_spelling(written, self.columns.iter().map(|column| column.name.as_str())) + } +} + +fn declared_spelling<'a>(written: &str, mut declared: impl Iterator) -> String { + declared + .find(|name| name.eq_ignore_ascii_case(written)) + .unwrap_or(written) + .to_owned() +} + +/// `None` when no table of that name is in the schema. +async fn declared_table( + connection: &mut SqlxConnection, + table: &TableRef, +) -> Result, DriverError> { + let schema = schema_of(table); + let name: Option = sqlx::query_scalar(catalog::DECLARED_TABLE_NAME_SQL) + .bind(table.name.as_str()) + .bind(schema) + .fetch_optional(&mut *connection) + .await + .map_err(map_error)?; + let Some(name) = name else { + return Ok(None); + }; + let columns = sqlx::query(catalog::TABLE_XINFO_SQL) + .bind(name.as_str()) + .bind(schema) + .fetch_all(&mut *connection) + .await + .map_err(map_error)? + .iter() + .map(SqliteColumnRow::from_row) + .collect::, _>>()?; + Ok(Some(DeclaredTable { name, columns })) +} + +pub(crate) async fn fetch_inbound_foreign_keys( + connection: &mut SqlxConnection, + table: &TableRef, +) -> Result, DriverError> { + let shape = table_shape(connection, table).await?; + inbound_keys(connection, table, &shape).await +} + +/// The keys of the schema's other tables that point at `table`, its +/// columns spelled as `shape` declares them, since a child's REFERENCES +/// clause can spell them otherwise. +async fn inbound_keys( + connection: &mut SqlxConnection, + table: &TableRef, + shape: &TableShape, +) -> Result, DriverError> { + let schema_sql = quote(schema_of(table)); + let rows = sqlx::query(AssertSqlSafe(catalog::inbound_keys_sql(&schema_sql))) + .bind(table.name.as_str()) + .bind(schema_of(table)) + .fetch_all(&mut *connection) + .await + .map_err(map_error)? + .iter() + .map(|row| { + Ok(InboundKeyRow { + child: text(row, "child")?, + child_sql: row.try_get("child_sql").map_err(map_error)?, + child_keys: usize::try_from(row.try_get::("child_keys").map_err(map_error)?).unwrap_or(0), + id: row.try_get("id").map_err(map_error)?, + from: text(row, "from")?, + to: row.try_get("to").map_err(map_error)?, + }) + }) + .collect::, DriverError>>()?; + let mut keys = inbound_foreign_keys(table.schema.as_deref(), rows, &shape.primary_key()); + for key in &mut keys { + for column in &mut key.referenced_columns { + *column = declared_spelling(column, shape.columns.iter().map(|declared| declared.name.as_str())); + } + } + Ok(keys) +} + /// How a row can be named again: the primary key, else a unique index /// over NOT NULL columns covering every row, else the rowid. pub(crate) async fn row_identity( @@ -287,33 +407,15 @@ pub(crate) async fn fetch_table_definition( }) .collect(); let key_columns = shape.primary_key(); - let primary_key = (!key_columns.is_empty()).then(|| PrimaryKeyConstraint { + let primary_key = (!key_columns.is_empty()).then_some(PrimaryKeyConstraint { name: None, - columns: key_columns.clone(), + columns: key_columns, }); let indexes = indexes(connection, table, &shape).await?; let foreign_keys = foreign_key_rows(connection, table).await?; let foreign_keys = resolve_foreign_keys(connection, table, foreign_keys, create_statement.as_deref()).await?; - let inbound_rows = sqlx::query(AssertSqlSafe(catalog::inbound_keys_sql(&schema_sql))) - .bind(table.name.as_str()) - .bind(schema_of(table)) - .fetch_all(&mut *connection) - .await - .map_err(map_error)? - .iter() - .map(|row| { - Ok(InboundKeyRow { - child: text(row, "child")?, - child_sql: row.try_get("child_sql").map_err(map_error)?, - child_keys: usize::try_from(row.try_get::("child_keys").map_err(map_error)?).unwrap_or(0), - id: row.try_get("id").map_err(map_error)?, - from: text(row, "from")?, - to: row.try_get("to").map_err(map_error)?, - }) - }) - .collect::, DriverError>>()?; - let inbound = inbound_foreign_keys(table.schema.as_deref(), inbound_rows, &key_columns); + let inbound = inbound_keys(connection, table, &shape).await?; let source = match create_statement { Some(create_statement) => Some(TableSource { diff --git a/linux/crates/drivers/sqlite/tests/integration/catalog.rs b/linux/crates/drivers/sqlite/tests/integration/catalog.rs index 1db4707b36..2d28c9a73a 100644 --- a/linux/crates/drivers/sqlite/tests/integration/catalog.rs +++ b/linux/crates/drivers/sqlite/tests/integration/catalog.rs @@ -132,6 +132,114 @@ async fn fetch_table_definition_lists_inbound_foreign_keys() { connection.close().await.unwrap(); } +/// The pragma repeats a REFERENCES clause as written; every other read, +/// and a filter on the parent's columns, names them as declared. +#[tokio::test] +async fn foreign_keys_name_the_parent_and_its_columns_as_declared() { + let file = TestFile::new().await; + let connection = file.open().await; + apply( + connection.as_ref(), + &[ + "CREATE TABLE customers (Id INTEGER, Region TEXT, PRIMARY KEY (Id, Region))", + "CREATE TABLE orders (id INTEGER PRIMARY KEY, customer_id INTEGER, region TEXT, \ + FOREIGN KEY (CUSTOMER_ID, region) REFERENCES Customers (ID, REGION))", + "CREATE TABLE invoices (id INTEGER PRIMARY KEY, customer_id INTEGER, region TEXT, \ + FOREIGN KEY (customer_id, region) REFERENCES CUSTOMERS)", + ], + ) + .await + .unwrap(); + + let outbound = connection + .fetch_foreign_keys(&TableRef::unqualified("orders"), call()) + .await + .unwrap(); + let implicit = connection + .fetch_foreign_keys(&TableRef::unqualified("invoices"), call()) + .await + .unwrap(); + let inbound = connection + .fetch_inbound_foreign_keys(&TableRef::unqualified("customers"), call()) + .await + .unwrap(); + + let declared = vec!["Id".to_owned(), "Region".to_owned()]; + assert_eq!(outbound[0].ref_table, "customers"); + assert_eq!(outbound[0].ref_columns, declared); + assert_eq!(outbound[0].columns, ["customer_id", "region"]); + assert_eq!(implicit[0].ref_table, "customers"); + assert_eq!(implicit[0].ref_columns, declared); + let pointing: Vec<(&str, &[String])> = inbound + .iter() + .map(|key| (key.table.name.as_str(), key.referenced_columns.as_slice())) + .collect(); + assert_eq!(pointing, [("invoices", &declared[..]), ("orders", &declared[..])]); + connection.close().await.unwrap(); +} + +/// SQLite accepts a key to a table that is not there, and keeps one +/// whose table was dropped while foreign keys were off. +#[tokio::test] +async fn a_key_to_a_missing_table_leaves_the_tables_other_keys_readable() { + let file = TestFile::new().await; + let connection = file.open().await; + apply( + connection.as_ref(), + &[ + "CREATE TABLE owners (id INTEGER PRIMARY KEY)", + "CREATE TABLE notes (id INTEGER PRIMARY KEY, owner INTEGER REFERENCES owners, \ + ghost_id INTEGER REFERENCES ghost, spook_id INTEGER REFERENCES Spook (Id))", + ], + ) + .await + .unwrap(); + let notes = TableRef::unqualified("notes"); + + let keys = connection.fetch_foreign_keys(¬es, call()).await.unwrap(); + let definition = connection.fetch_table_definition(¬es, call()).await.unwrap(); + + let mut read: Vec<(&str, &[String])> = keys + .iter() + .map(|key| (key.ref_table.as_str(), key.ref_columns.as_slice())) + .collect(); + read.sort_unstable(); + assert_eq!( + read, + [ + ("Spook", &["Id".to_owned()][..]), + ("ghost", &[][..]), + ("owners", &["id".to_owned()][..]), + ] + ); + assert_eq!(definition.foreign_keys, keys); + connection.close().await.unwrap(); +} + +#[tokio::test] +async fn inbound_keys_read_alone_are_the_definitions() { + let file = TestFile::new().await; + let connection = file.open().await; + apply( + connection.as_ref(), + &[ + "CREATE TABLE orders (id INTEGER PRIMARY KEY, code TEXT UNIQUE)", + "CREATE TABLE lines (id INTEGER PRIMARY KEY, order_id INTEGER REFERENCES orders)", + "CREATE TABLE notes (id INTEGER PRIMARY KEY, order_code TEXT REFERENCES \"ORDERS\" (CODE))", + ], + ) + .await + .unwrap(); + let orders = TableRef::unqualified("orders"); + + let inbound = connection.fetch_inbound_foreign_keys(&orders, call()).await.unwrap(); + let definition = connection.fetch_table_definition(&orders, call()).await.unwrap(); + + assert_eq!(inbound, definition.inbound_foreign_keys); + assert_eq!(inbound[1].referenced_columns, ["code"]); + connection.close().await.unwrap(); +} + #[tokio::test] async fn dependents_and_source_are_read() { let file = TestFile::new().await; diff --git a/linux/crates/drivers/sqlite/tests/integration/dialect.rs b/linux/crates/drivers/sqlite/tests/integration/dialect.rs index c55ab8a7f2..d4d671a2a9 100644 --- a/linux/crates/drivers/sqlite/tests/integration/dialect.rs +++ b/linux/crates/drivers/sqlite/tests/integration/dialect.rs @@ -53,6 +53,27 @@ async fn like_escape_matches_literal_underscore_and_percent() { connection.close().await.unwrap(); } +/// A binary key followed from another table arrives as its hex text. +#[tokio::test] +async fn a_blob_equals_its_hex_form() { + let file = TestFile::new().await; + let connection = file.open().await; + apply( + connection.as_ref(), + &[ + "CREATE TABLE names (name BLOB)", + "INSERT INTO names VALUES (X'0AFF'), (X'0A'), ('0x0aff')", + ], + ) + .await + .unwrap(); + let table = TableRef::unqualified("names"); + + assert_eq!(matching(connection.as_ref(), &table, FilterOp::Eq, "0x0aff").await, 1); + assert_eq!(matching(connection.as_ref(), &table, FilterOp::NotEq, "0x0A").await, 2); + connection.close().await.unwrap(); +} + #[tokio::test] async fn insensitive_filter_matches_mixed_case() { let file = TestFile::new().await; diff --git a/linux/crates/test-support/src/call_counters.rs b/linux/crates/test-support/src/call_counters.rs index 21d1e8a009..f3faba6da6 100644 --- a/linux/crates/test-support/src/call_counters.rs +++ b/linux/crates/test-support/src/call_counters.rs @@ -11,6 +11,7 @@ pub struct CallCounters { pub fetch_columns: Arc, pub fetch_indexes: Arc, pub fetch_foreign_keys: Arc, + pub fetch_inbound_foreign_keys: Arc, pub fetch_table_definition: Arc, pub row_identity: Arc, pub table_estimate: Arc, diff --git a/linux/crates/test-support/src/connection_script.rs b/linux/crates/test-support/src/connection_script.rs index 7a1806b27c..a0f063f2a3 100644 --- a/linux/crates/test-support/src/connection_script.rs +++ b/linux/crates/test-support/src/connection_script.rs @@ -1,6 +1,6 @@ use std::sync::Arc; -use tablepro_core::ddl::TableDefinition; +use tablepro_core::ddl::{InboundForeignKey, TableDefinition}; use tablepro_core::dialect::SqlDialect; use tablepro_core::meta::{ObjectInfo, RowIdentity, SchemaInfo, TableEstimate}; use tablepro_core::{ @@ -28,6 +28,7 @@ pub struct ConnectionScript { pub fetch_columns: Replies>>, pub fetch_indexes: Replies>>, pub fetch_foreign_keys: Replies>>, + pub fetch_inbound_foreign_keys: Replies>>, pub fetch_table_definition: Replies>, pub row_identity: Replies>, pub table_estimate: Replies>, @@ -55,6 +56,7 @@ impl Default for ConnectionScript { fetch_columns: Replies::default(), fetch_indexes: Replies::default(), fetch_foreign_keys: Replies::default(), + fetch_inbound_foreign_keys: Replies::default(), fetch_table_definition: Replies::default(), row_identity: Replies::default(), table_estimate: Replies::default(), diff --git a/linux/crates/test-support/src/fake_connection.rs b/linux/crates/test-support/src/fake_connection.rs index bbfea689f8..87d8ccf726 100644 --- a/linux/crates/test-support/src/fake_connection.rs +++ b/linux/crates/test-support/src/fake_connection.rs @@ -2,7 +2,7 @@ use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; use async_trait::async_trait; -use tablepro_core::ddl::TableDefinition; +use tablepro_core::ddl::{InboundForeignKey, TableDefinition}; use tablepro_core::dialect::SqlDialect; use tablepro_core::meta::{ObjectInfo, ObjectScope, RowIdentity, SchemaInfo, TableEstimate}; use tablepro_core::statement::Statement; @@ -260,6 +260,24 @@ impl Connection for FakeConnection { .await } + async fn fetch_inbound_foreign_keys( + &self, + table: &TableRef, + options: CallOptions, + ) -> Result, DriverError> { + let counter = &self.counters.fetch_inbound_foreign_keys; + let name = Some(table.name.as_str()); + self.answer( + "fetch_inbound_foreign_keys", + counter, + |s| &mut s.fetch_inbound_foreign_keys, + name, + &options, + TimeoutPhase::Statement, + ) + .await + } + async fn fetch_table_definition( &self, table: &TableRef, diff --git a/linux/crates/test-support/src/replies.rs b/linux/crates/test-support/src/replies.rs index 4de7b823ab..d4e8b5b7b5 100644 --- a/linux/crates/test-support/src/replies.rs +++ b/linux/crates/test-support/src/replies.rs @@ -26,6 +26,11 @@ impl Replies { self } + /// Whether every call has an answer once the queue is empty. + pub fn answers_always(&self) -> bool { + self.otherwise.is_some() + } + /// Answer every call whose SQL contains `fragment` with `reply`, /// before the queue is consulted. The first matching rule wins. pub fn when_sql_contains(&mut self, fragment: impl Into, reply: R) -> &mut Self { diff --git a/linux/data/resources/shortcuts-dialog.ui b/linux/data/resources/shortcuts-dialog.ui index 7a8aba7fbb..9437cc425d 100644 --- a/linux/data/resources/shortcuts-dialog.ui +++ b/linux/data/resources/shortcuts-dialog.ui @@ -216,6 +216,12 @@ <Primary>BackSpace + + + Open the Referenced Row + <Primary>j + + Widen Column diff --git a/linux/docs/adding-drivers.md b/linux/docs/adding-drivers.md index 4234ccf5e6..7682286d97 100644 --- a/linux/docs/adding-drivers.md +++ b/linux/docs/adding-drivers.md @@ -153,7 +153,7 @@ The `Connection` is a workspace's link to one server: the pool behind the sideba | `server_info`, `dialect`, `read_only` | Fixed at connect. | | `link_state`, `lost` | A `LinkWatch` shared by every call. `lost()` resolves when the link is gone: a failed probe, or a failed call that a check outside the pool confirmed. One dead backend in the pool does not resolve it, so the session monitor can act on it. | | `probe` | Whether the server answers. It never waits for a pool slot: with every slot busy it opens a connection of its own. | -| `list_schemas`, `list_objects`, `fetch_columns`, `fetch_indexes`, `fetch_foreign_keys`, `fetch_table_definition`, `row_identity`, `table_estimate` | Catalogue reads. `fetch_table_definition` reads columns, keys and dependents in one call so they come from the same moment. | +| `list_schemas`, `list_objects`, `fetch_columns`, `fetch_indexes`, `fetch_foreign_keys`, `fetch_inbound_foreign_keys`, `fetch_table_definition`, `row_identity`, `table_estimate` | Catalogue reads. `fetch_table_definition` reads columns, keys and dependents in one call so they come from the same moment. `fetch_inbound_foreign_keys` returns the inbound keys that definition lists, read alone for a table tab's cell menu. Each key names its other table as the sidebar does, and its columns as declared. | | `query_bound` | A generated read that keeps at most `limit` rows and reads one more to set `truncated`. | | `count_rows` | A generated count. | | `stream_read` | Every row of a read into a `ResultSink`, with no row limit, on a connection of its own, held read-only for its whole length. | diff --git a/linux/po/POTFILES.in b/linux/po/POTFILES.in index 8d0eff6045..c49cc8f372 100644 --- a/linux/po/POTFILES.in +++ b/linux/po/POTFILES.in @@ -162,8 +162,10 @@ crates/app/src/ui/browse_tab/save_input.rs crates/app/src/ui/browse_tab/selection_handlers.rs crates/app/src/ui/browse_tab/selection_input.rs crates/app/src/ui/browse_tab/selection_snapshot.rs +crates/app/src/ui/browse_tab/table_filter.rs crates/app/src/ui/browse_tab/tests/load.rs crates/app/src/ui/browse_tab/tests/mod.rs +crates/app/src/ui/browse_tab/tests/references.rs crates/app/src/ui/browse_tab/tests/total.rs crates/app/src/ui/clipboard/clipboard_copy.rs crates/app/src/ui/clipboard/clipboard_format.rs @@ -236,6 +238,7 @@ crates/app/src/ui/error_text/edit.rs crates/app/src/ui/error_text/files.rs crates/app/src/ui/error_text/mod.rs crates/app/src/ui/error_text/operation.rs +crates/app/src/ui/error_text/reference.rs crates/app/src/ui/error_text/saved_connection.rs crates/app/src/ui/error_text/script.rs crates/app/src/ui/error_text/secret.rs @@ -316,6 +319,7 @@ crates/app/src/ui/grid/inline_editor.rs crates/app/src/ui/grid/menu_rows.rs crates/app/src/ui/grid/mod.rs crates/app/src/ui/grid/multiline_editor.rs +crates/app/src/ui/grid/reference_menu.rs crates/app/src/ui/grid/result_grid.rs crates/app/src/ui/grid/result_sorters.rs crates/app/src/ui/grid/row_factory.rs @@ -328,6 +332,7 @@ crates/app/src/ui/grid/row_state_icon.rs crates/app/src/ui/grid/row_watches.rs crates/app/src/ui/grid/selection.rs crates/app/src/ui/grid/tab_grid_context.rs +crates/app/src/ui/grid/table_keys.rs crates/app/src/ui/grid/test_grid.rs crates/app/src/ui/grid/toggle_cell.rs crates/app/src/ui/grid/value_order.rs @@ -349,6 +354,7 @@ crates/app/src/ui/host_key_failure.rs crates/app/src/ui/host_key_review.rs crates/app/src/ui/icons.rs crates/app/src/ui/import_dialog.rs +crates/app/src/ui/menu_label.rs crates/app/src/ui/mod.rs crates/app/src/ui/name_entry.rs crates/app/src/ui/picker/connection_menu.rs @@ -458,6 +464,7 @@ crates/app/src/ui/workspace/imp.rs crates/app/src/ui/workspace/import.rs crates/app/src/ui/workspace/import_ledger.rs crates/app/src/ui/workspace/mod.rs +crates/app/src/ui/workspace/references.rs crates/app/src/ui/workspace/row_ops.rs crates/app/src/ui/workspace/saves.rs crates/app/src/ui/workspace/session_watches.rs @@ -490,6 +497,7 @@ crates/app/src/ui/workspace/table_naming.rs crates/app/src/ui/workspace/tests/close_tab.rs crates/app/src/ui/workspace/tests/large_catalog.rs crates/app/src/ui/workspace/tests/mod.rs +crates/app/src/ui/workspace/tests/references.rs crates/app/src/ui/workspace/tests/tab_title.rs crates/app/src/ui/workspace/transaction_banner.rs crates/app/src/ui/workspace/unsaved_icon.rs From be187394fe97fed9772ffe4e760a51472d99b8e1 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 00:44:56 +0700 Subject: [PATCH 2/9] docs(linux): changelog for opening rows from a foreign key --- linux/changelog.d/3797.added.1.md | 1 + linux/changelog.d/3797.added.md | 1 + linux/changelog.d/3797.fixed.1.md | 1 + linux/changelog.d/3797.fixed.2.md | 1 + linux/changelog.d/3797.fixed.md | 1 + 5 files changed, 5 insertions(+) create mode 100644 linux/changelog.d/3797.added.1.md create mode 100644 linux/changelog.d/3797.added.md create mode 100644 linux/changelog.d/3797.fixed.1.md create mode 100644 linux/changelog.d/3797.fixed.2.md create mode 100644 linux/changelog.d/3797.fixed.md diff --git a/linux/changelog.d/3797.added.1.md b/linux/changelog.d/3797.added.1.md new file mode 100644 index 0000000000..38fe31318c --- /dev/null +++ b/linux/changelog.d/3797.added.1.md @@ -0,0 +1 @@ +Filters compare a binary column to a 0x hex value. diff --git a/linux/changelog.d/3797.added.md b/linux/changelog.d/3797.added.md new file mode 100644 index 0000000000..37b018bd38 --- /dev/null +++ b/linux/changelog.d/3797.added.md @@ -0,0 +1 @@ +Open Referenced Row (Ctrl+J) and Show Referencing Rows in the cell menu open the rows a foreign key leads to. diff --git a/linux/changelog.d/3797.fixed.1.md b/linux/changelog.d/3797.fixed.1.md new file mode 100644 index 0000000000..bc7144a0d1 --- /dev/null +++ b/linux/changelog.d/3797.fixed.1.md @@ -0,0 +1 @@ +On SQLite, a foreign key naming no columns of a missing table kept the table's structure tab from opening. diff --git a/linux/changelog.d/3797.fixed.2.md b/linux/changelog.d/3797.fixed.2.md new file mode 100644 index 0000000000..cdfb926328 --- /dev/null +++ b/linux/changelog.d/3797.fixed.2.md @@ -0,0 +1 @@ +Two tabs on one table came back with the same filter after a restart or Reopen Closed Tab. diff --git a/linux/changelog.d/3797.fixed.md b/linux/changelog.d/3797.fixed.md new file mode 100644 index 0000000000..9adf8551b9 --- /dev/null +++ b/linux/changelog.d/3797.fixed.md @@ -0,0 +1 @@ +On PostgreSQL, the structure tab listed each partition's copy of a foreign key as a key that could not be dropped. From 21187ccaf04c76d410a1366064de8872b00e1f20 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 01:01:46 +0700 Subject: [PATCH 3/9] perf(linux-grid): copy only key cells when a cell menu offers foreign keys --- .../crates/app/src/ui/grid/reference_menu.rs | 71 ++++++++++++++----- 1 file changed, 54 insertions(+), 17 deletions(-) diff --git a/linux/crates/app/src/ui/grid/reference_menu.rs b/linux/crates/app/src/ui/grid/reference_menu.rs index 71ffd75c50..014ece5e72 100644 --- a/linux/crates/app/src/ui/grid/reference_menu.rs +++ b/linux/crates/app/src/ui/grid/reference_menu.rs @@ -4,8 +4,10 @@ use std::sync::Arc; use gtk4::prelude::*; use gtk4::{gio, glib}; +use tablepro_core::ForeignKeyInfo; use tablepro_core::browse::{ReferenceError, RowReference}; use tablepro_core::column::ResultColumn; +use tablepro_core::ddl::InboundForeignKey; use tablepro_core::meta::TableRef; use super::{GridCell, GridMsg, GridSink, KeysHandle, RowObject, TabGridContext, TableKeys}; @@ -67,27 +69,21 @@ impl ReferenceMenu { let Some(column) = self.columns.get(cell.column()) else { return Offers::default(); }; - let shown = self.tab.snapshot(&row).to_values(); let open = keys .outbound .iter() .enumerate() .filter(|(_, key)| key.columns.contains(&column.name)) - .filter(|(_, key)| leads_somewhere(RowReference::referenced(&keys.table, key, &self.columns, &shown))) + .filter(|(_, key)| leads_somewhere(self.referenced_by(&keys.table, key, &row))) + .map(|(index, _)| index) + .collect(); + let referencing = keys + .inbound + .iter() + .enumerate() + .filter(|(_, key)| self.referencing_by(&keys.table, key, &row).is_some_and(leads_somewhere)) .map(|(index, _)| index) .collect(); - // Other rows point at the row as it was saved; a draft is not - // saved, so nothing points at it yet. - let referencing = match row.cells() { - Some(saved) => keys - .inbound - .iter() - .enumerate() - .filter(|(_, key)| leads_somewhere(RowReference::referencing(&keys.table, key, &self.columns, &saved))) - .map(|(index, _)| index) - .collect(), - None => Vec::new(), - }; Offers { keys: Some(keys), open, @@ -145,8 +141,7 @@ impl ReferenceMenu { let armed = self.armed.borrow(); let keys = armed.keys.as_ref()?; let key = keys.outbound.get(index)?; - let shown = self.tab.snapshot(row).to_values(); - Some(RowReference::referenced(&keys.table, key, &self.columns, &shown)) + Some(self.referenced_by(&keys.table, key, row)) } /// The rows the inbound key at `index` holds that point at `row`. @@ -154,8 +149,50 @@ impl ReferenceMenu { let armed = self.armed.borrow(); let keys = armed.keys.as_ref()?; let key = keys.inbound.get(index)?; + self.referencing_by(&keys.table, key, row) + } + + /// The row `key` points at from `row` as the grid shows it, pending + /// edits included. + fn referenced_by( + &self, + source: &TableRef, + key: &ForeignKeyInfo, + row: &RowObject, + ) -> Result { + let (columns, values) = self.key_cells(&key.columns, |index| { + Some(self.tab.with_effective_value(row, index, |value, _| value.clone())) + }); + RowReference::referenced(source, key, &columns, &values) + } + + /// The rows of `key` that point at `row` as it was saved, since that + /// is what they reference. A draft is not saved, so `None`. + fn referencing_by( + &self, + source: &TableRef, + key: &InboundForeignKey, + row: &RowObject, + ) -> Option> { let saved = row.cells()?; - Some(RowReference::referencing(&keys.table, key, &self.columns, &saved)) + let (columns, values) = self.key_cells(&key.referenced_columns, |index| saved.get(index).cloned()); + Some(RowReference::referencing(source, key, &columns, &values)) + } + + /// The row's columns named `names` with their values, read through + /// `value`, so a row of large cells copies only its key's. + fn key_cells( + &self, + names: &[String], + value: impl Fn(usize) -> Option, + ) -> (Vec, Vec) { + names + .iter() + .filter_map(|name| { + let index = self.columns.iter().position(|column| column.name == *name)?; + Some((self.columns.get(index)?.clone(), value(index)?)) + }) + .unzip() } /// The one outbound key the armed menu offers, for its plain item. From 550ed947709cec824513ffdf49cdb95e8b0ea0af Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 01:10:26 +0700 Subject: [PATCH 4/9] fix(linux): follow a decimal key into an integer one --- linux/crates/core/src/browse/row_reference.rs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/linux/crates/core/src/browse/row_reference.rs b/linux/crates/core/src/browse/row_reference.rs index 42e9b5078b..b53234ada5 100644 --- a/linux/crates/core/src/browse/row_reference.rs +++ b/linux/crates/core/src/browse/row_reference.rs @@ -104,6 +104,10 @@ fn filter_text(value: &Value, column_type: &ColumnType) -> Option { Some(format!("0x{}", crate::hex::encode_lower(bytes))) } Value::Bytes(_) => None, + // A key can point at a key of another numeric type, such as an + // integer at a numeric(10,2), and the other column parses the + // text: the zeros the scale pads with would make it refuse 1.00. + Value::Decimal(decimal) if decimal.is_finite() => Some(without_trailing_zeros(decimal.to_string())), other => match edit_text(other, column_type) { EditText::Editable(text) => Some(text), EditText::ReadOnly(_) => None, @@ -111,6 +115,13 @@ fn filter_text(value: &Value, column_type: &ColumnType) -> Option { } } +fn without_trailing_zeros(text: String) -> String { + match text.contains('.') { + true => text.trim_end_matches('0').trim_end_matches('.').to_owned(), + false => text, + } +} + fn target(source: &TableRef, schema: Option<&str>, name: &str) -> Result { if source.schema.is_some() && schema.is_none() { return Err(ReferenceError::UnknownSchema { table: name.to_owned() }); @@ -239,6 +250,30 @@ mod tests { ); } + #[test] + fn a_decimal_key_is_written_without_the_zeros_of_its_scale() { + let columns = [typed( + "amount", + ColumnKind::Decimal { + precision: Some(10), + scale: Some(2), + }, + )]; + let key = key(&["amount"], Some("sales"), &["id"]); + let text_of = |mantissa| { + let value = Value::Decimal(crate::value::SqlDecimal::from_scaled_i128(mantissa, 2)); + RowReference::referenced(&orders(), &key, &columns, &[value]) + .expect("a decimal key") + .filter + .rules + }; + + assert_eq!(text_of(100), vec![eq("id", "1")]); + assert_eq!(text_of(150), vec![eq("id", "1.5")]); + assert_eq!(text_of(-1000), vec![eq("id", "-10")]); + assert_eq!(text_of(0), vec![eq("id", "0")]); + } + #[test] fn a_key_the_catalogue_could_not_pair_up_is_refused_and_shown() { let columns = [text_column("owner")]; From b2685e8317c26cdf5d9f82e2551c97058b5f94e7 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 01:18:40 +0700 Subject: [PATCH 5/9] fix(linux): compare SQL Server text, ntext and image columns, and follow real keys into double ones --- linux/changelog.d/3797.fixed.3.md | 1 + linux/crates/core/src/browse/row_reference.rs | 17 +++++++ linux/crates/core/src/dialect/sql_dialect.rs | 7 +++ linux/crates/core/src/filter/build_filter.rs | 7 +-- linux/crates/drivers/mssql/src/dialect.rs | 10 ++++ .../drivers/mssql/tests/integration/filter.rs | 50 +++++++++++-------- 6 files changed, 69 insertions(+), 23 deletions(-) create mode 100644 linux/changelog.d/3797.fixed.3.md diff --git a/linux/changelog.d/3797.fixed.3.md b/linux/changelog.d/3797.fixed.3.md new file mode 100644 index 0000000000..50de81ad93 --- /dev/null +++ b/linux/changelog.d/3797.fixed.3.md @@ -0,0 +1 @@ +On SQL Server, an equals, not equals or in filter on a text or ntext column failed. diff --git a/linux/crates/core/src/browse/row_reference.rs b/linux/crates/core/src/browse/row_reference.rs index b53234ada5..d1aeee169e 100644 --- a/linux/crates/core/src/browse/row_reference.rs +++ b/linux/crates/core/src/browse/row_reference.rs @@ -108,6 +108,10 @@ fn filter_text(value: &Value, column_type: &ColumnType) -> Option { // integer at a numeric(10,2), and the other column parses the // text: the zeros the scale pads with would make it refuse 1.00. Value::Decimal(decimal) if decimal.is_finite() => Some(without_trailing_zeros(decimal.to_string())), + // A real key can point at a double one, which holds the real's + // exact value; the real's shortest text would parse as another + // double. A real column reads the long text back as the same real. + Value::Float32(value) => Some(f64::from(*value).to_string()), other => match edit_text(other, column_type) { EditText::Editable(text) => Some(text), EditText::ReadOnly(_) => None, @@ -274,6 +278,19 @@ mod tests { assert_eq!(text_of(0), vec![eq("id", "0")]); } + #[test] + fn a_single_precision_key_is_written_as_the_double_it_widens_to() { + let columns = [typed("weight", ColumnKind::Float(crate::column::FloatWidth::F32))]; + let key = key(&["weight"], Some("sales"), &["weight"]); + + let reference = + RowReference::referenced(&orders(), &key, &columns, &[Value::Float32(0.1)]).expect("a real key"); + + assert_eq!(reference.filter.rules, vec![eq("weight", "0.10000000149011612")]); + let parsed: f32 = "0.10000000149011612".parse().expect("a real"); + assert_eq!(parsed, 0.1_f32, "a real column reads the text back as another value"); + } + #[test] fn a_key_the_catalogue_could_not_pair_up_is_refused_and_shown() { let columns = [text_column("owner")]; diff --git a/linux/crates/core/src/dialect/sql_dialect.rs b/linux/crates/core/src/dialect/sql_dialect.rs index 61d0d6e4b2..a3b137b49f 100644 --- a/linux/crates/core/src/dialect/sql_dialect.rs +++ b/linux/crates/core/src/dialect/sql_dialect.rs @@ -125,6 +125,13 @@ pub trait SqlDialect: Send + Sync + std::fmt::Debug { false } + /// The column where it meets a bound value of its own type, for a + /// type the engine compares only once it is cast to another. `None` + /// when the column compares as it is. + fn compared_operand(&self, _column: &ColumnInfo) -> Option { + None + } + /// `!` rather than a backslash: a backslash means something else /// inside a string literal under some PostgreSQL and MySQL session /// modes, and is no escape at all on SQLite and SQL Server. diff --git a/linux/crates/core/src/filter/build_filter.rs b/linux/crates/core/src/filter/build_filter.rs index 5ee11e3f7e..fd94d59ff1 100644 --- a/linux/crates/core/src/filter/build_filter.rs +++ b/linux/crates/core/src/filter/build_filter.rs @@ -64,6 +64,7 @@ fn rule_sql( ) -> Result { let column_sql = dialect.quote_identifier(&column.name); let text_sql = dialect.text_operand(column).unwrap_or_else(|| column_sql.clone()); + let compared_sql = dialect.compared_operand(column).unwrap_or_else(|| column_sql.clone()); let equality_as_text = dialect.equality_as_text(column); match rule.op { // A null test has no value to bind, so it never takes a marker. @@ -75,7 +76,7 @@ fn rule_sql( } FilterOp::Eq | FilterOp::NotEq | FilterOp::Lt | FilterOp::LtEq | FilterOp::Gt | FilterOp::GtEq => { let marker = bind_typed(builder, dialect, column, require_single(rule)?)?; - Ok(format!("{column_sql} {} {marker}", comparison_operator(rule.op))) + Ok(format!("{compared_sql} {} {marker}", comparison_operator(rule.op))) } FilterOp::Contains | FilterOp::StartsWith | FilterOp::EndsWith => { // The app is building the pattern, so what the user typed @@ -119,7 +120,7 @@ fn rule_sql( }; let operand = match equality_as_text { true => &text_sql, - false => &column_sql, + false => &compared_sql, }; Ok(format!("{operand} {keyword} ({})", markers.join(", "))) } @@ -127,7 +128,7 @@ fn rule_sql( let (low, high) = require_pair(rule)?; let low = bind_typed(builder, dialect, column, low)?; let high = bind_typed(builder, dialect, column, high)?; - Ok(format!("{column_sql} BETWEEN {low} AND {high}")) + Ok(format!("{compared_sql} BETWEEN {low} AND {high}")) } } } diff --git a/linux/crates/drivers/mssql/src/dialect.rs b/linux/crates/drivers/mssql/src/dialect.rs index e6bcee9ec5..43b57d12cc 100644 --- a/linux/crates/drivers/mssql/src/dialect.rs +++ b/linux/crates/drivers/mssql/src/dialect.rs @@ -253,6 +253,16 @@ impl SqlDialect for MssqlDialect { matches!(column.column_type.kind, ColumnKind::Xml) } + /// text, ntext and image take no `=`, `<` or IN until cast to the + /// max type that replaced each, which the column's cast target names. + fn compared_operand(&self, column: &ColumnInfo) -> Option { + let legacy = ["text", "ntext", "image"] + .iter() + .any(|name| column.column_type.name.eq_ignore_ascii_case(name)); + let target = column.column_type.catalog.as_ref()?.cast_target.as_sql(); + legacy.then(|| format!("CAST({} AS {target})", self.quote_identifier(&column.name))) + } + /// COUNT(*) is an int on SQL Server and overflows past two billion /// rows. fn count_all(&self) -> &'static str { diff --git a/linux/crates/drivers/mssql/tests/integration/filter.rs b/linux/crates/drivers/mssql/tests/integration/filter.rs index e678f8dad1..a484b4af92 100644 --- a/linux/crates/drivers/mssql/tests/integration/filter.rs +++ b/linux/crates/drivers/mssql/tests/integration/filter.rs @@ -96,39 +96,49 @@ async fn a_decimal_filter_keeps_the_digits_it_was_given() { } /// A binary key followed from another table arrives as its hex text. +/// text, ntext and image have no `=` of their own. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[ignore = "requires docker"] -async fn equals_on_varbinary_takes_the_hex_form() { +async fn equals_on_binary_and_legacy_large_types() { let fixture = start().await; let connection = connect(&fixture, false).await; setup( connection.as_ref(), &[ - "CREATE TABLE dbo.keys (id int PRIMARY KEY, k varbinary(16))", - "INSERT INTO dbo.keys VALUES (1, 0x0AFF), (2, 0x0A)", + "CREATE TABLE dbo.keys (id int PRIMARY KEY, k varbinary(16), i image, t text, n ntext)", + "INSERT INTO dbo.keys VALUES (1, 0x0AFF, 0x0AFF, 'red', N'red'), (2, 0x0A, 0x0A, 'blue', N'blue')", ], ) .await; let table = dbo("keys"); let columns = columns(connection.as_ref(), &table).await; let dialect = connection.dialect(); - let mut builder = StatementBuilder::new(); - builder.push_sql(&format!("SELECT id FROM {} WHERE ", dialect.qualify(&table))); - let filter = FilterSet { - rules: vec![FilterRule { - column: "k".to_owned(), - op: FilterOp::Eq, - value: Some(FilterValue::Single("0x0aff".to_owned())), - }], - ..FilterSet::default() - }; - build_filter(&mut builder, dialect.as_ref(), &columns, &filter).expect("a filter"); - let rows = connection - .query_bound(&builder.finish(), RowLimit::EDITOR_DEFAULT, within(CALL)) - .await - .expect("the filtered rows") - .rows; + for (column, op, text) in [ + ("k", FilterOp::Eq, "0x0aff"), + ("i", FilterOp::Eq, "0x0aff"), + ("t", FilterOp::Eq, "red"), + ("n", FilterOp::Eq, "red"), + ("n", FilterOp::NotEq, "blue"), + ] { + let mut builder = StatementBuilder::new(); + builder.push_sql(&format!("SELECT id FROM {} WHERE ", dialect.qualify(&table))); + let filter = FilterSet { + rules: vec![FilterRule { + column: column.to_owned(), + op, + value: Some(FilterValue::Single(text.to_owned())), + }], + ..FilterSet::default() + }; + build_filter(&mut builder, dialect.as_ref(), &columns, &filter).expect("a filter"); - assert_eq!(rows, vec![vec![Value::Int(1)]]); + let rows = connection + .query_bound(&builder.finish(), RowLimit::EDITOR_DEFAULT, within(CALL)) + .await + .unwrap_or_else(|error| panic!("{column} {op:?}: {error}")) + .rows; + + assert_eq!(rows, vec![vec![Value::Int(1)]], "{column} {op:?}"); + } } From d1fd0a2ebdd7851eae2b851c0c3b7c723d43656d Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 02:14:19 +0700 Subject: [PATCH 6/9] fix(linux): follow a date key into a timestamp one, and name each key by the rows it opens --- .../crates/app/src/ui/grid/reference_menu.rs | 15 ++--- .../core/src/edit/parse_literal_text.rs | 64 ++++++++++++++++++- 2 files changed, 68 insertions(+), 11 deletions(-) diff --git a/linux/crates/app/src/ui/grid/reference_menu.rs b/linux/crates/app/src/ui/grid/reference_menu.rs index 014ece5e72..bc023d14b1 100644 --- a/linux/crates/app/src/ui/grid/reference_menu.rs +++ b/linux/crates/app/src/ui/grid/reference_menu.rs @@ -101,7 +101,7 @@ impl ReferenceMenu { if let Some(key) = keys.outbound.get(index) { let target = TableRef::new(key.ref_schema.clone(), key.ref_table.clone()); self.choices.append_item(&choice( - &key_label(&keys.table, &target, &key.columns), + &key_label(&keys.table, &target, &key.ref_columns), OPEN_CHOICE, index, )); @@ -228,8 +228,9 @@ fn choice(label: &str, action: &str, index: usize) -> gio::MenuItem { item } -/// The other table, named with its schema where that is not the -/// source's, and the key's columns on the side that holds the key. +/// The rows an item opens: the other table, named with its schema where +/// that is not the source's, and the columns its filter is on. Two keys +/// from one column into one table differ there. fn key_label(source: &TableRef, table: &TableRef, columns: &[String]) -> String { let name = match (&table.schema, table.schema == source.schema) { (Some(schema), false) => format!("{schema}.{}", table.name), @@ -413,12 +414,8 @@ pub(super) mod tests { offered, [ item("Open Referenced Row", None, None), - item("tenants (tenant__id)", Some("cell.open-reference"), Some(0)), - item( - "crm.customers (tenant__id, customer__id)", - Some("cell.open-reference"), - Some(1) - ), + item("tenants (id)", Some("cell.open-reference"), Some(0)), + item("crm.customers (tenant__id, id)", Some("cell.open-reference"), Some(1)), ] ); assert_eq!(followed(&grid), [(crm, filter(&[("tenant_id", "7"), ("id", "42")]))]); diff --git a/linux/crates/core/src/edit/parse_literal_text.rs b/linux/crates/core/src/edit/parse_literal_text.rs index 3e3ba424ea..a639db939f 100644 --- a/linux/crates/core/src/edit/parse_literal_text.rs +++ b/linux/crates/core/src/edit/parse_literal_text.rs @@ -121,7 +121,8 @@ fn strict_value(input: &str, column: &ColumnType, purpose: Purpose) -> Result parse_date(trimmed) - .or_else(|error| far_temporal(trimmed, column, purpose, parse_date).unwrap_or(Err(error))), + .or_else(|error| far_temporal(trimmed, column, purpose, parse_date).unwrap_or(Err(error))) + .or_else(|error| compared_across(purpose, error, || midnight_date(trimmed))), ColumnKind::Time { with_offset: true, .. } => parse_time_with_offset(trimmed), ColumnKind::Time { range: range @ (TimeRange::Duration | TimeRange::LongDuration), @@ -134,7 +135,8 @@ fn strict_value(input: &str, column: &ColumnType, purpose: Purpose) -> Result parse_timestamp_with_offset(trimmed) .or_else(|error| far_temporal(trimmed, column, purpose, parse_timestamp_with_offset).unwrap_or(Err(error))), ColumnKind::Timestamp { .. } => parse_timestamp(trimmed) - .or_else(|error| far_temporal(trimmed, column, purpose, parse_timestamp).unwrap_or(Err(error))), + .or_else(|error| far_temporal(trimmed, column, purpose, parse_timestamp).unwrap_or(Err(error))) + .or_else(|error| compared_across(purpose, error, || date_at_midnight(trimmed))), ColumnKind::Interval => trimmed .parse::() .map(Value::Interval) @@ -294,6 +296,37 @@ fn parse_date(input: &str) -> Result { .map_err(|_| EditParseError::InvalidDate) } +/// A comparison takes a value of the neighbouring type as the server +/// casts it: a date is its midnight, and a midnight is its date. A key +/// can point from a date at a timestamp, so the value it carries is the +/// other type's. A stored value has to be the column's own. +fn compared_across( + purpose: Purpose, + error: EditParseError, + other: impl FnOnce() -> Option, +) -> Result { + match purpose { + Purpose::Compare => other().ok_or(error), + Purpose::Store => Err(error), + } +} + +fn midnight_date(input: &str) -> Option { + match parse_timestamp(input).ok()? { + Value::Timestamp(Temporal::Finite(stamp)) if stamp.time() == NaiveTime::MIN => { + Some(Value::Date(Temporal::Finite(stamp.date()))) + } + _ => None, + } +} + +fn date_at_midnight(input: &str) -> Option { + match parse_date(input).ok()? { + Value::Date(Temporal::Finite(date)) => Some(Value::Timestamp(Temporal::Finite(date.and_time(NaiveTime::MIN)))), + _ => None, + } +} + fn parse_time(input: &str) -> Result { if let Ok(time) = NaiveTime::parse_from_str(input, "%H:%M:%S%.f") { return Ok(Value::Time(SqlTime::from_time_of_day(time))); @@ -818,6 +851,33 @@ mod tests { } } + /// A key from a date column can point at a timestamp one, and back. + #[test] + fn a_comparison_reads_a_date_as_its_midnight_and_a_midnight_as_its_date() { + let date = column(ColumnKind::Date, false); + let stamp = column(stamp(TemporalZone::Naive), false); + let day = NaiveDate::from_ymd_opt(2026, 1, 1).expect("a date"); + + assert_eq!( + parse_filter_text("2026-01-01", &stamp), + Ok(Value::Timestamp(Temporal::Finite(day.and_time(NaiveTime::MIN)))) + ); + assert_eq!( + parse_filter_text("2026-01-01 00:00:00", &date), + Ok(Value::Date(Temporal::Finite(day))) + ); + assert_eq!( + parse_filter_text("2026-01-01 08:00:00", &date), + Err(EditParseError::InvalidDate), + "a time of day is no date" + ); + assert_eq!( + parse_literal_text("2026-01-01", &stamp), + Err(EditParseError::InvalidTimestamp), + "a stored timestamp took a bare date" + ); + } + #[test] fn a_filter_on_a_binary_column_reads_the_hex_form_an_export_writes() { let binary = column(bytes(), false); From e82bc8710a1b879be42a23c25fa88b016e4151cc Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 02:30:30 +0700 Subject: [PATCH 7/9] fix(linux): refuse SQLite referencing rows a key compares otherwise than its columns --- linux/changelog.d/3797.added.2.md | 1 + .../app/src/ui/browse_tab/tests/references.rs | 1 + .../crates/app/src/ui/error_text/reference.rs | 8 +++++ .../crates/app/src/ui/grid/reference_menu.rs | 1 + .../crates/app/src/ui/structure_tab/tests.rs | 1 + .../crates/app/src/ui/workspace/structure.rs | 1 + .../crates/core/src/browse/reference_error.rs | 4 +++ linux/crates/core/src/browse/row_reference.rs | 29 +++++++++++++++ .../core/src/ddl/inbound_foreign_key.rs | 5 +++ linux/crates/core/src/ddl/tests.rs | 1 + linux/crates/drivers/clickhouse/src/ddl.rs | 1 + .../mssql/src/catalog/table_definition.rs | 1 + linux/crates/drivers/mssql/src/ddl.rs | 1 + .../mysql/src/catalog/table_definition.rs | 1 + linux/crates/drivers/mysql/src/ddl/mod.rs | 1 + .../postgres/src/catalog/table_definition.rs | 1 + linux/crates/drivers/postgres/src/ddl.rs | 1 + .../crates/drivers/sqlite/src/catalog/mod.rs | 33 +++++++++++++++++ .../sqlite/src/catalog/table_definition.rs | 1 + .../sqlite/src/ddl/create_table_source.rs | 21 +++++++++++ linux/crates/drivers/sqlite/src/ddl/mod.rs | 2 +- linux/crates/drivers/sqlite/src/ddl/tests.rs | 1 + linux/crates/drivers/sqlite/src/metadata.rs | 32 +++++++++++++++-- .../sqlite/tests/integration/catalog.rs | 35 +++++++++++++++++++ 24 files changed, 181 insertions(+), 3 deletions(-) create mode 100644 linux/changelog.d/3797.added.2.md diff --git a/linux/changelog.d/3797.added.2.md b/linux/changelog.d/3797.added.2.md new file mode 100644 index 0000000000..90afda9205 --- /dev/null +++ b/linux/changelog.d/3797.added.2.md @@ -0,0 +1 @@ +A filter on a timestamp column takes a date as its midnight, and a date column takes a midnight. diff --git a/linux/crates/app/src/ui/browse_tab/tests/references.rs b/linux/crates/app/src/ui/browse_tab/tests/references.rs index b8f7b80983..70fa1ebcf2 100644 --- a/linux/crates/app/src/ui/browse_tab/tests/references.rs +++ b/linux/crates/app/src/ui/browse_tab/tests/references.rs @@ -30,6 +30,7 @@ fn name_key() -> ForeignKeyInfo { fn orders_key() -> InboundForeignKey { InboundForeignKey { + child_compares_alike: true, name: "orders_user_id_fkey".to_owned(), table: TableRef::unqualified("orders"), columns: vec!["user_id".to_owned()], diff --git a/linux/crates/app/src/ui/error_text/reference.rs b/linux/crates/app/src/ui/error_text/reference.rs index 6857cbbff9..48ad4db85f 100644 --- a/linux/crates/app/src/ui/error_text/reference.rs +++ b/linux/crates/app/src/ui/error_text/reference.rs @@ -15,6 +15,10 @@ pub fn reference_message(error: &ReferenceError) -> String { ReferenceError::NoFilterText { column } => { gettext_f("Can't filter {column} on this value", &[("column", column)]) } + ReferenceError::ComparedOtherwise { table } => gettext_f( + "Can't filter {table} the way its key compares values", + &[("table", table)], + ), } } @@ -40,6 +44,9 @@ mod tests { ReferenceError::NoFilterText { column: "outline".to_owned(), }, + ReferenceError::ComparedOtherwise { + table: "orders".to_owned(), + }, ]; let messages: Vec = errors.iter().map(reference_message).collect(); @@ -50,5 +57,6 @@ mod tests { assert!(messages[2].contains("fk_notes_0"), "{messages:?}"); assert!(messages[3].contains("customers"), "{messages:?}"); assert!(messages[4].contains("outline"), "{messages:?}"); + assert!(messages[5].contains("orders"), "{messages:?}"); } } diff --git a/linux/crates/app/src/ui/grid/reference_menu.rs b/linux/crates/app/src/ui/grid/reference_menu.rs index bc023d14b1..934e87455f 100644 --- a/linux/crates/app/src/ui/grid/reference_menu.rs +++ b/linux/crates/app/src/ui/grid/reference_menu.rs @@ -455,6 +455,7 @@ pub(super) mod tests { table: public("customers"), outbound: Vec::new(), inbound: vec![InboundForeignKey { + child_compares_alike: true, name: "orders_customer_id_fkey".to_owned(), table: orders.clone(), columns: vec!["customer_id".to_owned()], diff --git a/linux/crates/app/src/ui/structure_tab/tests.rs b/linux/crates/app/src/ui/structure_tab/tests.rs index 40ef444ee8..870ee62067 100644 --- a/linux/crates/app/src/ui/structure_tab/tests.rs +++ b/linux/crates/app/src/ui/structure_tab/tests.rs @@ -646,6 +646,7 @@ fn referenced_primary_key_switch_is_insensitive_with_foreign_key_names() { indexes: Vec::new(), foreign_keys: Vec::new(), inbound_foreign_keys: vec![InboundForeignKey { + child_compares_alike: true, name: "orders_user_fk".to_owned(), table: TableRef::unqualified("orders"), columns: vec!["user_id".to_owned()], diff --git a/linux/crates/app/src/ui/workspace/structure.rs b/linux/crates/app/src/ui/workspace/structure.rs index 942b2abb13..96157ccd26 100644 --- a/linux/crates/app/src/ui/workspace/structure.rs +++ b/linux/crates/app/src/ui/workspace/structure.rs @@ -651,6 +651,7 @@ mod tests { let mut users = definition(vec![text_column("id")]); let key = |name: &str, table: TableRef, column: &str| InboundForeignKey { + child_compares_alike: true, name: name.to_owned(), table, columns: vec![column.to_owned()], diff --git a/linux/crates/core/src/browse/reference_error.rs b/linux/crates/core/src/browse/reference_error.rs index 36047b4f59..38756b929b 100644 --- a/linux/crates/core/src/browse/reference_error.rs +++ b/linux/crates/core/src/browse/reference_error.rs @@ -19,6 +19,10 @@ pub enum ReferenceError { UnknownSchema { table: String }, #[error("the value of {column} has no text a filter can compare")] NoFilterText { column: String }, + /// The engine compares the key under rules the referencing columns + /// do not share, so a filter on them would miss rows or add some. + #[error("{table} compares its key otherwise than its columns")] + ComparedOtherwise { table: String }, } impl ReferenceError { diff --git a/linux/crates/core/src/browse/row_reference.rs b/linux/crates/core/src/browse/row_reference.rs index d1aeee169e..a2dc6e489f 100644 --- a/linux/crates/core/src/browse/row_reference.rs +++ b/linux/crates/core/src/browse/row_reference.rs @@ -42,6 +42,11 @@ impl RowReference { let cells = key_cells(&key.referenced_columns, columns, values)?; let filter = key_filter(&key.name, cells, &key.columns)?; let table = target(source, key.table.schema.as_deref(), &key.table.name)?; + if !key.child_compares_alike { + return Err(ReferenceError::ComparedOtherwise { + table: key.table.name.clone(), + }); + } Ok(Self { table, filter }) } } @@ -350,12 +355,36 @@ mod tests { assert!(error.leaves_nothing_to_follow(), "{error:?}"); } + #[test] + fn rows_a_key_compares_otherwise_than_their_columns_are_refused() { + let customers = TableRef::new(None, "customers".to_owned()); + let inbound = InboundForeignKey { + child_compares_alike: false, + name: "orders_customer_fkey".to_owned(), + table: TableRef::new(None, "orders".to_owned()), + columns: vec!["customer_code".to_owned()], + referenced_columns: vec!["code".to_owned()], + }; + + let error = RowReference::referencing(&customers, &inbound, &[text_column("code")], &[Value::Int(1)]) + .expect_err("a NOCASE parent and a BINARY child"); + + assert_eq!( + error, + ReferenceError::ComparedOtherwise { + table: "orders".to_owned() + } + ); + assert!(!error.leaves_nothing_to_follow()); + } + #[test] fn referencing_rows_filter_the_other_table_on_its_own_columns() { let customers = TableRef::new(Some("crm".to_owned()), "customers".to_owned()); let columns = [text_column("id"), text_column("region")]; let values = [Value::Int(42), Value::Text("eu".to_owned())]; let inbound = InboundForeignKey { + child_compares_alike: true, name: "orders_customer_fkey".to_owned(), table: orders(), columns: vec!["customer_id".to_owned(), "customer_region".to_owned()], diff --git a/linux/crates/core/src/ddl/inbound_foreign_key.rs b/linux/crates/core/src/ddl/inbound_foreign_key.rs index 52c1851516..373eb2610c 100644 --- a/linux/crates/core/src/ddl/inbound_foreign_key.rs +++ b/linux/crates/core/src/ddl/inbound_foreign_key.rs @@ -13,4 +13,9 @@ pub struct InboundForeignKey { pub columns: Vec, /// The columns of this table they point at, in the same order. pub referenced_columns: Vec, + /// Whether `=` on the referencing columns finds the rows the key + /// matches. SQLite compares a key under the referenced columns' + /// affinity and collation, which a referencing column can declare + /// otherwise; the other engines make the two sides agree. + pub child_compares_alike: bool, } diff --git a/linux/crates/core/src/ddl/tests.rs b/linux/crates/core/src/ddl/tests.rs index 2f9c487c6a..de12977b0a 100644 --- a/linux/crates/core/src/ddl/tests.rs +++ b/linux/crates/core/src/ddl/tests.rs @@ -399,6 +399,7 @@ fn a_nullable_key_read_back_as_it_is_saves() { fn inbound(name: &str, referenced: &str) -> InboundForeignKey { InboundForeignKey { + child_compares_alike: true, name: name.to_owned(), table: TableRef::new(Some("app".to_owned()), "orders"), columns: vec!["user_id".to_owned()], diff --git a/linux/crates/drivers/clickhouse/src/ddl.rs b/linux/crates/drivers/clickhouse/src/ddl.rs index 9e1ef7c5a4..b1bd4881c9 100644 --- a/linux/crates/drivers/clickhouse/src/ddl.rs +++ b/linux/crates/drivers/clickhouse/src/ddl.rs @@ -333,6 +333,7 @@ mod tests { /// a definition built elsewhere; the rule still holds. fn referenced(table: &mut TableDefinition, column: &str) { table.inbound_foreign_keys = vec![tablepro_core::ddl::InboundForeignKey { + child_compares_alike: true, name: "visits_event_fk".to_owned(), table: TableRef::new(Some("analytics".to_owned()), "visits"), columns: vec!["event_id".to_owned()], diff --git a/linux/crates/drivers/mssql/src/catalog/table_definition.rs b/linux/crates/drivers/mssql/src/catalog/table_definition.rs index 69b495d00c..fa09ef2eba 100644 --- a/linux/crates/drivers/mssql/src/catalog/table_definition.rs +++ b/linux/crates/drivers/mssql/src/catalog/table_definition.rs @@ -56,6 +56,7 @@ pub(crate) fn inbound_foreign_keys(rows: &[Vec]) -> Result keys.push(InboundForeignKey { + child_compares_alike: true, name, table, columns: vec![column], diff --git a/linux/crates/drivers/mssql/src/ddl.rs b/linux/crates/drivers/mssql/src/ddl.rs index 329147470a..48e5561601 100644 --- a/linux/crates/drivers/mssql/src/ddl.rs +++ b/linux/crates/drivers/mssql/src/ddl.rs @@ -1025,6 +1025,7 @@ mod tests { fn referenced(table: &mut TableDefinition, column: &str) { table.inbound_foreign_keys = vec![InboundForeignKey { + child_compares_alike: true, name: "lines_order_fk".to_owned(), table: TableRef::new(Some("dbo".to_owned()), "lines"), columns: vec!["order_id".to_owned()], diff --git a/linux/crates/drivers/mysql/src/catalog/table_definition.rs b/linux/crates/drivers/mysql/src/catalog/table_definition.rs index b1ce32ddc0..49b973d84a 100644 --- a/linux/crates/drivers/mysql/src/catalog/table_definition.rs +++ b/linux/crates/drivers/mysql/src/catalog/table_definition.rs @@ -160,6 +160,7 @@ pub(crate) fn inbound_foreign_keys(rows: &[CatalogCells]) -> Result keys.push(InboundForeignKey { + child_compares_alike: true, name, table: TableRef::new(Some(schema), table), columns: vec![column], diff --git a/linux/crates/drivers/mysql/src/ddl/mod.rs b/linux/crates/drivers/mysql/src/ddl/mod.rs index b5a415d7d1..fe5d6a5eb3 100644 --- a/linux/crates/drivers/mysql/src/ddl/mod.rs +++ b/linux/crates/drivers/mysql/src/ddl/mod.rs @@ -584,6 +584,7 @@ mod tests { fn referenced(table: &mut TableDefinition, column: &str) { table.inbound_foreign_keys = vec![InboundForeignKey { + child_compares_alike: true, name: "orders_user_fk".to_owned(), table: TableRef::new(Some("app".to_owned()), "orders"), columns: vec!["user_id".to_owned()], diff --git a/linux/crates/drivers/postgres/src/catalog/table_definition.rs b/linux/crates/drivers/postgres/src/catalog/table_definition.rs index f4e203a793..1c902f45e4 100644 --- a/linux/crates/drivers/postgres/src/catalog/table_definition.rs +++ b/linux/crates/drivers/postgres/src/catalog/table_definition.rs @@ -45,6 +45,7 @@ pub(crate) fn inbound_foreign_key(row: &Row) -> Result ColumnType { } } +/// How SQLite compares a column's values: by its affinity, numeric ones +/// alike, and by its collation, named in lower case. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct Comparison { + numeric: bool, + affinity: Affinity, + collation: String, +} + +impl Comparison { + /// `declared` and `collation` as a CREATE TABLE spells them; a + /// column with no COLLATE compares as BINARY. + pub(crate) fn of(declared: &str, collation: Option<&str>) -> Self { + let affinity = affinity(declared); + Self { + numeric: matches!(affinity, Affinity::Integer | Affinity::Real | Affinity::Numeric), + affinity, + collation: collation.unwrap_or("binary").to_ascii_lowercase(), + } + } + + /// Whether `=` on a column compared like `self` finds what a key + /// compared like `key` matches. Numeric affinities compare numbers + /// alike. + pub(crate) fn finds_as(&self, key: &Self) -> bool { + let affinity = match self.numeric { + true => key.numeric, + false => self.affinity == key.affinity, + }; + affinity && self.collation == key.collation + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum Affinity { Integer, diff --git a/linux/crates/drivers/sqlite/src/catalog/table_definition.rs b/linux/crates/drivers/sqlite/src/catalog/table_definition.rs index fa29813d7d..c21ce7d190 100644 --- a/linux/crates/drivers/sqlite/src/catalog/table_definition.rs +++ b/linux/crates/drivers/sqlite/src/catalog/table_definition.rs @@ -48,6 +48,7 @@ pub(crate) fn inbound_foreign_keys( .unwrap_or_else(|| foreign_key_name(&row.child, row.id)); let position = *positions.entry((row.child.clone(), row.id)).or_insert_with(|| { keys.push(InboundForeignKey { + child_compares_alike: true, name, table: TableRef::new(schema.map(str::to_owned), row.child.clone()), columns: Vec::new(), diff --git a/linux/crates/drivers/sqlite/src/ddl/create_table_source.rs b/linux/crates/drivers/sqlite/src/ddl/create_table_source.rs index fc41394b07..d41ceafa86 100644 --- a/linux/crates/drivers/sqlite/src/ddl/create_table_source.rs +++ b/linux/crates/drivers/sqlite/src/ddl/create_table_source.rs @@ -316,6 +316,27 @@ pub(crate) fn declared_foreign_key_names(sql: &str) -> Option Some(names) } +/// How a CREATE TABLE has `column` compare its values. `None` when the +/// text does not read or names no such column. +pub(crate) fn declared_comparison(sql: &str, column: &str) -> Option { + let source = CreateTableSource::read(sql).ok()?; + let (_, declared) = source + .columns() + .find(|(_, declared)| declared.name.eq_ignore_ascii_case(column))?; + let type_text = declared.type_range.clone().map_or("", |range| source.text(range)); + let collation = declared + .segments + .iter() + .filter(|segment| segment.kind == ColumnConstraintKind::Collate) + .filter_map(|segment| { + let text = source.text(segment.range.clone()).trim(); + let name = text.get("COLLATE".len()..).filter(|_| text.len() > "COLLATE".len())?; + Some(unquote(name.trim())) + }) + .next_back(); + Some(crate::catalog::Comparison::of(type_text, collation.as_deref())) +} + /// Whether a CREATE TABLE declares `column` `PRIMARY KEY DESC`. Text /// that does not read declares nothing. pub(crate) fn declared_descending_key(sql: &str, column: &str) -> bool { diff --git a/linux/crates/drivers/sqlite/src/ddl/mod.rs b/linux/crates/drivers/sqlite/src/ddl/mod.rs index f0c62d09ed..6b5127c9a1 100644 --- a/linux/crates/drivers/sqlite/src/ddl/mod.rs +++ b/linux/crates/drivers/sqlite/src/ddl/mod.rs @@ -24,7 +24,7 @@ use tablepro_core::statement::{Statement, StatementBuilder}; use tablepro_core::{ForeignKeyInfo, Value}; use self::create_table_source::CreateTableSource; -pub(crate) use self::create_table_source::{declared_descending_key, declared_foreign_key_names}; +pub(crate) use self::create_table_source::{declared_comparison, declared_descending_key, declared_foreign_key_names}; use self::declared_shape::readable; use crate::catalog::MAIN_SCHEMA; use crate::dialect::SqliteDialect; diff --git a/linux/crates/drivers/sqlite/src/ddl/tests.rs b/linux/crates/drivers/sqlite/src/ddl/tests.rs index 8df802fe29..17d5d1832d 100644 --- a/linux/crates/drivers/sqlite/src/ddl/tests.rs +++ b/linux/crates/drivers/sqlite/src/ddl/tests.rs @@ -108,6 +108,7 @@ fn foreign_key(name: &str, column: &str, parent: &str, parent_column: &str) -> F fn inbound(name: &str, child: &str, column: &str, parent_column: &str) -> InboundForeignKey { InboundForeignKey { + child_compares_alike: true, name: name.to_owned(), table: TableRef::unqualified(child), columns: vec![column.to_owned()], diff --git a/linux/crates/drivers/sqlite/src/metadata.rs b/linux/crates/drivers/sqlite/src/metadata.rs index ffa826a40f..a4ff5bac18 100644 --- a/linux/crates/drivers/sqlite/src/metadata.rs +++ b/linux/crates/drivers/sqlite/src/metadata.rs @@ -2,8 +2,8 @@ //! //! `catalog` owns the SQL and what the rows mean. This runs it. -use std::collections::BTreeMap; use std::collections::btree_map::Entry; +use std::collections::{BTreeMap, HashMap}; use sqlx::sqlite::{SqliteConnection as SqlxConnection, SqliteRow}; use sqlx::{AssertSqlSafe, Row}; @@ -22,7 +22,7 @@ use crate::catalog::{ self, InboundKeyRow, MAIN_SCHEMA, SchemaRow, SqliteColumnRow, TableFacts, foreign_key_names, generated_expressions, inbound_foreign_keys, select_dependents, }; -use crate::ddl::{declared_descending_key, declared_foreign_key_names}; +use crate::ddl::{declared_comparison, declared_descending_key, declared_foreign_key_names}; use crate::errors::{SQLITE_ERROR, map_error}; use crate::index_sql::read_index_sql; @@ -294,15 +294,43 @@ async fn inbound_keys( }) }) .collect::, DriverError>>()?; + let child_sql: HashMap> = rows + .iter() + .map(|row| (row.child.clone(), row.child_sql.clone())) + .collect(); + let parent_sql = match rows.is_empty() { + true => None, + false => create_statement(connection, table).await?, + }; let mut keys = inbound_foreign_keys(table.schema.as_deref(), rows, &shape.primary_key()); for key in &mut keys { for column in &mut key.referenced_columns { *column = declared_spelling(column, shape.columns.iter().map(|declared| declared.name.as_str())); } + let child = child_sql.get(&key.table.name).and_then(Option::as_deref); + key.child_compares_alike = compares_alike(parent_sql.as_deref(), child, key); } Ok(keys) } +/// SQLite compares a key under the parent columns' affinity and +/// collation, so `=` on a child column declared otherwise finds other +/// rows than the key matches. Text that does not read is taken at its +/// word: nothing says the two sides differ. +fn compares_alike(parent_sql: Option<&str>, child_sql: Option<&str>, key: &InboundForeignKey) -> bool { + key.columns + .iter() + .zip(&key.referenced_columns) + .all(|(child_column, parent_column)| { + let child = child_sql.and_then(|sql| declared_comparison(sql, child_column)); + let parent = parent_sql.and_then(|sql| declared_comparison(sql, parent_column)); + match (child, parent) { + (Some(child), Some(parent)) => child.finds_as(&parent), + _ => true, + } + }) +} + /// How a row can be named again: the primary key, else a unique index /// over NOT NULL columns covering every row, else the rowid. pub(crate) async fn row_identity( diff --git a/linux/crates/drivers/sqlite/tests/integration/catalog.rs b/linux/crates/drivers/sqlite/tests/integration/catalog.rs index 2d28c9a73a..dd6f7bb0b6 100644 --- a/linux/crates/drivers/sqlite/tests/integration/catalog.rs +++ b/linux/crates/drivers/sqlite/tests/integration/catalog.rs @@ -216,6 +216,41 @@ async fn a_key_to_a_missing_table_leaves_the_tables_other_keys_readable() { connection.close().await.unwrap(); } +/// SQLite matches a key under the parent column's affinity and +/// collation, which `=` on a child column declared otherwise does not. +#[tokio::test] +async fn an_inbound_key_says_whether_its_columns_compare_as_it_does() { + let file = TestFile::new().await; + let connection = file.open().await; + apply( + connection.as_ref(), + &[ + "CREATE TABLE users (email TEXT PRIMARY KEY COLLATE NOCASE, id INTEGER UNIQUE)", + "CREATE TABLE posts (id INTEGER PRIMARY KEY, author TEXT REFERENCES users (email))", + "CREATE TABLE notes (id INTEGER PRIMARY KEY, author TEXT COLLATE \"NoCase\" REFERENCES users (email))", + "CREATE TABLE logs (id INTEGER PRIMARY KEY, user_id TEXT REFERENCES users (id))", + "CREATE TABLE visits (id INTEGER PRIMARY KEY, user_id INT REFERENCES users (id))", + ], + ) + .await + .unwrap(); + + let inbound = connection + .fetch_inbound_foreign_keys(&TableRef::unqualified("users"), call()) + .await + .unwrap(); + + let alike: Vec<(&str, bool)> = inbound + .iter() + .map(|key| (key.table.name.as_str(), key.child_compares_alike)) + .collect(); + assert_eq!( + alike, + [("logs", false), ("notes", true), ("posts", false), ("visits", true)] + ); + connection.close().await.unwrap(); +} + #[tokio::test] async fn inbound_keys_read_alone_are_the_definitions() { let file = TestFile::new().await; From 002fdf7cfc47077a44f347b0eabb36090aeaf19c Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 02:30:45 +0700 Subject: [PATCH 8/9] docs(linux): note how an inbound key says it compares like its columns --- linux/docs/adding-drivers.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux/docs/adding-drivers.md b/linux/docs/adding-drivers.md index 7682286d97..a6191ac3c5 100644 --- a/linux/docs/adding-drivers.md +++ b/linux/docs/adding-drivers.md @@ -153,7 +153,7 @@ The `Connection` is a workspace's link to one server: the pool behind the sideba | `server_info`, `dialect`, `read_only` | Fixed at connect. | | `link_state`, `lost` | A `LinkWatch` shared by every call. `lost()` resolves when the link is gone: a failed probe, or a failed call that a check outside the pool confirmed. One dead backend in the pool does not resolve it, so the session monitor can act on it. | | `probe` | Whether the server answers. It never waits for a pool slot: with every slot busy it opens a connection of its own. | -| `list_schemas`, `list_objects`, `fetch_columns`, `fetch_indexes`, `fetch_foreign_keys`, `fetch_inbound_foreign_keys`, `fetch_table_definition`, `row_identity`, `table_estimate` | Catalogue reads. `fetch_table_definition` reads columns, keys and dependents in one call so they come from the same moment. `fetch_inbound_foreign_keys` returns the inbound keys that definition lists, read alone for a table tab's cell menu. Each key names its other table as the sidebar does, and its columns as declared. | +| `list_schemas`, `list_objects`, `fetch_columns`, `fetch_indexes`, `fetch_foreign_keys`, `fetch_inbound_foreign_keys`, `fetch_table_definition`, `row_identity`, `table_estimate` | Catalogue reads. `fetch_table_definition` reads columns, keys and dependents in one call so they come from the same moment. `fetch_inbound_foreign_keys` returns the inbound keys that definition lists, read alone for a table tab's cell menu. Each key names its other table as the sidebar does, and its columns as declared. An inbound key's `child_compares_alike` is false when `=` on the referencing columns finds other rows than the key matches, as on SQLite when the two sides differ in affinity or collation. | | `query_bound` | A generated read that keeps at most `limit` rows and reads one more to set `truncated`. | | `count_rows` | A generated count. | | `stream_read` | Every row of a read into a `ResultSink`, with no row limit, on a connection of its own, held read-only for its whole length. | From 04140a8ac5dd47402be358eab97dfee6f96add54 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 02:44:37 +0700 Subject: [PATCH 9/9] fix(linux-grid): name keys by their own columns and their constraint when two clash --- .../crates/app/src/ui/grid/reference_menu.rs | 121 ++++++++++++++---- 1 file changed, 94 insertions(+), 27 deletions(-) diff --git a/linux/crates/app/src/ui/grid/reference_menu.rs b/linux/crates/app/src/ui/grid/reference_menu.rs index 934e87455f..fb504bcaf9 100644 --- a/linux/crates/app/src/ui/grid/reference_menu.rs +++ b/linux/crates/app/src/ui/grid/reference_menu.rs @@ -97,15 +97,17 @@ impl ReferenceMenu { self.section.remove_all(); self.choices.remove_all(); if let Some(keys) = offers.keys.as_deref() { - for &index in &offers.open { - if let Some(key) = keys.outbound.get(index) { + let open: Vec<(usize, KeyName)> = offers + .open + .iter() + .filter_map(|&index| { + let key = keys.outbound.get(index)?; let target = TableRef::new(key.ref_schema.clone(), key.ref_table.clone()); - self.choices.append_item(&choice( - &key_label(&keys.table, &target, &key.ref_columns), - OPEN_CHOICE, - index, - )); - } + Some((index, KeyName::new(&keys.table, &target, &key.columns, &key.name))) + }) + .collect(); + for (index, label) in labelled(open) { + self.choices.append_item(&choice(&label, OPEN_CHOICE, index)); } match offers.open.len() { 0 => {} @@ -118,14 +120,16 @@ impl ReferenceMenu { .append_submenu(Some(&crate::i18n::gettext("Open Referenced Row")), &self.choices), } let referencing = gio::Menu::new(); - for &index in &offers.referencing { - if let Some(key) = keys.inbound.get(index) { - referencing.append_item(&choice( - &key_label(&keys.table, &key.table, &key.columns), - REFERENCING, - index, - )); - } + let inbound: Vec<(usize, KeyName)> = offers + .referencing + .iter() + .filter_map(|&index| { + let key = keys.inbound.get(index)?; + Some((index, KeyName::new(&keys.table, &key.table, &key.columns, &key.name))) + }) + .collect(); + for (index, label) in labelled(inbound) { + referencing.append_item(&choice(&label, REFERENCING, index)); } if referencing.n_items() > 0 { self.section @@ -228,15 +232,44 @@ fn choice(label: &str, action: &str, index: usize) -> gio::MenuItem { item } -/// The rows an item opens: the other table, named with its schema where -/// that is not the source's, and the columns its filter is on. Two keys -/// from one column into one table differ there. -fn key_label(source: &TableRef, table: &TableRef, columns: &[String]) -> String { - let name = match (&table.schema, table.schema == source.schema) { - (Some(schema), false) => format!("{schema}.{}", table.name), - _ => table.name.clone(), - }; - menu_label(&format!("{name} ({})", columns.join(", "))) +/// How an item names its key: the other table, with its schema where +/// that is not the source's, and the key's columns on the side that +/// holds it, which tell two keys into one table apart. +struct KeyName { + short: String, + constraint: String, +} + +impl KeyName { + fn new(source: &TableRef, table: &TableRef, columns: &[String], constraint: &str) -> Self { + let name = match (&table.schema, table.schema == source.schema) { + (Some(schema), false) => format!("{schema}.{}", table.name), + _ => table.name.clone(), + }; + Self { + short: format!("{name} ({})", columns.join(", ")), + constraint: constraint.to_owned(), + } + } +} + +/// Each key's label. Two keys from the same columns into the same table +/// also name their constraint, since they point at different columns. +fn labelled(names: Vec<(usize, KeyName)>) -> Vec<(usize, String)> { + let shared = |short: &str| names.iter().filter(|(_, name)| name.short == short).count() > 1; + names + .iter() + .map(|(index, name)| { + let label = match shared(&name.short) { + true => crate::i18n::gettext_f( + "{key} through {constraint}", + &[("key", &name.short), ("constraint", &name.constraint)], + ), + false => name.short.clone(), + }; + (*index, menu_label(&label)) + }) + .collect() } /// The index a parameterized item carries. @@ -414,13 +447,47 @@ pub(super) mod tests { offered, [ item("Open Referenced Row", None, None), - item("tenants (id)", Some("cell.open-reference"), Some(0)), - item("crm.customers (tenant__id, id)", Some("cell.open-reference"), Some(1)), + item("tenants (tenant__id)", Some("cell.open-reference"), Some(0)), + item( + "crm.customers (tenant__id, customer__id)", + Some("cell.open-reference"), + Some(1) + ), ] ); assert_eq!(followed(&grid), [(crm, filter(&[("tenant_id", "7"), ("id", "42")]))]); } + /// One column pointing at two columns of one table gives the two + /// keys the same columns, so their constraints tell them apart. + #[gtk4::test] + fn keys_from_the_same_columns_into_one_table_name_their_constraints() { + let mut by_id = key(&["customer_id"], public("customers"), &["id"]); + by_id.name = "by_id".to_owned(); + let mut by_code = key(&["customer_id"], public("customers"), &["code"]); + by_code.name = "by_code".to_owned(); + let grid = orders_grid(vec![by_id, by_code]); + + open_menu(&grid.cell_of(1, 1)); + + assert_eq!( + reference_items(&grid), + [ + item("Open Referenced Row", None, None), + item( + "customers (customer__id) through by__id", + Some("cell.open-reference"), + Some(0) + ), + item( + "customers (customer__id) through by__code", + Some("cell.open-reference"), + Some(1) + ), + ] + ); + } + #[gtk4::test] fn a_key_the_catalogue_could_not_pair_is_offered_and_says_why_it_cannot_open() { let grid = orders_grid(vec![key(&["customer_id"], public("ghost"), &[])]);