rzo1 opened a new pull request, #216:
URL: https://github.com/apache/openjpa/pull/216
A bulk `UPDATE`/`DELETE` that cannot be expressed as one statement is
executed in memory, so the result is a modified persistence context rather than
a modified database. The flush added in 4.2.0 is what carries the row count
`executeUpdate()` reports to the database — measured on Derby: without it only
the candidate `SELECT` is emitted, and an `em.clear()` discards the operation
after the count was already reported. Section 4.10 mandates neither behaviour
("the persistence context is not synchronized with the result of the bulk
update or delete"), so the flush stays the default and becomes escapable
through the new `FlushAfterInMemoryBulkOperation` compatibility option.
Suppressing it does not suppress the in-memory application: the instances are
still modified and written at commit.
Review turned up something worse than the ticket. `Broker.flush()` *rejects*
an incremental flush when the store does not support `OPTION_INC_FLUSH` — which
`AbstractStoreManager` does not by default, so **openjpa-xmlstore failed every
bulk delete** — and when a savepoint is set that the `SavepointManager` cannot
flush incrementally. Both rejections marked the transaction rollback-only, so
where 4.1.x applied the operation, 4.2.0 lost the transaction. The operation
now checks first and skips the flush with an INFO log.
Two things worth your attention:
1. **It adds kernel API.** `Broker.isFlushSupported()`, a `default` method
returning true, overridden in `BrokerImpl` and `DelegatingBroker`. A
third-party `Broker` keeps the default and would still hit the exception. I
would rather have avoided new API, but discriminating the rejection after the
fact is not possible — `DelegatingBroker.flush()` wraps the
`UnsupportedException` into a generic `PersistenceException`, so catching it
does not work.
2. **A targeted flush is not feasible today.** `BrokerImpl.flush(int)` is
protected and always operates on the whole transactional cache, firing the
flush events over it and clearing the flush flags, and the `Broker` interface
exposes only `flush()`/`preFlush()`. Flushing the affected instances alone
needs more kernel API; left as a follow-up rather than bodged, so unrelated
pending changes still ride along — which the migration note now says.
35 tests in the bulk package pass on both the Derby and H2 profiles; 12 are
new, and 6 of those fail with the kernel part reverted (the other 6 pin
behaviour 4.2.0 already introduced). The documented savepoint fallback covers
the flush *after* the operation only — the pre-flush before the candidate query
is not guarded against a savepoint, so that case still fails as before, and
both the guide and the javadoc now say so.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
