Modified the fact manifest to remove the dups #685
Conversation
Reviewer's GuideUpdated load_fact_manifest.sql to eliminate duplicate manifest entries by refactoring the Fact_manifest CTE to use SELECT DISTINCT and max-date subqueries instead of GROUP BY aggregates, introduced NOLOCK table hints, and removed redundant COLLATE clauses in partner/agency joins. Entity relationship diagram for deduplication in Fact_manifest CTEerDiagram
CT_FacilityManifest {
Id int
DateRecieved date
SiteCode int
Start date
End date
}
HTS_FacilityManifest {
Id int
DateArrived date
SiteCode int
Start date
End date
}
CBS_FacilityManifest {
Id int
DateArrived date
SiteCode int
Start date
End date
}
ALL_EMRSites {
MFL_Code int
emr varchar
}
CT_FacilityManifest ||--|| ALL_EMRSites : "SiteCode = MFL_Code"
HTS_FacilityManifest ||--|| ALL_EMRSites : "SiteCode = MFL_Code"
CBS_FacilityManifest ||--|| ALL_EMRSites : "SiteCode = MFL_Code"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @DennisGibz - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| SDP_Agency as Agency | ||
| from ODS.Care.All_EMRSites | ||
| ), | ||
| Fact_manifest as( |
There was a problem hiding this comment.
@DennisGibz it's a bit hard to see the changes from the diff.
What is the main change to take care of the dups ?
| ,cast(getdate() as date) as LoadDate | ||
| FROM ODS.Care.CBS_FacilityManifest(NoLock) m | ||
| INNER JOIN ( | ||
| SELECT [SiteCode] |
There was a problem hiding this comment.
The manifest is supposed to return unduplicated records for each site on the date it uploads and not the latest record that a site uploads .
…Fact_Manifest_Dups_Alignment
…ed is the same and IDs are unique
|
@nobert-mumo / @Marymary-dev the changes are a bit clear now. The dups were caused by the ID column which is usually generated by DWAPI when inserting int the dwapicentral. |
DennisGibz
changed the title
|
@Marymary-dev I hope your concerns were addressed by the most recent adjustments to the query. |
| cbs_fm.[Id] = tm.MaxmanifestId | ||
| ) | ||
| SELECT | ||
| FactKey= IDENTITY(INT,1,1), |
There was a problem hiding this comment.
@DennisGibz a suggestion, instead on the extra changes on each of the union, could we just select the max(manifestId) & then group by the columns?
There was a problem hiding this comment.
@nobert-mumo every union has its own source of data and the group by will be duplicated across the unions.
I think its just the approach of writing the query which at the end of the day gives the same results,ama?
I know you aren't the best friend of subqueries..
There was a problem hiding this comment.
@DennisGibz I meant introducing a group by on this final select. You will only need one group by to eliminate the dups on manifest_id
|
@nobert-mumo above is now factor in. The query gives same results as the one for the subquery. |
DennisGibz
requested review from
Marymary-dev
and removed request for
kevlanyo
Summary by Sourcery
Refactor fact_manifest CTE to remove duplicate manifest records by selecting the latest entry per site and streamline joins.
Enhancements: