Skip to content

get_rows() applies pagination to a caller-supplied stmnt, silently truncating results #11

Description

@fsecada01

Problem

get_rows() (sqlmodel_crud_utils/a_sync.py:204, mirrored in sync.py:201) accepts an optional stmnt: SelectOfScalar | None so callers can pass a fully custom select(...) — e.g. a single-column projection instead of select(model). But regardless of whether stmnt was supplied or built internally, the function unconditionally applies pagination before executing it:

# a_sync.py:406-408
stmnt = stmnt.offset((page - 1) * page_size).limit(
    page_size
)  # Corrected offset calculation

page_size defaults to 100. So any custom stmnt= passed in silently gets capped at 100 rows unless the caller also remembers to pass a large enough page_size — there's no way to opt out of pagination, and no warning/error when the true result set exceeds it. This is easy to hit: e.g. building an in-memory set of existing IDs from a large table to diff against an incoming batch — the caller gets back a plausible-looking but incomplete result and has no signal that rows were dropped.

Impact

This isn't a style issue — it's silent data loss. A caller reasonably expects a custom stmnt= to mean "run exactly this query," not "run this query, but also cap it at 100 rows unless I remembered to override page_size." We hit this while diffing ~1,500 existing rows against an external ID list (ranked_jobs_microservice#46) and had to bypass get_rows() entirely in favor of a raw select() to avoid the risk.

Suggested fix

One of:

  • When a caller-supplied stmnt is provided, skip the .offset()/.limit() application entirely — the caller owns pagination for a custom statement.
  • Or add a paginate: bool = True flag so callers can explicitly opt out.
  • At minimum, document the current behavior loudly in the docstring/README so it isn't discovered via a truncated result set in production.

Repro

# Table has 1500 rows matching the filter
success, rows = await get_rows(session_inst=session, model=SomeModel, some_field="x")
assert len(rows) == 100  # not 1500 — silently truncated by the default page_size

Filed from work on https://github.com/fsecada01/Ranked-Jobs-API-Micro-Service (see PR #47 discussion).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions