Re-factored the scripts that fetch the data from the central Db and r… #713
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…emoved the sub-query which was elementing some records into the ODS esspeciary the ct_Patient
Reviewer's GuideThis PR refactors the ODS CT_DOCKET SQL scripts by replacing embedded subqueries used for deduplication and max-date selection with more readable CTE-based pipelines that use row_number() partitions to identify and merge only the latest records. Sequence diagram for the new CTE-based ETL pipeline in CT_PatientVisitssequenceDiagram
participant DWAPICentral
participant CTEs
participant ODS_Care_CT_PatientVisits
DWAPICentral->>CTEs: Extract PatientVisit data
CTEs->>CTEs: Partition and rank records by SiteCode, PatientPK, VisitDate, VisitID
CTEs->>CTEs: Select latest record per partition (rank=1)
CTEs->>ODS_Care_CT_PatientVisits: Merge latest records into ODS table
Sequence diagram for the new CTE-based ETL pipeline in Load_CT_ARTPatientsequenceDiagram
participant DWAPICentral
participant CTEs
participant ODS_Care_CT_ARTPatients
DWAPICentral->>CTEs: Extract PatientArt data
CTEs->>CTEs: Partition and rank records by SiteCode, PatientPK
CTEs->>CTEs: Select latest record per partition (rank=1)
CTEs->>ODS_Care_CT_ARTPatients: Merge latest records into ODS table
Sequence diagram for the new CTE-based ETL pipeline in CT_PatientPharmacysequenceDiagram
participant DWAPICentral
participant CTEs
participant ODS_Care_CT_PatientPharmacy
DWAPICentral->>CTEs: Extract PatientPharmacy data
CTEs->>CTEs: Partition and rank records by SiteCode, PatientPK, DispenseDate, Drug, VisitID
CTEs->>CTEs: Select latest record per partition (rank=1)
CTEs->>ODS_Care_CT_PatientPharmacy: Merge latest records into ODS table
Sequence diagram for the new CTE-based ETL pipeline in CT_PatientStatussequenceDiagram
participant DWAPICentral
participant CTEs
participant ODS_Care_CT_PatientStatus
DWAPICentral->>CTEs: Extract PatientStatus data
CTEs->>CTEs: Partition and rank records by SiteCode, PatientPK, ExitDate, ExitReason
CTEs->>CTEs: Select latest record per partition (rank=1)
CTEs->>ODS_Care_CT_PatientStatus: Merge latest records into ODS table
Class diagram for refactored ETL CTE pipeline structureclassDiagram
class DWAPICentral {
+PatientExtract
+PatientVisitExtract
+PatientArtExtract
+PatientPharmacyExtract
+PatientStatusExtract
+Facility
}
class CTE_Pipeline {
+row_number() partitioning
+rank selection
+deduplication
}
class ODS_Care_CT_Patient {
+PatientID
+PatientPK
+SiteCode
+FacilityName
+... (other patient attributes)
}
class ODS_Care_CT_PatientVisits {
+PatientID
+PatientPK
+SiteCode
+FacilityName
+VisitID
+VisitDate
+... (other visit attributes)
}
class ODS_Care_CT_ARTPatients {
+PatientID
+PatientPK
+SiteCode
+FacilityName
+... (other ART attributes)
}
class ODS_Care_CT_PatientPharmacy {
+PatientID
+PatientPK
+SiteCode
+FacilityName
+Drug
+DispenseDate
+... (other pharmacy attributes)
}
class ODS_Care_CT_PatientStatus {
+PatientID
+PatientPK
+SiteCode
+FacilityName
+ExitDate
+ExitReason
+... (other status attributes)
}
DWAPICentral --> CTE_Pipeline
CTE_Pipeline --> ODS_Care_CT_Patient
CTE_Pipeline --> ODS_Care_CT_PatientVisits
CTE_Pipeline --> ODS_Care_CT_ARTPatients
CTE_Pipeline --> ODS_Care_CT_PatientPharmacy
CTE_Pipeline --> ODS_Care_CT_PatientStatus
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 there - I've reviewed your changes - here's some feedback:
- In Load_CT_ARTPatient.sql the final MERGE is using the Ordered_ct_PatientArt_source CTE instead of the MaxOrdered_ct_PatientArt_source CTE, so you’re merging all rows rather than only the latest ones.
- In CT_PatientPharmacy.sql the CTE names (e.g. Ordered_ct_patient_source) don’t clearly reflect the pharmacy context—consider renaming them to Ordered_ct_PatientPharmacy_source to avoid confusion or collision.
- All scripts repeat large column lists in multiple CTEs and the MERGE—consider centralizing or auto-generating the column list (e.g. via a view or script) to reduce duplication and maintenance overhead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In Load_CT_ARTPatient.sql the final MERGE is using the Ordered_ct_PatientArt_source CTE instead of the MaxOrdered_ct_PatientArt_source CTE, so you’re merging all rows rather than only the latest ones.
- In CT_PatientPharmacy.sql the CTE names (e.g. Ordered_ct_patient_source) don’t clearly reflect the pharmacy context—consider renaming them to Ordered_ct_PatientPharmacy_source to avoid confusion or collision.
- All scripts repeat large column lists in multiple CTEs and the MERGE—consider centralizing or auto-generating the column list (e.g. via a view or script) to reduce duplication and maintenance overhead.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@DennisGibz just to confirm is the main change replacing the sub queries with the new CTEs? The sourcery-ai gives some overview but it would be nice to provide a context what necessitated the change for better review. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…emoved the sub-query which was elementing some records into the ODS esspeciary the ct_Patient
Summary by Sourcery
Refactor the CT_DOCKET ODS scripts to replace nested subqueries with CTE-driven pipelines that leverage row_number for record deduplication and reintroduce previously omitted records by removing the faulty filtering subquery.
Bug Fixes:
Enhancements: