Repository navigation
Description
Problem
The /api/v1/data query has no ORDER BY, so pages fetched with limit/offset aren't guaranteed to fit together: across a sequence of requests a client can get some records twice and miss others.
get_filtered_data (app/db/queries.py:124–134 on main) builds:
SELECT {columns}
FROM oed.oed_data
WHERE {conditions}
LIMIT {limit} OFFSET {offset}Postgres doesn't promise any row order without ORDER BY, and its docs warn that LIMIT/OFFSET then return unpredictable subsets: https://www.postgresql.org/docs/current/queries-limit.html. In practice a plain sequential scan usually returns rows in the same physical order, which is why this hasn't been noticed. Things that can change the order between two requests include:
- a different query plan (for example once an index exists, or after
ANALYZE); - synchronized sequential scans, where a scan that starts while another is running joins it partway through the table (
synchronize_seqscans, on by default); - reads served by a different instance.
moldb-postgrescurrently has one instance, so this doesn't apply today, but the chart's default host ismoldb-postgres-r, which reads from any instance.
Not reproduced. I haven't observed inconsistent pages; this is from reading the query and the Postgres documentation. With 35,753 rows and one database instance it's unlikely to show up often.
Why it matters more now
Every way to read more than one page depends on offset:
- JSON responses over 5,000 records are auto-paginated, and clients follow the
nextlinks (fix: cap explicit page sizes and stop printing query results #8, fix: reject out-of-range limit and offset; previous past the end goes to the last records #10). - With feat: return full CSV results up to 100,000 rows; cap JSON limit at 5,000 #11, a JSON
limitabove 5,000 gets a 422, and a CSV query matching more than 100,000 records gets a 400 telling the caller to "page with limit and offset".
Why it isn't a one-line fix
oed.oed_data has no key. The table definition in db_migration/oed_03_data.sql (and db_init/oed_02_data.sql) has no primary key, unique constraint or identity column (I grepped every migration), and the table is filled by an INSERT … SELECT joining data_df_kcat, data_df_km and data_df_kcatkm. Ordering by the existing columns isn't a fix:
- no combination of them is known to be unique, so ties can still come back in any order;
- sorting all 35,753 rows by many text columns on every page is expensive without an index to match.
Proposal
- Add a surrogate key in a new migration, for example
id bigint GENERATED ALWAYS AS IDENTITY PRIMARY KEY. Existing rows get numbered when the column is added, and theINSERT … SELECTinoed_03_data.sqlfills it automatically on future loads. - Add
ORDER BY idto the data query built inbuild_data_query/get_filtered_data(feat: return full CSV results up to 100,000 rows; cap JSON limit at 5,000 #11 moves the query there), so JSON pages, CSV exports andnext/previouslinks all use one stable order.get_total_countdoesn't need it. - Decide whether
idappears in responses. Leaving it out keeps the JSON and CSV columns unchanged. Including it gives clients a stable record identifier, but it changes every response and thecolumnsoptions.
Reloading the table would renumber id, so pages are stable within one dataset version, not across reloads. That seems acceptable for paging.
Out of scope
A user-selectable sort order, like CLEANDB-api's ordering parameter (whose default order ends with predictions_uniprot_annot_id ASC for the same reason). That could be built on top of this.
References
- Raised in review of fix: reject out-of-range limit and offset; previous past the end goes to the last records #10 and feat: return full CSV results up to 100,000 rows; cap JSON limit at 5,000 #11.
- CLEANDB-api orders its
/searchresults with a key as the final tiebreaker (DEFAULT_ORDER_BYin itsapp/db/queries.py).
Activity