Fix before pagination (#676)

* add reverse helper * fix * fix encoding * chore: changeset Co-authored-by: Destiner <[email protected]>
ponder-sh · Mar 1, 2024 · 695fe00 · 695fe00
1 parent b0c39a8
commit 695fe00
Show file tree

Hide file tree

Showing 6 changed files with 103 additions and 32 deletions.
diff --git a/.changeset/giant-deers-burn.md b/.changeset/giant-deers-burn.md
@@ -0,0 +1,5 @@
+---
+"@ponder/core": patch
+---
+
+Fixed a bug where paginated queries using `before` did not behave correctly.
diff --git a/packages/core/src/indexing-store/postgres/store.ts b/packages/core/src/indexing-store/postgres/store.ts
@@ -12,7 +12,10 @@ import {
 } from "../utils/cursor.js";
 import { decodeRow, encodeRow, encodeValue } from "../utils/encoding.js";
 import { buildWhereConditions } from "../utils/filter.js";
-import { buildOrderByConditions } from "../utils/sort.js";
+import {
+  buildOrderByConditions,
+  reverseOrderByConditions,
+} from "../utils/sort.js";
 
 const MAX_BATCH_SIZE = 1_000 as const;
 
@@ -342,12 +345,7 @@ export class PostgresIndexingStore implements IndexingStore {
 
       if (where) {
         query = query.where((eb) =>
-          buildWhereConditions({
-            eb,
-            where,
-            table,
-            encoding: "postgres",
-          }),
+          buildWhereConditions({ eb, where, table, encoding: "postgres" }),
         );
       }
 
@@ -377,7 +375,7 @@ export class PostgresIndexingStore implements IndexingStore {
       if (after === null && before === null) {
         query = query.limit(limit + 1);
         const rows = await query.execute();
-        const records = rows.map((row) => decodeRow(row, table, "sqlite"));
+        const records = rows.map((row) => decodeRow(row, table, "postgres"));
 
         if (records.length === limit + 1) {
           records.pop();
@@ -466,8 +464,19 @@ export class PostgresIndexingStore implements IndexingStore {
           )
           .limit(limit + 2);
 
+        // Reverse the order by conditions to get the previous page.
+        query = query.clearOrderBy();
+        const reversedOrderByConditions =
+          reverseOrderByConditions(orderByConditions);
+        for (const [column, direction] of reversedOrderByConditions) {
+          query = query.orderBy(column, direction);
+        }
+
         const rows = await query.execute();
-        const records = rows.map((row) => decodeRow(row, table, "postgres"));
+        const records = rows
+          .map((row) => decodeRow(row, table, "postgres"))
+          // Reverse the records again, back to the original order.
+          .reverse();
 
         if (records.length === 0) {
           return {

diff --git a/packages/core/src/indexing-store/sqlite/store.ts b/packages/core/src/indexing-store/sqlite/store.ts
@@ -12,7 +12,10 @@ import {
 } from "../utils/cursor.js";
 import { decodeRow, encodeRow, encodeValue } from "../utils/encoding.js";
 import { buildWhereConditions } from "../utils/filter.js";
-import { buildOrderByConditions } from "../utils/sort.js";
+import {
+  buildOrderByConditions,
+  reverseOrderByConditions,
+} from "../utils/sort.js";
 
 const MAX_BATCH_SIZE = 1_000 as const;
 
@@ -312,12 +315,7 @@ export class SqliteIndexingStore implements IndexingStore {
 
       if (where) {
         query = query.where((eb) =>
-          buildWhereConditions({
-            eb,
-            where,
-            table,
-            encoding: "sqlite",
-          }),
+          buildWhereConditions({ eb, where, table, encoding: "sqlite" }),
         );
       }
 
@@ -435,8 +433,19 @@ export class SqliteIndexingStore implements IndexingStore {
           )
           .limit(limit + 2);
 
+        // Reverse the order by conditions to get the previous page.
+        query = query.clearOrderBy();
+        const reversedOrderByConditions =
+          reverseOrderByConditions(orderByConditions);
+        for (const [column, direction] of reversedOrderByConditions) {
+          query = query.orderBy(column, direction);
+        }
+
         const rows = await query.execute();
-        const records = rows.map((row) => decodeRow(row, table, "sqlite"));
+        const records = rows
+          .map((row) => decodeRow(row, table, "sqlite"))
+          // Reverse the records again, back to the original order.
+          .reverse();
 
         if (records.length === 0) {
           return {

diff --git a/packages/core/src/indexing-store/store.test.ts b/packages/core/src/indexing-store/store.test.ts
@@ -861,25 +861,32 @@ test("findMany() cursor pagination ascending", async (context) => {
     tableName: "Pet",
     checkpoint: createCheckpoint(10),
     data: [
-      { id: "id1", name: "Skip", bigAge: 105n },
-      { id: "id2", name: "Foo", bigAge: 10n },
-      { id: "id3", name: "Bar", bigAge: 190n },
+      { id: "id1", name: "Skip" },
+      { id: "id2", name: "Foo" },
+      { id: "id3", name: "Bar" },
       { id: "id4", name: "Zarbar" },
-      { id: "id5", name: "Winston", age: 12 },
+      { id: "id5", name: "Winston" },
+      { id: "id6", name: "Book" },
+      { id: "id7", name: "Shea" },
+      { id: "id8", name: "Snack" },
+      { id: "id9", name: "Last" },
     ],
   });
 
   const resultOne = await indexingStore.findMany({
     tableName: "Pet",
-    orderBy: { name: "asc" },
-    limit: 2,
+    orderBy: { id: "asc" },
+    limit: 5,
   });
 
   expect(
     resultOne.items.map((i) => ({ id: i.id, name: i.name })),
   ).toMatchObject([
-    { id: "id3", name: "Bar" },
+    { id: "id1", name: "Skip" },
     { id: "id2", name: "Foo" },
+    { id: "id3", name: "Bar" },
+    { id: "id4", name: "Zarbar" },
+    { id: "id5", name: "Winston" },
   ]);
   expect(resultOne.pageInfo).toMatchObject({
     startCursor: expect.any(String),
@@ -890,16 +897,17 @@ test("findMany() cursor pagination ascending", async (context) => {
 
   const resultTwo = await indexingStore.findMany({
     tableName: "Pet",
-    orderBy: { name: "asc" },
+    orderBy: { id: "asc" },
     after: resultOne.pageInfo.endCursor,
   });
 
   expect(
     resultTwo.items.map((i) => ({ id: i.id, name: i.name })),
   ).toMatchObject([
-    { id: "id1", name: "Skip" },
-    { id: "id5", name: "Winston" },
-    { id: "id4", name: "Zarbar" },
+    { id: "id6", name: "Book" },
+    { id: "id7", name: "Shea" },
+    { id: "id8", name: "Snack" },
+    { id: "id9", name: "Last" },
   ]);
   expect(resultTwo.pageInfo).toMatchObject({
     startCursor: expect.any(String),
@@ -910,14 +918,17 @@ test("findMany() cursor pagination ascending", async (context) => {
 
   const resultThree = await indexingStore.findMany({
     tableName: "Pet",
-    orderBy: { name: "asc" },
+    orderBy: { id: "asc" },
     before: resultTwo.pageInfo.startCursor,
-    limit: 1,
+    limit: 2,
   });
 
   expect(
     resultThree.items.map((i) => ({ id: i.id, name: i.name })),
-  ).toMatchObject([{ id: "id2", name: "Foo" }]);
+  ).toMatchObject([
+    { id: "id4", name: "Zarbar" },
+    { id: "id5", name: "Winston" },
+  ]);
   expect(resultThree.pageInfo).toMatchObject({
     startCursor: expect.any(String),
     endCursor: expect.any(String),

diff --git a/packages/core/src/indexing-store/utils/sort.test.ts b/packages/core/src/indexing-store/utils/sort.test.ts
@@ -1,6 +1,6 @@
 import { createSchema } from "@/schema/schema.js";
 import { expect, test } from "vitest";
-import { buildOrderByConditions } from "./sort.js";
+import { buildOrderByConditions, reverseOrderByConditions } from "./sort.js";
 
 const schema = createSchema((p) => ({
   PetKind: p.createEnum(["CAT", "DOG"]),
@@ -87,3 +87,33 @@ test("buildOrderByConditions throws for invalid order direction", () => {
     }),
   ).toThrow("Invalid sort direction. Got 'aaaaasc', expected 'asc' or 'desc'");
 });
+
+test("reverseOrderByConditions reverses with one condition", () => {
+  const conditions = buildOrderByConditions({
+    orderBy: undefined,
+    table: schema.tables.Pet,
+  });
+
+  expect(conditions).toEqual([["id", "asc"]]);
+
+  const reversedConditions = reverseOrderByConditions(conditions);
+  expect(reversedConditions).toEqual([["id", "desc"]]);
+});
+
+test("reverseOrderByConditions reverses with two conditions", () => {
+  const conditions = buildOrderByConditions({
+    orderBy: { names: "desc" },
+    table: schema.tables.Pet,
+  });
+
+  expect(conditions).toEqual([
+    ["names", "desc"],
+    ["id", "desc"],
+  ]);
+
+  const reversedConditions = reverseOrderByConditions(conditions);
+  expect(reversedConditions).toEqual([
+    ["names", "asc"],
+    ["id", "asc"],
+  ]);
+});
diff --git a/packages/core/src/indexing-store/utils/sort.ts b/packages/core/src/indexing-store/utils/sort.ts
@@ -57,3 +57,10 @@ export function buildOrderByConditions({
 
   return orderByConditions;
 }
+
+export function reverseOrderByConditions(orderByConditions: OrderByConditions) {
+  return orderByConditions.map(([columnName, direction]) => [
+    columnName,
+    direction === "asc" ? "desc" : "asc",
+  ]) satisfies OrderByConditions;
+}