fix(oracle): sequence keywords, create order, and tolerant index/trigger drops - #266
Merged
Merged
Conversation
Same treatment DB2 got: the live DDL suite plus the shipped DEMO_A/DEMO_B samples, migrated into a throwaway schema and re-compared. Oracle passed every CREATE, ALTER and routine round trip already; the samples found three more. **`NO CYCLE` / `NO CACHE` are ORA-03049 on Oracle.** It spells them `NOCYCLE` and `NOCACHE`, one word. The spaced form Postgres and DB2 accept killed the CREATE SEQUENCE, and everything that leaned on it went down too: the table whose column default calls the sequence (ORA-02289), then the views over that table. Now behind `unspacedSequenceNoKeywords`, applied to both the create and alter paths. **Tables were created before the sequences they default to.** `sortAddedByDependency` only understands foreign keys, so `DEFAULT order_seq.NEXTVAL` was not a dependency it could see. Sequences, types and roles now go ahead of tables — none of them can depend on a table, so first is always safe. **Index and trigger drops were the only intolerant ones.** The dialect already has `oracleDrop` (version-aware: `IF EXISTS` on 23+, a SQLCODE guard below) and uses it for TABLE, VIEW, SEQUENCE, FUNCTION and PROCEDURE — but DROP INDEX and DROP TRIGGER were emitted bare, so ORA-01418 / ORA-04080 on an object that had already gone with its table failed the step. Also: a function-based index reads as its hidden `SYS_NC00006$` placeholder in ALL_IND_COLUMNS (the expression lives in ALL_IND_EXPRESSIONS, which the provider does not read). Emitting that name is ORA-00904, so the generator now skips it with `-- review:` instead of shipping DDL that cannot run. Capturing the real expression is the deeper fix and is not attempted here. End state on the Oracle samples: 18/18 steps execute and re-comparing after the migration reports **no differences**. All five utilities work, and DEMO_A is untouched. One environment note, not a code change: `docker/init/oracle/01_seed.sh` seeds `FREEPDB1` while compose provisions `ORACLE_DATABASE: FOXDB`, so the demo schemas land in a different PDB from the one the app config points at. The samples exist — just not where FOXDB is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_6c4a1db9-e091-4cf1-a1c1-df898baf0e63) |
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.
Follow-on to #264 (merged while this was in progress). Same harness, pointed at
Oracle — the last dialect in
docker composewith genuinely distinctive DDL.Oracle already passed every CREATE, ALTER and procedure/function round trip the
moment it was added to the live suite. The shipped DEMO_A/DEMO_B samples
found three bugs that the synthetic cases could not.
Bugs
NO CYCLE/NO CACHEare ORA-03049 on Oracle. Oracle spells themNOCYCLEandNOCACHE, one word each. The spaced form — correct on Postgresand DB2 — killed
CREATE SEQUENCE, and everything leaning on it went downafter: the table whose column default calls the sequence (ORA-02289), then the
views over that table. Now behind an
unspacedSequenceNoKeywordsdialect flag,applied to both the create and alter paths.
Tables were created before the sequences they default to.
sortAddedByDependencyunderstands foreign keys and nothing else, soDEFAULT order_seq.NEXTVALwas invisible to it. Sequences, types and roles nowsort ahead of tables; none of them can depend on a table, so first is always
safe.
Index and trigger drops were the only intolerant ones left. The dialect
already has
oracleDrop— version-aware,DROP … IF EXISTSon 23+ and aSQLCODE guard below — and uses it for TABLE, VIEW, SEQUENCE, FUNCTION and
PROCEDURE.
DROP INDEXandDROP TRIGGERwere emitted bare, so ORA-01418 /ORA-04080 on an object that had already gone with its table failed the step.
Known limitation, made visible instead of fatal
A function-based index reads as its hidden
SYS_NC00006$placeholder inALL_IND_COLUMNS; the real expression lives inALL_IND_EXPRESSIONS, which theprovider does not read. Emitting that placeholder is ORA-00904, so the generator
now skips such an index with a
-- review:note rather than shipping DDL thatcannot run. Capturing the actual expression is the deeper fix and is not
attempted here.
Verification
migration reports no differences. All five DBA utilities (pool, sessions,
system, sizes, index-fragmentation) return real data. DEMO_A is untouched.
npx vitest run— 1798 passedcd apps/web && npx tsc --noEmitclean; eslint 0 errorsEnvironment note (no code change)
docker/init/oracle/01_seed.shseedsFREEPDB1, while compose provisionsORACLE_DATABASE: FOXDB. The demo schemas do get created — just in a differentPDB from the one the app config points at, so
FOXDBlooks empty. Worthreconciling separately; I left both alone.
🤖 Generated with Claude Code
Note
Medium Risk
Changes core migration ordering and sequence rendering for all dialects that opt into the flag; Oracle-specific drop behavior is broader but aligned with existing tolerant drops. Low risk for non-Oracle engines unless they set the new flag.
Overview
Fixes Oracle migration failures surfaced by DEMO_A/DEMO_B samples and extends the live DDL harness to Oracle.
Sequence DDL now uses
NOCYCLE/NOCACHEwhen the dialect setsunspacedSequenceNoKeywords(Oracle), on both CREATE and ALTER paths—replacingNO CYCLE/NO CACHE, which Oracle rejects as ORA-03049 and breaks dependent tables and views.CREATE ordering for added objects runs sequences, types, and roles before tables (and MQTs), so defaults like
order_seq.NEXTVALdo not hit ORA-02289 because FK-only sorting missed sequence dependencies.Oracle drops for indexes and triggers use the same
oracleDropSQLCODE-guard pattern as other object types, avoiding ORA-01418 / ORA-04080 when the object was already removed with its table.Function-based indexes whose introspection only yields hidden
SYS_NC…$placeholders are skipped with a-- review:comment instead of emitting invalid CREATE INDEX DDL.The live integration test adds Oracle (DUAL probe) and a routine round-trip spec (second user via
system).Reviewed by Cursor Bugbot for commit 722eab2. Bugbot is set up for automated code reviews on this repo. Configure here.