Skip to content

feat(oracle): add Oracle provider with row-level and advisory locks#8

Closed
mnbuhl wants to merge 4 commits intomainfrom
feature/oracle-provider
Closed

feat(oracle): add Oracle provider with row-level and advisory locks#8
mnbuhl wants to merge 4 commits intomainfrom
feature/oracle-provider

Conversation

@mnbuhl
Copy link
Copy Markdown
Owner

@mnbuhl mnbuhl commented Apr 25, 2026

Summary

Adds a fourth database provider (EntityFrameworkCore.Locking.Oracle) following the same 7-file shape as PostgreSQL, MySQL, and SQL Server.

  • Row-level locking via SELECT ... FOR UPDATE [NOWAIT | WAIT n | SKIP LOCKED]. Oracle has no row-level shared lock, so ForShare() throws LockingConfigurationException. Sub-second WAIT timeouts round up to integer seconds.
  • Distributed/advisory locks via DBMS_LOCK.ALLOCATE_UNIQUE + REQUEST/RELEASE (session-scoped, release_on_commit => FALSE). Long keys (>64 UTF-8 bytes) are SHA-256-hashed to lock:<hex58> to stay within the 128-byte ALLOCATE_UNIQUE limit.
  • Prerequisite: the database user needs GRANT EXECUTE ON DBMS_LOCK. Without it, ORA-06550 surfaces and is translated to LockingConfigurationException with remediation text. The integration test fixture performs the grant via SYSTEM.
  • Exception mapping: ORA-00054/ORA-30006LockTimeoutException, ORA-00060DeadlockException, ORA-06550LockingConfigurationException.
  • Integration tests inherit IntegrationTestsBase, ConcurrencyTestsBase, and DistributedLockIntegrationTestsBase. WaitTimeout and DistributedLockAcquireTimeout are overridden to 1s (integer-second granularity).
  • CI: new test-oracle job against gvenzl/oracle-free:23-slim-faststart with a 30-minute timeout for container startup.
  • README/CLAUDE.md updated with Oracle limitations table, DBMS_LOCK grant prerequisite, and supported version (19c+).

Reviewed internally for code quality, security, and architecture — all three perspectives approved after applying fixes: TryAcquireAsync now registers cmd.Cancel() on cancellation for parity with AcquireAsync, and the test fixture pins the admin password explicitly via WithPassword(...) so the grant step cannot silently break on future image changes.

mnbuhl and others added 2 commits April 25, 2026 18:37
- Row-level locking via SELECT ... FOR UPDATE [NOWAIT | WAIT n | SKIP LOCKED].
  Oracle has no row-level shared lock; ForShare throws LockingConfigurationException.
  Sub-second WAIT timeouts round up to integer seconds.
- Distributed/advisory locks via DBMS_LOCK.ALLOCATE_UNIQUE + REQUEST/RELEASE
  (session-scoped, release_on_commit => FALSE). Requires GRANT EXECUTE ON DBMS_LOCK;
  the integration test fixture performs the grant via SYSTEM.
- Exception translator maps ORA-00054/30006 -> LockTimeoutException,
  ORA-00060 -> DeadlockException, ORA-06550 -> LockingConfigurationException.
- Extends the 7-file provider shape used by PostgreSQL/MySQL/SQL Server.
- Integration tests inherit shared bases and run against gvenzl/oracle-free:23-slim-faststart
  via Testcontainers.Oracle.
- CI adds test-oracle job with 30-minute timeout for container startup.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb3d45dc9c

ℹ️ 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".

private const string AcquirePlSql =
"DECLARE lh VARCHAR2(128); rc INTEGER;\n"
+ "BEGIN\n"
+ " DBMS_LOCK.ALLOCATE_UNIQUE(lockname => :name, lockhandle => lh);\n"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid implicit commits in advisory lock SQL

