From 8c5a884b9fcbcc96a3575737b142b88ce0656050 Mon Sep 17 00:00:00 2001 From: Thijs van den Anker Date: Wed, 27 Mar 2024 21:43:36 +0100 Subject: [PATCH 1/2] Cursor paginate uses incorrect column name for where on union --- tests/Database/DatabaseQueryBuilderTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index 40500d9ed514..ccf2f9ca00a9 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -5272,7 +5272,7 @@ public function testCursorPaginateWithUnionWheres() $builder->shouldReceive('get')->once()->andReturnUsing(function () use ($builder, $results, $ts) { $this->assertEquals( - '(select "id", "start_time" as "created_at", \'video\' as type from "videos" where ("start_time" > ?)) union (select "id", "created_at", \'news\' as type from "news" where ("start_time" > ?)) order by "created_at" asc limit 17', + '(select "id", "start_time" as "created_at", \'video\' as type from "videos" where ("start_time" > ?)) union (select "id", "created_at", \'news\' as type from "news" where ("created_at" > ?)) order by "created_at" asc limit 17', $builder->toSql()); $this->assertEquals([$ts], $builder->bindings['where']); $this->assertEquals([$ts], $builder->bindings['union']); @@ -5319,7 +5319,7 @@ public function testCursorPaginateWithUnionWheresWithRawOrderExpression() $builder->shouldReceive('get')->once()->andReturnUsing(function () use ($builder, $results, $ts) { $this->assertEquals( - '(select "id", "is_published", "start_time" as "created_at", \'video\' as type from "videos" where "is_published" = ? and ("start_time" > ?)) union (select "id", "is_published", "created_at", \'news\' as type from "news" where "is_published" = ? and ("start_time" > ?)) order by case when (id = 3 and type="news" then 0 else 1 end), "created_at" asc limit 17', + '(select "id", "is_published", "start_time" as "created_at", \'video\' as type from "videos" where "is_published" = ? and ("start_time" > ?)) union (select "id", "is_published", "created_at", \'news\' as type from "news" where "is_published" = ? and ("created_at" > ?)) order by case when (id = 3 and type="news" then 0 else 1 end), "created_at" asc limit 17', $builder->toSql()); $this->assertEquals([true, $ts], $builder->bindings['where']); $this->assertEquals([true, $ts], $builder->bindings['union']); @@ -5366,7 +5366,7 @@ public function testCursorPaginateWithUnionWheresReverseOrder() $builder->shouldReceive('get')->once()->andReturnUsing(function () use ($builder, $results, $ts) { $this->assertEquals( - '(select "id", "start_time" as "created_at", \'video\' as type from "videos" where ("start_time" < ?)) union (select "id", "created_at", \'news\' as type from "news" where ("start_time" < ?)) order by "created_at" desc limit 17', + '(select "id", "start_time" as "created_at", \'video\' as type from "videos" where ("start_time" < ?)) union (select "id", "created_at", \'news\' as type from "news" where ("created_at" < ?)) order by "created_at" desc limit 17', $builder->toSql()); $this->assertEquals([$ts], $builder->bindings['where']); $this->assertEquals([$ts], $builder->bindings['union']); @@ -5413,7 +5413,7 @@ public function testCursorPaginateWithUnionWheresMultipleOrders() $builder->shouldReceive('get')->once()->andReturnUsing(function () use ($builder, $results, $ts) { $this->assertEquals( - '(select "id", "start_time" as "created_at", \'video\' as type from "videos" where ("start_time" < ? or ("start_time" = ? and ("id" > ?)))) union (select "id", "created_at", \'news\' as type from "news" where ("start_time" < ? or ("start_time" = ? and ("id" > ?)))) order by "created_at" desc, "id" asc limit 17', + '(select "id", "start_time" as "created_at", \'video\' as type from "videos" where ("start_time" < ? or ("start_time" = ? and ("id" > ?)))) union (select "id", "created_at", \'news\' as type from "news" where ("created_at" < ? or ("created_at" = ? and ("id" > ?)))) order by "created_at" desc, "id" asc limit 17', $builder->toSql()); $this->assertEquals([$ts, $ts, 1], $builder->bindings['where']); $this->assertEquals([$ts, $ts, 1], $builder->bindings['union']); From d4e0d301a96ad2f549b759db22267e10b97a1d46 Mon Sep 17 00:00:00 2001 From: Thijs van den Anker Date: Wed, 27 Mar 2024 22:07:29 +0100 Subject: [PATCH 2/2] Set the right column names for the cursor on the unions --- .../Database/Concerns/BuildsQueries.php | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/Illuminate/Database/Concerns/BuildsQueries.php b/src/Illuminate/Database/Concerns/BuildsQueries.php index 0d45b0a6f6fc..4c77c964f78e 100644 --- a/src/Illuminate/Database/Concerns/BuildsQueries.php +++ b/src/Illuminate/Database/Concerns/BuildsQueries.php @@ -379,11 +379,11 @@ protected function paginateUsingCursor($perPage, $columns = ['*'], $cursorName = $orders = $this->ensureOrderForCursorPagination(! is_null($cursor) && $cursor->pointsToPreviousItems()); if (! is_null($cursor)) { - $addCursorConditions = function (self $builder, $previousColumn, $i) use (&$addCursorConditions, $cursor, $orders) { + $addCursorConditions = function (self $builder, $previousColumn, $originalColumn, $i) use (&$addCursorConditions, $cursor, $orders) { $unionBuilders = isset($builder->unions) ? collect($builder->unions)->pluck('query') : collect(); if (! is_null($previousColumn)) { - $originalColumn = $this->getOriginalColumnNameForCursorPagination($this, $previousColumn); + $originalColumn ??= $this->getOriginalColumnNameForCursorPagination($this, $previousColumn); $builder->where( Str::contains($originalColumn, ['(', ')']) ? new Expression($originalColumn) : $originalColumn, @@ -393,7 +393,7 @@ protected function paginateUsingCursor($perPage, $columns = ['*'], $cursorName = $unionBuilders->each(function ($unionBuilder) use ($previousColumn, $cursor) { $unionBuilder->where( - $this->getOriginalColumnNameForCursorPagination($this, $previousColumn), + $this->getOriginalColumnNameForCursorPagination($unionBuilder, $previousColumn), '=', $cursor->parameter($previousColumn) ); @@ -414,22 +414,23 @@ protected function paginateUsingCursor($perPage, $columns = ['*'], $cursorName = ); if ($i < $orders->count() - 1) { - $builder->orWhere(function (self $builder) use ($addCursorConditions, $column, $i) { - $addCursorConditions($builder, $column, $i + 1); + $builder->orWhere(function (self $builder) use ($addCursorConditions, $column, $originalColumn, $i) { + $addCursorConditions($builder, $column, $originalColumn, $i + 1); }); } $unionBuilders->each(function ($unionBuilder) use ($column, $direction, $cursor, $i, $orders, $addCursorConditions) { $unionBuilder->where(function ($unionBuilder) use ($column, $direction, $cursor, $i, $orders, $addCursorConditions) { + $originalColumn = $this->getOriginalColumnNameForCursorPagination($unionBuilder, $column); $unionBuilder->where( - $this->getOriginalColumnNameForCursorPagination($this, $column), + $originalColumn, $direction === 'asc' ? '>' : '<', $cursor->parameter($column) ); if ($i < $orders->count() - 1) { - $unionBuilder->orWhere(function (self $builder) use ($addCursorConditions, $column, $i) { - $addCursorConditions($builder, $column, $i + 1); + $unionBuilder->orWhere(function (self $builder) use ($addCursorConditions, $column, $originalColumn, $i) { + $addCursorConditions($builder, $column, $originalColumn, $i + 1); }); } @@ -439,7 +440,7 @@ protected function paginateUsingCursor($perPage, $columns = ['*'], $cursorName = }); }; - $addCursorConditions($this, null, 0); + $addCursorConditions($this, null, null, 0); } $this->limit($perPage + 1);