Skip to content

Mark triggers / routines / events as out of scope#9

Merged
winebarrel merged 3 commits into
mainfrom
drop-trigger-routine-todos
May 2, 2026
Merged

Mark triggers / routines / events as out of scope#9
winebarrel merged 3 commits into
mainfrom
drop-trigger-routine-todos

Conversation

@winebarrel

Copy link
Copy Markdown
Owner

Summary

Triggers, stored procedures / functions, and events are imperative version-tagged code rather than declarative schema. Trying to diff them inside a schema-management tool tends to surface more confusion than value, so they're a non-goal — not "v2 work" sitting on TODO.md.

  • TODO.md: drop the three checkbox items and add a callout block recording the rationale.
  • AGENTS.md: views are now the only object-coverage gap; same out-of-scope callout added.
  • -- myschema:execute entry reframed as the sanctioned escape hatch for trigger/routine/event SQL alongside myschema-managed tables (matches pistachio's pattern for non-modelled objects).
  • Cross-references that mentioned triggers as a future filter target (--enable, --split per object) trimmed accordingly.

Test plan

  • No source changes; go test ./... unaffected
  • Markdown reads cleanly

🤖 Generated with Claude Code

Triggers, stored procedures / functions, and events are imperative
version-tagged code, not declarative schema. Diffing them inside a
schema-management tool tends to surface more confusion than value
(when does a body change matter? how do we order CREATE/DROP next to
DDL?), so they're a non-goal — not "v2 work".

- Drop the three checkbox items from TODO.md and add a quote block
  pointing managers at out-of-band tooling.
- Update AGENTS.md "not yet implemented" so views are the only
  object-coverage gap; add the same out-of-scope callout.
- Reframe the `-- myschema:execute` directive entry as the sanctioned
  escape hatch for trigger/routine/event SQL, since that's the
  pistachio pattern users would copy.
- Trim cross-references that mentioned triggers as a future object
  (filter --enable, --split-per-object).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented May 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 38.75%. Comparing base (c0570c5) to head (3980a49).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main       #9   +/-   ##
=======================================
  Coverage   38.75%   38.75%           
=======================================
  Files          20       20           
  Lines        1321     1321           
=======================================
  Hits          512      512           
  Misses        702      702           
  Partials      107      107           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

MySQL auto-commits DDL, so wrapping plan/apply in BEGIN/COMMIT changes
nothing meaningful. The flag was a documented no-op since day one and
keeping it on the TODO as "implement or remove" was just deferring the
decision. Remove it everywhere:

- ApplyOptions.WithTx field gone from apply.go (kong won't surface
  --with-tx anymore).
- TODO.md: drop both entries (the "implement properly" item under
  "Diff ordering and safety" and the "implement or remove" item under
  "Cleanup").
- AGENTS.md: drop the matching bullet from the "not yet implemented"
  list.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Remove --with-tx flag (out of scope)
@winebarrel winebarrel enabled auto-merge May 2, 2026 02:54
@winebarrel winebarrel merged commit 3ad9d19 into main May 2, 2026
2 checks passed
@winebarrel winebarrel deleted the drop-trigger-routine-todos branch May 2, 2026 02:55
winebarrel added a commit that referenced this pull request May 2, 2026
Two inline comments. Both turn into doc-only changes — no behaviour
change — because the surfaced concerns are intentional design choices
already consistent with the rest of myschema.

diff/rename.go
  - applyColumnRenames doc-comment now states explicitly that
    `RENAME COLUMN` is MySQL 8.0+ syntax, and notes that this matches
    myschema's existing 8.0+ baseline (INVISIBLE indexes, CHECK
    constraints, etc.). Apply against an older server fails loudly
    with a syntax error rather than silently. (Copilot #1.)
  - rewriteIndexColumnRefs doc-comment now documents that
    IndexPart.Expr (functional / expression indexes) is intentionally
    NOT rewritten: rewriting embedded column references inside
    arbitrary SQL expressions requires a real parser, and getting it
    subtly wrong is more dangerous than the alternative. An
    expression index that references a renamed column will surface
    as DROP+CREATE on that one index — functionally correct, rare
    enough that the documented limitation suffices. (Copilot #2.)

AGENTS.md
  - "In scope (v1)" entry for renamed-from now spells out the MySQL
    8.0+ requirement, the full list of cross-cutting rewrites
    (index parts, FK Columns, cross-table FK RefCols, self-ref FK
    RefCols, PK constraint columns), and the documented expression-
    index limitation. Also clarifies that constraint / FK *name*
    renames are out of scope (MySQL has no in-place RENAME for
    these), distinct from constraint definition changes which
    already DROP+ADD.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant