Merge pull request #7 from plank/fix-newest-scopes

fix: newest subquery failing in sqlite + improve its execution plan
plank · Dec 15, 2021 · a58cdb1 · a58cdb1
2 parents 5637b0f + 39e6762
commit a58cdb1
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 8 deletions.
diff --git a/src/Concerns/HasRevisions.php b/src/Concerns/HasRevisions.php
@@ -147,20 +147,39 @@ public function scopeWithNext(Builder $builder)
     }
 
     /**
-     * Get the id of the initial revisioned item
+     * Get the id of the most recent revisioned item
      *
+     * @param Builder|self $builder
      * @return mixed
      */
     public function scopeWithNewestAt($builder, $until = null, $since = null)
     {
+        /**
+         * @var Revision $revision
+         */
         $revision = config('checkpoint.models.revision');
-        return $builder->withInitial()->addSelect([
-            'newest_id' => $revision::select('revisionable_id')
-                ->whereIn('id', $revision::latestIds($until, $since)
-                    ->whereColumn('original_revisionable_id', 'initial_id')
-                    ->where('revisionable_type', get_class($this))
-                )
-        ])->with('newest');
+
+        $newestIdQuery = $revision::select('revisionable_id')
+            ->whereColumn('original_revisionable_id', 'initial_id')
+            ->whereType($this)
+            ->whereIn((new $revision)->getKeyName(), $revision::latestIds($until, $since)
+                ->whereColumn('original_revisionable_id', 'initial_id')
+                ->whereType($this)
+            );
+
+        // The SQL Standard does not allow referencing aliases in the SELECT clause,
+        // Postgres and SQLite follow this convention, mysql does not. When the
+        // driver is mysql, prefer 2 SELECT sub-queries over a nested FROM sub-query
+        // TODO: confirm this works on other db drivers (postgres, mssql, mongo?)
+        if($builder->getConnection()->getDriverName() === 'mysql') {
+            $withNewest = $builder->withInitial()->addSelect(['newest_id' => $newestIdQuery]);
+        } else {
+            $model = $builder->getModel();
+            $withNewest = $model->newQueryWithoutScopes()
+                ->addSelect(['newest_id' => $newestIdQuery])
+                ->from($builder->withInitial(), $model->getTable());
+        }
+        return $withNewest->with('newest', 'initial');
     }
 
     /**

diff --git a/src/Models/Revision.php b/src/Models/Revision.php
@@ -32,6 +32,7 @@
  * @method static Builder|Revision newModelQuery()
  * @method static Builder|Revision newQuery()
  * @method static Builder|Revision query()
+ * @method static \Illuminate\Database\Query\Builder|Revision select($columns = ['*'])
  * @method static Builder|Revision whereId($value)
  * @method static Builder|Revision whereType($value)
  * @method static Builder|Revision whereRevisionableId($value)

diff --git a/tests/Feature/RevisionableRelationsTest.php b/tests/Feature/RevisionableRelationsTest.php
@@ -91,6 +91,27 @@ public function retrieve_next_revisionable_if_available(): void
         $this->assertEquals($post->id, $revision2->revisionable_id);
     }
 
+    /**
+     * @test
+     */
+    public function retrieve_newest_revisionable(): void
+    {
+        $post = factory(Post::class)->create();
+        $revision1 = $post->revision;
+        $original = clone $post;
+
+        $original_newest_id = $original->newest()->first()->id;
+        $this->assertEquals($post->id, $original_newest_id);
+
+        $post->performRevision();
+        $revision2 = $post->revision;
+        $this->assertEquals($post->id, Post::withNewest()->find($post->id)->newest->id);
+        $this->assertNotEquals($original_newest_id, Post::withNewest()->find($post->id)->newest->id);
+
+        $this->assertEquals($original->id, $revision1->revisionable_id);
+        $this->assertEquals($post->id, $revision2->revisionable_id);
+    }
+
     /**
      * @test
      */