Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plausible before birth checks #561

Open
MelaniePhilofsky opened this issue Jul 29, 2024 · 2 comments
Open

Plausible before birth checks #561

MelaniePhilofsky opened this issue Jul 29, 2024 · 2 comments
Labels
check bug/enhancement DQ check SQL logic has a bug or needs refinement

Comments

@MelaniePhilofsky
Copy link

First, thank you @katy-sadowski, @MaximMoinat and all who update, maintain and support the DQD! This is a very useful tool and we rely on it to ensure our data are of high quality.

One suggestion, the new plausible before birth checks require an even start before birth. However, in our source data some of our events start at birth. So, a baby has a birthdatetime of 10am on December 31, 2024, their visit_start_datime is also at 10am on December 31, 2024. The DQD requires birthdatetime < visit_start_datime. I think changing the check of birthdatetime < or = visit_start_datime would be more appropriate since visits (and possibly other clinical events) can start at birthdatetime.

@MaximMoinat
Copy link
Collaborator

MaximMoinat commented Jul 30, 2024

That makes sense to me. The check is already implemented such that a record violates it if event_date < birth_date. So same date is allowed.

CAST(cdmTable.@cdmFieldName AS DATE) < COALESCE(
p.birth_datetime,

When looking at event datetime, things indeed go wrong. The event is cast to a date, but the birth_datetime is not and will still have a time component. This explains your findings; all datetime of events happening at same date as birth will violated the check.

Two possible solutions:

  • Cast birth_datetime to date, allowing all events that are on the same day as birth (disregarding time of day)
  • Add separate clause when dealing with datetime fields to take the birth time into account.

I would propose the first as it is simple to implement and consistent.

@katy-sadowski
Copy link
Collaborator

I concur with your suggestion @MaximMoinat - thanks! And thanks Melanie for the report :)

@katy-sadowski katy-sadowski added the check bug/enhancement DQ check SQL logic has a bug or needs refinement label Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
check bug/enhancement DQ check SQL logic has a bug or needs refinement
Projects
None yet
Development

No branches or pull requests

3 participants