feat(oracle): add Oracle provider with row-level and advisory locks#8
feat(oracle): add Oracle provider with row-level and advisory locks#8
Conversation
- 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>
There was a problem hiding this comment.
💡 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" |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
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.
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>
|
Not looking to support Oracle unless someone specifically requests it |
Summary
Adds a fourth database provider (
EntityFrameworkCore.Locking.Oracle) following the same 7-file shape as PostgreSQL, MySQL, and SQL Server.SELECT ... FOR UPDATE [NOWAIT | WAIT n | SKIP LOCKED]. Oracle has no row-level shared lock, soForShare()throwsLockingConfigurationException. Sub-secondWAITtimeouts round up to integer seconds.DBMS_LOCK.ALLOCATE_UNIQUE+REQUEST/RELEASE(session-scoped,release_on_commit => FALSE). Long keys (>64 UTF-8 bytes) are SHA-256-hashed tolock:<hex58>to stay within the 128-byteALLOCATE_UNIQUElimit.GRANT EXECUTE ON DBMS_LOCK. Without it,ORA-06550surfaces and is translated toLockingConfigurationExceptionwith remediation text. The integration test fixture performs the grant viaSYSTEM.ORA-00054/ORA-30006→LockTimeoutException,ORA-00060→DeadlockException,ORA-06550→LockingConfigurationException.IntegrationTestsBase,ConcurrencyTestsBase, andDistributedLockIntegrationTestsBase.WaitTimeoutandDistributedLockAcquireTimeoutare overridden to 1s (integer-second granularity).test-oraclejob againstgvenzl/oracle-free:23-slim-faststartwith a 30-minute timeout for container startup.DBMS_LOCKgrant prerequisite, and supported version (19c+).Reviewed internally for code quality, security, and architecture — all three perspectives approved after applying fixes:
TryAcquireAsyncnow registerscmd.Cancel()on cancellation for parity withAcquireAsync, and the test fixture pins the admin password explicitly viaWithPassword(...)so the grant step cannot silently break on future image changes.