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 repositoryItems in list order:
|
// put the concept information into the expression along with the concept set item information |
|
for (ConceptSetItem repositoryItem : repositoryItems) { |
|
ConceptSetExpression.ConceptSetItem currentItem = new ConceptSetExpression.ConceptSetItem(); |
|
currentItem.concept = map.get(repositoryItem.getConceptId()); |
|
currentItem.includeDescendants = (repositoryItem.getIncludeDescendants() == 1); |
|
currentItem.includeMapped = (repositoryItem.getIncludeMapped() == 1); |
|
currentItem.isExcluded = (repositoryItem.getIsExcluded() == 1); |
|
expressionItems.add(currentItem); |
|
} |
|
expression.items = expressionItems.toArray(new ConceptSetExpression.ConceptSetItem[0]); // this will return a new array |
-
repositoryItems is populated from getConceptSetItems(id):
|
List<ConceptSetItem> repositoryItems = new ArrayList<>(); |
|
if (Objects.isNull(version)) { |
|
getConceptSetItems(id).forEach(repositoryItems::add); |
|
} else { |
|
ConceptSetVersionFullDTO dto = getVersion(id, version); |
which delegates to the repository:
|
@Path("{id}/items") |
|
@Produces(MediaType.APPLICATION_JSON) |
|
public Iterable<ConceptSetItem> getConceptSetItems(@PathParam("id") final int id) { |
|
return getConceptSetItemRepository().findAllByConceptSetId(id); |
|
} |
-
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:
|
List<ConceptSetItem> findAllByConceptSetId(Integer conceptSetId); |
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.
-
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:
|
CREATE TABLE ${ohdsiSchema}.concept_set_item ( |
|
concept_set_item_id INTEGER NOT NULL DEFAULT NEXTVAL('concept_set_item_sequence'), |
|
concept_set_id INTEGER NOT NULL, |
|
concept_id INTEGER NOT NULL, |
|
is_excluded INTEGER NOT NULL, |
|
include_descendants INTEGER NOT NULL, |
|
include_mapped INTEGER NOT NULL |
|
); |
|
ALTER TABLE ${ohdsiSchema}.concept_set_item ADD CONSTRAINT PK_concept_set_item PRIMARY KEY (concept_set_item_id); |
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.
-
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:
|
List<ConceptSetItem> items = conceptSetItemRepository.findAllByConceptSetId(source.getId()); |
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.
Summary
GET /conceptset/{id}/expressioncan 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 ofexpression.itemsdiffers.This has been confirmed first-hand against a live ATLAS instance.
Environment
Observed: repeated
GET /conceptset/{id}/expressionfor the same concept set, with no edit to that concept set in between, returneditemsin 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:
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}/itemsWhere this appears to originate
Reading the source on
master(a9720e7), the order ofexpression.itemsis 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:WebAPI/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java
Lines 336 to 345 in a9720e7
repositoryItemsis populated fromgetConceptSetItems(id):WebAPI/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java
Lines 296 to 300 in a9720e7
which delegates to the repository:
WebAPI/src/main/java/org/ohdsi/webapi/service/ConceptSetService.java
Lines 202 to 206 in a9720e7
findAllByConceptSetIdis a Spring Data derived query with noOrderByclause and noSortargument, so the generated SQL has noORDER BYand the row order is unspecified:WebAPI/src/main/java/org/ohdsi/webapi/conceptset/ConceptSetItemRepository.java
Line 26 in a9720e7
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.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:WebAPI/src/main/resources/db/migration/postgresql/V1.0.1.0__conceptsets.sql
Lines 12 to 19 in a9720e7
WebAPI/src/main/resources/db/migration/postgresql/V2.3.0.20180412000001__constraints.sql
Line 5 in a9720e7
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_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:WebAPI/src/main/java/org/ohdsi/webapi/conceptset/converter/ConceptSetToConceptSetVersionConverter.java
Line 26 in a9720e7
There is precedent for caring about deterministic serialization here — OHDSI/circe-be#48 explicitly pinned property order for
Conceptserialization 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:
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.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.