Repository navigation
fix(test): oracle populate using oracle 19c and lower - #2864
Merged
bpamiri merged 3 commits intoJun 6, 2026
Merged
Conversation
…ersions don't have that syntax
mikegrogan
changed the title
mikegrogan
changed the title
bpamiri
reviewed
Jun 5, 2026
bpamiri
left a comment
bpamiri
left a comment
Collaborator
There was a problem hiding this comment.
👋 Thanks for the fix, @mikegrogan — and welcome as a first-time contributor! Solid catch: Oracle only added IF EXISTS DDL modifiers in 23c, so DROP TABLE IF EXISTS is a hard parse error on 19c/21c.
Verdict: looks good — one process item before merge
The logic is correct, and the Lucee 7 + SQLite CI job passed on your latest commit bd2d84a. (It currently shows skipping only because editing the PR title re-runs CI as an edited event, which intentionally bypasses the test job — the earlier opened run on the same commit is green. The one earlier red was the PR-title commitlint check, which you've already fixed.)
Correctness ✅
local.dbinfoandlocal.dbare both set earlier in the file (lines 4 and 12), anddatabase_versionis a validcfdbinfo type="version"key — the existing H2 branch already reads it.Val(ListFirst(...))keeps the parse safe on every engine, and it's only consulted in the Oracle branch.- Both
DROP TABLE IF EXISTSsites in this file are covered, and you correctly leftvendor/wheels/tests/populate.cfmalone — that one already drops Oracle tables viacfdbinfo type="tables"+CASCADE CONSTRAINTS PURGE, so it never had this bug.
Optional polish (non-blocking)
The guard is inlined twice; if you feel like it you could compute it once near dbMajorVersion:
<cfset local.dropIfExists = (local.db IS NOT "oracle" OR local.dbMajorVersion GTE 23) ? " IF EXISTS" : "">
...
<cfquery ...>DROP TABLE#local.dropIfExists# c_o_r_e_posts</cfquery>Purely cosmetic — happy to merge as-is.
Before merge
- DCO sign-off — our contribution flow needs every commit to carry
Signed-off-by:. Please re-commit with sign-off (e.g.git rebase --signoff developthen force-push the branch). See CONTRIBUTING.md.
ℹ️ Heads-up (not for this PR): the same Oracle-<23 IF EXISTS pattern lives in framework code at vendor/wheels/databaseAdapters/Oracle/OracleMigrator.cfc:96 — we'll patch that separately so migrate down / rollback works on 19c/21c too.
Thanks again for contributing! 🙏
Contributor
Author
|
I actually had a commit like 10 years ago, but it's been a while! Thanks. |
This was referenced Jun 6, 2026
bpamiri
added a commit
that referenced
this pull request
Jun 6, 2026
* fix(migrator): make Oracle DROP TABLE/VIEW work on Oracle <23c Oracle only added the IF EXISTS DDL modifier in 23c, so OracleMigrator's dropTable() — which emitted "DROP TABLE IF EXISTS ... CASCADE CONSTRAINTS" unconditionally — produced a hard parse error (ORA-00933) on 19c/21c. The remove-table migration template re-throws on error, so the whole migration failed. dropView() inherited the same IF EXISTS bug from Abstract. Wrap both in the version-agnostic Oracle PL/SQL idiom that runs the bare DROP and swallows ORA-00942, preserving "drop if exists" semantics on every supported Oracle version. $execute never splits on ';' and skips the trailing-semicolon append for Oracle, so the anonymous block reaches the driver intact. Validated on Lucee 7 + SQLite via a new unit spec asserting the generated SQL (OracleMigratorDropSpec); real-Oracle execution is covered by the soft-fail Oracle compat-matrix job. Framework-side counterpart to the demo-app test-populate fix in #2864. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * docs: add CHANGELOG entry for Oracle <23c migrator DROP fix Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * test(migrator): assert ORA-00942 guard in OracleMigrator dropView spec Addresses Reviewer A's nit on #2869: the dropView assertions mirror the dropTable block but omitted the -942 (table/view-does-not-exist) guard check. Verified 252/0/0 on the migrator bundle (Lucee 7 + SQLite). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> --------- Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.
Summary
fix(test): resolve oracle ddl syntax errors in test suite population. Tested against Ran against oracle 19c. Oracle 23c and beyond there is no needed fix. the "if exists" got added to their create table syntax.
https://database.guide/oracle-adds-support-for-if-exists-and-if-not-exists-syntax-modifiers/
Related Issue
Closes #
Type of Change
Feature Completeness Checklist
Signed-off-by:(usegit commit -s); see CONTRIBUTING.mdweb/sites/guides/src/content/docs/v4-0-0-snapshot/with matching sidebar entry.ai/wheels/directory[Unreleased]section/wheels/tests/core?format=json)Test Plan
Ran against oracle 19c
Screenshots / Output