feat(structure): create, edit, and drop triggers across all engines#1708
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d940c09d2b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| supportsDropIndex: driverType.supportsDropIndex, | ||
| supportsModifyPrimaryKey: driverType.supportsModifyPrimaryKey, | ||
| supportsTriggers: driverType.supportsTriggers, | ||
| supportsTriggerEditing: driverType.supportsTriggerEditing, |
There was a problem hiding this comment.
This is a new DriverPlugin requirement, but the registry is reading it directly from every loaded driver type. Existing downloadable/third-party driver plugins compiled against the previous PluginKit will not have a witness-table entry for this requirement; the nearby supportsColumnReorder comment already documents that reading newly added requirements from driverType can crash stale plugins. In an install with any older external driver plugin, registration can fail/crash before the app can even fall back to the default false capability, so this needs the same compatibility treatment as the preserved snapshot fields.
Useful? React with 👍 / 👎.
| static let parameterStyle: ParameterStyle = .dollar | ||
| static let supportsDropDatabase = true | ||
| static let supportsTriggers = true | ||
| static let supportsTriggerEditing = true |
There was a problem hiding this comment.
Because this plugin also advertises additionalDatabaseTypeIds for Redshift and CockroachDB, and DatabaseConnection.supportsTriggerEditing resolves aliases through the PostgreSQL plugin type, this single flag exposes the New/Edit/Delete trigger UI for those variants too. The Redshift and Cockroach driver classes do not implement the trigger create/fetch/drop hooks, so those connections fall back to generic trigger DDL (or empty trigger lists) and the new actions can only fail or run inappropriate PostgreSQL-oriented SQL; gate this capability per variant instead of enabling it on the shared plugin descriptor.
Useful? React with 👍 / 👎.
…QL rollback, guard PG constraint triggers
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1809fe30aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sql: sql, | ||
| kind: .schemaMutation, |
There was a problem hiding this comment.
When editing on non-replace drivers such as MySQL, SQLite/libSQL, or D1, the code authorizes only the user-supplied CREATE text, but then lines 78 and 82-85 execute a generated DROP TRIGGER before it. Because DefaultExecutionGate treats destructive SQL as always confirmation-worthy even in Silent mode, these edit flows can drop an existing trigger without the destructive confirmation or audit coverage that schema changes with DROP normally get; include the generated drop in the authorized SQL/kind before executing it.
Useful? React with 👍 / 👎.
|
|
||
| static let isDownloadable = true | ||
| static let supportsTriggers = true | ||
| static let supportsTriggerEditing = true |
There was a problem hiding this comment.
For Oracle connections, enabling the edit UI here exposes Edit/Save for existing triggers, but OraclePluginDriver does not override fetchTriggerDefinition and its fetchTriggers query builds statement from only TRIGGER_NAME, type, event, status, and WHEN clause, omitting the trigger body. As a result, editing an existing Oracle trigger opens and applies an incomplete CREATE OR REPLACE TRIGGER header rather than the real definition, so saves either fail or replace the trigger incorrectly; keep this disabled until the driver returns full trigger DDL.
Useful? React with 👍 / 👎.
| return triggers | ||
| } | ||
|
|
||
| var triggerEditUsesReplace: Bool { true } |
There was a problem hiding this comment.
This advertises replace-based trigger editing for every PostgreSQL server, and the editor later emits CREATE OR REPLACE TRIGGER; PostgreSQL 13 and older do not accept that syntax, while this driver already supports/version-gates much older server versions via serverVersionNumber. On those connections the new Create/Edit Trigger UI will consistently fail instead of using a drop-and-create fallback or hiding editing.
Useful? React with 👍 / 👎.
| func createTriggerTemplate(table: String, schema: String?) -> String? { | ||
| let resolved = schema ?? _currentSchema | ||
| return """ | ||
| CREATE OR ALTER TRIGGER \(quoteIdentifier("trigger_name")) |
There was a problem hiding this comment.
The new SQL Server trigger template unconditionally uses CREATE OR ALTER TRIGGER, and fetched definitions are rewritten to the same syntax, but older SQL Server versions that the driver can connect to reject CREATE OR ALTER. The existing view template already checks MSSQLCapabilities before using this syntax, so trigger editing should do the same or fall back to drop/create; otherwise New/Edit Trigger fails on those servers even though the UI is enabled.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40675990bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| var supportsTransactionalDDL: Bool { true } |
There was a problem hiding this comment.
On libSQL/Turso connections using the default remote mode, this flag makes TriggerApplyStrategy choose the transactional path for both trigger creates and SQLite-style edits, so TriggerEditing.runInTransaction calls beginTransaction() before running the DDL. I checked LibSQLPlugin.swift and libsqlMode defaults to remote, while executeTransactionStatement throws unless the backend is .local, so New/Edit Trigger always fails on the main remote/Turso path before the trigger SQL is executed; make this capability backend-dependent or avoid the transaction wrapper for remote libSQL.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4306cf6066
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| static let supportsDropDatabase = true | ||
| static let supportsTriggers = true | ||
| static let supportsTriggerEditing = true |
There was a problem hiding this comment.
Enabling editing here makes MySQL/MariaDB edits use TriggerDetailView.editTrigger's fallback because MySQLPluginDriver does not implement fetchTriggerDefinition; that fallback TriggerInfo.statement is reconstructed from information_schema.TRIGGERS.ACTION_STATEMENT and omits full SHOW CREATE TRIGGER metadata such as the original DEFINER. Since the apply path drops and recreates on edit, even a no-op save can change the trigger to run under the current user (and rollback restores the same lossy definition), so fetch the full trigger DDL before advertising editing.
Useful? React with 👍 / 👎.
| let fetched = try? await driver?.fetchTriggerDefinition(name: trigger.name, table: tableName) | ||
| let sql = (fetched ?? nil) ?? trigger.statement | ||
| editorSheet = TriggerEditorSheetItem( | ||
| mode: .edit(originalName: trigger.name, originalDefinition: trigger.statement), |
There was a problem hiding this comment.
When the selected SQL Server trigger is disabled (TriggerInfo.enabled == false), this edit mode drops that state: only the name and definition are carried into the sheet, and the SQL Server driver rewrites the definition to CREATE OR ALTER TRIGGER. Running ALTER TRIGGER/CREATE OR ALTER TRIGGER enables disabled triggers, so saving a disabled trigger, even without semantic changes, makes it fire again; carry the enabled flag through and re-disable after apply or block editing disabled triggers.
Useful? React with 👍 / 👎.
1 participant
PR 2 of the v2 trigger work (follows #1703). Adds create / edit / drop for triggers across all 7 supported engines, using the determined DDL-first design (research-backed, not a tool clone).
Design
pg_get_functiondef+pg_get_triggerdef), since the logic lives in the function.CREATE OR REPLACE/CREATE OR ALTERwhere the engine supports it, else drop-and-recreate. Transactional-DDL engines (PostgreSQL, SQLite, SQL Server) wrap the change in a transaction; non-transactional engines (MySQL, Oracle) use a rollback buffer that restores the original on failure.ExecutionGate(confirmation + safe-mode), records to query history, and broadcastsrefreshData(the triggers tab auto-reloads).Scope
createTriggerTemplate,fetchTriggerDefinition,generateDropTriggerSQL,triggerEditUsesReplace,supportsTransactionalDDL, and asupportsTriggerEditingcapability.DatabaseDriver/adapter bridges, capability gating, theTriggerEditingorchestrator, theTriggerEditorViewsheet (editableCodeEditSourceEditor), and a New/Edit/Delete action bar in the Triggers tab.Engine notes
LONG/DBMS_METADATAwould crash the connection).Status
Built in a worktree; not yet compiled (no Xcode here). App-target SwiftLint
--strictis clean. Please build and report any errors — highest-risk spots are theTriggerEditorVieweditableSourceEditorand theconfirmationDialog/sheet(item:)wiring inTriggerDetailView.