fix(datagrid): open materialized views read-only in the data grid - #3072
Merged
Merged
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #3063
Summary
A materialized view opened from the sidebar was an editable grid. Cell edits, Add Row, Duplicate, Paste and Delete all queued, and Save then failed, because PostgreSQL refuses every row write on a materialized view. It now opens read-only from the sidebar, from Open Quickly and from a restored tab, on every engine that lists the kind, ClickHouse included.
Root cause
TableInfo.TableType.allowsRowEditingreturnedtruefor.materializedView, on the assumption that an object holding rows accepts row DML. The sidebar derives the tab'sisViewfrom that rule, while Open Quickly hard-codesisView: truefor anything it groups with views, so the same matview was writable from one route and read-only from the other. The two gates (canEditActiveResult,canAddRow) read!isViewand ignored theobjectTypethe tab already carries, so a tab saved withisView == falsestayed writable after any rule fix: persisted tabs, back/forward history, recently closed tabs and window payloads all carry the old Bool.Two more leaks sat behind the gates.
RowEditingCoordinator's add, delete, duplicate and paste guards checkedisEditablealone, which a table-tab load setstruefor every object, views included, so a delete that reached it on a view tab staged a DELETE. And the grid's empty-space Add Row read a closure cached on fouronChangehooks that never watched the object kind, the result mode or the schema.What changed
allowsRowEditingisfalsefor.materializedView, matching the iOS model (TableKind.allowsRowEditingwas already false).TabTableContext.allowsRowEditingis!isView && resolvedObjectKind().allowsRowEditing. A kind can refuse rows the Bool allows; it can never re-enable a tab the Bool marks read-only.canEditActiveResultreads it.canAddRownow narrowscanEditActiveResultrather than repeating half of it, so the toolbar never offers Add Row on a query result the grid refuses.RowEditingCoordinator.addNewRow,deleteSelectedRows,duplicateSelectedRowandpasteRowsguard oncanEditActiveResult, the gate the Edit menu validates on.canAddRowis not used there because it is Data-mode only, and the Edit menu's Add Row and Duplicate Row run in JSON mode too (pinned byRowEditingCoordinatorJsonModeTests).coordinator.canAddRowlive. The cachedonAddRownil-ing inMainEditorContentViewand its fouronChangehooks are gone;onAddRowis wired once with the other stable refs.TabTableContext.objectType,EditorTabPayload.objectType,StructureEditEligibility,ForeignKeyEditPolicy,TableStructureView.objectKind, thecanAddRowdoc, and theTabObjectKindTestsheader.databases/clickhouse.mdx(Limitations),databases/postgresql.mdx(next to the matview Structure note),features/change-tracking.mdx.Scope is row writes only. Row counts,
Cmd+R, export, TRUNCATE eligibility (TableOperationEligibility, kind-based) and Structure gating (resolvedObjectKind()) read neitherisViewnorallowsRowEditingand are unchanged. The one visible side effect: MCPlist_recent_tablesreportsis_view: truefor a matview opened from the sidebar, as it already did for one opened from Open Quickly.Measured
PostgreSQL 17.11 (Homebrew, aarch64), schema
matview_rows, dropped after:UPDATE,INSERT,DELETEERROR: cannot change materialized view "mv"TRUNCATEERROR: "mv" is not a tableMERGEERROR: cannot execute MERGE on relation "mv", not supported for materialized viewsCOPY mv FROM STDINERROR: cannot copy to materialized view "mv"pg_relation_is_updatable('mv', false)0(a table and a simple view:28)INSTEAD OF / BEFORE triggers and rules are refused on a matview too, so there is no way to make one writable.
ClickHouse 26.10.1.488 (
clickhouse local):INSERTALTER TABLE … UPDATE/DELETEENGINE = MergeTree … POPULATE)TO tgtCode: 80. MATERIALIZED VIEW targets existing table matview_rows.tgt. Execute the statement directly on it. (INCORRECT_QUERY)Both list as engine
MaterializedViewinsystem.tables, so the object list cannot tell them apart, and an editable grid over aTOview is this bug again. Editingtgtshows throughmv_toat once.Tests
New and changed cases:
MaterializedViewRowWriteTests(new, 11 cases): the sidebar route, sidebar and Open Quickly parity, a restored tab that keptisView == false, a restored tab with no kind, Add/Duplicate/Paste/Delete throughRowEditingCoordinatoron a matview and on a loaded view, and the grid's empty-space menu. EverycanAddRowassertion seeds authoritative rows in Data mode, and each refusal has a table positive control.TabObjectKindTests: the matview case asserts!allowsRowEditingwithisView == false; three new cases pin the gate in both directions (a refusing kind wins over the Bool, the Bool wins over a writable kind, writable kinds and no kind stay writable).TableInfoTests.materializedViewDisallowsRowEditing(new); matview removed fromlocalRelationsAllowRowEditing.QuickSwitcherObjectKindTests: the matview fixture isisReadOnly: true, and the cross-connection builder is asserted to produce[true, false]for a matview and a partitioned table.MainContentCoordinatorAddRowTests.queryTabFollowsTheResultRefusal(new): a query result with resolved keys offers a row, one with unresolved keys does not.Run through
verify.sh, 24 suites (MaterializedViewRowWriteTests TableInfoTests TabObjectKindTests QuickSwitcherObjectKindTests QuickSwitcherItemIdentityTests QuickSwitcherViewModelTests MainContentCoordinatorAddRowTests MainContentCommandActionsResultViewTests RowEditingCoordinatorJsonModeTests RowEditingCoordinatorValueFilterTests RowEditingCoordinatorCopyTests SaveCompletionTests ValueFilterEditedRowTests CommandActionsDispatchTests PluginDriverAdapterTableTypeMappingTests EditorTabPayloadTests MultiConnectionNavigationTests TabPersistenceCoordinatorTests MainWindowToolbarValidationTests ToolbarContextResolverTests MenuContentModeParityTests MainContentCoordinatorGridSelectionTests ResultSwitchIdentityTests ResultEditabilityTests): 376 executed, 366 passed, 10 failed. All 10 areQuickSwitcherViewModelTestscases that sleep 200 ms for a search debounce and miss it under the machine's load (load average 130 to 230 from parallel builds); the count moved between runs (17, then 10), and the suite run alone passes 57 of 57. An unrelated branch on the same machine hit 18 of the same failures.Each new test was turned red by reverting the edit it guards (measured):
allowsRowEditingback totruefor.materializedViewTableInfoTests.materializedViewDisallowsRowEditing,QuickSwitcherObjectKindTests.crossConnectionItemsCarryTheType,TabObjectKindTests.materializedViewKeepsItsKind,.refusingKindDecidesRowEditing,MaterializedViewRowWriteTests.sidebarMaterializedViewIsReadOnly,.sidebarAndOpenQuicklyAgree,.restoredStaleTabIsReadOnly,.rowCommandsRefuseAMaterializedView,.emptySpaceMenuFollowsTheGate(9)TabTableContext.allowsRowEditingback to!isViewrestoredStaleTabIsReadOnly,rowCommandsRefuseAMaterializedView,emptySpaceMenuFollowsTheGate,TabObjectKindTests.refusingKindDecidesRowEditing,.materializedViewKeepsItsKind(5)TabTableContext.allowsRowEditingas the kind alone, without!isViewTabObjectKindTests.readOnlyMarkOutranksTheKindRowEditingCoordinatorguards back toisEditable, empty-space menu back to the closure alone,canAddRownot built oncanEditActiveResultrowCommandsRefuseAMaterializedView,deleteRefusesALoadedView,emptySpaceMenuFollowsTheGate,MainContentCoordinatorAddRowTests.queryTabFollowsTheResultRefusalThe last two ran together (5 red, each tied to one mutation). Positive controls stayed green throughout:
sidebarTableStaysWritable,restoredTableTabStaysWritable,restoredTabWithoutAKindFollowsTheBool,rowCommandsStillWorkOnATable,deleteStillStagesOnATable,writableKindsKeepRowEditing.Other checks:
verify.sh generate,buildanddocspass.verify.sh lintover the 16 touched Swift files reports one violation,sorted_importsatTableProTests/Models/Query/TabObjectKindTests.swift:9, on an import line this change does not touch (the file sits outside SwiftLint'sincluded:).Before / After
Screenshots to be added. States to capture, on PostgreSQL with a materialized view
daily_totalsinpublic:daily_totalsopened from the sidebar, a cell double-clicked into edit mode, toolbar Add Row enabled.cannot change materialized view).Critique points not taken
None. Every objection in the critique is applied:
RowEditingCoordinatorguards read the gate, and the empty-space menu readscanAddRowlive (the cache is removed rather than taught one more key).!isView && resolvedObjectKind().allowsRowEditing, tested in both directions.canAddRowassertion seeds authoritative rows in Data mode and has a positive control.QuickSwitcherObjectKindTestsusesisReadOnly: truefor the matview item and asserts the production builder produces it.Deliberately not fixed here
TableInfo.TableType.allowsRowEditingistruefor.foreignTable(TablePro/Models/Query/QueryResult.swift:171). Measured on PostgreSQL 17.11: afile_fdwforeign table refuses UPDATE, INSERT and DELETE andpg_relation_is_updatableanswers0, while apostgres_fdwone answers28. Writability there belongs to the object, not its kind, so it needs a driver-sourced per-object flag frompg_relation_is_updatable. The same flag would let ClickHouse report which materialized views store their own rows and give inner-storage MVs back their grid editing.open_table_tab, AppleScript, deeplinks, a URLtable=and a query tab runningSELECT * FROM mvopen withobjectType == nilandisView == false(openTableTab(_:schema:database:showStructure:isView:objectType:…)defaults,MainContentCoordinator+Navigation.swift:48;resolveTableEditability,MainContentCoordinator.swift:1483). Fixing them needs a catalog kind lookup, which the Structure gate needs too.TABLE(the container table), so they stay editable and fail with ORA-01732 on a read-only MV. That is a listing defect in those plugins.