From c6c6434e2c1c14f1c66cd91184e33beed488bee7 Mon Sep 17 00:00:00 2001 From: Abdulrahman Date: Mon, 4 Dec 2017 18:36:12 +0300 Subject: [PATCH 1/4] Fix SQLite delete for complex delete statements --- .../Database/Query/Grammars/SQLiteGrammar.php | 42 +++++++++++++++++++ tests/Database/DatabaseQueryBuilderTest.php | 11 +++++ 2 files changed, 53 insertions(+) diff --git a/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php b/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php index 77aa46e99882..be06f79a31ea 100755 --- a/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php +++ b/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php @@ -174,6 +174,48 @@ public function compileInsert(Builder $query, array $values) return "insert into $table ($names) select ".implode(' union all select ', $columns); } + + /** + * Compile a delete statement into SQL. + * + * @param \Illuminate\Database\Query\Builder $query + * @return string + */ + public function compileDelete(Builder $query) + { + // SQLite delete statment doesn't fully support complex keywords like joins .. + // We use select statment as a subquery to overcome this delimma. + if (isset($query->joins) || isset($query->limit)) { + // Since rowid is common column between all SQLite tables, + // we use it in select subquery and in delete where-in statment. + $selectSql = parent::compileSelect($query->select("{$query->from}.rowid")); + + return trim( + "delete from {$this->wrapTable($query->from)} where {$this->wrap('rowid')} in ({$selectSql})" + ); + } + + $wheres = is_array($query->wheres) ? $this->compileWheres($query) : ''; + + return trim("delete from {$this->wrapTable($query->from)} $wheres"); + } + + /** + * Prepare the bindings for a delete statement. + * + * @param array $bindings + * @param array $values + * @return array + */ + public function prepareBindingsForDelete(array $bindings) + { + $cleanBindings = Arr::except($bindings, ['join', 'select']); + + return array_values( + array_merge($bindings['join'], Arr::flatten($cleanBindings)) + ); + } + /** * Compile a truncate table statement into SQL. * diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index 982dd7db467f..7527c3f9a242 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -1578,6 +1578,12 @@ public function testDeleteMethod() $result = $builder->from('users')->delete(1); $this->assertEquals(1, $result); + $builder = $this->getSqliteBuilder(); + $builder->getConnection()->shouldReceive('delete')->once()->with('delete from "users" where "rowid" in (select "users"."rowid" from "users" where "email" = ? order by "id" asc limit 1)', ['foo'])->andReturn(1); + $result = $builder->from('users')->where('email', '=', 'foo')->orderBy('id')->take(1)->delete(); + $this->assertEquals(1, $result); + + $builder = $this->getMySqlBuilder(); $builder->getConnection()->shouldReceive('delete')->once()->with('delete from `users` where `email` = ? order by `id` asc limit 1', ['foo'])->andReturn(1); $result = $builder->from('users')->where('email', '=', 'foo')->orderBy('id')->take(1)->delete(); @@ -1591,6 +1597,11 @@ public function testDeleteMethod() public function testDeleteWithJoinMethod() { + $builder = $this->getSqliteBuilder(); + $builder->getConnection()->shouldReceive('delete')->once()->with('delete from "users" where "rowid" in (select "users"."rowid" from "users" inner join "contacts" on "users"."id" = "contacts"."id" where "users"."email" = ? order by "users"."id" asc limit 1)', ['foo'])->andReturn(1); + $result = $builder->from('users')->join('contacts', 'users.id', '=', 'contacts.id')->where('users.email', '=', 'foo')->orderBy('users.id')->limit(1)->delete(); + $this->assertEquals(1, $result); + $builder = $this->getMySqlBuilder(); $builder->getConnection()->shouldReceive('delete')->once()->with('delete `users` from `users` inner join `contacts` on `users`.`id` = `contacts`.`id` where `email` = ?', ['foo'])->andReturn(1); $result = $builder->from('users')->join('contacts', 'users.id', '=', 'contacts.id')->where('email', '=', 'foo')->orderBy('id')->limit(1)->delete(); From 54f23fe1e08b25405b1ceaae1b7389dd572bc400 Mon Sep 17 00:00:00 2001 From: Abdulrahman Date: Mon, 4 Dec 2017 19:16:35 +0300 Subject: [PATCH 2/4] add a missing import --- src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php b/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php index be06f79a31ea..c4f55b6da59a 100755 --- a/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php +++ b/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php @@ -2,6 +2,7 @@ namespace Illuminate\Database\Query\Grammars; +use Illuminate\Support\Arr; use Illuminate\Database\Query\Builder; class SQLiteGrammar extends Grammar From fb367c331745fb282e45ebeda5c24f00bd4cec3e Mon Sep 17 00:00:00 2001 From: Abdulrahman Date: Tue, 5 Dec 2017 10:14:16 +0300 Subject: [PATCH 3/4] typos --- src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php b/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php index c4f55b6da59a..fec0f365d5dc 100755 --- a/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php +++ b/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php @@ -184,11 +184,11 @@ public function compileInsert(Builder $query, array $values) */ public function compileDelete(Builder $query) { - // SQLite delete statment doesn't fully support complex keywords like joins .. - // We use select statment as a subquery to overcome this delimma. + // SQLite delete statement doesn't fully support complex keywords like joins .. + // We use select statement as a sub-query to overcome this dilemma. if (isset($query->joins) || isset($query->limit)) { // Since rowid is common column between all SQLite tables, - // we use it in select subquery and in delete where-in statment. + // we use it in select sub-query and in delete where-in statement. $selectSql = parent::compileSelect($query->select("{$query->from}.rowid")); return trim( From 9abcab4d36fdba8897c01aac6d39deadf3e61156 Mon Sep 17 00:00:00 2001 From: Abdulrahman Date: Tue, 5 Dec 2017 11:25:56 +0300 Subject: [PATCH 4/4] Integration tests --- .../Database/Query/Grammars/SQLiteGrammar.php | 5 +- .../Database/EloquentDeleteTest.php | 70 +++++++++++++++++++ 2 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 tests/Integration/Database/EloquentDeleteTest.php diff --git a/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php b/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php index fec0f365d5dc..44129e43c777 100755 --- a/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php +++ b/src/Illuminate/Database/Query/Grammars/SQLiteGrammar.php @@ -175,7 +175,6 @@ public function compileInsert(Builder $query, array $values) return "insert into $table ($names) select ".implode(' union all select ', $columns); } - /** * Compile a delete statement into SQL. * @@ -191,9 +190,7 @@ public function compileDelete(Builder $query) // we use it in select sub-query and in delete where-in statement. $selectSql = parent::compileSelect($query->select("{$query->from}.rowid")); - return trim( - "delete from {$this->wrapTable($query->from)} where {$this->wrap('rowid')} in ({$selectSql})" - ); + return "delete from {$this->wrapTable($query->from)} where {$this->wrap('rowid')} in ({$selectSql})"; } $wheres = is_array($query->wheres) ? $this->compileWheres($query) : ''; diff --git a/tests/Integration/Database/EloquentDeleteTest.php b/tests/Integration/Database/EloquentDeleteTest.php new file mode 100644 index 000000000000..67acae6135ba --- /dev/null +++ b/tests/Integration/Database/EloquentDeleteTest.php @@ -0,0 +1,70 @@ +set('app.debug', 'true'); + + $app['config']->set('database.default', 'testbench'); + + $app['config']->set('database.connections.testbench', [ + 'driver' => 'sqlite', + 'database' => ':memory:', + 'prefix' => '', + ]); + } + + public function setUp() + { + parent::setUp(); + + Schema::create('posts', function ($table) { + $table->increments('id'); + $table->string('title')->nullable(); + $table->timestamps(); + }); + + Schema::create('comments', function ($table) { + $table->increments('id'); + $table->string('body')->nullable(); + $table->integer('post_id'); + $table->timestamps(); + }); + } + + public function testOnlyDeleteWhatGiven() + { + for($i = 1; $i <= 10; $i++) { + Comment::create([ + 'post_id' => Post::create()->id + ]); + } + + Post::latest('id')->limit(1)->delete(); + $this->assertEquals(9, Post::all()->count()); + + Post::join('comments', 'comments.post_id', '=', 'posts.id')->where('posts.id', '>', 1)->orderBy('posts.id')->limit(1)->delete(); + $this->assertEquals(8, Post::all()->count()); + } +} + +class Post extends Model +{ + public $table = 'posts'; +} + +class Comment extends Model +{ + public $table = 'comments'; + protected $fillable = ['post_id']; +}