Cursors fixes: only include cursors where supported and fix admin UI's cursor state handling.

crates/assets/js/admin/src/components/Table.tsx

@@ -39,6 +39,8 @@ import { Checkbox } from "@/components/ui/checkbox";
 import { Skeleton } from "@/components/ui/skeleton";
 import { createIsMobile } from "@/lib/signals";
 
+export type { Updater } from "@tanstack/solid-table";
+
 type TableOptions<TData, TValue> = {
   data: TData[] | undefined;
   columns: ColumnDef<TData, TValue>[];
@@ -57,7 +59,9 @@ export function buildTable<TData, TValue>(
   opts: TableOptions<TData, TValue>,
   overrides?: Partial<SolidTableOptions<TData>>,
 ) {
-  console.debug("buildTable: ", opts);
+  console.debug(
+    `buildTable(): columns=${opts.columns.length}, rows=${opts.data?.length}`,
+  );
 
   function buildColumns() {
     const onRowSelection = opts.onRowSelection;

crates/assets/js/admin/src/components/logs/LogsPage.tsx

@@ -10,13 +10,12 @@ import {
   createMemo,
 } from "solid-js";
 import { useSearchParams } from "@solidjs/router";
-import { createWritableMemo } from "@solid-primitives/memo";
 import type {
   ColumnDef,
   PaginationState,
   SortingState,
 } from "@tanstack/solid-table";
-import { useQuery, useQueryClient } from "@tanstack/solid-query";
+import { useQuery } from "@tanstack/solid-query";
 import { Chart } from "chart.js/auto";
 import type {
   ChartData,
@@ -44,6 +43,7 @@ import {
   DialogFooter,
 } from "@/components/ui/dialog";
 import { Table, buildTable } from "@/components/Table";
+import type { Updater } from "@/components/Table";
 import { FilterBar } from "@/components/FilterBar";
 
 import { fetchLogs } from "@/lib/api/logs";
@@ -164,74 +164,98 @@ const columns: ColumnDef<LogJson>[] = [
   },
 ];
 
+type SearchParams = {
+  filter?: string;
+  pageSize?: string;
+  pageIndex?: string;
+};
+
 // Value is the previous value in case this isn't the first fetch.
 export function LogsPage() {
-  const [searchParams, setSearchParams] = useSearchParams<{
-    filter?: string;
-    pageSize?: string;
-    pageIndex?: string;
-  }>();
-  const [cursors, setCursors] = createWritableMemo<string[]>(() => {
-    // Reset when search params change
-    const _ = [searchParams.pageSize, searchParams.filter];
-    console.debug("cursor reset");
-    return [];
+  // IMPORTANT: We need to memo the search params to treat absence and defaults
+  // consistently, otherwise `undefined`->`default` may invalidate the cursors.
+  const [searchParams, setSearchParams] = useSearchParams<SearchParams>();
+  const filter = createMemo(() => searchParams.filter);
+  const pageSize = createMemo(() => safeParseInt(searchParams.pageSize) ?? 20);
+  const pageIndex = createMemo(() => safeParseInt(searchParams.pageIndex) ?? 0);
+
+  const pagination = (): PaginationState => ({
+    pageIndex: pageIndex(),
+    pageSize: pageSize(),
   });
-
-  const pagination = (): PaginationState => {
-    return {
-      pageSize: safeParseInt(searchParams.pageSize) ?? 20,
-      pageIndex: safeParseInt(searchParams.pageIndex) ?? 0,
-    };
-  };
-
-  const setFilter = (filter: string | undefined) => {
+  const setPagination = (s: PaginationState) => {
     setSearchParams({
       ...searchParams,
+      pageIndex: s.pageIndex,
+      pageSize: s.pageSize,
+    });
+  };
+  const setFilter = (filter: string | undefined) => {
+    // Reset pagination.
+    setSearchParams({
+      pageIndex: undefined,
+      pageSize: undefined,
       filter,
-      // Reset
-      pageIndex: "0",
     });
   };
 
-  const [sorting, setSorting] = createSignal<SortingState>([]);
+  const [sorting, setSortingImpl] = createSignal<SortingState>([]);
+  const setSorting = (s: Updater<SortingState>) => {
+    // Reset pagination.
+    setSearchParams({
+      ...searchParams,
+      pageIndex: undefined,
+      pageSize: undefined,
+    });
+    setSortingImpl(s);
+  };
+
+  const cursors = createMemo<Map<number, string>>(() => {
+    // Reset cursor whenever table or search params change. This is basically
+    // the same as `queryKey` below minus `pageIndex`.
+    const _ = [pageSize(), filter(), sorting()];
+    console.debug("resetting cursor");
+    return new Map();
+  });
 
   // NOTE: admin user endpoint doesn't support offset, we have to cursor through
   // and cannot just jump to page N.
   const logsFetch = useQuery(() => ({
-    queryKey: [
-      "logs",
-      searchParams.filter,
-      pagination().pageSize,
-      pagination().pageIndex,
-      sorting(),
-    ],
-    queryFn: async () => {
-      const p = pagination();
-      const c = cursors();
-      const s = sorting();
+    queryKey: [pagination(), filter(), sorting()],
+    queryFn: async ({ queryKey }) => {
+      console.debug(`Fetching data with key: ${queryKey}`);
 
-      const response = await fetchLogs(
-        p.pageSize,
-        searchParams.filter,
-        c[p.pageIndex - 1],
-        formatSortingAsOrder(s),
-      );
+      try {
+        const { pageSize, pageIndex } = pagination();
+        const cursor = cursors().get(pageIndex - 1);
 
-      const cursor = response.cursor;
-      if (cursor && p.pageIndex >= c.length) {
-        setCursors([...c, cursor]);
+        const response = await fetchLogs(
+          pageSize,
+          pageIndex,
+          filter(),
+          cursor,
+          formatSortingAsOrder(sorting()),
+        );
+
+        // Update cursors.
+        if (sorting().length === 0 && response.cursor) {
+          cursors().set(pageIndex, response.cursor);
+        }
+
+        return response;
+      } catch (err) {
+        // Reset.
+        setSearchParams({
+          filter: undefined,
+          pageSize: undefined,
+          pageIndex: undefined,
+        });
+
+        throw err;
       }
-
-      return response;
     },
   }));
-  const client = useQueryClient();
-  const refetch = () => {
-    client.invalidateQueries({
-      queryKey: ["logs"],
-    });
-  };
+  const refetch = () => logsFetch.refetch();
 
   const [showMap, setShowMap] = createSignal(true);
   const [showGeoipDialog, setShowGeoipDialog] = createSignal(false);
@@ -246,13 +270,7 @@ export function LogsPage() {
         onColumnPinningChange: setColumnPinningState,
         rowCount: Number(logsFetch.data?.total_row_count ?? -1),
         pagination: pagination(),
-        onPaginationChange: (s: PaginationState) => {
-          setSearchParams({
-            ...searchParams,
-            pageIndex: s.pageIndex,
-            pageSize: s.pageSize,
-          });
-        },
+        onPaginationChange: setPagination,
       },
       {
         manualSorting: true,
@@ -338,9 +356,9 @@ export function LogsPage() {
             </Show>
 
             <FilterBar
-              initial={searchParams.filter}
+              initial={filter()}
               onSubmit={(value: string) => {
-                if (value === searchParams.filter) {
+                if (value === filter()) {
                   refetch();
                 } else {
                   setFilter(value);

crates/assets/js/admin/src/components/tables/TablePane.tsx

@@ -1,6 +1,5 @@
 import { Match, Show, Switch, createMemo, createSignal, JSX } from "solid-js";
 import type { Signal } from "solid-js";
-import { createWritableMemo } from "@solid-primitives/memo";
 import { TbRefresh, TbTable, TbTrash, TbColumns } from "solid-icons/tb";
 import { useSearchParams } from "@solidjs/router";
 import { useQuery } from "@tanstack/solid-query";
@@ -43,6 +42,7 @@ import { DebugDialogButton } from "@/components/tables/SchemaDownload";
 import { CreateAlterTableForm } from "@/components/tables/CreateAlterTable";
 import { CreateAlterIndexForm } from "@/components/tables/CreateAlterIndex";
 import { Table as TableComponent, buildTable } from "@/components/Table";
+import type { Updater } from "@/components/Table";
 import { FilterBar } from "@/components/FilterBar";
 import { DestructiveActionButton } from "@/components/DestructiveActionButton";
 import { IconButton } from "@/components/IconButton";
@@ -935,6 +935,12 @@ function TriggerTable(props: { table: Table; schemas: ListSchemasResponse }) {
   );
 }
 
+type SearchParams = {
+  filter?: string;
+  pageSize?: string;
+  pageIndex?: string;
+};
+
 export function TablePane(props: {
   selectedTable: [Table, string] | [View, string];
   schemas: ListSchemasResponse;
@@ -943,52 +949,57 @@ export function TablePane(props: {
   const selectedSchema = () => props.selectedTable[0];
   const isTable = () => tableType(selectedSchema()) === "table";
 
-  const [searchParams, setSearchParams] = useSearchParams<{
-    filter?: string;
-    pageSize?: string;
-    pageIndex?: string;
-  }>();
+  // IMPORTANT: We need to memo the search params to treat absence and defaults
+  // consistently, otherwise `undefined`->`default` may invalidate the cursors.
+  const [searchParams, setSearchParams] = useSearchParams<SearchParams>();
+  const filter = createMemo(() => searchParams.filter);
+  const pageSize = createMemo(() => safeParseInt(searchParams.pageSize) ?? 20);
+  const pageIndex = createMemo(() => safeParseInt(searchParams.pageIndex) ?? 0);
 
-  const [cursors, setCursors] = createWritableMemo<string[]>(() => {
-    // Reset cursor whenever table or search params change.
-    const _ = [props.selectedTable, searchParams.pageSize, searchParams.filter];
-    console.debug("resetting cursor");
-    return [];
+  const pagination = (): PaginationState => ({
+    pageIndex: pageIndex(),
+    pageSize: pageSize(),
   });
+  const setPagination = (s: PaginationState) => {
+    setSearchParams({
+      ...searchParams,
+      pageIndex: s.pageIndex,
+      pageSize: s.pageSize,
+    });
+  };
+  const setFilter = (filter: string | undefined) => {
+    // Reset pagination.
+    setSearchParams({
+      pageIndex: undefined,
+      pageSize: undefined,
+      filter,
+    });
+  };
 
-  const [filter, setFilter] = [
-    () => searchParams.filter,
-    (filter: string | undefined) => {
-      setSearchParams({
-        ...searchParams,
-        filter,
-      });
-    },
-  ];
+  const [sorting, setSortingImpl] = createSignal<SortingState>([]);
+  const setSorting = (s: Updater<SortingState>) => {
+    // Reset pagination.
+    setSearchParams({
+      ...searchParams,
+      pageIndex: undefined,
+      pageSize: undefined,
+    });
+    setSortingImpl(s);
+  };
 
-  const [pagination, setPagination] = [
-    (): PaginationState => {
-      return {
-        pageSize: safeParseInt(searchParams.pageSize) ?? 20,
-        pageIndex: safeParseInt(searchParams.pageIndex) ?? 0,
-      };
-    },
-    (s: PaginationState) => {
-      setSearchParams({
-        ...searchParams,
-        pageSize: s.pageSize,
-        pageIndex: s.pageIndex,
-      });
-    },
-  ];
-
-  const [sorting, setSorting] = createSignal<SortingState>([]);
+  const cursors = createMemo<Map<number, string>>(() => {
+    // Reset cursor whenever table or search params change. This is basically
+    // the same as `queryKey` below minus `pageIndex`.
+    const _ = [selectedSchema(), pageSize(), filter(), sorting()];
+    console.debug("resetting cursor");
+    return new Map();
+  });
 
   const records: QueryObserverResult<ListRowsResponse> = useQuery(() => ({
     queryKey: [
-      selectedSchema().name,
-      searchParams.filter,
+      selectedSchema(),
       pagination(),
+      filter(),
       sorting(),
     ] as ReadonlyArray<unknown>,
     queryFn: async ({ queryKey }) => {
@@ -996,19 +1007,20 @@ export function TablePane(props: {
 
       try {
         const { pageSize, pageIndex } = pagination();
+        const cursor = cursors().get(pageIndex - 1);
 
         const response = await fetchRows(
           selectedSchema().name,
-          searchParams.filter ?? null,
+          filter() ?? null,
           pageSize,
           pageIndex,
-          cursors()[pageIndex - 1],
+          cursor ?? null,
           formatSortingAsOrder(sorting()),
         );
 
-        const newCursor = response.cursor;
-        if (newCursor && pageIndex >= cursors().length) {
-          setCursors([...cursors(), newCursor]);
+        // Update cursors.
+        if (sorting().length === 0 && response.cursor) {
+          cursors().set(pageIndex, response.cursor);
         }
 
         return response;
@@ -1054,22 +1066,10 @@ export function TablePane(props: {
             </div>
           </Match>
 
-          <Match when={records.isLoading}>
+          <Match when={true}>
             <RecordTable
               selectedSchema={selectedSchema()}
-              records={undefined}
-              pagination={[pagination, setPagination]}
-              filter={[filter, setFilter]}
-              columnPinningState={[columnPinningState, setColumnPinningState]}
-              sorting={[sorting, setSorting]}
-              rowsRefetch={rowsRefetch}
-            />
-          </Match>
-
-          <Match when={records.isSuccess}>
-            <RecordTable
-              selectedSchema={selectedSchema()}
-              records={records.data}
+              records={records.isSuccess ? records.data : undefined}
               pagination={[pagination, setPagination]}
               filter={[filter, setFilter]}
               columnPinningState={[columnPinningState, setColumnPinningState]}

crates/assets/js/admin/src/lib/api/logs.ts

@@ -5,6 +5,7 @@ import type { ListLogsResponse } from "@bindings/ListLogsResponse";
 
 export async function fetchLogs(
   pageSize: number,
+  pageIndex: number,
   filter?: string,
   cursor?: string | null,
   order?: string,
@@ -12,6 +13,7 @@ export async function fetchLogs(
   const params = buildListSearchParams({
     filter,
     pageSize,
+    pageIndex,
     cursor,
     order,
   });

crates/assets/js/admin/src/lib/list.ts

@@ -53,10 +53,8 @@ export function buildListSearchParams({
 
   if (cursor) {
     params.set("cursor", cursor);
-  } else {
-    if (pageIndex) {
-      params.set("offset", `${pageIndex * pageSize}`);
-    }
+  } else if (pageIndex) {
+    params.set("offset", `${pageIndex * pageSize}`);
   }
 
   if (order) {

crates/core/src/admin/list_logs.rs

@@ -142,6 +142,7 @@ pub async fn list_logs_handler(
     cursor,
     order,
     filter: filter_params,
+    offset,
     ..
   } = raw_url_query
     .as_ref()
@@ -197,6 +198,7 @@ pub async fn list_logs_handler(
     geoip_db_type.clone(),
     filter_where_clause.clone(),
     cursor,
+    offset,
     order.as_ref().unwrap_or_else(|| &DEFAULT_ORDERING),
     limit_or_default(limit, None).map_err(|err| Error::BadRequest(err.into()))?,
   )
@@ -208,7 +210,7 @@ pub async fn list_logs_handler(
     }
   }
 
-  let first_page = cursor.is_none();
+  let first_page = cursor.is_none() && offset.unwrap_or(0) == 0;
   let stats = if first_page {
     let now = Utc::now();
     let args = FetchAggregateArgs {
@@ -234,15 +236,22 @@ pub async fn list_logs_handler(
     None
   };
 
-  let response = ListLogsResponse {
-    total_row_count,
-    cursor: logs.last().map(|log| {
+  let next_cursor = if order.is_none() {
+    logs.last().map(|log| {
+      #[cfg(debug_assertions)]
       if let Some(old_cursor) = cursor {
         assert!(old_cursor > log.id);
       }
 
       return log.id.to_string();
-    }),
+    })
+  } else {
+    None
+  };
+
+  let response = ListLogsResponse {
+    total_row_count,
+    cursor: next_cursor,
     entries: logs
       .into_iter()
       .map(|log| log.into())
@@ -258,6 +267,7 @@ async fn fetch_logs(
   geoip_db_type: Option<DatabaseType>,
   filter_where_clause: WhereClause,
   cursor: Option<i64>,
+  offset: Option<usize>,
   order: &Order,
   limit: usize,
 ) -> Result<Vec<LogEntry>, Error> {
@@ -268,6 +278,11 @@ async fn fetch_logs(
     trailbase_sqlite::Value::Integer(limit as i64),
   ));
 
+  params.push((
+    Cow::Borrowed(":offset"),
+    trailbase_sqlite::Value::Integer(offset.map_or(0, |o| o.try_into().unwrap_or(0))),
+  ));
+
   if let Some(cursor) = cursor {
     params.push((
       Cow::Borrowed(":cursor"),
@@ -301,6 +316,7 @@ async fn fetch_logs(
       ORDER BY
         {order_clause}
       LIMIT :limit
+      OFFSET :offset
     "#,
     geoip = match geoip_db_type {
       Some(DatabaseType::GeoLite2Country) => "geoip_country(log.client_ip) AS client_geoip_cc",

crates/core/src/admin/rows/list_rows.rs

@@ -107,18 +107,22 @@ pub async fn list_rows_handler(
   )
   .await?;
 
-  let next_cursor = cursor_column.and_then(|(col_idx, _col)| {
-    let row = rows.last()?;
-    assert!(row.len() > col_idx);
-    match &row[col_idx] {
-      SqlValue::Integer(n) => Some(n.to_string()),
-      SqlValue::Blob(b) => {
-        // Should be a base64 encoded [u8; 16] id.
-        b.to_b64_url_safe().ok()
+  let next_cursor = if order.is_none() {
+    cursor_column.and_then(|(col_idx, _col)| {
+      let row = rows.last()?;
+      assert!(row.len() > col_idx);
+      match &row[col_idx] {
+        SqlValue::Integer(n) => Some(n.to_string()),
+        SqlValue::Blob(b) => {
+          // Should be a base64 encoded [u8; 16] id.
+          b.to_b64_url_safe().ok()
+        }
+        _ => None,
       }
-      _ => None,
-    }
-  });
+    })
+  } else {
+    None
+  };
 
   return Ok(Json(ListRowsResponse {
     total_row_count,

crates/core/src/records/list_records.rs

@@ -11,7 +11,6 @@ use std::convert::TryInto;
 use std::sync::LazyLock;
 use trailbase_qs::{OrderPrecedent, Query};
 use trailbase_schema::QualifiedNameEscaped;
-use trailbase_schema::sqlite::ColumnDataType;
 use trailbase_sqlite::Value;
 
 use crate::app_state::AppState;
@@ -127,13 +126,19 @@ pub async fn list_records_handler(
     ));
   }
 
+  // NOTE: We lost the ability to cursor VIEWs when we moved to `_rowid_`. They currently only
+  // support OFFSET. We could restore functionality where a cursor-able PK is included.
+  // NOTE: We cannot use cursors if there's a custom order/sorting defined.
+  // NOTE: Multiple order criteria only matter for non-unique columns, i.e. ordering on PK
+  // and then on another column makes no difference.
+  let supports_cursor = is_table
+    && order
+      .as_ref()
+      .is_none_or(|o| o.columns.is_empty() || (pk_column.name == o.columns[0].0));
   let cursor_clause = if let Some(encrypted_cursor) = cursor {
-    if !is_table {
-      // TODO: When we moved to _rowid_ for cursoring, we lost the ability to cursor VIEWs. They
-      // currently only support OFFSET. We could restore cursoring for cases where a cursorable
-      // PK column is included. We also need a test-case to cover this.
+    if !supports_cursor {
       return Err(RecordError::BadRequest(
-        "Only TABLEs support cursors. Use offset for VIEWs.",
+        "Only TABLEs with PK primary order support cursors. Use offset instead.",
       ));
     }
 
@@ -146,37 +151,29 @@ pub async fn list_records_handler(
       )?),
     ));
 
-    let mut pk_order = OrderPrecedent::Descending;
-    if let Some(ref order) = order
-      && let Some((col, ord)) = order.columns.first()
-      && *ord == OrderPrecedent::Ascending
-    {
-      if pk_column.data_type != ColumnDataType::Integer || *col != pk_column.name {
-        // NOTE: This relies on the fact that _rowid_ is an alias for integer primary key
-        // columns.
-        return Err(RecordError::BadRequest(
-          "Cannot cursor on queries where the primary order criterion is not an integer primary key",
-        ));
+    // We already check above that primary order criteria must be none or PK.
+    match order.as_ref() {
+      Some(ord)
+        if ord
+          .columns
+          .first()
+          .is_some_and(|(_c, o)| *o == OrderPrecedent::Ascending) =>
+      {
+        Some("_ROW_._rowid_ > :cursor".to_string())
       }
-
-      pk_order = OrderPrecedent::Ascending;
-    }
-
-    match pk_order {
-      OrderPrecedent::Descending => Some("_ROW_._rowid_ < :cursor".to_string()),
-      OrderPrecedent::Ascending => Some("_ROW_._rowid_ > :cursor".to_string()),
+      // Descending:
+      _ => Some("_ROW_._rowid_ < :cursor".to_string()),
     }
   } else {
     None
   };
 
-  let order_clause = order.map_or_else(
+  let order_clause = order.as_ref().map_or_else(
     || fmt_order(&pk_column.name, OrderPrecedent::Descending),
-    |order| {
-      order
-        .columns
-        .into_iter()
-        .map(|(col, ord)| fmt_order(&col, ord))
+    |o| {
+      o.columns
+        .iter()
+        .map(|(col, ord)| fmt_order(col, ord.clone()))
         .join(",")
     },
   );
@@ -265,7 +262,7 @@ pub async fn list_records_handler(
     }));
   }
 
-  let cursor: Option<String> = if is_table {
+  let cursor: Option<String> = if supports_cursor {
     // The SQL query template returns thw row id as the last column.
     let value = &last_row[last_row.len() - 1];
     let Value::Integer(rowid) = value else {

crates/core/src/records/subscribe.rs

@@ -823,7 +823,8 @@ struct SubscriptionQuery {
 
 impl SubscriptionQuery {
   fn parse(query: &str) -> Result<SubscriptionQuery, RecordError> {
-    // NOTE: We rely on form-encoding to properly parse ampersands, e.g.: `filter[col0]=a&b%filter[col1]=c`.
+    // NOTE: We rely on form-encoding to properly parse ampersands, e.g.:
+    // `filter[col0]=a&b%filter[col1]=c`.
     let qs = serde_qs::Config::new().max_depth(9).use_form_encoding(true);
     return qs
       .deserialize_bytes::<SubscriptionQuery>(query.as_bytes())