GET /conceptset/{id}/expression returns items in nondeterministic order (query has no ORDER BY)

Inizia da ConceptSetItemRepository.java e dai punti di ingresso getConceptSetItems e getConceptSetExpression in ConceptSetService.java. Segui le quattro letture di concept set elencate e il percorso di ConceptSetToConceptSetVersionConverter.java, quindi verifica come i test esistenti coprono l’ordine serializzato dei relativi elementi. Il lavoro è completato quando le risposte interessate e gli snapshot delle versioni hanno un ordine documentato e deterministico senza modificarne il contenuto.

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}/expression
• GET /conceptset/{id}/expression/{sourceKey}
• GET /conceptset/{id}/version/{version}/expression
• GET /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:

1. The expression is assembled by iterating repositoryItems in list order:
   https://github.com/OHDSI/WebAPI/blob/a9720e7bc0bec454d3363a61999b10677e448af6/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java#L336-L345

2. repositoryItems is populated from getConceptSetItems(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

3. findAllByConceptSetId is a Spring Data derived query with no OrderBy clause and no Sort argument, so the generated SQL has no ORDER BY and the row order is unspecified:
   https://github.com/OHDSI/WebAPI/blob/a9720e7bc0bec454d3363a61999b10677e448af6/src/main/java/org/ohdsi/webapi/conceptset/ConceptSetItemRepository.java#L26

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

4. concept_set_item carries only a primary key on concept_set_item_id; I could not find any index on concept_set_id in the migrations, so the lookup is a full scan filtered on concept_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#L5

   A 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_seqscans is 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, a VACUUM 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 large concept_set_item is relative to shared_buffers on the affected instance — is that something you would expect to be significant on a typical deployment? On other supported platforms the missing ORDER BY still leaves the order unspecified, though the practical trigger would differ. Happy to confirm the platform and table size if that helps narrow it down.

5. Nothing downstream restores an order: circe's ConceptSetExpression.items is a plain ConceptSetItem[] 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 a Sort. Ordering by concept_id is stable across databases and survives a delete-and-reinsert of the items on save, which concept_set_item_id would not.
• Or sort expressionItems in ConceptSetService.getConceptSetExpression before assigning expression.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.