GET /conceptset/{id}/expression returns items in nondeterministic order (query has no ORDER BY)
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 151
- Forks
- 183
- Avg merge
- 14m
- Merged PRs (30d)
- 2
Description
Summary
GET /conceptset/{id}/expression can return the items of an unmodified concept set in a different order across identical requests. The response content is otherwise identical — same concepts, same flags — only the order of expression.items differs.
This has been confirmed first-hand against a live ATLAS instance.
Environment
- ATLAS 2.15.0, WebAPI 2.15.1
- Also present on 2.14, so this is not a regression introduced in 2.15 — it looks like long-standing behavior, which is consistent with the code history below (the relevant query and schema have been unchanged since the original migrations).
Observed: repeated GET /conceptset/{id}/expression for the same concept set, with no edit to that concept set in between, returned items in different orders.
Why this matters
Downstream tooling that persists or hashes the concept set expression treats the response as content. A pure reordering is indistinguishable from a real edit:
- the serialized JSON on disk changes,
- its content hash changes, so change-detection reports a modification that did not happen,
- study repositories under version control accumulate spurious diffs and uncommitted changes.
This is reported from downstream use in OHDSI/Picard, which imports concept sets and cohort definitions from ATLAS into a study repository and uses content hashing to decide whether an input has actually changed. Any reproducible-study tooling that round-trips concept sets through WebAPI has the same exposure, and the same applies to cohort definitions once a re-saved cohort picks up a reordered embedded concept set expression.
Endpoints involved
GET /conceptset/{id}/expressionGET /conceptset/{id}/expression/{sourceKey}GET /conceptset/{id}/version/{version}/expressionGET /conceptset/{id}/items
Where this appears to originate
Reading the source on master (a9720e7), the order of expression.items is simply the row order the database happens to return, and nothing in the chain imposes a deterministic order:
-
The expression is assembled by iterating
repositoryItemsin list order:
https://github.com/OHDSI/WebAPI/blob/a9720e7bc0bec454d3363a61999b10677e448af6/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java#L336-L345 -
repositoryItemsis populated fromgetConceptSetItems(id):
https://github.com/OHDSI/WebAPI/blob/a9720e7bc0bec454d3363a61999b10677e448af6/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java#L296-L300
which delegates to the repository:
https://github.com/OHDSI/WebAPI/blob/a9720e7bc0bec454d3363a61999b10677e448af6/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java#L202-L206 -
findAllByConceptSetIdis a Spring Data derived query with noOrderByclause and noSortargument, so the generated SQL has noORDER BYand the row order is unspecified:
https://github.com/OHDSI/WebAPI/blob/a9720e7bc0bec454d3363a61999b10677e448af6/src/main/java/org/ohdsi/webapi/conceptset/ConceptSetItemRepository.java#L26This is the platform-independent core of the report: without an
ORDER BY, the row order is formally unspecified on any database, and a client cannot rely on it being stable between two identical requests. -
concept_set_itemcarries only a primary key onconcept_set_item_id; I could not find any index onconcept_set_idin the migrations, so the lookup is a full scan filtered onconcept_set_id:
https://github.com/OHDSI/WebAPI/blob/a9720e7bc0bec454d3363a61999b10677e448af6/src/main/resources/db/migration/postgresql/V1.0.1.0__conceptsets.sql#L12-L19
https://github.com/OHDSI/WebAPI/blob/a9720e7bc0bec454d3363a61999b10677e448af6/src/main/resources/db/migration/postgresql/V2.3.0.20180412000001__constraints.sql#L5A candidate mechanism, if the affected instance runs on PostgreSQL. I have not confirmed which database platform this particular WebAPI instance uses, so I offer this conditionally rather than as a diagnosis. On PostgreSQL, an unordered sequential scan is exactly the situation where identical queries can legitimately return rows in different orders with no data change at all —
synchronize_seqscansis on by default, so a scan may start at an arbitrary block and wrap around, and a parallel sequential scan interleaves worker output nondeterministically. A plan change, aVACUUM FULL/CLUSTER, or restoring the OHDSI schema from a dump will also change heap order. Whether that specific mechanism applies would also depend on how largeconcept_set_itemis relative toshared_bufferson the affected instance — is that something you would expect to be significant on a typical deployment? On other supported platforms the missingORDER BYstill leaves the order unspecified, though the practical trigger would differ. Happy to confirm the platform and table size if that helps narrow it down. -
Nothing downstream restores an order: circe's
ConceptSetExpression.itemsis a plainConceptSetItem[]and faithfully serializes whatever order it is handed, so this does not look like a circe-be or ATLAS issue.
One related observation: the same unordered query is used when a concept set version snapshot is created, so an arbitrary row order is frozen into concept_set_version.asset_json:
https://github.com/OHDSI/WebAPI/blob/a9720e7bc0bec454d3363a61999b10677e448af6/src/main/java/org/ohdsi/webapi/conceptset/converter/ConceptSetToConceptSetVersionConverter.java#L26
There is precedent for caring about deterministic serialization here — OHDSI/circe-be#48 explicitly pinned property order for Concept serialization for the same class of reason.
Possible fix — question for maintainers
Would you be open to giving these reads a deterministic order? A couple of options, and I'd defer to you on which fits best:
- Add an explicit ordering to the repository method, e.g.
findAllByConceptSetIdOrderByConceptIdAsc, or pass aSort. Ordering byconcept_idis stable across databases and survives a delete-and-reinsert of the items on save, whichconcept_set_item_idwould not. - Or sort
expressionItemsinConceptSetService.getConceptSetExpressionbefore assigningexpression.items, which localizes the change to the expression endpoints.
An accompanying index on concept_set_item(concept_set_id) would probably be worthwhile regardless, both for the sort and for the lookup itself.
A related question: is item order intended to be semantically meaningful anywhere (i.e. does anything rely on the current insertion order coming back), or is a concept set expression conceptually an unordered set? If it is unordered, normalizing the order on read seems safe and would make the endpoint's output content-addressable.
Happy to supply more
We can attach an example concept set that exhibits the reordering, along with the responses from two consecutive identical requests, so you have something concrete to reproduce against — I'll follow up with that below. If there is anything else that would be more useful to see, just say. We're also glad to test a fix against our downstream usage.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with ConceptSetItemRepository.java and the getConceptSetItems and getConceptSetExpression entry points in ConceptSetService.java. Trace the four listed concept-set reads and the ConceptSetToConceptSetVersionConverter.java path, then check how existing tests cover their serialized item order. Done means the affected responses and version snapshots have a documented, deterministic order without changing their contents.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, spring
- Domain
- api, backend, database
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 67/100