DBMS_LOCK.ALLOCATE_UNIQUE performs a commit, and this block runs it on every lock acquire (and similarly on release), so distributed lock operations are not transaction-neutral. When callers hold an open EF transaction and then call AcquireDistributedLock*/TryAcquireDistributedLock*, pending DML can be committed prematurely and a later rollback will not undo those changes. Please switch to a non-committing path (for example ALLOCATE_UNIQUE_AUTONOMOUS on supported Oracle versions, or a lock-id strategy that avoids ALLOCATE_UNIQUE in the request/release path).

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — confirmed and fixed in 43407d4.

DBMS_LOCK.ALLOCATE_UNIQUE does commit (it may insert into DBMS_LOCK_ALLOCATED), which would cause pending DML on the caller's EF transaction to be committed before the advisory lock even runs. Rolling back later would leave those writes behind.

Rather than using ALLOCATE_UNIQUE_AUTONOMOUS (which would still involve allocation state and the session-level dictionary), I switched the provider to the integer-id overload of DBMS_LOCK.REQUEST/RELEASE, which does not commit. The string key is hashed with XxHash32 into Oracle's user id range [0, 2^30-1] with a namespace prefix — same strategy the PostgreSQL provider already uses for pg_advisory_lock.

Added a regression test (Acquire_InsideOpenTransaction_DoesNotCommitPendingDml) that inserts a row, acquires the lock, releases it, rolls back the transaction, and asserts the row is gone. All 30 Oracle integration tests pass.

README and CLAUDE.md now call out this as an explicit design choice with a "transaction-neutral" guarantee.

mnbuhl and others added 2 commits April 25, 2026 19:12
Tests (29/29 passing locally):
- OracleFixture: build connection string against FREEPDB1 so the app user
  (which gvenzl creates there) can connect. The default Testcontainers.Oracle
  connection string routes to a service the listener does not register (ORA-12514).
- OracleFixture: run GRANT EXECUTE ON SYS.DBMS_LOCK via sqlplus inside the
  container as SYS AS SYSDBA. SYSTEM cannot grant without WITH GRANT OPTION
  (ORA-01031), and running externally hits listener/service-name issues.
- OracleFixture: pin the admin password explicitly via ORACLE_PASSWORD env.
- Mark 4 query-shape tests virtual so Oracle can override them with
  LockingConfigurationException assertions — Oracle rejects FOR UPDATE on
  collection Include, Skip/Take, and OrderBy+Take queries (ORA-02014).

Exception translator:
- Walk the full InnerException chain (not just one level) to find OracleException.
- Map ORA-02014 to LockingConfigurationException with a helpful message.

Benchmarks:
- Add EntityFrameworkCore.Locking.Oracle project reference.
- Add OracleBenchmarkDbContext and Oracle_* benchmarks in SqlGenerationBenchmarks.
- Centralize Oracle.EntityFrameworkCore version in Directory.Packages.props.

README:
- Document Oracle-rejected query shapes (ORA-02014).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DBMS_LOCK.ALLOCATE_UNIQUE performs an implicit commit (it may insert into
DBMS_LOCK_ALLOCATED). Calling it from AcquireDistributedLock* while an EF
transaction was open would commit pending DML prematurely, and a later
rollback would not undo those writes — breaking transaction neutrality.

Switch to the integer-id overload of DBMS_LOCK.REQUEST/RELEASE, which does
not commit. String keys are hashed via XxHash32 into Oracle's user id range
[0, 2^30-1] with a namespace prefix.

Codex P1 feedback on PR #8.

- OracleAdvisoryLockProvider: drop ALLOCATE_UNIQUE; use id-based REQUEST/RELEASE.
- Add System.IO.Hashing package reference (matches PostgreSQL provider).
- Regression test: Acquire_InsideOpenTransaction_DoesNotCommitPendingDml
  asserts an INSERT made before the acquire is rolled back after the acquire.
- Update README and CLAUDE.md to describe the new mechanism and transaction
  neutrality guarantee.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mnbuhl
Copy link
Copy Markdown
Owner Author

mnbuhl commented Apr 27, 2026

Not looking to support Oracle unless someone specifically requests it

@mnbuhl mnbuhl closed this Apr 27, 2026
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