Add OracleDB support to golang-migrate#1230
Add OracleDB support to golang-migrate#1230JailtonJunior94 wants to merge 2 commits intogolang-migrate:masterfrom
Conversation
| var ( | ||
| specs = []dktesting.ContainerSpec{ | ||
| { | ||
| ImageName: "gvenzl/oracle-free:23.5-slim", Options: oracleOptions(), |
There was a problem hiding this comment.
I'm not familiar with OracleDB, but is it possible to use an official Docker image? Most of the other database tests use official images.
|
Good one, waiting to be approved. |
|
follow up on this |
|
@dhui what needs to be done to make this happen? This feature has a lot of interest, it would surely make many people happy. My team is currently working on a project where Oracle Database is meant to store critical data. We chose to run Flyway in an init container (in Kubernetes environment). While it sure works, it makes lots of things way more complicated. Is there any way that we can help with this PR? |
|
@mauri870 Oracle does not publish an official Docker image on Docker Hub. The Other Oracle images available (like Given these constraints, |
There was a problem hiding this comment.
Pull request overview
Adds an OracleDB database driver to golang-migrate (plus CLI wiring, docs, and integration-style tests) so users can run migrations against Oracle using the standard database.Driver interface.
Changes:
- Introduces a new
database/oracledriver implementing the migrate DB driver interface (Open/Close/Lock/Unlock/Run/Version/SetVersion/Drop). - Adds Oracle integration tests + example migrations (single-statement and multi-statement styles).
- Wires the driver into the CLI via a build tag and documents Oracle usage.
Reviewed changes
Copilot reviewed 36 out of 37 changed files in this pull request and generated 21 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/cli/build_oracle.go | Adds CLI build-tag import so the Oracle driver is included when building with -tags oracle. |
| go.mod | Adds/updates dependencies required for Oracle support and tests. |
| go.sum | Records checksums for newly added/updated dependencies. |
| database/oracle/oracle.go | New Oracle database driver implementation. |
| database/oracle/oracle_test.go | New test suite using dktest + dockerized Oracle for driver behavior coverage. |
| database/oracle/testdata/init.sql | Container init SQL for the Oracle test environment. |
| database/oracle/README.md | Oracle driver documentation, configuration options, and testing instructions. |
| database/oracle/examples/migrations/1085649617_create_users_table.up.sql | Example “up” migration for single-statement mode. |
| database/oracle/examples/migrations/1085649617_create_users_table.down.sql | Example “down” migration for single-statement mode. |
| database/oracle/examples/migrations/1185749658_add_city_to_users.up.sql | Example “up” migration for single-statement mode. |
| database/oracle/examples/migrations/1185749658_add_city_to_users.down.sql | Example “down” migration for single-statement mode. |
| database/oracle/examples/migrations/1285849751_add_index_on_user_emails.up.sql | Example “up” index migration for single-statement mode. |
| database/oracle/examples/migrations/1285849751_add_index_on_user_emails.down.sql | Example “down” index migration for single-statement mode. |
| database/oracle/examples/migrations/1385949617_create_books_table.up.sql | Example “up” migration for single-statement mode. |
| database/oracle/examples/migrations/1385949617_create_books_table.down.sql | Example “down” migration for single-statement mode. |
| database/oracle/examples/migrations/1485949617_create_movies_table.up.sql | Example “up” migration for single-statement mode. |
| database/oracle/examples/migrations/1485949617_create_movies_table.down.sql | Example “down” migration for single-statement mode. |
| database/oracle/examples/migrations/1585849751_just_a_comment.up.sql | Example no-op/comment migration (single-statement set). |
| database/oracle/examples/migrations/1685849751_another_comment.up.sql | Example no-op/comment migration (single-statement set). |
| database/oracle/examples/migrations/1785849751_another_comment.up.sql | Example no-op/comment migration (single-statement set). |
| database/oracle/examples/migrations/1885849751_another_comment.up.sql | Example no-op/comment migration (single-statement set). |
| database/oracle/examples/migrations-multistmt/1085649617_create_users_table.up.sql | Example multi-statement migration exercising separator-based splitting. |
| database/oracle/examples/migrations-multistmt/1085649617_create_users_table.down.sql | Example “down” migration for multi-statement mode. |
| database/oracle/examples/migrations-multistmt/1185749658_add_city_to_users.up.sql | Example multi-statement migration (multiple ALTERs). |
| database/oracle/examples/migrations-multistmt/1185749658_add_city_to_users.down.sql | Example “down” migration for multi-statement mode. |
| database/oracle/examples/migrations-multistmt/1285849751_add_index_on_user_emails.up.sql | Example “up” index migration for multi-statement mode. |
| database/oracle/examples/migrations-multistmt/1285849751_add_index_on_user_emails.down.sql | Example “down” index migration for multi-statement mode. |
| database/oracle/examples/migrations-multistmt/1385949617_create_books_table.up.sql | Example “up” migration for multi-statement mode. |
| database/oracle/examples/migrations-multistmt/1385949617_create_books_table.down.sql | Example “down” migration for multi-statement mode. |
| database/oracle/examples/migrations-multistmt/1485949617_create_movies_table.up.sql | Example “up” migration for multi-statement mode. |
| database/oracle/examples/migrations-multistmt/1485949617_create_movies_table.down.sql | Example “down” migration for multi-statement mode. |
| database/oracle/examples/migrations-multistmt/1585849751_just_a_comment.up.sql | Example no-op/comment migration (multi-statement set). |
| database/oracle/examples/migrations-multistmt/1685849751_another_comment.up.sql | Example no-op/comment migration (multi-statement set). |
| database/oracle/examples/migrations-multistmt/1785849751_another_comment.up.sql | Example no-op/comment migration (multi-statement set). |
| database/oracle/examples/migrations-multistmt/1885849751_another_comment.up.sql | Example no-op/comment migration (multi-statement set). |
| database/oracle/README.md | Documents Oracle driver options, migration-file format, and local/docker test instructions. |
| README.md | Adds Oracle to the top-level list of supported database drivers. |
| Makefile | Adds Oracle to the set of DBs exercised by make test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
database/oracle/oracle.go
Outdated
| // https://docs.oracle.com/cd/B28359_01/appdev.111/b28419/d_lock.htm#ARPLS021 | ||
| query := ` | ||
| declare | ||
| v_lockhandle varchar2(200); | ||
| v_result number; | ||
| begin | ||
| dbms_lock.allocate_unique('control_lock', v_lockhandle); | ||
| v_result := dbms_lock.request(v_lockhandle, dbms_lock.x_mode); | ||
| if v_result <> 0 then | ||
| dbms_output.put_line( | ||
| case | ||
| when v_result=1 then 'Timeout' | ||
| when v_result=2 then 'Deadlock' | ||
| when v_result=3 then 'Parameter Error' | ||
| when v_result=4 then 'Already owned' | ||
| when v_result=5 then 'Illegal Lock Handle' | ||
| end); | ||
| end if; | ||
| end; | ||
| ` | ||
| if _, err := ora.conn.ExecContext(context.Background(), query); err != nil { | ||
| return &database.Error{OrigErr: err, Err: "try lock failed", Query: []byte(query)} | ||
| } | ||
|
|
||
| ora.isLocked = true | ||
| return nil |
There was a problem hiding this comment.
Lock() always sets ora.isLocked = true even when dbms_lock.request fails (non-zero v_result). Because the PL/SQL block only writes to dbms_output and doesn’t raise, ExecContext will still succeed and migrations can proceed without actually holding the lock. Capture v_result and raise an exception (or return it via an OUT bind) when it’s non-zero so the Go code can return an error.
There was a problem hiding this comment.
Fixed. The PL/SQL block now uses raise_application_error instead of dbms_output.put_line, so any non-zero v_result makes ExecContext return an error. Additionally, isLocked was changed to sync/atomic.Bool and the lock/unlock logic uses database.CasRestoreOnErr, which only mutates the field when the PL/SQL call succeeds.
| @@ -0,0 +1 @@ | |||
| DROP TABLE IF EXISTS MOVIES No newline at end of file | |||
There was a problem hiding this comment.
Oracle SQL doesn’t support DROP TABLE IF EXISTS .... This down migration will fail during rollback. Use a PL/SQL block that catches ORA-00942 (or query USER_TABLES) to drop conditionally.
| DROP TABLE IF EXISTS MOVIES | |
| BEGIN | |
| EXECUTE IMMEDIATE 'DROP TABLE MOVIES'; | |
| EXCEPTION | |
| WHEN OTHERS THEN | |
| IF SQLCODE != -942 THEN | |
| RAISE; | |
| END IF; | |
| END; | |
| / |
There was a problem hiding this comment.
Fixed. Replaced with a PL/SQL BEGIN...EXECUTE IMMEDIATE...EXCEPTION WHEN OTHERS block that ignores ORA-00942 (table or view does not exist) and re-raises any other error.
|
|
||
| --- | ||
|
|
||
| DROP TABLE IF EXISTS USERS_MS; |
There was a problem hiding this comment.
Oracle SQL doesn’t support DROP TABLE IF EXISTS .... This statement will fail if the table is absent (and will also make this multi-stmt example misleading). Use a PL/SQL conditional drop that ignores ORA-00942 instead.
| DROP TABLE IF EXISTS USERS_MS; | |
| BEGIN | |
| EXECUTE IMMEDIATE 'DROP TABLE USERS_MS'; | |
| EXCEPTION | |
| WHEN OTHERS THEN | |
| IF SQLCODE != -942 THEN | |
| RAISE; | |
| END IF; | |
| END; | |
| / |
There was a problem hiding this comment.
Fixed. Replaced with a PL/SQL conditional drop block that catches ORA-00942 and re-raises any other exception.
| //go:build oracle | ||
| // +build oracle | ||
|
|
There was a problem hiding this comment.
Other internal/cli/build_*.go files use only the //go:build <tag> constraint. Keeping the legacy // +build oracle line here is inconsistent and can cause build-constraint drift over time; consider removing it to match the repo’s convention.
There was a problem hiding this comment.
Fixed. The legacy // +build oracle line has been removed. The file now contains only the //go:build oracle constraint, consistent with all other internal/cli/build_*.go files.
database/oracle/oracle.go
Outdated
| dbms_lock.allocate_unique('control_lock', v_lockhandle); | ||
| v_result := dbms_lock.request(v_lockhandle, dbms_lock.x_mode); |
There was a problem hiding this comment.
The lock name 'control_lock' is a single global identifier. This can cause unrelated migrations (different databases/schemas/migrations tables) to block each other. Use a lock name derived from the target (e.g., database/service + migrations table) so independent migration sets don’t contend.
There was a problem hiding this comment.
Fixed. The lock name is now derived from the migrations table: <table>_migrate_lock. We also added a dbmsLockName() helper that caps the prefix to 115 chars (128 − len("_migrate_lock")) before appending the suffix, so the total never exceeds Oracle's 128-char limit for dbms_lock.allocate_unique.
| @@ -0,0 +1 @@ | |||
| DROP TABLE IF EXISTS MOVIES_MS No newline at end of file | |||
There was a problem hiding this comment.
Oracle SQL doesn’t support DROP TABLE IF EXISTS .... This down migration will fail during rollback. Use a PL/SQL block that catches ORA-00942 (or checks USER_TABLES) to drop conditionally.
| DROP TABLE IF EXISTS MOVIES_MS | |
| BEGIN | |
| EXECUTE IMMEDIATE 'DROP TABLE MOVIES_MS'; | |
| EXCEPTION | |
| WHEN OTHERS THEN | |
| IF SQLCODE != -942 THEN | |
| RAISE; | |
| END IF; | |
| END; | |
| / |
There was a problem hiding this comment.
Fixed. Replaced with a PL/SQL block that catches ORA-00942 and re-raises for any other error.
| @@ -0,0 +1,2 @@ | |||
| DROP TABLE IF EXISTS USERS_MS; | |||
There was a problem hiding this comment.
Oracle SQL doesn’t support DROP TABLE IF EXISTS .... This down migration will fail during rollback. Use a PL/SQL block that catches ORA-00942 (or checks USER_TABLES) to drop conditionally.
| DROP TABLE IF EXISTS USERS_MS; | |
| BEGIN | |
| EXECUTE IMMEDIATE 'DROP TABLE USERS_MS'; | |
| EXCEPTION | |
| WHEN OTHERS THEN | |
| IF SQLCODE != -942 THEN | |
| RAISE; | |
| END IF; | |
| END; | |
| / |
There was a problem hiding this comment.
Fixed. Replaced with a PL/SQL block that catches ORA-00942 and re-raises for any other error.
| func (ora *Oracle) Version() (version int, dirty bool, err error) { | ||
| query := "SELECT VERSION, DIRTY FROM " + ora.config.MigrationsTable + " WHERE ROWNUM = 1 ORDER BY VERSION desc" | ||
| err = ora.conn.QueryRowContext(context.Background(), query).Scan(&version, &dirty) | ||
| switch { | ||
| case err == sql.ErrNoRows: | ||
| return database.NilVersion, false, nil | ||
| case err != nil: | ||
| return 0, false, &database.Error{OrigErr: err, Query: []byte(query)} | ||
| default: | ||
| return version, dirty, nil | ||
| } |
There was a problem hiding this comment.
In Oracle, ROWNUM = 1 is applied before ORDER BY, so this query won’t reliably return the highest version (and can even be rejected depending on the optimizer/version). Use a subquery (ORDER BY inside) and apply ROWNUM = 1 on the outer query to reliably fetch the latest version.
There was a problem hiding this comment.
Fixed. Replaced the incorrect WHERE ROWNUM = 1 ORDER BY VERSION DESC (ROWNUM is applied before ORDER BY in Oracle, so it would return an arbitrary row, not necessarily the latest) with ORDER BY VERSION DESC FETCH FIRST 1 ROW ONLY, which correctly retrieves the highest version. Note: FETCH FIRST requires Oracle 12c+, which matches the minimum version documented in the README.
| @@ -0,0 +1 @@ | |||
| DROP TABLE IF EXISTS BOOKS No newline at end of file | |||
There was a problem hiding this comment.
Oracle SQL doesn’t support DROP TABLE IF EXISTS .... This down migration will fail during rollback. Use a PL/SQL block that catches ORA-00942 (or query USER_TABLES) to drop conditionally.
| DROP TABLE IF EXISTS BOOKS | |
| BEGIN | |
| EXECUTE IMMEDIATE 'DROP TABLE BOOKS'; | |
| EXCEPTION | |
| WHEN OTHERS THEN | |
| IF SQLCODE != -942 THEN | |
| RAISE; | |
| END IF; | |
| END; | |
| / |
There was a problem hiding this comment.
Fixed. Replaced with a PL/SQL block that catches ORA-00942 and re-raises for any other error.
database/oracle/README.md
Outdated
| 2. Go into the sqlplus console | ||
|
|
||
| ```bash | ||
| $ docker exec -it orclxe bash |
There was a problem hiding this comment.
This docker-compose example uses container_name: oracle-db, but the next step runs docker exec -it orclxe ..., which won’t work as written. Update the command to match the declared container name (or update the compose snippet) so the instructions are runnable.
| $ docker exec -it orclxe bash | |
| $ docker exec -it oracle-db bash |
There was a problem hiding this comment.
Fixed. Updated the docker exec command to use oracle-db to match the container_name declared in the compose snippet.
## Bug fixes
### oracle.go
- Fix Lock()/Unlock() silently ignoring dbms_lock errors: replaced
dbms_output.put_line with raise_application_error so any non-zero
v_result causes ExecContext to return an error to the Go caller
- Fix Lock() setting isLocked=true even when the PL/SQL lock request
fails: changed isLocked from plain bool to sync/atomic.Bool and
replaced manual state mutation with database.CasRestoreOnErr, which
performs a compare-and-swap only when the inner function succeeds
- Fix global lock name 'control_lock' causing unrelated migration sets
to block each other: lock name is now derived from the migrations
table (<table>_migrate_lock). Added dbmsLockName() helper that
truncates the prefix to 115 chars so the total never exceeds the
128-char limit of dbms_lock.allocate_unique
- Fix Version() returning wrong row: WHERE ROWNUM = 1 ORDER BY VERSION
DESC is incorrect in Oracle because ROWNUM filtering is applied before
ORDER BY, potentially returning an arbitrary row. Replaced with
ORDER BY VERSION DESC FETCH FIRST 1 ROW ONLY (requires Oracle 12c+,
which is the documented minimum)
- Fix SetVersion() using TRUNCATE inside a transaction: TRUNCATE is DDL
in Oracle and issues an implicit commit, breaking the atomicity of the
clear + insert pair. Changed to DELETE FROM
- Fix scanner.Err() unchecked in removeComments and parseMultiStatements:
a read error or token-too-long condition would silently return partial
SQL with a nil error. Both functions now check and return scanner.Err()
after the scan loop
- Fix tables.Err() unchecked in Drop(): added post-loop rows.Err() check
to detect iteration errors before executing DROP TABLE statements
- Add input validation for migrations table name via validateMigrationsTable
(regex ^[A-Z][A-Z0-9_$#]{0,127}$) to prevent SQL injection through
the x-migrations-table URL parameter
- Add compile-time interface assertion: var _ database.Driver = (*Oracle)(nil)
- Replace hashicorp/go-multierror with stdlib errors.Join (Go 1.20+),
reducing external dependencies
- Extract parseURLParams helper from Open() for cleaner separation and
independent testability
- Fix Open() not closing db on WithInstance failure (resource leak)
## Migration file fixes (Oracle compatibility)
All .down.sql files used DROP TABLE IF EXISTS / DROP INDEX IF EXISTS,
which Oracle does not support. Replaced every occurrence with a PL/SQL
BEGIN...EXECUTE IMMEDIATE...EXCEPTION WHEN OTHERS block:
- ORA-00942 (table or view does not exist) is silenced for DROP TABLE
- ORA-01418 (specified index does not exist) is silenced for DROP INDEX
- All other error codes are re-raised
Affected files:
examples/migrations/1085649617_create_users_table.down.sql
examples/migrations/1285849751_add_index_on_user_emails.down.sql
examples/migrations/1385949617_create_books_table.down.sql
examples/migrations/1485949617_create_movies_table.down.sql
examples/migrations-multistmt/1085649617_create_users_table.down.sql
examples/migrations-multistmt/1085649617_create_users_table.up.sql
examples/migrations-multistmt/1385949617_create_books_table.down.sql
examples/migrations-multistmt/1485949617_create_movies_table.down.sql
Also added missing trailing --- separator to
examples/migrations-multistmt/1485949617_create_movies_table.down.sql
for consistency with all other multi-statement files.
## Test improvements (oracle_test.go)
- Fix HostPort binding: changed "0/tcp" to "0" — the /tcp suffix is
for the container port key, not the host binding value
- Add TestOpen_InvalidURL: ensures malformed URLs return an error
- Add TestOpen_CustomParams: table-driven tests for x-migrations-table,
x-multi-stmt-enabled, and x-multi-stmt-separator URL parameters
- Add TestOpen_InvalidMultiStmtEnabled: ensures invalid bool value errors
- Add TestOpen_InvalidMigrationsTable: injection/invalid-identifier cases
(single quote, starts with digit, space, semicolon)
- Add TestRemoveComments: table-driven unit tests for the comment-stripping
helper (empty input, only comments, mixed, no comments, inline dash)
- Fix double blank line before TestRemoveComments
## Documentation (database/oracle/README.md)
- Correct driver references from godror/godror to sijms/go-ora throughout
- Fix typos: "spilt" → "split", "some case" → "some cases",
"very time expense" → "very time-consuming"
- Update tested version from "18-xe" to "Oracle 23c Free (23.5)" and
document Oracle 12c as minimum required for FETCH FIRST syntax
- Fix docker exec example: "orclxe" → "oracle-db" to match the
container_name declared in the compose snippet
## Other
- README.md: remove stray blank bullet point after Oracle entry
- Makefile: add oracle to DATABASE list so it is included in
make build / build-cli (sijms/go-ora is pure Go, CGO_ENABLED=0 safe)
- internal/cli/build_oracle.go: remove legacy // +build oracle line,
keeping only the modern //go:build oracle constraint, consistent
with all other build_*.go files in the package
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add OracleDB support to golang-migrate
This PR implements support for OracleDB in golang-migrate. The main changes include:
Open,Close,Lock,Unlock,Run,SetVersion,Version, andDrop.How to Test: