Skip to content

Commit e73cbe2

Browse files
tpetrytpetry
committed
fix: insertOrIgnoreReturning work with many unique keys
1 parent cfef6cd commit e73cbe2

File tree

7 files changed

+72
-41
lines changed

7 files changed

+72
-41
lines changed

‎src/Illuminate/Database/Eloquent/Model.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1543,7 +1543,7 @@ protected function performInsertOrIgnore(Builder $query, array|string $uniqueBy)
15431543
return true;
15441544
}
15451545

1546-
$result = $query->toBase()->insertOrIgnoreReturning($attributes, $uniqueBy);
1546+
$result = $query->toBase()->insertOrIgnoreReturning($attributes, ['*'], $uniqueBy);
15471547

15481548
if ($result->isEmpty()) {
15491549
return false;

‎src/Illuminate/Database/Query/Builder.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4156,13 +4156,13 @@ public function insertOrIgnore(array $values)
41564156
}
41574157

41584158
/**
4159-
* Insert new records into the database while ignoring specific conflicts and returning specified columns.
4159+
* Insert new records into the database and returning specified columns with optional ignoring specific conflicts.
41604160
*
4161-
* @param non-empty-string|non-empty-array<non-empty-string> $uniqueBy
41624161
* @param non-empty-array<non-empty-string> $returning
4162+
* @param non-empty-string|non-empty-array<non-empty-string>|null $uniqueBy
41634163
* @return \Illuminate\Support\Collection
41644164
*/
4165-
public function insertOrIgnoreReturning(array $values, array|string $uniqueBy, array $returning = ['*'])
4165+
public function insertOrIgnoreReturning(array $values, array $returning = ['*'], array|string|null $uniqueBy = null)
41664166
{
41674167
if (empty($values)) {
41684168
return new Collection;
@@ -4188,7 +4188,7 @@ public function insertOrIgnoreReturning(array $values, array|string $uniqueBy, a
41884188

41894189
$this->applyBeforeQueryCallbacks();
41904190

4191-
$sql = $this->grammar->compileInsertOrIgnoreReturning($this, $values, (array) $uniqueBy, $returning);
4191+
$sql = $this->grammar->compileInsertOrIgnoreReturning($this, $values, $returning, $uniqueBy === null ? null : Arr::wrap($uniqueBy));
41924192

41934193
$result = new Collection(
41944194
$this->connection->selectFromWriteConnection($sql, $this->cleanBindings(Arr::flatten($values, 1)))

‎src/Illuminate/Database/Query/Grammars/Grammar.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1255,13 +1255,13 @@ public function compileInsertOrIgnore(Builder $query, array $values)
12551255
*
12561256
* @param \Illuminate\Database\Query\Builder $query
12571257
* @param array $values
1258-
* @param array $uniqueBy
12591258
* @param array $returning
1259+
* @param array|null $uniqueBy
12601260
* @return string
12611261
*
12621262
* @throws \RuntimeException
12631263
*/
1264-
public function compileInsertOrIgnoreReturning(Builder $query, array $values, array $uniqueBy, array $returning)
1264+
public function compileInsertOrIgnoreReturning(Builder $query, array $values, array $returning, array|null $uniqueBy)
12651265
{
12661266
throw new RuntimeException('This database engine does not support insert or ignore with returning.');
12671267
}

‎src/Illuminate/Database/Query/Grammars/PostgresGrammar.php‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -378,15 +378,18 @@ public function compileInsertOrIgnore(Builder $query, array $values)
378378
*
379379
* @param \Illuminate\Database\Query\Builder $query
380380
* @param array $values
381-
* @param array $uniqueBy
382381
* @param array $returning
382+
* @param array|null $uniqueBy
383383
* @return string
384384
*/
385-
public function compileInsertOrIgnoreReturning(Builder $query, array $values, array $uniqueBy, array $returning)
385+
public function compileInsertOrIgnoreReturning(Builder $query, array $values, array $returning, array|null $uniqueBy)
386386
{
387-
return $this->compileInsert($query, $values)
388-
.' on conflict ('.$this->columnize($uniqueBy).') do nothing'
389-
.' returning '.$this->columnize($returning);
387+
$insert = $this->compileInsert($query, $values);
388+
389+
return match ($uniqueBy) {
390+
null => "{$insert} on conflict do nothing returning {$this->columnize($returning)}",
391+
default => "{$insert} on conflict ({$this->columnize($uniqueBy)}) do nothing returning {$this->columnize($returning)}",
392+
};
390393
}
391394

392395
/**

‎src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -293,15 +293,18 @@ public function compileInsertOrIgnore(Builder $query, array $values)
293293
*
294294
* @param \Illuminate\Database\Query\Builder $query
295295
* @param array $values
296-
* @param array $uniqueBy
297296
* @param array $returning
297+
* @param array|null $uniqueBy
298298
* @return string
299299
*/
300-
public function compileInsertOrIgnoreReturning(Builder $query, array $values, array $uniqueBy, array $returning)
300+
public function compileInsertOrIgnoreReturning(Builder $query, array $values, array $returning, array|null $uniqueBy)
301301
{
302-
return $this->compileInsert($query, $values)
303-
.' on conflict ('.$this->columnize($uniqueBy).') do nothing'
304-
.' returning '.$this->columnize($returning);
302+
$insert = $this->compileInsert($query, $values);
303+
304+
return match ($uniqueBy) {
305+
null => "{$insert} on conflict do nothing returning {$this->columnize($returning)}",
306+
default => "{$insert} on conflict ({$this->columnize($uniqueBy)}) do nothing returning {$this->columnize($returning)}",
307+
};
305308
}
306309

307310
/**

‎tests/Database/DatabaseEloquentModelTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,7 +1118,7 @@ public function testInsertOrIgnoreProcessWithIncrementing()
11181118
$query = m::mock(Builder::class);
11191119
$baseQuery = m::mock(BaseBuilder::class);
11201120
$query->shouldReceive('toBase')->once()->andReturn($baseQuery);
1121-
$baseQuery->shouldReceive('insertOrIgnoreReturning')->once()->with(['name' => 'taylor'], ['name'])->andReturn(new BaseCollection([(object) ['id' => 1, 'name' => 'taylor']]));
1121+
$baseQuery->shouldReceive('insertOrIgnoreReturning')->once()->with(['name' => 'taylor'], ['*'], ['name'])->andReturn(new BaseCollection([(object) ['id' => 1, 'name' => 'taylor']]));
11221122
$query->shouldReceive('getConnection')->once();
11231123
$model->expects($this->once())->method('newModelQuery')->willReturn($query);
11241124
$model->expects($this->once())->method('updateTimestamps');
@@ -1143,7 +1143,7 @@ public function testInsertOrIgnoreProcessWithConflict()
11431143
$query = m::mock(Builder::class);
11441144
$baseQuery = m::mock(BaseBuilder::class);
11451145
$query->shouldReceive('toBase')->once()->andReturn($baseQuery);
1146-
$baseQuery->shouldReceive('insertOrIgnoreReturning')->once()->with(['name' => 'taylor'], ['name'])->andReturn(new BaseCollection);
1146+
$baseQuery->shouldReceive('insertOrIgnoreReturning')->once()->with(['name' => 'taylor'], ['*'], ['name'])->andReturn(new BaseCollection);
11471147
$query->shouldReceive('getConnection')->once();
11481148
$model->expects($this->once())->method('newModelQuery')->willReturn($query);
11491149
$model->expects($this->once())->method('updateTimestamps');
@@ -1165,7 +1165,7 @@ public function testInsertOrIgnoreProcessWithNonIncrementing()
11651165
$query = m::mock(Builder::class);
11661166
$baseQuery = m::mock(BaseBuilder::class);
11671167
$query->shouldReceive('toBase')->once()->andReturn($baseQuery);
1168-
$baseQuery->shouldReceive('insertOrIgnoreReturning')->once()->with(['name' => 'taylor'], ['name'])->andReturn(new BaseCollection([(object) ['name' => 'taylor']]));
1168+
$baseQuery->shouldReceive('insertOrIgnoreReturning')->once()->with(['name' => 'taylor'], ['*'], ['name'])->andReturn(new BaseCollection([(object) ['name' => 'taylor']]));
11691169
$query->shouldReceive('getConnection')->once();
11701170
$model->expects($this->once())->method('newModelQuery')->willReturn($query);
11711171
$model->expects($this->once())->method('updateTimestamps');

‎tests/Database/DatabaseQueryBuilderTest.php‎

Lines changed: 46 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4111,13 +4111,13 @@ public function testInsertOrIgnoreReturningMethod()
41114111
$this->expectException(RuntimeException::class);
41124112
$this->expectExceptionMessage('does not support');
41134113
$builder = $this->getBuilder();
4114-
$builder->from('users')->insertOrIgnoreReturning(['email' => 'foo'], 'email');
4114+
$builder->from('users')->insertOrIgnoreReturning(['email' => 'foo']);
41154115
}
41164116

41174117
public function testInsertOrIgnoreReturningMethodWithEmptyValues()
41184118
{
41194119
$builder = $this->getPostgresBuilder();
4120-
$result = $builder->from('users')->insertOrIgnoreReturning([], 'email');
4120+
$result = $builder->from('users')->insertOrIgnoreReturning([]);
41214121
$this->assertInstanceOf(Collection::class, $result);
41224122
$this->assertTrue($result->isEmpty());
41234123
}
@@ -4127,31 +4127,44 @@ public function testMySqlInsertOrIgnoreReturningMethod()
41274127
$this->expectException(RuntimeException::class);
41284128
$this->expectExceptionMessage('does not support');
41294129
$builder = $this->getMySqlBuilder();
4130-
$builder->from('users')->insertOrIgnoreReturning(['email' => 'foo'], 'email');
4130+
$builder->from('users')->insertOrIgnoreReturning(['email' => 'foo']);
41314131
}
41324132

41334133
public function testPostgresInsertOrIgnoreReturningMethod()
41344134
{
41354135
$builder = $this->getPostgresBuilder();
41364136
$builder->getConnection()->shouldReceive('recordsHaveBeenModified')->once();
41374137
$builder->getConnection()->shouldReceive('selectFromWriteConnection')->once()->with(
4138-
'insert into "users" ("email") values (?) on conflict ("email") do nothing returning "id"',
4138+
'insert into "users" ("email") values (?) on conflict do nothing returning "id"',
41394139
['foo']
41404140
)->andReturn([['id' => 1]]);
4141-
$result = $builder->from('users')->insertOrIgnoreReturning(['email' => 'foo'], 'email', ['id']);
4141+
$result = $builder->from('users')->insertOrIgnoreReturning(['email' => 'foo'], ['id']);
41424142
$this->assertInstanceOf(Collection::class, $result);
41434143
$this->assertEquals([['id' => 1]], $result->all());
41444144
}
41454145

4146-
public function testPostgresInsertOrIgnoreReturningMethodWithMultipleUniqueByColumns()
4146+
public function testPostgresInsertOrIgnoreReturningMethodWithUniqueByColumn()
4147+
{
4148+
$builder = $this->getPostgresBuilder();
4149+
$builder->getConnection()->shouldReceive('recordsHaveBeenModified')->once();
4150+
$builder->getConnection()->shouldReceive('selectFromWriteConnection')->once()->with(
4151+
'insert into "users" ("email", "name") values (?, ?) on conflict ("email") do nothing returning *',
4152+
['foo', 'bar']
4153+
)->andReturn([['id' => 1, 'email' => 'foo', 'name' => 'bar']]);
4154+
$result = $builder->from('users')->insertOrIgnoreReturning(['email' => 'foo', 'name' => 'bar'], ['*'], 'email');
4155+
$this->assertInstanceOf(Collection::class, $result);
4156+
$this->assertEquals([['id' => 1, 'email' => 'foo', 'name' => 'bar']], $result->all());
4157+
}
4158+
4159+
public function testPostgresInsertOrIgnoreReturningMethodWithUniqueByColumns()
41474160
{
41484161
$builder = $this->getPostgresBuilder();
41494162
$builder->getConnection()->shouldReceive('recordsHaveBeenModified')->once();
41504163
$builder->getConnection()->shouldReceive('selectFromWriteConnection')->once()->with(
41514164
'insert into "users" ("email", "name") values (?, ?) on conflict ("email", "name") do nothing returning *',
41524165
['foo', 'bar']
41534166
)->andReturn([['id' => 1, 'email' => 'foo', 'name' => 'bar']]);
4154-
$result = $builder->from('users')->insertOrIgnoreReturning(['email' => 'foo', 'name' => 'bar'], ['email', 'name']);
4167+
$result = $builder->from('users')->insertOrIgnoreReturning(['email' => 'foo', 'name' => 'bar'], ['*'], ['email', 'name']);
41554168
$this->assertInstanceOf(Collection::class, $result);
41564169
$this->assertEquals([['id' => 1, 'email' => 'foo', 'name' => 'bar']], $result->all());
41574170
}
@@ -4161,12 +4174,11 @@ public function testPostgresInsertOrIgnoreReturningMethodWithMultipleRecords()
41614174
$builder = $this->getPostgresBuilder();
41624175
$builder->getConnection()->shouldReceive('recordsHaveBeenModified')->once();
41634176
$builder->getConnection()->shouldReceive('selectFromWriteConnection')->once()->with(
4164-
'insert into "users" ("email") values (?), (?) on conflict ("email") do nothing returning "id", "email"',
4177+
'insert into "users" ("email") values (?), (?) on conflict do nothing returning "id", "email"',
41654178
['foo', 'bar']
41664179
)->andReturn([['id' => 1, 'email' => 'foo']]);
41674180
$result = $builder->from('users')->insertOrIgnoreReturning(
41684181
[['email' => 'foo'], ['email' => 'bar']],
4169-
'email',
41704182
['id', 'email']
41714183
);
41724184
$this->assertInstanceOf(Collection::class, $result);
@@ -4178,23 +4190,37 @@ public function testSqliteInsertOrIgnoreReturningMethod()
41784190
$builder = $this->getSQLiteBuilder();
41794191
$builder->getConnection()->shouldReceive('recordsHaveBeenModified')->once();
41804192
$builder->getConnection()->shouldReceive('selectFromWriteConnection')->once()->with(
4181-
'insert into "users" ("email") values (?) on conflict ("email") do nothing returning "id"',
4193+
'insert into "users" ("email") values (?) on conflict do nothing returning "id"',
41824194
['foo']
41834195
)->andReturn([['id' => 1]]);
4184-
$result = $builder->from('users')->insertOrIgnoreReturning(['email' => 'foo'], 'email', ['id']);
4196+
$result = $builder->from('users')->insertOrIgnoreReturning(['email' => 'foo'], ['id']);
41854197
$this->assertInstanceOf(Collection::class, $result);
41864198
$this->assertEquals([['id' => 1]], $result->all());
41874199
}
41884200

4189-
public function testSqliteInsertOrIgnoreReturningMethodWithMultipleUniqueByColumns()
4201+
4202+
public function testSqliteInsertOrIgnoreReturningMethodWithUniqueByColumn()
4203+
{
4204+
$builder = $this->getSQLiteBuilder();
4205+
$builder->getConnection()->shouldReceive('recordsHaveBeenModified')->once();
4206+
$builder->getConnection()->shouldReceive('selectFromWriteConnection')->once()->with(
4207+
'insert into "users" ("email", "name") values (?, ?) on conflict ("email") do nothing returning *',
4208+
['foo', 'bar']
4209+
)->andReturn([['id' => 1, 'email' => 'foo', 'name' => 'bar']]);
4210+
$result = $builder->from('users')->insertOrIgnoreReturning(['email' => 'foo', 'name' => 'bar'], ['*'], 'email');
4211+
$this->assertInstanceOf(Collection::class, $result);
4212+
$this->assertEquals([['id' => 1, 'email' => 'foo', 'name' => 'bar']], $result->all());
4213+
}
4214+
4215+
public function testSqliteInsertOrIgnoreReturningMethodWithUniqueByColumns()
41904216
{
41914217
$builder = $this->getSQLiteBuilder();
41924218
$builder->getConnection()->shouldReceive('recordsHaveBeenModified')->once();
41934219
$builder->getConnection()->shouldReceive('selectFromWriteConnection')->once()->with(
41944220
'insert into "users" ("email", "name") values (?, ?) on conflict ("email", "name") do nothing returning *',
41954221
['foo', 'bar']
41964222
)->andReturn([['id' => 1, 'email' => 'foo', 'name' => 'bar']]);
4197-
$result = $builder->from('users')->insertOrIgnoreReturning(['email' => 'foo', 'name' => 'bar'], ['email', 'name']);
4223+
$result = $builder->from('users')->insertOrIgnoreReturning(['email' => 'foo', 'name' => 'bar'], ['*'], ['email', 'name']);
41984224
$this->assertInstanceOf(Collection::class, $result);
41994225
$this->assertEquals([['id' => 1, 'email' => 'foo', 'name' => 'bar']], $result->all());
42004226
}
@@ -4204,12 +4230,11 @@ public function testSqliteInsertOrIgnoreReturningMethodWithMultipleRecords()
42044230
$builder = $this->getSQLiteBuilder();
42054231
$builder->getConnection()->shouldReceive('recordsHaveBeenModified')->once();
42064232
$builder->getConnection()->shouldReceive('selectFromWriteConnection')->once()->with(
4207-
'insert into "users" ("email") values (?), (?) on conflict ("email") do nothing returning "id", "email"',
4233+
'insert into "users" ("email") values (?), (?) on conflict do nothing returning "id", "email"',
42084234
['foo', 'bar']
42094235
)->andReturn([['id' => 1, 'email' => 'foo']]);
42104236
$result = $builder->from('users')->insertOrIgnoreReturning(
42114237
[['email' => 'foo'], ['email' => 'bar']],
4212-
'email',
42134238
['id', 'email']
42144239
);
42154240
$this->assertInstanceOf(Collection::class, $result);
@@ -4221,42 +4246,42 @@ public function testSqlServerInsertOrIgnoreReturningMethod()
42214246
$this->expectException(RuntimeException::class);
42224247
$this->expectExceptionMessage('does not support');
42234248
$builder = $this->getSqlServerBuilder();
4224-
$builder->from('users')->insertOrIgnoreReturning(['email' => 'foo'], 'email');
4249+
$builder->from('users')->insertOrIgnoreReturning(['email' => 'foo']);
42254250
}
42264251

42274252
public function testInsertOrIgnoreReturningWithEmptyUniqueByArray()
42284253
{
42294254
$this->expectException(InvalidArgumentException::class);
42304255
$this->expectExceptionMessage('The unique columns must not be empty.');
42314256
$builder = $this->getPostgresBuilder();
4232-
$builder->from('users')->insertOrIgnoreReturning(['email' => 'foo'], []);
4257+
$builder->from('users')->insertOrIgnoreReturning(['email' => 'foo'], ['*'], []);
42334258
}
42344259

42354260
public function testInsertOrIgnoreReturningWithEmptyUniqueByString()
42364261
{
42374262
$this->expectException(InvalidArgumentException::class);
42384263
$this->expectExceptionMessage('The unique columns must not be empty.');
42394264
$builder = $this->getPostgresBuilder();
4240-
$builder->from('users')->insertOrIgnoreReturning(['email' => 'foo'], '');
4265+
$builder->from('users')->insertOrIgnoreReturning(['email' => 'foo'], ['*'], '');
42414266
}
42424267

42434268
public function testInsertOrIgnoreReturningWithEmptyReturning()
42444269
{
42454270
$this->expectException(InvalidArgumentException::class);
42464271
$this->expectExceptionMessage('The returning columns must not be empty.');
42474272
$builder = $this->getPostgresBuilder();
4248-
$builder->from('users')->insertOrIgnoreReturning(['email' => 'foo'], 'email', []);
4273+
$builder->from('users')->insertOrIgnoreReturning(['email' => 'foo'], []);
42494274
}
42504275

42514276
public function testInsertOrIgnoreReturningDoesNotMarkRecordsModifiedWhenNoRowsWereInserted()
42524277
{
42534278
$builder = $this->getPostgresBuilder();
42544279
$builder->getConnection()->shouldReceive('selectFromWriteConnection')->once()->with(
4255-
'insert into "users" ("email") values (?) on conflict ("email") do nothing returning *',
4280+
'insert into "users" ("email") values (?) on conflict do nothing returning *',
42564281
['foo']
42574282
)->andReturn([]);
42584283
$builder->getConnection()->shouldReceive('recordsHaveBeenModified')->once()->with(false);
4259-
$result = $builder->from('users')->insertOrIgnoreReturning(['email' => 'foo'], 'email');
4284+
$result = $builder->from('users')->insertOrIgnoreReturning(['email' => 'foo']);
42604285
$this->assertInstanceOf(Collection::class, $result);
42614286
$this->assertTrue($result->isEmpty());
42624287
}

0 commit comments

Comments
 (0)