From d5bcd18e050f3de33dc81a6b8ba64315aeb71ba4 Mon Sep 17 00:00:00 2001 From: Lukas Eder Date: Mon, 23 Oct 2023 16:29:45 +0200 Subject: [PATCH] [jOOQ/jOOQ#8283] BatchCRUD does not update optimistic locking version and timestamp values in UpdatableRecord --- .../main/java/org/jooq/impl/BatchCRUD.java | 45 +++++++++++++++++-- .../java/org/jooq/impl/TableRecordImpl.java | 31 ++++++++----- .../org/jooq/impl/UpdatableRecordImpl.java | 26 +++++++---- 3 files changed, 80 insertions(+), 22 deletions(-) diff --git a/jOOQ/src/main/java/org/jooq/impl/BatchCRUD.java b/jOOQ/src/main/java/org/jooq/impl/BatchCRUD.java index 44ef4e6afc..1e56874cc7 100644 --- a/jOOQ/src/main/java/org/jooq/impl/BatchCRUD.java +++ b/jOOQ/src/main/java/org/jooq/impl/BatchCRUD.java @@ -37,8 +37,11 @@ */ package org.jooq.impl; +import static java.lang.Boolean.TRUE; import static org.jooq.conf.SettingsTools.executeStaticStatements; +import java.math.BigInteger; +import java.sql.Timestamp; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; @@ -114,7 +117,9 @@ final class BatchCRUD extends AbstractBatch { } private final int[] executePrepared() { + boolean optimisticLocking = TRUE.equals(configuration.settings().isExecuteWithOptimisticLocking()); Map> queries = new LinkedHashMap<>(); + List signals = new ArrayList<>(); QueryCollector collector = new QueryCollector(); // Add the QueryCollector to intercept query execution after rendering @@ -126,8 +131,14 @@ final class BatchCRUD extends AbstractBatch { try { records[i].attach(local); executeAction(i); + + if (optimisticLocking) + signals.add(null); } catch (QueryCollectorSignal e) { + if (optimisticLocking) + signals.add(e); + Query query = e.getQuery(); String sql = e.getSQL(); @@ -162,12 +173,18 @@ final class BatchCRUD extends AbstractBatch { for (int i = 0; i < result.size(); i++) array[i] = result.get(i); + // [#8283] Store back optimistic locking values to updated records + if (optimisticLocking) + updateRecordVersionsAndTimestamps(signals, array); + updateChangedFlag(); return array; } private final int[] executeStatic() { + boolean optimisticLocking = TRUE.equals(configuration.settings().isExecuteWithOptimisticLocking()); List queries = new ArrayList<>(); + List signals = new ArrayList<>(); QueryCollector collector = new QueryCollector(); Configuration local = deriveConfiguration(collector); @@ -177,8 +194,14 @@ final class BatchCRUD extends AbstractBatch { try { records[i].attach(local); executeAction(i); + + if (optimisticLocking) + signals.add(null); } catch (QueryCollectorSignal e) { + if (optimisticLocking) + signals.add(e); + Query query = e.getQuery(); if (query.isExecutable()) @@ -191,10 +214,24 @@ final class BatchCRUD extends AbstractBatch { // Resulting statements can be batch executed in their requested order int[] result = dsl.batch(queries).execute(); + + // [#8283] Store back optimistic locking values to updated records + if (optimisticLocking) + updateRecordVersionsAndTimestamps(signals, result); + updateChangedFlag(); return result; } + private final void updateRecordVersionsAndTimestamps(List signals, int[] array) { + for (int i = 0; i < records.length && i < array.length; i++) { + QueryCollectorSignal signal = signals.get(i); + + if (signal != null && array[i] > 0) + ((TableRecordImpl) records[i]).setRecordVersionAndTimestamp(signal.version, signal.timestamp); + } + } + private final void executeAction(int i) { switch (action) { case STORE: @@ -281,9 +318,11 @@ final class BatchCRUD extends AbstractBatch { * This exception is used as a signal for jOOQ's internals to abort query * execution, and return generated SQL back to batch execution. */ - private static class QueryCollectorSignal extends ControlFlowSignal { - private final String sql; - private final Query query; + static class QueryCollectorSignal extends ControlFlowSignal { + final String sql; + final Query query; + BigInteger version; + Timestamp timestamp; QueryCollectorSignal(String sql, Query query) { this.sql = sql; diff --git a/jOOQ/src/main/java/org/jooq/impl/TableRecordImpl.java b/jOOQ/src/main/java/org/jooq/impl/TableRecordImpl.java index 93c97de95e..776f680982 100644 --- a/jOOQ/src/main/java/org/jooq/impl/TableRecordImpl.java +++ b/jOOQ/src/main/java/org/jooq/impl/TableRecordImpl.java @@ -97,8 +97,8 @@ import org.jooq.conf.Settings; import org.jooq.conf.SettingsTools; import org.jooq.conf.WriteIfReadonly; import org.jooq.exception.DataTypeException; +import org.jooq.impl.BatchCRUD.QueryCollectorSignal; import org.jooq.tools.JooqLogger; -import org.jooq.tools.StringUtils; /** * A record implementation for a record originating from a single table @@ -194,22 +194,31 @@ public class TableRecordImpl> extends AbstractQualified // [#1002] Consider also identity columns of non-updatable records // [#1537] Avoid refreshing identity columns on batch inserts Collection> key = setReturningIfNeeded(insert); - int result = insert.execute(); + try { + int result = insert.execute(); - if (result > 0) { - for (Field changedField : changedFields) - changed(changedField, false); + if (result > 0) { + for (Field changedField : changedFields) + changed(changedField, false); - // [#1596] If insert was successful, update timestamp and/or version columns - setRecordVersionAndTimestamp(version, timestamp); + // [#1596] If insert was successful, update timestamp and/or version columns + setRecordVersionAndTimestamp(version, timestamp); - // [#1859] If an insert was successful try fetching the generated values. - getReturningIfNeeded(insert, key); + // [#1859] If an insert was successful try fetching the generated values. + getReturningIfNeeded(insert, key); - fetched = true; + fetched = true; + } + + return result; } - return result; + // [#8283] Pass optimistic locking information on to BatchCRUD, if applicable + catch (QueryCollectorSignal e) { + e.version = version; + e.timestamp = timestamp; + throw e; + } } final void getReturningIfNeeded(StoreQuery query, Collection> key) { diff --git a/jOOQ/src/main/java/org/jooq/impl/UpdatableRecordImpl.java b/jOOQ/src/main/java/org/jooq/impl/UpdatableRecordImpl.java index dcfce8cf2f..19eea78015 100644 --- a/jOOQ/src/main/java/org/jooq/impl/UpdatableRecordImpl.java +++ b/jOOQ/src/main/java/org/jooq/impl/UpdatableRecordImpl.java @@ -86,6 +86,7 @@ import org.jooq.UpdatableRecord; import org.jooq.conf.UpdateUnchangedRecords; import org.jooq.exception.DataChangedException; import org.jooq.exception.NoDataFoundException; +import org.jooq.impl.BatchCRUD.QueryCollectorSignal; import org.jooq.tools.JooqLogger; import org.jooq.tools.StringUtils; @@ -351,18 +352,27 @@ public class UpdatableRecordImpl> extends TableReco ? null : setReturningIfNeeded(query); - int result = query.execute(); - checkIfChanged(result, version, timestamp); + try { + int result = query.execute(); + checkIfChanged(result, version, timestamp); - if (result > 0) { - for (Field changedField : changedFields) - changed(changedField, false); + if (result > 0) { + for (Field changedField : changedFields) + changed(changedField, false); - // [#1859] If an update was successful try fetching the generated - getReturningIfNeeded(query, key); + // [#1859] If an update was successful try fetching the generated + getReturningIfNeeded(query, key); + } + + return result; } - return result; + // [#8283] Pass optimistic locking information on to BatchCRUD, if applicable + catch (QueryCollectorSignal e) { + e.version = version; + e.timestamp = timestamp; + throw e; + } } @Override