feat(datagrid): show a table's properties and let its comment be edited - #2578
Open
datlechin wants to merge 1 commit into
Open
feat(datagrid): show a table's properties and let its comment be edited#2578datlechin wants to merge 1 commit into
datlechin wants to merge 1 commit into
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Member
Author
|
x |
Member
Author
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
注册 for free
to join this conversation on GitHub.
Already have an account?
登录 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.


Closes #2555.
What was missing
There was no surface for a table's own properties, and no way to edit a table comment anywhere in the app, on any engine. The right-sidebar inspector showed a read-only subset (size, rows, engine, collation, timestamps, comment) and only while no row was selected. The reporter stores a JSON payload in a table comment and needed to read and edit it without writing
COMMENT ON.What this adds
A Properties tab, first in the structure editor's sub-tab picker.
fetchTableMetadatathroughTableStructureLoader, so the tab's own session owns the data rather than the coordinator's latest-wins inspector slot.PluginTableMetadatagainsattributes: [PluginObjectAttribute], which lets each driver name its own vocabulary without the app modelling any of it: PostgreSQL sends Owner, Tablespace, Persistence and a Kind for anything that is not an ordinary table; MySQL sends Row Format, Auto Increment and Create Options.SchemaChange.modifyTableCommentinto the sameStructureChangeManagerthe Columns grid writes to, so it inherits Safe Mode, the authorization gate, the transaction, query history, undo and redo, and the unsaved-changes prompt on close. The editor is the existingTextValueEditor: a scrollingNSTextViewwith AppKit's quote and dash substitutions off, which is the answer to "how do I open the comment bigger".Writability is decided twice, and both gates have to agree.
DriverPlugin.supportsTableComment(defaultfalse) says whether the engine has table comments at all.PluginTableMetadata.commentIsReadOnly(defaulttrue) says whether this relation takes one: PostgreSQL lowers it frompg_class.relkind, MySQL fromSHOW TABLE STATUSnaming no engine, which is how it reports a view. A driver that cannot establish the relation kind offers no edit, so a materialized view or a foreign table never reaches aCOMMENT ON TABLEthat its engine would refuse.The PostgreSQL literal is dollar-quoted rather than
'...'. A comment is arbitrary user text, the driver's shared escape helper doubles apostrophes but leaves backslashes, and withstandard_conforming_stringsoff PostgreSQL reads backslash escapes inside an ordinary literal, so a backslash in front of a doubled apostrophe closes the literal early and the rest of the comment parses as SQL. A dollar-quoted body is not scanned for escapes under either setting.One undo step per editing episode, not per keystroke: the field writes on every character, and a per-character entry would fill all 100 levels of undo from one paragraph and evict the column edits staged beside it. Replaying an undo or redo always registers, so
Cmd+Shift+Zputs the comment back.MySQLTableStatusalso fills inavgRowLength,collation,createTimeandupdateTime, whichSHOW TABLE STATUShad always returned and the driver had never read.PluginKit ABI
Additive, no
currentPluginKitVersionbump, and no registry plugin binary changes at all: the two engines that gain comment editing are both bundled.PluginTableMetadata.attributesandcommentIsReadOnlyarrive through a new init overload; the original keeps its exact signature and is marked@_disfavoredOverload, the same shapePluginTableInfoalready uses. Adding a parameter to the existing init would have replaced its mangled symbol and broken every shipped plugin.Decodabledoes not fall back to an initializer's default value, so a payload written before these fields existed throwskeyNotFound.commentIsReadOnly = true, so an already-built plugin presents its comment read-only rather than being assumed writable.generateSetTableCommentSQLandsupportsTableCommentare protocol requirements with default implementations.Scope and non-goals
COMMENT ONkeyword (Oracle reads materialized-view container rows throughALL_TABLES; Trino's grammar has no materialized-view form), and ClickHouse'sMODIFY COMMENTneeds 23.9 or later, which the app does not currently require. They are also all registry-only, so enabling them means publishing five plugin artifacts in lockstep with the app.LibPQBackedDriverrather thanPostgreSQLPluginDriver, so they inherit no comment generator and stay read-only.Verified
Every step through
verify.sh, plus an end-to-end check against a local PostgreSQL 17.buildtest(the suites owning the changed types)build MySQLDriver,build PostgreSQLDriverplugins(AllPlugins)oracle-niofails withunknown attribute 'usableFromInlinenonisolated'from its own@TaskLocalmacro, which blocks the aggregate on this toolchain for any change underPlugins/. CI runs it.lintdocsabiEnd to end, driving a Debug build against
postgresql://127.0.0.1:5432/postgres:Cmd+S, thenselect obj_description('public.orders'::regclass, 'pg_class')returned the typed text;Cmd+S, and the same query returnedNULL, so theIS NULLbranch is exercised too.Tests
StructureChangeManagerTableCommentTests: staging, clearing, returning to baseline, undo coalescing, redo, discard, and the baseline-refresh cases (a staged edit is kept; a baseline that catches up drops the change).SchemaStatementGeneratorPluginTests: the plugin-delegated statement, the nil-returns-throws case, the clear passingnil, and the ordering after every structural change.PluginTableMetadataCodingTests: decoding a payload written before the new fields existed, the round trip, and that both compatibility paths report the comment read-only.PostgreSQLDollarQuotedLiteralTests: a backslash-and-apostrophe payload survives untouched, and the tag grows until the body cannot close it.PostgreSQLTableAttributeTestsandMySQLTableStatusTests: the two read-side parsers, including which relations take a writable comment.StructurePropertiesTabUITests: the tab is the leading segment and opens, and the comment is read-only on SQLite, which stores none.Review
Codex read the diff cold twice, once as a defect review and once adversarially against the design. Everything it raised that survived checking is fixed:
loadInitialDatareachedloadSchemaForEditinglet an edit be staged and then dropped by the re-baseline. The field is disabled until the initial load and any post-save reload have finished.fetchTableMetadatawithSELECT COUNT(*), and SQLite scans up to 100,001 rows, which put a scan in front of the Columns tab for tables nobody opened Properties on. The fetch is lazy, and a refresh only repeats it for a tab that already loaded it.commentIsReadOnlydefaulted to false, so a driver that could not classify the relation still got an editor. It defaults to read-only now, and the six engines that cannot yet classify are out of the shipped set entirely.pg_defaultfor every inheriting table, which is wrong in a database created with its own tablespace. It resolvespg_database.dattablespace.sales.orderswould overwrite what was cached forarchive.orders. Adoption now matches the wholeDatabaseScopeand only publishes to the visible panel when the originating tab is the selected one.Two findings were not acted on, with reasons:
CLAUDE.mdbans comments that describe what code does or explain callers; the surrounding files (StructureEditingSession,TableStructureLoader,PluginTableMetadata) all state the invariant and the defect it prevents, which is what these do. The one clause that did explain a caller was removed.applyStagedChangessnapshots the queue and then clears the whole manager on success, so an edit made during a slow DDL is lost. That is real, but it predates this change and applies to column, index and key edits equally; it is a separate fix.Screenshots
There is no before for a new tab. Both shots were captured on a 1x display, so they are 1512x861 rather than the 3024x1722 the other docs images use; they are worth re-taking on a Retina machine.