Skip to content

Commit 486803a

Browse files
KocKoc
committed
feat(db): add conflict tolerant inserts to query builder and QBMapper (code review)
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
1 parent bd1714f commit 486803a

15 files changed

Lines changed: 215 additions & 257 deletions

File tree

‎lib/private/DB/Adapter.php‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010

1111
use Doctrine\DBAL\Exception;
1212
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
13+
use OC\DB\Exceptions\DbalException;
14+
use OCP\DB\QueryBuilder\IQueryBuilder;
1315

1416
/**
1517
* This handles the way we use to write queries, into something that can be
@@ -111,22 +113,19 @@ public function insertIfNotExist($table, $input, ?array $compare = null) {
111113
/**
112114
* @throws \OCP\DB\Exception
113115
*/
114-
public function insertIgnoreConflict(string $table, array $values) : int {
115-
$builder = $this->conn->getQueryBuilder();
116-
$builder->insert($table);
117-
foreach ($values as $key => $value) {
118-
$builder->setValue($key, $builder->createNamedParameter($value));
116+
public function insertIgnoreConflict(string $table, array $values, array $types = []) : int {
117+
try {
118+
$builder = $this->conn->getQueryBuilder();
119+
$builder->insert($table);
120+
foreach ($values as $key => $value) {
121+
$builder->setValue($key, $builder->createNamedParameter($value, $types[$key] ?? IQueryBuilder::PARAM_STR));
122+
}
123+
return $builder->executeStatement();
124+
} catch (DbalException $e) {
125+
if ($e->getReason() === \OCP\DB\Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
126+
return 0;
127+
}
128+
throw $e;
119129
}
120-
$builder->ignoreConflictsOnInsert();
121-
return $builder->executeStatement();
122-
}
123-
124-
/**
125-
* Transform an INSERT statement into a conflict tolerant one. Platforms
126-
* without native support return the statement unchanged, the resulting
127-
* constraint violation is handled by the query builder.
128-
*/
129-
public function getInsertIgnoreConflictSql(string $sql): string {
130-
return $sql;
131130
}
132131
}

‎lib/private/DB/AdapterMySQL.php‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
namespace OC\DB;
1010

11+
use OCP\DB\QueryBuilder\IQueryBuilder;
12+
1113
class AdapterMySQL extends Adapter {
1214
/** @var string */
1315
protected $collation;
@@ -40,19 +42,32 @@ protected function getCollation(): string {
4042
return $this->collation;
4143
}
4244

43-
/**
44-
* We can't use ON DUPLICATE KEY UPDATE here because Nextcloud use the CLIENT_FOUND_ROWS flag
45-
* With this flag the MySQL returns the number of selected rows
46-
* instead of the number of affected/modified rows
47-
* It's impossible to change this behaviour at runtime or for a single query
48-
* Then, the result is 1 if a row is inserted and also 1 if a row is updated with same or different values
49-
*
50-
* With INSERT IGNORE, the result is 1 when a row is inserted, 0 otherwise
51-
*
52-
* Risk: it can also ignore other errors like type mismatch or truncated data…
53-
*/
5445
#[\Override]
55-
public function getInsertIgnoreConflictSql(string $sql): string {
56-
return preg_replace('/^INSERT/i', 'INSERT IGNORE', $sql) ?? $sql;
46+
public function insertIgnoreConflict(string $table, array $values, array $types = []): int {
47+
$builder = $this->conn->getQueryBuilder();
48+
$builder->insert($table);
49+
$updates = [];
50+
foreach ($values as $key => $value) {
51+
$builder->setValue($key, $builder->createNamedParameter($value, $types[$key] ?? IQueryBuilder::PARAM_STR));
52+
}
53+
54+
/*
55+
* We can't use ON DUPLICATE KEY UPDATE here because Nextcloud use the CLIENT_FOUND_ROWS flag
56+
* With this flag the MySQL returns the number of selected rows
57+
* instead of the number of affected/modified rows
58+
* It's impossible to change this behaviour at runtime or for a single query
59+
* Then, the result is 1 if a row is inserted and also 1 if a row is updated with same or different values
60+
*
61+
* With INSERT IGNORE, the result is 1 when a row is inserted, 0 otherwise
62+
*
63+
* Risk: it can also ignore other errors like type mismatch or truncated data…
64+
*/
65+
$res = $this->conn->executeStatement(
66+
preg_replace('/^INSERT/i', 'INSERT IGNORE', $builder->getSQL()),
67+
$builder->getParameters(),
68+
$builder->getParameterTypes()
69+
);
70+
71+
return $res;
5772
}
5873
}

‎lib/private/DB/AdapterPgSql.php‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
namespace OC\DB;
1010

11+
use OCP\DB\QueryBuilder\IQueryBuilder;
12+
1113
class AdapterPgSql extends Adapter {
1214

1315
#[\Override]
@@ -26,12 +28,16 @@ public function fixupStatement($statement) {
2628
return $statement;
2729
}
2830

29-
/**
30-
* "upsert" is only available since PgSQL 9.5, but the generic way
31-
* would leave error logs in the DB.
32-
*/
3331
#[\Override]
34-
public function getInsertIgnoreConflictSql(string $sql): string {
35-
return $sql . ' ON CONFLICT DO NOTHING';
32+
public function insertIgnoreConflict(string $table, array $values, array $types = []) : int {
33+
// "upsert" is only available since PgSQL 9.5, but the generic way
34+
// would leave error logs in the DB.
35+
$builder = $this->conn->getQueryBuilder();
36+
$builder->insert($table);
37+
foreach ($values as $key => $value) {
38+
$builder->setValue($key, $builder->createNamedParameter($value, $types[$key] ?? IQueryBuilder::PARAM_STR));
39+
}
40+
$queryString = $builder->getSQL() . ' ON CONFLICT DO NOTHING';
41+
return $this->conn->executeStatement($queryString, $builder->getParameters(), $builder->getParameterTypes());
3642
}
3743
}

‎lib/private/DB/AdapterSqlite.php‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace OC\DB;
1010

1111
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
12+
use OCP\DB\QueryBuilder\IQueryBuilder;
1213

1314
class AdapterSqlite extends Adapter {
1415
/**
@@ -83,7 +84,18 @@ public function insertIfNotExist($table, $input, ?array $compare = null) {
8384
}
8485

8586
#[\Override]
86-
public function getInsertIgnoreConflictSql(string $sql): string {
87-
return $sql . ' ON CONFLICT DO NOTHING';
87+
public function insertIgnoreConflict(string $table, array $values, array $types = []): int {
88+
$builder = $this->conn->getQueryBuilder();
89+
$builder->insert($table);
90+
$updates = [];
91+
foreach ($values as $key => $value) {
92+
$builder->setValue($key, $builder->createNamedParameter($value, $types[$key] ?? IQueryBuilder::PARAM_STR));
93+
}
94+
95+
return $this->conn->executeStatement(
96+
$builder->getSQL() . ' ON CONFLICT DO NOTHING',
97+
$builder->getParameters(),
98+
$builder->getParameterTypes()
99+
);
88100
}
89101
}

‎lib/private/DB/Connection.php‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -565,9 +565,9 @@ public function insertIfNotExist($table, $input, ?array $compare = null) {
565565
}
566566
}
567567

568-
public function insertIgnoreConflict(string $table, array $values) : int {
568+
public function insertIgnoreConflict(string $table, array $values, array $types = []) : int {
569569
try {
570-
return $this->adapter->insertIgnoreConflict($table, $values);
570+
return $this->adapter->insertIgnoreConflict($table, $values, $types);
571571
} catch (\Exception $e) {
572572
$this->logDatabaseException($e);
573573
throw $e;
@@ -991,8 +991,4 @@ public function getShardDefinition(string $name): ?ShardDefinition {
991991
public function getCrossShardMoveHelper(): CrossShardMoveHelper {
992992
return new CrossShardMoveHelper($this->shardConnectionManager);
993993
}
994-
995-
public function getInsertIgnoreConflictSql(string $sql): string {
996-
return $this->adapter->getInsertIgnoreConflictSql($sql);
997-
}
998994
}

‎lib/private/DB/ConnectionAdapter.php‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,9 @@ public function insertIfNotExist(string $table, array $input, ?array $compare =
9999
}
100100

101101
#[\Override]
102-
public function insertIgnoreConflict(string $table, array $values): int {
102+
public function insertIgnoreConflict(string $table, array $values, array $types = []): int {
103103
try {
104-
return $this->inner->insertIgnoreConflict($table, $values);
104+
return $this->inner->insertIgnoreConflict($table, $values, $types);
105105
} catch (Exception $e) {
106106
throw DbalException::wrap($e);
107107
}
@@ -299,8 +299,4 @@ public function getShardDefinition(string $name): ?ShardDefinition {
299299
public function getCrossShardMoveHelper(): CrossShardMoveHelper {
300300
return $this->inner->getCrossShardMoveHelper();
301301
}
302-
303-
public function getInsertIgnoreConflictSql(string $sql): string {
304-
return $this->inner->getInsertIgnoreConflictSql($sql);
305-
}
306302
}

‎lib/private/DB/QueryBuilder/ExtendedQueryBuilder.php‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -359,10 +359,4 @@ public function forUpdate(ConflictResolutionMode $conflictResolutionMode = Confl
359359
$this->builder->forUpdate($conflictResolutionMode);
360360
return $this;
361361
}
362-
363-
#[\Override]
364-
public function ignoreConflictsOnInsert(): self {
365-
$this->builder->ignoreConflictsOnInsert();
366-
return $this;
367-
}
368362
}

‎lib/private/DB/QueryBuilder/QueryBuilder.php‎

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
use OC\DB\QueryBuilder\FunctionBuilder\PgSqlFunctionBuilder;
2020
use OC\DB\QueryBuilder\FunctionBuilder\SqliteFunctionBuilder;
2121
use OC\SystemConfig;
22-
use OCP\DB\Exception;
2322
use OCP\DB\IResult;
2423
use OCP\DB\QueryBuilder\ConflictResolutionMode;
2524
use OCP\DB\QueryBuilder\ICompositeExpression;
@@ -41,7 +40,6 @@ class QueryBuilder extends TypedQueryBuilder {
4140
private bool $nonEmptyWhere = false;
4241
protected ?string $lastInsertedTable = null;
4342
private array $selectedColumns = [];
44-
private bool $insertIgnoreConflicts = false;
4543

4644
/**
4745
* Initializes a new QueryBuilder.
@@ -279,29 +277,11 @@ public function executeStatement(?IDBConnection $connection = null): int {
279277
$connection = $this->connection;
280278
}
281279

282-
try {
283-
return $connection->executeStatement(
284-
$this->getSQL(),
285-
$this->getParameters(),
286-
$this->getParameterTypes(),
287-
);
288-
} catch (Exception $e) {
289-
// fallback for platforms without native conflict tolerant inserts
290-
if ($this->insertIgnoreConflicts
291-
&& $e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
292-
return 0;
293-
}
294-
throw $e;
295-
}
296-
}
297-
298-
#[\Override]
299-
public function ignoreConflictsOnInsert(): self {
300-
if ($this->getType() !== \Doctrine\DBAL\Query\QueryBuilder::INSERT) {
301-
throw new \LogicException('ignoreConflictsOnInsert() can only be used on INSERT queries');
302-
}
303-
$this->insertIgnoreConflicts = true;
304-
return $this;
280+
return $connection->executeStatement(
281+
$this->getSQL(),
282+
$this->getParameters(),
283+
$this->getParameterTypes(),
284+
);
305285
}
306286

307287
/**
@@ -318,12 +298,7 @@ public function ignoreConflictsOnInsert(): self {
318298
*/
319299
#[\Override]
320300
public function getSQL() {
321-
$sql = $this->queryBuilder->getSQL();
322-
if ($this->insertIgnoreConflicts
323-
&& $this->getType() === \Doctrine\DBAL\Query\QueryBuilder::INSERT) {
324-
return $this->connection->getInsertIgnoreConflictSql($sql);
325-
}
326-
return $sql;
301+
return $this->queryBuilder->getSQL();
327302
}
328303

329304
/**

‎lib/public/AppFramework/Db/QBMapper.php‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,8 @@ public function insertIgnoreConflict(Entity $entity): int {
173173
// be saved
174174
$properties = $entity->getUpdatedFields();
175175

176-
$qb = $this->db->getQueryBuilder();
177-
$qb->insert($this->tableName);
178-
$qb->ignoreConflictsOnInsert();
176+
$values = [];
177+
$types = [];
179178

180179
// build the fields
181180
foreach ($properties as $property => $updated) {
@@ -185,11 +184,11 @@ public function insertIgnoreConflict(Entity $entity): int {
185184

186185
$column = $entity->propertyToColumn($property);
187186
$getter = 'get' . ucfirst($property);
188-
$type = $this->getParameterTypeForProperty($entity, $property);
189-
$qb->setValue($column, $qb->createNamedParameter($entity->$getter(), $type));
187+
$values[$column] = $entity->$getter();
188+
$types[$column] = $this->getParameterTypeForProperty($entity, $property);
190189
}
191190

192-
return $qb->executeStatement();
191+
return $this->db->insertIgnoreConflict($this->tableName, $values, $types);
193192
}
194193

195194
/**

‎lib/public/DB/QueryBuilder/IQueryBuilder.php‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -225,23 +225,6 @@ public function executeQuery(?IDBConnection $connection = null): IResult;
225225
*/
226226
public function executeStatement(?IDBConnection $connection = null): int;
227227

228-
/**
229-
* Ignore unique constraint conflicts for INSERT queries.
230-
*
231-
* Rows conflicting with an existing row on a unique constraint are skipped
232-
* instead of raising an error, the row count returned by
233-
* {@see self::executeStatement()} is reduced accordingly. On platforms
234-
* without native support for conflict tolerant inserts the resulting
235-
* constraint violation is caught and reported as 0 affected rows.
236-
*
237-
* Must only be called on INSERT queries.
238-
*
239-
* @return $this
240-
* @since 36.0.0
241-
* @throws \LogicException when called on a non-INSERT query
242-
*/
243-
public function ignoreConflictsOnInsert(): self;
244-
245228
/**
246229
* Gets the complete SQL string formed by the current specifications of this QueryBuilder.
247230
*

0 commit comments

Comments
 (0)