Repository navigation
feat(sidebar): list PostgreSQL sequences and set or edit them - #3722
Conversation
…quences # Conflicts: # apple/TablePro/Core/Database/DatabaseObjectToolEligibility.swift
…nner change on main
…quences # Conflicts: # apple/TablePro/Views/Main/Child/MainEditorContentView.swift
…ce kind and menu items moved
…quences # Conflicts: # apple/TablePro/Core/Database/DatabaseObjectToolEligibility.swift
…quences # Conflicts: # apple/TablePro/Resources/Localizable.xcstrings
…ommandActions+DatabaseTools
J2TeamNNL
left a comment
There was a problem hiding this comment.
Review notes. Not built or run locally.
| let afterColumn = "COALESCE(pg_catalog.\(descending ? "min" : "max")(\(quotedColumn))::bigint + \(increment), \(start))" | ||
| // Never behind where the sequence already is: another session may hold values cached past it. | ||
| let current = "(SELECT CASE WHEN is_called THEN last_value + \(increment) ELSE last_value END FROM \(relation))" | ||
| let value = "\(descending ? "LEAST" : "GREATEST")(\(afterColumn), \(current))" |
There was a problem hiding this comment.
The option "After the values already in " wraps the value in GREATEST (LEAST when descending) with the current position. It can never move the sequence back. After a delete or truncate, picking it reports success and the sequence stays high. Only the typed value can lower it.
| // and with row security off a policy that would hide rows fails the read instead. | ||
| let rowSecurity = hasRowSecurity ? "SET LOCAL row_security = off;\n" : "" | ||
| return rowSecurity | ||
| + "LOCK TABLE \(source) IN SHARE ROW EXCLUSIVE MODE;\n" |
There was a problem hiding this comment.
LOCK TABLE ... IN SHARE ROW EXCLUSIVE MODE needs UPDATE, DELETE or TRUNCATE on the table. A role with only INSERT or SELECT fails with permission denied, and this is the default option for owned sequences. There is no fallback and the sheet gives no hint why.
| let kind = PluginExportObjectKind.from(tableType: table.type.rawValue) | ||
| guard request.kinds.contains(kind) else { return nil } | ||
| // A sequence holds no rows worth a file, and a role with USAGE alone cannot read it. | ||
| if table.type == .sequence, !request.kinds.contains(.sequence) { return nil } |
There was a problem hiding this comment.
PluginExportObjectKind.from(tableType:) has no sequence case, so a SEQUENCE row is still classified as a table. The name dedupe only runs when loadSequences returns rows. If it returns [] (fetch failed or pg_sequences not visible), the sequence is exported as SELECT * data. Fixing the classification in from(tableType:) would remove the need for the guard.
| return definitions.map { PluginSequenceInfo(name: $0.name, ddl: $0.ddl, schema: schemaName) } | ||
| } | ||
|
|
||
| func fetchSequenceDetails(sequence: String, schema: String?) async throws -> PluginSequenceDetails? { |
There was a problem hiding this comment.
fetchSequenceDetails returns nil when no definition matches, and SequenceEditing.details turns that into unsupported. If another session drops the sequence, or the role cannot see it, the user gets an unsupported error. Not found and not supported should be different errors. Also, opening the sheet runs up to three sequential round trips, and the position query does not depend on the definition, so it could run concurrently.
| // 'a' for OWNED BY, 'i' for an identity column's own sequence. | ||
| var ownerDependency: String? | ||
|
|
||
| func withLastValue(_ value: String?) -> PostgreSQLSequenceDefinition { |
There was a problem hiding this comment.
The five new fields are vars with no initializer parameters, so withLastValue has to copy each by hand. A field added later and missed there is dropped silently on the pre-10 path. Making them let initializer parameters, or copying self and replacing lastValue, avoids that.
| } | ||
| } | ||
|
|
||
| private static func ownerColumns(sequence: String) -> String { |
There was a problem hiding this comment.
ownerColumns(sequence:) rewrites the owner subqueries that PostgreSQLTableListing.sequenceOwnerColumns already holds. A change to how ownership is read must be made in two places, and the sidebar tooltip and the sheet can then disagree.
| action: #selector(MainSplitViewController.editObjectComment(_:)) | ||
| ), | ||
| MenuItemFactory.item( | ||
| String(localized: "Set Next Value…"), |
There was a problem hiding this comment.
Three always-present items (New Sequence, Set Next Value, Edit Sequence) are added to the Database menu for every engine, and the menu now scrolls on the 1024x768 UI test runner. The PR rewrites DiagramPointerUITests to work around it. AGENTS.md says to fix the code, not the test, and a deliberate test change must be named in the description.
| var result: PluginQueryResult? | ||
| var answered = PostgreSQLTableListingLadder.leastCapableAttempt | ||
| for attempt in PostgreSQLTableListingLadder.degradableAttempts where result == nil { | ||
| do { |
There was a problem hiding this comment.
Sequences now come back from fetchTables, which every consumer shares. Compare, ObjectCopyCatalog, export and transfer were patched, but autocomplete (SQLSchemaProvider), MCP list_tables and schema search, AI QueryContextBuilder and RowImportSheet were not. They now see tables whose columns are last_value, log_cnt and is_called. Each future consumer needs the same filter.
|
Fixes for the review notes, in one commit: J2TeamNNL@db8800a Fixed:
Skipped:
Not built or run locally; only swiftc -parse on the touched files. To take it: |
…quences # Conflicts: # apple/TablePro/Core/Plugins/PluginDriverAdapter.swift # apple/TablePro/Models/Query/QueryResult.swift
…quences # Conflicts: # apple/TablePro/Resources/Localizable.xcstrings # apple/TablePro/Views/Sidebar/DatabaseTreeOutlineCoordinator+Commands.swift
…quences # Conflicts: # apple/TablePro/Views/Main/MainContentCommandActions.swift
Summary
Son macOS, so each schema gets a Sequences section; the row tooltip names the owning column (OWNED BY).setval, to a typed value or past the values already in the owning column.ALTER SEQUENCEfor increment, min, max, cache and cycle.CREATE SEQUENCEtemplate.PluginTableInfoowner fields,PluginSequenceDetails, and driver hooks with defaults for details, setval, ALTER SEQUENCE and the create template.AS <type>andCACHEout, so an export restored it as bigint with cache 1.MainContentCommandActions+DatabaseTools.swift: New Sequence put the class over SwiftLint's 1100-line body limit.PostgreSQLFetchTablesAllSchemasTests: the macOS listing reads relkindS.SidebarObjectKindTests:.sequencejoins the table category.WindowSidebarStateTests(unknown object kind): the fixture's unknown kind issynonym, sincesequenceis now known.DatabaseMenuCommandTests(New Folder): New Folder follows New Sequence.DiagramPointerUITests: opens the diagram from the sidebar's empty-area menu; the longer Database menu puts View ER Diagram under its scroll arrow on the 1024x768 runner.Test plan
PostgreSQLSequenceBrowsingTests,SequenceSidebarTests, the newDatabaseTreeMenuSpecTestsandMainMenuValidationTestscases fail on main and pass here.setvalpast the column's max.Docs: TableProApp/docs#78
Fixes #3605