From 32b316493c9484dae12a204e29397a95f814595e Mon Sep 17 00:00:00 2001 From: Kostiantyn Miakshyn Date: Sat, 26 Sep 2026 00:54:38 +0200 Subject: [PATCH 1/3] feat(db): add conflict tolerant inserts to query builder and QBMapper Signed-off-by: Kostiantyn Miakshyn --- lib/private/DB/Adapter.php | 10 ++ lib/private/DB/AdapterMySQL.php | 36 ++++---- lib/private/DB/AdapterPgSql.php | 9 +- lib/private/DB/AdapterSqlite.php | 12 +-- lib/private/DB/Connection.php | 7 ++ lib/private/DB/ConnectionAdapter.php | 7 ++ .../DB/QueryBuilder/ExtendedQueryBuilder.php | 6 ++ lib/private/DB/QueryBuilder/QueryBuilder.php | 19 +++- lib/public/AppFramework/Db/QBMapper.php | 47 ++++++++++ lib/public/DB/QueryBuilder/IQueryBuilder.php | 18 ++++ .../DB/QueryBuilder/ITypedQueryBuilder.php | 8 ++ tests/lib/AppFramework/Db/QBMapperDBTest.php | 18 ++++ tests/lib/AppFramework/Db/QBMapperTest.php | 91 +++++++++++++++++++ .../lib/DB/QueryBuilder/QueryBuilderTest.php | 47 ++++++++++ 14 files changed, 307 insertions(+), 28 deletions(-) diff --git a/lib/private/DB/Adapter.php b/lib/private/DB/Adapter.php index 0e379d5627f9a..6f08fabccd741 100644 --- a/lib/private/DB/Adapter.php +++ b/lib/private/DB/Adapter.php @@ -127,4 +127,14 @@ public function insertIgnoreConflict(string $table, array $values) : int { throw $e; } } + + /** + * Return a transformer turning an INSERT query into a conflict tolerant + * one, or null when the platform has no native support for it. + * + * @return null|callable(string): string + */ + public function getInsertIgnoreSqlTransformer(): ?callable { + return null; + } } diff --git a/lib/private/DB/AdapterMySQL.php b/lib/private/DB/AdapterMySQL.php index a4312a26d9047..408dd90f35b57 100644 --- a/lib/private/DB/AdapterMySQL.php +++ b/lib/private/DB/AdapterMySQL.php @@ -44,28 +44,26 @@ protected function getCollation(): string { public function insertIgnoreConflict(string $table, array $values): int { $builder = $this->conn->getQueryBuilder(); $builder->insert($table); - $updates = []; foreach ($values as $key => $value) { $builder->setValue($key, $builder->createNamedParameter($value)); } + $builder->ignoreConflictsOnInsert(); + return $builder->executeStatement(); + } - /* - * We can't use ON DUPLICATE KEY UPDATE here because Nextcloud use the CLIENT_FOUND_ROWS flag - * With this flag the MySQL returns the number of selected rows - * instead of the number of affected/modified rows - * It's impossible to change this behaviour at runtime or for a single query - * Then, the result is 1 if a row is inserted and also 1 if a row is updated with same or different values - * - * With INSERT IGNORE, the result is 1 when a row is inserted, 0 otherwise - * - * Risk: it can also ignore other errors like type mismatch or truncated data… - */ - $res = $this->conn->executeStatement( - preg_replace('/^INSERT/i', 'INSERT IGNORE', $builder->getSQL()), - $builder->getParameters(), - $builder->getParameterTypes() - ); - - return $res; + /** + * We can't use ON DUPLICATE KEY UPDATE here because Nextcloud use the CLIENT_FOUND_ROWS flag + * With this flag the MySQL returns the number of selected rows + * instead of the number of affected/modified rows + * It's impossible to change this behaviour at runtime or for a single query + * Then, the result is 1 if a row is inserted and also 1 if a row is updated with same or different values + * + * With INSERT IGNORE, the result is 1 when a row is inserted, 0 otherwise + * + * Risk: it can also ignore other errors like type mismatch or truncated data… + */ + #[\Override] + public function getInsertIgnoreSqlTransformer(): callable { + return fn (string $sql) => preg_replace('/^INSERT/i', 'INSERT IGNORE', $sql) ?? $sql; } } diff --git a/lib/private/DB/AdapterPgSql.php b/lib/private/DB/AdapterPgSql.php index 3c0617651c60f..c7d6d4f8b9001 100644 --- a/lib/private/DB/AdapterPgSql.php +++ b/lib/private/DB/AdapterPgSql.php @@ -35,7 +35,12 @@ public function insertIgnoreConflict(string $table, array $values) : int { foreach ($values as $key => $value) { $builder->setValue($key, $builder->createNamedParameter($value)); } - $queryString = $builder->getSQL() . ' ON CONFLICT DO NOTHING'; - return $this->conn->executeStatement($queryString, $builder->getParameters(), $builder->getParameterTypes()); + $builder->ignoreConflictsOnInsert(); + return $builder->executeStatement(); + } + + #[\Override] + public function getInsertIgnoreSqlTransformer(): callable { + return fn (string $sql) => $sql . ' ON CONFLICT DO NOTHING'; } } diff --git a/lib/private/DB/AdapterSqlite.php b/lib/private/DB/AdapterSqlite.php index 223a839c6d8fc..f623912ae68c5 100644 --- a/lib/private/DB/AdapterSqlite.php +++ b/lib/private/DB/AdapterSqlite.php @@ -86,15 +86,15 @@ public function insertIfNotExist($table, $input, ?array $compare = null) { public function insertIgnoreConflict(string $table, array $values): int { $builder = $this->conn->getQueryBuilder(); $builder->insert($table); - $updates = []; foreach ($values as $key => $value) { $builder->setValue($key, $builder->createNamedParameter($value)); } + $builder->ignoreConflictsOnInsert(); + return $builder->executeStatement(); + } - return $this->conn->executeStatement( - $builder->getSQL() . ' ON CONFLICT DO NOTHING', - $builder->getParameters(), - $builder->getParameterTypes() - ); + #[\Override] + public function getInsertIgnoreSqlTransformer(): callable { + return fn (string $sql) => $sql . ' ON CONFLICT DO NOTHING'; } } diff --git a/lib/private/DB/Connection.php b/lib/private/DB/Connection.php index 0ad9886e7c05d..008d0e7bb4aba 100644 --- a/lib/private/DB/Connection.php +++ b/lib/private/DB/Connection.php @@ -991,4 +991,11 @@ public function getShardDefinition(string $name): ?ShardDefinition { public function getCrossShardMoveHelper(): CrossShardMoveHelper { return new CrossShardMoveHelper($this->shardConnectionManager); } + + /** + * @return null|callable(string): string + */ + public function getInsertIgnoreSqlTransformer(): ?callable { + return $this->adapter->getInsertIgnoreSqlTransformer(); + } } diff --git a/lib/private/DB/ConnectionAdapter.php b/lib/private/DB/ConnectionAdapter.php index 11ac4b3809e81..fc6cd3816c437 100644 --- a/lib/private/DB/ConnectionAdapter.php +++ b/lib/private/DB/ConnectionAdapter.php @@ -299,4 +299,11 @@ public function getShardDefinition(string $name): ?ShardDefinition { public function getCrossShardMoveHelper(): CrossShardMoveHelper { return $this->inner->getCrossShardMoveHelper(); } + + /** + * @return null|callable(string): string + */ + public function getInsertIgnoreSqlTransformer(): ?callable { + return $this->inner->getInsertIgnoreSqlTransformer(); + } } diff --git a/lib/private/DB/QueryBuilder/ExtendedQueryBuilder.php b/lib/private/DB/QueryBuilder/ExtendedQueryBuilder.php index 5b63e1792bacb..e7515d09f25b0 100644 --- a/lib/private/DB/QueryBuilder/ExtendedQueryBuilder.php +++ b/lib/private/DB/QueryBuilder/ExtendedQueryBuilder.php @@ -359,4 +359,10 @@ public function forUpdate(ConflictResolutionMode $conflictResolutionMode = Confl $this->builder->forUpdate($conflictResolutionMode); return $this; } + + #[\Override] + public function ignoreConflictsOnInsert(): self { + $this->builder->ignoreConflictsOnInsert(); + return $this; + } } diff --git a/lib/private/DB/QueryBuilder/QueryBuilder.php b/lib/private/DB/QueryBuilder/QueryBuilder.php index 11264d28361e3..2c99c1dfb16ab 100644 --- a/lib/private/DB/QueryBuilder/QueryBuilder.php +++ b/lib/private/DB/QueryBuilder/QueryBuilder.php @@ -40,6 +40,7 @@ class QueryBuilder extends TypedQueryBuilder { private bool $nonEmptyWhere = false; protected ?string $lastInsertedTable = null; private array $selectedColumns = []; + private bool $insertIgnoreConflicts = false; /** * Initializes a new QueryBuilder. @@ -284,6 +285,15 @@ public function executeStatement(?IDBConnection $connection = null): int { ); } + #[\Override] + public function ignoreConflictsOnInsert(): self { + if ($this->getType() !== \Doctrine\DBAL\Query\QueryBuilder::INSERT) { + throw new \LogicException('ignoreConflictsOnInsert() can only be used on INSERT queries'); + } + $this->insertIgnoreConflicts = true; + return $this; + } + /** * Gets the complete SQL string formed by the current specifications of this QueryBuilder. * @@ -298,7 +308,14 @@ public function executeStatement(?IDBConnection $connection = null): int { */ #[\Override] public function getSQL() { - return $this->queryBuilder->getSQL(); + $sql = $this->queryBuilder->getSQL(); + if ($this->insertIgnoreConflicts) { + $transformer = $this->connection->getInsertIgnoreSqlTransformer(); + if ($transformer !== null) { + return $transformer($sql); + } + } + return $sql; } /** diff --git a/lib/public/AppFramework/Db/QBMapper.php b/lib/public/AppFramework/Db/QBMapper.php index 1fde5c71aaa04..76fb2354a5cfe 100644 --- a/lib/public/AppFramework/Db/QBMapper.php +++ b/lib/public/AppFramework/Db/QBMapper.php @@ -153,6 +153,53 @@ public function insertOrUpdate(Entity $entity): Entity { } } + /** + * Creates a new entry in the db from an entity, ignoring conflicts on + * unique constraints + * + * @param Entity $entity the entity that should be created + * @psalm-param T $entity the entity that should be created + * @return int number of inserted rows (0 if a conflicting row already exists) + * @throws Exception + * @since 36.0.0 + */ + public function insertIgnoreConflict(Entity $entity): int { + if ($entity instanceof SnowflakeAwareEntity) { + /** @psalm-suppress DocblockTypeContradiction */ + $entity->generateId(); + } + + // get updated fields to save, fields have to be set using a setter to + // be saved + $properties = $entity->getUpdatedFields(); + + $qb = $this->db->getQueryBuilder(); + $qb->insert($this->tableName); + $qb->ignoreConflictsOnInsert(); + + // build the fields + foreach ($properties as $property => $updated) { + if ($property === 'id' && $entity->id === null) { + continue; + } + + $column = $entity->propertyToColumn($property); + $getter = 'get' . ucfirst($property); + $type = $this->getParameterTypeForProperty($entity, $property); + $qb->setValue($column, $qb->createNamedParameter($entity->$getter(), $type)); + } + + try { + return $qb->executeStatement(); + } catch (Exception $ex) { + // fallback for platforms without native conflict tolerant inserts + if ($ex->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + return 0; + } + throw $ex; + } + } + /** * Updates an entry in the db from an entity * diff --git a/lib/public/DB/QueryBuilder/IQueryBuilder.php b/lib/public/DB/QueryBuilder/IQueryBuilder.php index 8cb89d8f976d4..a3b431cb4f9cc 100644 --- a/lib/public/DB/QueryBuilder/IQueryBuilder.php +++ b/lib/public/DB/QueryBuilder/IQueryBuilder.php @@ -225,6 +225,24 @@ public function executeQuery(?IDBConnection $connection = null): IResult; */ public function executeStatement(?IDBConnection $connection = null): int; + /** + * Ignore unique constraint conflicts for INSERT queries. + * + * Rows conflicting with an existing row on a unique constraint are skipped + * instead of raising an error, the row count returned by + * {@see self::executeStatement()} is reduced accordingly. + * + * On platforms without native support for conflict tolerant inserts the + * query is executed as a regular INSERT. + * + * Must only be called on INSERT queries. + * + * @return $this + * @since 36.0.0 + * @throws \LogicException when called on a non-INSERT query + */ + public function ignoreConflictsOnInsert(): self; + /** * Gets the complete SQL string formed by the current specifications of this QueryBuilder. * diff --git a/lib/public/DB/QueryBuilder/ITypedQueryBuilder.php b/lib/public/DB/QueryBuilder/ITypedQueryBuilder.php index 823cb862786e8..39453475987c2 100644 --- a/lib/public/DB/QueryBuilder/ITypedQueryBuilder.php +++ b/lib/public/DB/QueryBuilder/ITypedQueryBuilder.php @@ -347,4 +347,12 @@ public function runAcrossAllShards(): self; */ #[Override] public function forUpdate(ConflictResolutionMode $conflictResolutionMode = ConflictResolutionMode::Ordinary): self; + + /** + * @inheritDoc + * @return $this + * @since 36.0.0 + */ + #[Override] + public function ignoreConflictsOnInsert(): self; } diff --git a/tests/lib/AppFramework/Db/QBMapperDBTest.php b/tests/lib/AppFramework/Db/QBMapperDBTest.php index 24b8f06e6ce63..c691761a9914d 100644 --- a/tests/lib/AppFramework/Db/QBMapperDBTest.php +++ b/tests/lib/AppFramework/Db/QBMapperDBTest.php @@ -127,6 +127,24 @@ public function testUpdateDateTime(): void { $this->assertEquals($datetime->format('Y-m-d H:i:s'), $dbEntity->getDatetime()->format('Y-m-d H:i:s')); } + public function testInsertIgnoreConflict(): void { + $mapper = new QBDBTestMapper($this->connection); + + $entity = new QBDBTestEntity(); + $entity->setId(200001); + $entity->setDatetime(new \DateTimeImmutable('2000-01-01 23:45:00')); + $this->assertSame(1, $mapper->insertIgnoreConflict($entity)); + + $conflicting = new QBDBTestEntity(); + $conflicting->setId(200001); + $conflicting->setDatetime(new \DateTimeImmutable('2010-05-05 05:05:05')); + $this->assertSame(0, $mapper->insertIgnoreConflict($conflicting)); + + // the conflicting insert must not have modified the existing row + $dbEntity = $mapper->getById(200001); + $this->assertEquals('2000-01-01 23:45:00', $dbEntity->getDatetime()->format('Y-m-d H:i:s')); + } + protected function prepareTestingTable(): void { if ($this->schemaSetup) { $this->connection->getQueryBuilder()->delete('testing')->executeStatement(); diff --git a/tests/lib/AppFramework/Db/QBMapperTest.php b/tests/lib/AppFramework/Db/QBMapperTest.php index 09cfee14720d7..76dcd3e1b10a4 100644 --- a/tests/lib/AppFramework/Db/QBMapperTest.php +++ b/tests/lib/AppFramework/Db/QBMapperTest.php @@ -212,6 +212,97 @@ public function testUpdateEntityParameterTypeMapping(): void { $this->mapper->update($entity); } + public function testInsertIgnoreConflictEntityParameterTypeMapping(): void { + $datetime = new \DateTimeImmutable(); + $entity = new QBTestEntity(); + $entity->setIntProp(123); + $entity->setBoolProp(true); + $entity->setStringProp('string'); + $entity->setDatetimeProp($datetime); + + $intParam = $this->qb->createNamedParameter('int_prop', IQueryBuilder::PARAM_INT); + $boolParam = $this->qb->createNamedParameter('bool_prop', IQueryBuilder::PARAM_BOOL); + $stringParam = $this->qb->createNamedParameter('string_prop', IQueryBuilder::PARAM_STR); + $datetimeParam = $this->qb->createNamedParameter('datetime_prop', IQueryBuilder::PARAM_DATETIME_IMMUTABLE); + + $this->qb->expects($this->once()) + ->method('insert') + ->with($this->equalTo('table')); + $this->qb->expects($this->once()) + ->method('ignoreConflictsOnInsert'); + + $createNamedParameterCalls = [ + [123, IQueryBuilder::PARAM_INT, null], + [true, IQueryBuilder::PARAM_BOOL, null], + ['string', IQueryBuilder::PARAM_STR, null], + [$datetime, IQueryBuilder::PARAM_DATETIME_IMMUTABLE, null], + ]; + $this->qb->expects($this->exactly(4)) + ->method('createNamedParameter') + ->willReturnCallback(function () use (&$createNamedParameterCalls): void { + $expected = array_shift($createNamedParameterCalls); + $this->assertEquals($expected, func_get_args()); + }); + + $setValueCalls = [ + ['int_prop', $intParam], + ['bool_prop', $boolParam], + ['string_prop', $stringParam], + ['datetime_prop', $datetimeParam], + ]; + $this->qb->expects($this->exactly(4)) + ->method('setValue') + ->willReturnCallback(function () use (&$setValueCalls): void { + $expected = array_shift($setValueCalls); + $this->assertEquals($expected, func_get_args()); + }); + + $this->qb->expects($this->once()) + ->method('executeStatement') + ->willReturn(1); + + $this->assertSame(1, $this->mapper->insertIgnoreConflict($entity)); + } + + public function testInsertIgnoreConflictReturnsZeroOnUniqueViolation(): void { + $entity = new QBTestEntity(); + $entity->setStringProp('string'); + + $exception = new class extends \OCP\DB\Exception { + #[\Override] + public function getReason(): ?int { + return self::REASON_UNIQUE_CONSTRAINT_VIOLATION; + } + }; + + $this->qb->method('executeStatement') + ->willThrowException($exception); + + $this->assertSame(0, $this->mapper->insertIgnoreConflict($entity)); + } + + public function testInsertIgnoreConflictRethrowsOtherErrors(): void { + $entity = new QBTestEntity(); + $entity->setStringProp('string'); + + $exception = new class extends \OCP\DB\Exception { + #[\Override] + public function getReason(): ?int { + return self::REASON_DEADLOCK; + } + }; + + $this->qb->method('executeStatement') + ->willThrowException($exception); + + try { + $this->mapper->insertIgnoreConflict($entity); + $this->fail('Expected the exception to be rethrown'); + } catch (\OCP\DB\Exception $e) { + $this->assertSame($exception, $e); + } + } + public function testGetParameterTypeForProperty(): void { $entity = new QBTestEntity(); diff --git a/tests/lib/DB/QueryBuilder/QueryBuilderTest.php b/tests/lib/DB/QueryBuilder/QueryBuilderTest.php index e39ba0b78c62c..8e9b26931ff82 100644 --- a/tests/lib/DB/QueryBuilder/QueryBuilderTest.php +++ b/tests/lib/DB/QueryBuilder/QueryBuilderTest.php @@ -1449,4 +1449,51 @@ public function testExecuteWithParametersTooMany(): void { $this->invokePrivate($this->queryBuilder, 'connection', [$this->getConnection()]); $this->queryBuilder->executeQuery(); } + + public function testIgnoreConflictsOnInsertSql(): void { + $this->queryBuilder->insert('appconfig') + ->setValue('appid', $this->queryBuilder->createNamedParameter('testIgnoreConflicts')) + ->setValue('configkey', $this->queryBuilder->createNamedParameter('testing')); + $this->queryBuilder->ignoreConflictsOnInsert(); + + $sql = $this->queryBuilder->getSQL(); + match ($this->connection->getDatabaseProvider()) { + IDBConnection::PLATFORM_MYSQL, + IDBConnection::PLATFORM_MARIADB => $this->assertStringStartsWith('INSERT IGNORE INTO', $sql), + IDBConnection::PLATFORM_POSTGRES, + IDBConnection::PLATFORM_SQLITE => $this->assertStringEndsWith('ON CONFLICT DO NOTHING', $sql), + default => $this->assertStringStartsWith('INSERT INTO', $sql), + }; + } + + public function testIgnoreConflictsOnInsertExecution(): void { + $buildInsert = function (IQueryBuilder $qb): void { + $qb->insert('*PREFIX*appconfig') + ->values([ + 'appid' => $qb->createNamedParameter('testIgnoreConflicts'), + 'configkey' => $qb->createNamedParameter('testing'), + 'configvalue' => $qb->createNamedParameter('42'), + ]) + ->ignoreConflictsOnInsert(); + }; + + $qb = $this->connection->getQueryBuilder(); + $buildInsert($qb); + $this->assertSame(1, $qb->executeStatement()); + + $qb = $this->connection->getQueryBuilder(); + $buildInsert($qb); + $this->assertSame(0, $qb->executeStatement()); + + $qb = $this->connection->getQueryBuilder(); + $qb->delete('*PREFIX*appconfig') + ->where($qb->expr()->eq('appid', $qb->createNamedParameter('testIgnoreConflicts'))) + ->executeStatement(); + } + + public function testIgnoreConflictsOnInsertOnSelect(): void { + $this->queryBuilder->select('*')->from('appconfig'); + $this->expectException(\LogicException::class); + $this->queryBuilder->ignoreConflictsOnInsert(); + } } From b0453d0ea5c4eaa02f3e5ed1a2da32a079f646ad Mon Sep 17 00:00:00 2001 From: Kostiantyn Miakshyn Date: Sat, 26 Sep 2026 17:06:43 +0200 Subject: [PATCH 2/3] feat(db): add conflict tolerant inserts to query builder and QBMapper Signed-off-by: Kostiantyn Miakshyn --- lib/private/DB/Adapter.php | 19 ++----- lib/private/DB/AdapterMySQL.php | 11 ---- lib/private/DB/AdapterPgSql.php | 17 ++---- lib/private/DB/AdapterSqlite.php | 11 ---- lib/private/DB/QueryBuilder/QueryBuilder.php | 23 ++++++-- lib/public/AppFramework/Db/QBMapper.php | 10 +--- lib/public/DB/QueryBuilder/IQueryBuilder.php | 7 +-- tests/lib/AppFramework/Db/QBMapperTest.php | 39 ------------- .../lib/DB/QueryBuilder/QueryBuilderTest.php | 57 +++++++++++++++++++ 9 files changed, 88 insertions(+), 106 deletions(-) diff --git a/lib/private/DB/Adapter.php b/lib/private/DB/Adapter.php index 6f08fabccd741..9ce502e76ccf0 100644 --- a/lib/private/DB/Adapter.php +++ b/lib/private/DB/Adapter.php @@ -10,7 +10,6 @@ use Doctrine\DBAL\Exception; use Doctrine\DBAL\Exception\UniqueConstraintViolationException; -use OC\DB\Exceptions\DbalException; /** * This handles the way we use to write queries, into something that can be @@ -113,19 +112,13 @@ public function insertIfNotExist($table, $input, ?array $compare = null) { * @throws \OCP\DB\Exception */ public function insertIgnoreConflict(string $table, array $values) : int { - try { - $builder = $this->conn->getQueryBuilder(); - $builder->insert($table); - foreach ($values as $key => $value) { - $builder->setValue($key, $builder->createNamedParameter($value)); - } - return $builder->executeStatement(); - } catch (DbalException $e) { - if ($e->getReason() === \OCP\DB\Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { - return 0; - } - throw $e; + $builder = $this->conn->getQueryBuilder(); + $builder->insert($table); + foreach ($values as $key => $value) { + $builder->setValue($key, $builder->createNamedParameter($value)); } + $builder->ignoreConflictsOnInsert(); + return $builder->executeStatement(); } /** diff --git a/lib/private/DB/AdapterMySQL.php b/lib/private/DB/AdapterMySQL.php index 408dd90f35b57..8a49c86ecd4fb 100644 --- a/lib/private/DB/AdapterMySQL.php +++ b/lib/private/DB/AdapterMySQL.php @@ -40,17 +40,6 @@ protected function getCollation(): string { return $this->collation; } - #[\Override] - public function insertIgnoreConflict(string $table, array $values): int { - $builder = $this->conn->getQueryBuilder(); - $builder->insert($table); - foreach ($values as $key => $value) { - $builder->setValue($key, $builder->createNamedParameter($value)); - } - $builder->ignoreConflictsOnInsert(); - return $builder->executeStatement(); - } - /** * We can't use ON DUPLICATE KEY UPDATE here because Nextcloud use the CLIENT_FOUND_ROWS flag * With this flag the MySQL returns the number of selected rows diff --git a/lib/private/DB/AdapterPgSql.php b/lib/private/DB/AdapterPgSql.php index c7d6d4f8b9001..4ce9a737ca131 100644 --- a/lib/private/DB/AdapterPgSql.php +++ b/lib/private/DB/AdapterPgSql.php @@ -26,19 +26,10 @@ public function fixupStatement($statement) { return $statement; } - #[\Override] - public function insertIgnoreConflict(string $table, array $values) : int { - // "upsert" is only available since PgSQL 9.5, but the generic way - // would leave error logs in the DB. - $builder = $this->conn->getQueryBuilder(); - $builder->insert($table); - foreach ($values as $key => $value) { - $builder->setValue($key, $builder->createNamedParameter($value)); - } - $builder->ignoreConflictsOnInsert(); - return $builder->executeStatement(); - } - + /** + * "upsert" is only available since PgSQL 9.5, but the generic way + * would leave error logs in the DB. + */ #[\Override] public function getInsertIgnoreSqlTransformer(): callable { return fn (string $sql) => $sql . ' ON CONFLICT DO NOTHING'; diff --git a/lib/private/DB/AdapterSqlite.php b/lib/private/DB/AdapterSqlite.php index f623912ae68c5..9ecdbe34bfc26 100644 --- a/lib/private/DB/AdapterSqlite.php +++ b/lib/private/DB/AdapterSqlite.php @@ -82,17 +82,6 @@ public function insertIfNotExist($table, $input, ?array $compare = null) { } } - #[\Override] - public function insertIgnoreConflict(string $table, array $values): int { - $builder = $this->conn->getQueryBuilder(); - $builder->insert($table); - foreach ($values as $key => $value) { - $builder->setValue($key, $builder->createNamedParameter($value)); - } - $builder->ignoreConflictsOnInsert(); - return $builder->executeStatement(); - } - #[\Override] public function getInsertIgnoreSqlTransformer(): callable { return fn (string $sql) => $sql . ' ON CONFLICT DO NOTHING'; diff --git a/lib/private/DB/QueryBuilder/QueryBuilder.php b/lib/private/DB/QueryBuilder/QueryBuilder.php index 2c99c1dfb16ab..67a535bd5d0fd 100644 --- a/lib/private/DB/QueryBuilder/QueryBuilder.php +++ b/lib/private/DB/QueryBuilder/QueryBuilder.php @@ -19,6 +19,7 @@ use OC\DB\QueryBuilder\FunctionBuilder\PgSqlFunctionBuilder; use OC\DB\QueryBuilder\FunctionBuilder\SqliteFunctionBuilder; use OC\SystemConfig; +use OCP\DB\Exception; use OCP\DB\IResult; use OCP\DB\QueryBuilder\ConflictResolutionMode; use OCP\DB\QueryBuilder\ICompositeExpression; @@ -278,11 +279,20 @@ public function executeStatement(?IDBConnection $connection = null): int { $connection = $this->connection; } - return $connection->executeStatement( - $this->getSQL(), - $this->getParameters(), - $this->getParameterTypes(), - ); + try { + return $connection->executeStatement( + $this->getSQL(), + $this->getParameters(), + $this->getParameterTypes(), + ); + } catch (Exception $e) { + // fallback for platforms without native conflict tolerant inserts + if ($this->insertIgnoreConflicts + && $e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + return 0; + } + throw $e; + } } #[\Override] @@ -309,7 +319,8 @@ public function ignoreConflictsOnInsert(): self { #[\Override] public function getSQL() { $sql = $this->queryBuilder->getSQL(); - if ($this->insertIgnoreConflicts) { + if ($this->insertIgnoreConflicts + && $this->getType() === \Doctrine\DBAL\Query\QueryBuilder::INSERT) { $transformer = $this->connection->getInsertIgnoreSqlTransformer(); if ($transformer !== null) { return $transformer($sql); diff --git a/lib/public/AppFramework/Db/QBMapper.php b/lib/public/AppFramework/Db/QBMapper.php index 76fb2354a5cfe..6402bcbb6bf59 100644 --- a/lib/public/AppFramework/Db/QBMapper.php +++ b/lib/public/AppFramework/Db/QBMapper.php @@ -189,15 +189,7 @@ public function insertIgnoreConflict(Entity $entity): int { $qb->setValue($column, $qb->createNamedParameter($entity->$getter(), $type)); } - try { - return $qb->executeStatement(); - } catch (Exception $ex) { - // fallback for platforms without native conflict tolerant inserts - if ($ex->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { - return 0; - } - throw $ex; - } + return $qb->executeStatement(); } /** diff --git a/lib/public/DB/QueryBuilder/IQueryBuilder.php b/lib/public/DB/QueryBuilder/IQueryBuilder.php index a3b431cb4f9cc..edb59c644ecab 100644 --- a/lib/public/DB/QueryBuilder/IQueryBuilder.php +++ b/lib/public/DB/QueryBuilder/IQueryBuilder.php @@ -230,10 +230,9 @@ public function executeStatement(?IDBConnection $connection = null): int; * * Rows conflicting with an existing row on a unique constraint are skipped * instead of raising an error, the row count returned by - * {@see self::executeStatement()} is reduced accordingly. - * - * On platforms without native support for conflict tolerant inserts the - * query is executed as a regular INSERT. + * {@see self::executeStatement()} is reduced accordingly. On platforms + * without native support for conflict tolerant inserts the resulting + * constraint violation is caught and reported as 0 affected rows. * * Must only be called on INSERT queries. * diff --git a/tests/lib/AppFramework/Db/QBMapperTest.php b/tests/lib/AppFramework/Db/QBMapperTest.php index 76dcd3e1b10a4..fd3485c80eb55 100644 --- a/tests/lib/AppFramework/Db/QBMapperTest.php +++ b/tests/lib/AppFramework/Db/QBMapperTest.php @@ -264,45 +264,6 @@ public function testInsertIgnoreConflictEntityParameterTypeMapping(): void { $this->assertSame(1, $this->mapper->insertIgnoreConflict($entity)); } - public function testInsertIgnoreConflictReturnsZeroOnUniqueViolation(): void { - $entity = new QBTestEntity(); - $entity->setStringProp('string'); - - $exception = new class extends \OCP\DB\Exception { - #[\Override] - public function getReason(): ?int { - return self::REASON_UNIQUE_CONSTRAINT_VIOLATION; - } - }; - - $this->qb->method('executeStatement') - ->willThrowException($exception); - - $this->assertSame(0, $this->mapper->insertIgnoreConflict($entity)); - } - - public function testInsertIgnoreConflictRethrowsOtherErrors(): void { - $entity = new QBTestEntity(); - $entity->setStringProp('string'); - - $exception = new class extends \OCP\DB\Exception { - #[\Override] - public function getReason(): ?int { - return self::REASON_DEADLOCK; - } - }; - - $this->qb->method('executeStatement') - ->willThrowException($exception); - - try { - $this->mapper->insertIgnoreConflict($entity); - $this->fail('Expected the exception to be rethrown'); - } catch (\OCP\DB\Exception $e) { - $this->assertSame($exception, $e); - } - } - public function testGetParameterTypeForProperty(): void { $entity = new QBTestEntity(); diff --git a/tests/lib/DB/QueryBuilder/QueryBuilderTest.php b/tests/lib/DB/QueryBuilder/QueryBuilderTest.php index 8e9b26931ff82..b19db7d1d94c0 100644 --- a/tests/lib/DB/QueryBuilder/QueryBuilderTest.php +++ b/tests/lib/DB/QueryBuilder/QueryBuilderTest.php @@ -1496,4 +1496,61 @@ public function testIgnoreConflictsOnInsertOnSelect(): void { $this->expectException(\LogicException::class); $this->queryBuilder->ignoreConflictsOnInsert(); } + + private function prepareInsertThrowing(\OCP\DB\Exception $exception): void { + $this->queryBuilder->insert('appconfig') + ->setValue('appid', $this->queryBuilder->createNamedParameter('testIgnoreConflicts')); + + $connection = $this->createMock(ConnectionAdapter::class); + $connection->method('executeStatement') + ->willThrowException($exception); + $this->invokePrivate($this->queryBuilder, 'connection', [$connection]); + } + + public function testIgnoreConflictsOnInsertCatchesUniqueViolation(): void { + $this->prepareInsertThrowing(new class extends \OCP\DB\Exception { + #[\Override] + public function getReason(): ?int { + return self::REASON_UNIQUE_CONSTRAINT_VIOLATION; + } + }); + $this->queryBuilder->ignoreConflictsOnInsert(); + + $this->assertSame(0, $this->queryBuilder->executeStatement()); + } + + public function testIgnoreConflictsOnInsertRethrowsOtherErrors(): void { + $exception = new class extends \OCP\DB\Exception { + #[\Override] + public function getReason(): ?int { + return self::REASON_DEADLOCK; + } + }; + $this->prepareInsertThrowing($exception); + $this->queryBuilder->ignoreConflictsOnInsert(); + + try { + $this->queryBuilder->executeStatement(); + $this->fail('Expected the exception to be rethrown'); + } catch (\OCP\DB\Exception $e) { + $this->assertSame($exception, $e); + } + } + + public function testUniqueViolationNotCaughtWithoutFlag(): void { + $exception = new class extends \OCP\DB\Exception { + #[\Override] + public function getReason(): ?int { + return self::REASON_UNIQUE_CONSTRAINT_VIOLATION; + } + }; + $this->prepareInsertThrowing($exception); + + try { + $this->queryBuilder->executeStatement(); + $this->fail('Expected the exception to be rethrown'); + } catch (\OCP\DB\Exception $e) { + $this->assertSame($exception, $e); + } + } } From bd1714fa02f040f5a959969b725991d78f170965 Mon Sep 17 00:00:00 2001 From: Kostiantyn Miakshyn Date: Sat, 26 Sep 2026 17:13:33 +0200 Subject: [PATCH 3/3] feat(db): add conflict tolerant inserts to query builder and QBMapper Signed-off-by: Kostiantyn Miakshyn --- lib/private/DB/Adapter.php | 11 +++++------ lib/private/DB/AdapterMySQL.php | 4 ++-- lib/private/DB/AdapterPgSql.php | 4 ++-- lib/private/DB/AdapterSqlite.php | 4 ++-- lib/private/DB/Connection.php | 7 ++----- lib/private/DB/ConnectionAdapter.php | 7 ++----- lib/private/DB/QueryBuilder/QueryBuilder.php | 5 +---- 7 files changed, 16 insertions(+), 26 deletions(-) diff --git a/lib/private/DB/Adapter.php b/lib/private/DB/Adapter.php index 9ce502e76ccf0..e0edb7ba9faa7 100644 --- a/lib/private/DB/Adapter.php +++ b/lib/private/DB/Adapter.php @@ -122,12 +122,11 @@ public function insertIgnoreConflict(string $table, array $values) : int { } /** - * Return a transformer turning an INSERT query into a conflict tolerant - * one, or null when the platform has no native support for it. - * - * @return null|callable(string): string + * Transform an INSERT statement into a conflict tolerant one. Platforms + * without native support return the statement unchanged, the resulting + * constraint violation is handled by the query builder. */ - public function getInsertIgnoreSqlTransformer(): ?callable { - return null; + public function getInsertIgnoreConflictSql(string $sql): string { + return $sql; } } diff --git a/lib/private/DB/AdapterMySQL.php b/lib/private/DB/AdapterMySQL.php index 8a49c86ecd4fb..6b0fab0560e03 100644 --- a/lib/private/DB/AdapterMySQL.php +++ b/lib/private/DB/AdapterMySQL.php @@ -52,7 +52,7 @@ protected function getCollation(): string { * Risk: it can also ignore other errors like type mismatch or truncated data… */ #[\Override] - public function getInsertIgnoreSqlTransformer(): callable { - return fn (string $sql) => preg_replace('/^INSERT/i', 'INSERT IGNORE', $sql) ?? $sql; + public function getInsertIgnoreConflictSql(string $sql): string { + return preg_replace('/^INSERT/i', 'INSERT IGNORE', $sql) ?? $sql; } } diff --git a/lib/private/DB/AdapterPgSql.php b/lib/private/DB/AdapterPgSql.php index 4ce9a737ca131..79cf74f3a7a7e 100644 --- a/lib/private/DB/AdapterPgSql.php +++ b/lib/private/DB/AdapterPgSql.php @@ -31,7 +31,7 @@ public function fixupStatement($statement) { * would leave error logs in the DB. */ #[\Override] - public function getInsertIgnoreSqlTransformer(): callable { - return fn (string $sql) => $sql . ' ON CONFLICT DO NOTHING'; + public function getInsertIgnoreConflictSql(string $sql): string { + return $sql . ' ON CONFLICT DO NOTHING'; } } diff --git a/lib/private/DB/AdapterSqlite.php b/lib/private/DB/AdapterSqlite.php index 9ecdbe34bfc26..35a74181a51cc 100644 --- a/lib/private/DB/AdapterSqlite.php +++ b/lib/private/DB/AdapterSqlite.php @@ -83,7 +83,7 @@ public function insertIfNotExist($table, $input, ?array $compare = null) { } #[\Override] - public function getInsertIgnoreSqlTransformer(): callable { - return fn (string $sql) => $sql . ' ON CONFLICT DO NOTHING'; + public function getInsertIgnoreConflictSql(string $sql): string { + return $sql . ' ON CONFLICT DO NOTHING'; } } diff --git a/lib/private/DB/Connection.php b/lib/private/DB/Connection.php index 008d0e7bb4aba..aee103bf0342b 100644 --- a/lib/private/DB/Connection.php +++ b/lib/private/DB/Connection.php @@ -992,10 +992,7 @@ public function getCrossShardMoveHelper(): CrossShardMoveHelper { return new CrossShardMoveHelper($this->shardConnectionManager); } - /** - * @return null|callable(string): string - */ - public function getInsertIgnoreSqlTransformer(): ?callable { - return $this->adapter->getInsertIgnoreSqlTransformer(); + public function getInsertIgnoreConflictSql(string $sql): string { + return $this->adapter->getInsertIgnoreConflictSql($sql); } } diff --git a/lib/private/DB/ConnectionAdapter.php b/lib/private/DB/ConnectionAdapter.php index fc6cd3816c437..5910d4de5c16e 100644 --- a/lib/private/DB/ConnectionAdapter.php +++ b/lib/private/DB/ConnectionAdapter.php @@ -300,10 +300,7 @@ public function getCrossShardMoveHelper(): CrossShardMoveHelper { return $this->inner->getCrossShardMoveHelper(); } - /** - * @return null|callable(string): string - */ - public function getInsertIgnoreSqlTransformer(): ?callable { - return $this->inner->getInsertIgnoreSqlTransformer(); + public function getInsertIgnoreConflictSql(string $sql): string { + return $this->inner->getInsertIgnoreConflictSql($sql); } } diff --git a/lib/private/DB/QueryBuilder/QueryBuilder.php b/lib/private/DB/QueryBuilder/QueryBuilder.php index 67a535bd5d0fd..5d13b090db36d 100644 --- a/lib/private/DB/QueryBuilder/QueryBuilder.php +++ b/lib/private/DB/QueryBuilder/QueryBuilder.php @@ -321,10 +321,7 @@ public function getSQL() { $sql = $this->queryBuilder->getSQL(); if ($this->insertIgnoreConflicts && $this->getType() === \Doctrine\DBAL\Query\QueryBuilder::INSERT) { - $transformer = $this->connection->getInsertIgnoreSqlTransformer(); - if ($transformer !== null) { - return $transformer($sql); - } + return $this->connection->getInsertIgnoreConflictSql($sql); } return $sql; }