diff --git a/pom.xml b/pom.xml index af70ccef..5a8fbc89 100644 --- a/pom.xml +++ b/pom.xml @@ -151,5 +151,13 @@ 5.0.1 true + + + + com.h2database + h2 + 2.2.224 + test + diff --git a/src/main/java/org/datanucleus/store/rdbms/adapter/DatastoreAdapter.java b/src/main/java/org/datanucleus/store/rdbms/adapter/DatastoreAdapter.java index eb5d64ea..81fb0195 100644 --- a/src/main/java/org/datanucleus/store/rdbms/adapter/DatastoreAdapter.java +++ b/src/main/java/org/datanucleus/store/rdbms/adapter/DatastoreAdapter.java @@ -410,6 +410,9 @@ public interface DatastoreAdapter public static final String INCLUDE_TABLE_INDEX_STATISTICS = "IncludeTableIndexStatistics"; + /** Whether this datastore supports ORDER BY in UPDATE statements (required to avoid unique constraint violations on row-order-dependent updates). */ + public static final String ORDER_BY_IN_UPDATE_STATEMENT = "OrderByInUpdateStatement"; + /** * Initialise the datastore adapter. * @param handler SchemaHandler that we initialise the types for diff --git a/src/main/java/org/datanucleus/store/rdbms/adapter/H2Adapter.java b/src/main/java/org/datanucleus/store/rdbms/adapter/H2Adapter.java index 8801f4b3..65684605 100644 --- a/src/main/java/org/datanucleus/store/rdbms/adapter/H2Adapter.java +++ b/src/main/java/org/datanucleus/store/rdbms/adapter/H2Adapter.java @@ -101,6 +101,7 @@ public H2Adapter(DatabaseMetaData metadata) // Create index before FK to avoid duplication since H2 automatically creates index for FK supportedOptions.add(CREATE_INDEXES_BEFORE_FOREIGN_KEYS); + supportedOptions.add(ORDER_BY_IN_UPDATE_STATEMENT); } /** diff --git a/src/main/java/org/datanucleus/store/rdbms/adapter/MySQLAdapter.java b/src/main/java/org/datanucleus/store/rdbms/adapter/MySQLAdapter.java index 2e5c5309..c87b3826 100644 --- a/src/main/java/org/datanucleus/store/rdbms/adapter/MySQLAdapter.java +++ b/src/main/java/org/datanucleus/store/rdbms/adapter/MySQLAdapter.java @@ -165,6 +165,7 @@ else if (!isMariaDB && (datastoreMajorVersion < 5 || (datastoreMajorVersion == 5 supportedOptions.add(OPERATOR_BITWISE_OR); supportedOptions.add(OPERATOR_BITWISE_XOR); supportedOptions.add(PARAMETER_IN_CASE_IN_UPDATE_CLAUSE); + supportedOptions.add(ORDER_BY_IN_UPDATE_STATEMENT); // supportedOptions.add(NATIVE_ENUM_TYPE); // There is no point to supporting this since "CHECK IN(...)" is ANSI standard and does the same diff --git a/src/main/java/org/datanucleus/store/rdbms/scostore/AbstractListStore.java b/src/main/java/org/datanucleus/store/rdbms/scostore/AbstractListStore.java index 3edc657f..c0df0a2a 100644 --- a/src/main/java/org/datanucleus/store/rdbms/scostore/AbstractListStore.java +++ b/src/main/java/org/datanucleus/store/rdbms/scostore/AbstractListStore.java @@ -39,6 +39,7 @@ import org.datanucleus.metadata.CollectionMetaData; import org.datanucleus.state.DNStateManager; import org.datanucleus.store.connection.ManagedConnection; +import org.datanucleus.store.rdbms.adapter.DatastoreAdapter; import org.datanucleus.store.rdbms.mapping.java.ReferenceMapping; import org.datanucleus.store.types.scostore.ListStore; import org.datanucleus.store.rdbms.JDBCUtils; @@ -59,6 +60,7 @@ public abstract class AbstractListStore extends AbstractCollectionStore im protected String removeAtStmt; protected String shiftStmt; protected String shiftBulkStmt; + protected String shiftBulkUpStmt; /** * Constructor. Protected to prevent instantiation. @@ -455,7 +457,7 @@ protected void internalRemoveAt(DNStateManager ownerSM, int index, String stmt, */ protected int[] internalShiftBulk(DNStateManager ownerSM, int start, int amount, ManagedConnection conn, boolean batched, boolean executeNow) { - String shiftBulkStmt = getShiftBulkStmt(); + String shiftBulkStmt = (amount > 0) ? getShiftBulkUpStmt() : getShiftBulkStmt(); try { SQLController sqlControl = storeMgr.getSQLController(); @@ -482,7 +484,7 @@ protected int[] internalShiftBulk(DNStateManager ownerSM, int start, int amount, } catch (SQLException sqle) { - throw new NucleusDataStoreException(Localiser.msg("056012", shiftStmt), sqle); + throw new NucleusDataStoreException(Localiser.msg("056012", shiftBulkStmt), sqle); } } @@ -837,4 +839,64 @@ protected String getShiftBulkStmt() } return shiftBulkStmt; } + + /** + * Generate statement for bulk shifting rows UP (positive amount). + * When the datastore supports ORDER BY in UPDATE statements, adds ORDER BY DESC + * to avoid unique constraint violations when shifting indices upward. + *
+     * UPDATE LISTTABLE SET INDEXCOL = INDEXCOL + ?
+     * WHERE OWNERCOL = ?
+     * AND INDEXCOL > ?
+     * [AND DISTINGUISHER=?]
+     * [ORDER BY INDEXCOL DESC]
+     * 
+ * @return The Statement for shifting elements up in bulk + */ + protected String getShiftBulkUpStmt() + { + if (shiftBulkUpStmt == null) + { + synchronized (this) + { + StringBuilder stmt = new StringBuilder("UPDATE ").append(containerTable.toString()).append(" SET "); + + for (int i = 0; i < orderMapping.getNumberOfColumnMappings(); i++) + { + if (i > 0) + { + stmt.append(","); + } + stmt.append(orderMapping.getColumnMapping(i).getColumn().getIdentifier().toString()); + stmt.append(" = "); + stmt.append(orderMapping.getColumnMapping(i).getColumn().getIdentifier().toString()); + stmt.append(" + "); + stmt.append(orderMapping.getColumnMapping(i).getUpdateInputParameter()); + } + + stmt.append(" WHERE "); + BackingStoreHelper.appendWhereClauseForMapping(stmt, ownerMapping, null, true); + + stmt.append(" AND "); + stmt.append(orderMapping.getColumnMapping(0).getColumn().getIdentifier().toString()); + stmt.append(">"); + stmt.append(orderMapping.getColumnMapping(0).getInsertionInputParameter()); // Start position + + if (relationDiscriminatorMapping != null) + { + BackingStoreHelper.appendWhereClauseForMapping(stmt, relationDiscriminatorMapping, null, false); + } + + if (dba.supportsOption(DatastoreAdapter.ORDER_BY_IN_UPDATE_STATEMENT)) + { + stmt.append(" ORDER BY "); + stmt.append(orderMapping.getColumnMapping(0).getColumn().getIdentifier().toString()); + stmt.append(" DESC"); + } + + shiftBulkUpStmt = stmt.toString(); + } + } + return shiftBulkUpStmt; + } } \ No newline at end of file diff --git a/src/test/java/org/datanucleus/store/rdbms/scostore/AbstractListStoreOrderTest.java b/src/test/java/org/datanucleus/store/rdbms/scostore/AbstractListStoreOrderTest.java new file mode 100644 index 00000000..ab0a48b8 --- /dev/null +++ b/src/test/java/org/datanucleus/store/rdbms/scostore/AbstractListStoreOrderTest.java @@ -0,0 +1,84 @@ +/********************************************************************** +Copyright (c) 2024 Contributors. All rights reserved. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. + +Contributors: + ... +**********************************************************************/ +package org.datanucleus.store.rdbms.scostore; + +import java.sql.Connection; +import java.sql.DatabaseMetaData; +import java.sql.DriverManager; +import java.sql.SQLException; + +import junit.framework.TestCase; + +import org.datanucleus.store.rdbms.adapter.DatastoreAdapter; +import org.datanucleus.store.rdbms.adapter.H2Adapter; + +/** + * Tests for the bulk shift ordering fix in AbstractListStore. + * Verifies that H2Adapter registers the ORDER_BY_IN_UPDATE_STATEMENT capability, + * which enables the ORDER BY DESC clause on bulk shift UPDATEs for positive (upward) shifts. + * + *

Background: On MySQL, when shifting list indices upward (e.g. inserting at the start), + * the UPDATE processes rows in ascending PK/storage order. Without ORDER BY DESC, the lowest + * index row is updated first, colliding with a not-yet-shifted higher index row, causing + * a duplicate key violation. The fix adds ORDER BY idx DESC to the bulk UPDATE for positive + * shifts, gated behind the ORDER_BY_IN_UPDATE_STATEMENT adapter capability.

+ * + *

H2 does not exhibit the bug (it processes rows in a different order), but enabling the + * capability ensures the fix is exercised and tested on H2.

+ */ +public class AbstractListStoreOrderTest extends TestCase +{ + private Connection conn; + + @Override + protected void setUp() throws Exception + { + super.setUp(); + Class.forName("org.h2.Driver"); + conn = DriverManager.getConnection("jdbc:h2:mem:testOrderBy;DB_CLOSE_DELAY=-1", "sa", ""); + } + + @Override + protected void tearDown() throws Exception + { + if (conn != null && !conn.isClosed()) + { + conn.close(); + } + super.tearDown(); + } + + /** + * Verify that H2Adapter registers ORDER_BY_IN_UPDATE_STATEMENT capability. + */ + public void testH2AdapterSupportsOrderByInUpdate() throws SQLException + { + DatabaseMetaData md = conn.getMetaData(); + H2Adapter adapter = new H2Adapter(md); + assertTrue("H2Adapter should support ORDER_BY_IN_UPDATE_STATEMENT", + adapter.supportsOption(DatastoreAdapter.ORDER_BY_IN_UPDATE_STATEMENT)); + } + + /** + * Verify that the ORDER_BY_IN_UPDATE_STATEMENT constant is defined and has the expected value. + */ + public void testOrderByInUpdateConstantDefined() + { + assertEquals("OrderByInUpdateStatement", DatastoreAdapter.ORDER_BY_IN_UPDATE_STATEMENT); + } +}