Skip to content

Fix cursor pagination across nullable order values #970

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
HarshMN2345 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix cursor pagination across nullable order values #970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions src/Database/Adapter/Mongo.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2565,13 +2565,13 @@ public function find(Document $collection, array $queries = [], ?int $limit = 25
for ($j = 0; $j < $i; $j++) {
$originalPrev = $orderAttributes[$j];
$prevAttr = $this->filter($this->getInternalKeyForAttribute($originalPrev));
$tmp = $cursor[$originalPrev];
$tmp = $cursor[$originalPrev] ?? null;
$andConditions[] = [
$prevAttr => $tmp
];
}

$tmp = $cursor[$originalAttribute];
$tmp = $cursor[$originalAttribute] ?? null;

if ($originalAttribute === '$sequence') {
/** If there is only $sequence attribute in $orderAttributes skip Or And operators **/
Expand All @@ -2583,11 +2583,18 @@ public function find(Document $collection, array $queries = [], ?int $limit = 25
}
}

$andConditions[] = [
$attribute => [
$operator => $tmp
]
];
if ($tmp === null) {
if ($direction === Database::ORDER_DESC) {
// Only a later tie-breaker can follow a null sorted last.
continue;
}
$andConditions[] = [$attribute => ['$ne' => null]];
} else {
$comparison = [$attribute => [$operator => $tmp]];
$andConditions[] = $direction === Database::ORDER_DESC
? ['$or' => [$comparison, [$attribute => null]]]
: $comparison;
}

$orFilters[] = [
'$and' => $andConditions
Expand Down
5 changes: 5 additions & 0 deletions src/Database/Adapter/Postgres.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1996,6 +1996,11 @@ protected function getRandomOrder(): string
return 'RANDOM()';
}

protected function getNullOrder(): string
{
return Database::ORDER_DESC;
}

/**
* Size of POINT spatial type
*
Expand Down
30 changes: 27 additions & 3 deletions src/Database/Adapter/SQL.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2156,6 +2156,14 @@ abstract protected function getPDOType(mixed $value): int;
*/
abstract protected function getRandomOrder(): string;

/**
* Sort direction that places null values before non-null values.
*/
protected function getNullOrder(): string
{
return Database::ORDER_ASC;
}

