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).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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