OHDSI / CommonDataModel

Definition and DDLs for the OMOP Common Data Model (CDM)
https://ohdsi.github.io/CommonDataModel
896 stars 451 forks source link

Fact Relationship Table #152

Closed survitup closed 3 years ago

survitup commented 6 years ago

The fact relationship table contains Domain Concept ID as an INTEGER but the Domain Concept ID has been changed to a VARCHAR(20). The fact relationship documentation and code needs to be updated to reflect the change to VARCHAR(20).

gowthamrao commented 6 years ago

@cgreich why is this a varchar and not an integer. Integer is more efficient to use. @cukarthik you had a similar concern?

cgreich commented 6 years ago

@gowthamrao, @anthonyjsl

We already pushed it out? We don't have the concepts ready yet for tables and fields. Dammit.

cgreich commented 6 years ago

@clairblacketer:

@anthonyjsl made a point that's wrong, but another point related to it is there: We changed the references to tables to Concepts? I thought that's a V6 thing?

survitup commented 6 years ago

@clairblacketer @cgreich

Yes the issue I opened was about me confusing the Domain ID with the Domain Concept ID. I closed it after I realized that that it was the domain concept id. So in V6 the change will be to concepts rather than domains correct?

gowthamrao commented 6 years ago

@cgreich @clairblacketer @vojtechhuser

yes - we need to have concept_id for each omop table - cdm, derived, metadata. we need it for a lot of reasons

survitup commented 6 years ago

@gowthamrao

Shall I change the details of the issue to reflect the change of Domain to Concept ID?

cgreich commented 6 years ago

@clairblacketer: Can you make a list of all tables and fields, in all versions you have? So we can make Concepts?

cgreich commented 6 years ago

@anthonyjsl: I realized what you were doing, but I reopened it, because you drew the attention on the other issue. Yes, that's the idea. But Clair pushed it already out. So, we either scramble to make the Concepts, or we pull it and wait till V6.

gowthamrao commented 6 years ago

So is it varchar or Integer (please say integer)

clairblacketer commented 6 years ago

@cgreich

I didn't make any changes to the fact_relationship table for v5.3.0, it has always used DOMAIN_CONCEPT_ID. The vocabulary_id 'Domain' contains concepts that represent each domain in the vocabulary. For example, 19 represents 'Condition' and 17 represents 'Device'. I don't think there is an error here, I think there is just some confusing naming conventions because DOMAIN_ID is a varchar and DOMAIN_CONCEPT_ID is an integer.

clairblacketer commented 6 years ago

@gowthamrao it is an integer :)

clairblacketer commented 6 years ago

@cgreich

I will make a list of all recently added tables so we can get them in the 'Domain' vocabulary

gowthamrao commented 6 years ago

@clairblacketer if it helps, here are all the tables that need concept_id https://docs.google.com/spreadsheets/d/1wFbq6Y7JbL5UjhUBKLE4hniDEdMnyDGGlyeew38u3NM/edit?usp=drivesdk

gowthamrao commented 6 years ago

@cgreich

Assign every table a concept_id and map that to parent domain_concept_id.