diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index 760e9e79c..cdd0941a6 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -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 **/ @@ -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 diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 3004e9780..83cc1fa1d 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -1996,6 +1996,11 @@ protected function getRandomOrder(): string return 'RANDOM()'; } + protected function getNullOrder(): string + { + return Database::ORDER_DESC; + } + /** * Size of POINT spatial type * diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index 9ca4c1aee..11434de9e 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -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 * @@ -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]; @@ -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) . ')'; } diff --git a/src/Database/Database.php b/src/Database/Database.php index 4c3fab055..ed34a1336 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -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; @@ -8704,17 +8703,6 @@ public function find(string $collection, array $queries = [], string $forPermiss } } - 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."); } diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 2b6d37822..2cddfc1b1 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -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 */