feat: implement Oracle upsert functionality#7068
feat: implement Oracle upsert functionality#7068tkudlicka wants to merge 2 commits intomikro-orm:oracledbfrom
Conversation
|
|
||
| // Oracle returns RETURNING values in outBinds (not rows) | ||
| // Transform: { out_foo: 1, out_bar: 2 } -> [{ foo: 1, bar: 2 }] | ||
| // Multi-row: { out_foo__0: 1, out_bar__0: 2, out_foo__1: 3 } -> [{ foo: 1, bar: 2 }, { foo: 3, ... }] |
There was a problem hiding this comment.
this is already resolved in the OracleConnection
| block2 += ` execute immediate '${sql}' using ${using.join(', ')};\n`; | ||
| } | ||
|
|
||
| block += ' end;'; | ||
| block2 += ' end;'; | ||
|
|
||
| // save raw query without interpolation for logging, | ||
| Object.defineProperty(outBindings, '__rawQuery', { value: block2, writable: true, configurable: true, enumerable: false }); |
|
|
||
| query = this.config.get('onQuery')(query, params); | ||
| const formatted = this.platform.formatQuery(query, params); | ||
| const sql = this.getSql(rawQuery ?? query, formatted, loggerContext); |
There was a problem hiding this comment.
there was a reason for this, cant remember it yet though 🙃
|
some of the existing tests are failing because of the changes in the raw query handling. claude is great, but it tends to run just a subset of tests, so its easy for it to miss it broke some of the existing ones. next time ask it to verify all tests are passing before opening a PR 🙃 |
B4nan
force-pushed
the
oracledb
branch
4 times, most recently
from
aec6553 to
4150fe8
Compare
Implemented Oracle upsert using MERGE statements with RETURNING support via PL/SQL blocks. All 23 upsert tests pass for Oracle. Changes: - Fix embeddable field mapping in MERGE queries by including all update fields in USING clause - Add outBinds to rows transformation to handle Oracle RETURNING values correctly - Use unified PL/SQL block approach for all Oracle versions (11g through 23ai+) - Support ignore mode, merge fields, exclude fields, and WHERE conditions - Optimize single-row vs multi-row RETURNING value transformation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Remove duplicate outBinds transformation from OracleDialect (already in OracleConnection) - Restore addOutputClause for consistency with SQL Server MERGE implementation - This addresses code review feedback on PR mikro-orm#7068 Co-authored-by: Martin Adámek <banan23@gmail.com>
| // Oracle multi-row MERGE doesn't support RETURNING, so extra SELECTs are needed for batch operations | ||
| const expectedCalls = orm.em.getPlatform().usesReturningStatement() ? 10 : 20; | ||
| const oracleMultiplier = type === 'oracledb' ? 2 : 1; | ||
| expect(mock).toHaveBeenCalledTimes(expectedCalls * oracleMultiplier); |
There was a problem hiding this comment.
I've implemented Oracle upsert functionality using MERGE statements. One thing I wanted to run by you:
Current implementation:
- Uses plain MERGE queries (no RETURNING clause)
- This means Oracle needs extra SELECT queries to get the returned data (hence the oracleMultiplier * 2 in tests)
Rationale:
- Oracle 18c/19c/21c don't support MERGE...RETURNING at all
- Oracle 23ai added this support (https://blog.sqlora.com/en/merge-and-dml-returning-clause-in-oracle-23ai/), but most users aren't on it yet
- TypeORM takes the same approach (no RETURNING with MERGE)
Is this approach acceptable, or would you prefer we add version detection for Oracle 23ai+ to use native MERGE...RETURNING?
There was a problem hiding this comment.
I would be fine supporting only the latest version with the modern syntax.
There was a problem hiding this comment.
If something, the old (and I'd imagine less performant) variant should be opt-in. I wouldn't care about feature detection unless there is a simple way to do it (I would rather not query the db because of that, I surely don't want any try/catch logic).
There was a problem hiding this comment.
Sounds good. I’ll implement this only for the newer Oracle versions using the modern syntax and keep the implementation simple, without version detection or fallback logic.
2 participants
Summary
Implements Oracle upsert functionality using MERGE statements.
Implementation
MERGE RETURNING Limitation
Oracle versions before 23ai don't support RETURNING with MERGE statements. The implementation uses plain MERGE queries without RETURNING, requiring extra SELECT queries for batch operations to retrieve inserted/updated data. This is the same approach TypeORM uses.
Note: This can be optimized in the future by detecting Oracle 23ai+ and using native MERGE...RETURNING support (ref).