search

OHDSI / circe-be

CIRCE is a cohort definition and syntax compiler tool for OMOP CDMv5
Apache License 2.0
9 stars 13 forks source link

Error when using conceptset for standard concept ID and conceptset for source concept ID #54

Closed pbr6cornell closed 6 years ago

pbr6cornell commented 6 years ago

If user creates criteria that requires conceptset A in standard concept id field AND conceptset B in source concept id field, I would have thought the query should be requiring BOTH criteria to be satisfied.

However, looking through SQL, it appears the join to codeset makes this an OR so that either of two conceptsets will be satisfied.

Aside from ambiguity, the other issue I experienced is this OR is quite an expensive join, so probably want to think about a different way of implementing it if OR is the desired behavior (though I suspect we want AND)

gowthamrao commented 6 years ago

I was thinking of AND vs OR. While, I don't have a preference - I was able to come with one significant examples for OR.

This was when the source_concept was a "dirty" source concept that had two standard concepts.

chrisknoll commented 6 years ago

By design, it's an OR. I wouldn't call this a bug, as having 'AND' or 'OR' semantics is just up to the decision in the original design which was: if you have 2 concept sets, one with standard concepts, another with source concepts, you would look for records that have either the standard concept from the standard concept set OR the source_concepts from the source concept set.

We can change this behavior, of course, but we need to be sensitive of existing cohort definitions that depend on this behavior. Do we have the need to support the choice of if the standard concepts and source concepts should be OR'd vs. AND'd?

Additionally: when looking for concept IDs, it's using an IN() to match a set of concepts (yielded by a concept set expression), so the functionality that you are asking about sounds like you would want it to behave as:

concept_id in (concept set A) AND source_concept_id in (concept set B)

I'm just pointing out it is not single concept to single concept matching, unless your concept set has exactly one concept in it. For this reason, it made more sense to default to OR behavior over AND.

chrisknoll commented 6 years ago

@gowthamrao : in your usecase, would it be possible to look across 2 different records for the OR condition? The AND case enforces that the same record be evaluated when looking for both the concept_id and source_concept_id. I'm thinking that the OR case doesn't have to look within the same record, it's just saying 'one or the other'.

If that's the case, we can change the current behavior of 'either concept_id or source_concept_id' into 'both concept_id and source_concept_id', with the workaround that if you want an 'OR', then construct your criteria using an OR criteria group.

@pbr6cornell , is that in line with your thinking?