plausibleTemporalAfter only correctly works with same or person table

[MaximMoinat]

Description

The check plausibleTemporalAfter will only work when plausibleTemporalAfterTableName refers to the same table or the person table. Currently, this is the case: the check either checks whether end_date > start_date or start_date > birth_datetime. However, in other cases the behaviour of this check is not well defined.

Underlying issues

1. Join of cdmTable and plausibleTable (plausibleTemporalAfterTableName) on person_id:

   DataQualityDashboard/inst/sql/sql_server/field_plausible_temporal_after.sql

   Lines 35 to 37 in 1197d5d

   	FROM @cdmDatabaseSchema.@cdmTableName cdmTable
   	{@cdmDatabaseSchema.@cdmTableName != @cdmDatabaseSchema.@plausibleTemporalAfterTableName}?{
   	JOIN @cdmDatabaseSchema.@plausibleTemporalAfterTableName plausibleTable ON cdmTable.person_id = plausibleTable.person_id}

   
   If a person has multiple records in plausibleTable it is not defined against which record to compare. This could e.g. result in more violations than records in the cdmTable. An example: comparing an event date before observation period start date in a database where a person can have more than one observation_period.
2. Bug when comparing date
   

   DataQualityDashboard/inst/sql/sql_server/field_plausible_temporal_after.sql

   Lines 49 to 50 in 1197d5d

   	CAST(cdmTable.@plausibleTemporalAfterFieldName AS DATE)
   	} > CAST(cdmTable.@cdmFieldName AS DATE)

   
   Line 49 should be CAST(plausibleTable.@plausibleTemporalAfterFieldName AS DATE)

Currently this does not cause issues as the join is either against itself or against the person table.

Proposed solution

Short-term

At least it would be good to document this behaviour in the check description:

plausibleTemporalAfterTableName should only reference the same table as cdmTableName or the PERSON table. Referencing other tables might cause confusing behaviour.

Long-term

In the long term we might want to split things up to avoid confusion:

1. Simplify plausibleTemporalAfter by removing option to specify a tableName, making it only possible to compare within the same table.
2. Introduce new checks for other specific temporal after/during checks:
   a. plausibleAfterBirth (or combine with plausibleDuringLife), see also Rewrite temporalPlausibleAfter for checking events before birth #445
   b. plausibleDuringObservationPeriod
   c. plausibleDuringVisit (e.g. visit_detail must be during linked visit_occurrence)
   d. ...

[katy-sadowski]

Good catch on this. I think I might vote for the short term solution to be # 1 from the the long term ideas! Simplify plausibleTemporalAfter by removing option to specify a tableName, making it only possible to compare within the same table. (and at the same time refactor plausibleDuringLife to include the DOB check). I'm happy to help with this :)