/**
* Returns default PDO configuration
*
Expand Down Expand Up @@ -3047,6 +3055,11 @@ public function find(Document $collection, array $queries = [], ?int $limit = 25
$prevOriginal = $orderAttributes[$j];
$prevAttr = $this->filter($this->getInternalKeyForAttribute($prevOriginal));

if (($cursor[$prevOriginal] ?? null) === null) {
$conditions[] = "{$this->quote($alias)}.{$this->quote($prevAttr)} IS NULL";
continue;
}

$bindName = ":cursor_{$j}";
$binds[$bindName] = $cursor[$prevOriginal];

Expand All @@ -3058,10 +3071,21 @@ public function find(Document $collection, array $queries = [], ?int $limit = 25
? Query::TYPE_LESSER
: Query::TYPE_GREATER;

$bindName = ":cursor_{$i}";
$binds[$bindName] = $cursor[$originalAttribute];
$column = "{$this->quote($alias)}.{$this->quote($attribute)}";
$nullsFirst = $direction === $this->getNullOrder();

$conditions[] = "{$this->quote($alias)}.{$this->quote($attribute)} {$this->getSQLOperator($operator)} {$bindName}";
if (($cursor[$originalAttribute] ?? null) === null) {
if (!$nullsFirst) {
// Only a later tie-breaker can follow a null sorted last.
continue;
}
$conditions[] = "{$column} IS NOT NULL";
} else {
$bindName = ":cursor_{$i}";
$binds[$bindName] = $cursor[$originalAttribute];
$comparison = "{$column} {$this->getSQLOperator($operator)} {$bindName}";
$conditions[] = $nullsFirst ? $comparison : "({$comparison} OR {$column} IS NULL)";
}

$cursorWhere[] = '(' . implode(' AND ', $conditions) . ')';
}
Expand Down
12 changes: 0 additions & 12 deletions src/Database/Database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use Utopia\Database\Exception\Index as IndexException;
use Utopia\Database\Exception\Limit as LimitException;
use Utopia\Database\Exception\NotFound as NotFoundException;
use Utopia\Database\Exception\Order as OrderException;
use Utopia\Database\Exception\Query as QueryException;
use Utopia\Database\Exception\Relationship as RelationshipException;
use Utopia\Database\Exception\Restricted as RestrictedException;
Expand Down Expand Up @@ -8704,17 +8703,6 @@ public function find(string $collection, array $queries = [], string $forPermiss
}

@greptile-apps greptile-apps Bot Sep 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Partial cursors become null

Removing this validation also accepts partial cursor documents that omit required order attributes. Cursor validation checks only the document ID, so a caller can provide a cursor with the correct collection and ID but without the ordered field. The SQL and MongoDB adapters then treat the missing value as null and paginate from the null partition instead of the referenced document's actual position, returning the wrong page rather than rejecting the malformed cursor. Please continue allowing genuinely nullable values while rejecting absent required order fields.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/Database/Database.php
Line: 8703

Comment:
**Partial cursors become null**

Removing this validation also accepts partial cursor documents that omit required order attributes. Cursor validation checks only the document ID, so a caller can provide a cursor with the correct collection and ID but without the ordered field. The SQL and MongoDB adapters then treat the missing value as null and paginate from the null partition instead of the referenced document's actual position, returning the wrong page rather than rejecting the malformed cursor. Please continue allowing genuinely nullable values while rejecting absent required order fields.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

}

if (!empty($cursor)) {
foreach ($orderAttributes as $order) {
if ($cursor->getAttribute($order) === null) {
throw new OrderException(
message: "Order attribute '{$order}' is empty",
attribute: $order
);
}
}
}

if (!empty($cursor) && $cursor->getCollection() !== $collection->getId()) {
throw new DatabaseException("cursor Document must be from the same Collection.");
}
Expand Down
65 changes: 65 additions & 0 deletions tests/e2e/Adapter/Scopes/DocumentTests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3794,6 +3794,71 @@ public function testFindOrderByAndCursor(): void

$this->assertEquals($documentsTest[1]['$id'], $documents[0]['$id']);
}

public function testFindCursorWithNullOrderValues(): void
{
$database = $this->getDatabase();
$collection = __FUNCTION__;
$database->createCollection($collection);
$database->createAttribute($collection, 'value', Database::VAR_INTEGER, 4, false);
$database->createAttribute($collection, 'rank', Database::VAR_INTEGER, 4, false);

foreach ([[4, 2], [1, null], [2, 1], [null, 2], [null, null], [-1, 0], [0, -1], [2, null], [null, -1]] as [$value, $rank]) {
$database->createDocument($collection, new Document([
'$id' => ID::unique(),
'$permissions' => [Permission::read(Role::any())],
'value' => $value,
'rank' => $rank,
]));
}

foreach ([
[Query::orderAsc('value')],
[Query::orderDesc('value')],
[Query::orderAsc('value'), Query::orderDesc('rank')],
[Query::orderDesc('value'), Query::orderAsc('rank')],
] as $order) {
// Preserve each adapter's native null ordering and compare cursor
// pages against the same query without pagination.
$documents = $database->find($collection, $order);
$ids = \array_map(fn (Document $document) => $document->getId(), $documents);
$this->assertCount(9, $ids);

foreach ($documents as $index => $cursor) {
$after = $database->find($collection, [...$order, Query::cursorAfter($cursor)]);
$this->assertSame(
\array_slice($ids, $index + 1),
\array_map(fn (Document $document) => $document->getId(), $after)
);

$before = $database->find($collection, [...$order, Query::cursorBefore($cursor)]);
$this->assertSame(
\array_slice($ids, 0, $index),
\array_map(fn (Document $document) => $document->getId(), $before)
);
}

$paged = [];
$cursor = null;
do {
$queries = [...$order, Query::limit(2)];
if ($cursor !== null) {
$queries[] = Query::cursorAfter($cursor);
}
$page = $database->find($collection, $queries);
foreach ($page as $document) {
$paged[] = $document->getId();
}
$this->assertLessThanOrEqual(\count($ids), \count($paged));
$cursor = empty($page) ? null : $page[\count($page) - 1];
} while ($cursor !== null);

$this->assertSame($ids, $paged);
}

$database->deleteCollection($collection);
}

public function testFindOrderByIdAndCursor(): void
{
/** @var Database $database */
Expand Down
Loading