Skip to content

Use age_in_days for age calculations#1119

Open
labkey-martyp wants to merge 3 commits intorelease26.3-SNAPSHOTfrom
26.3_fb_tnprc_migration
Open

Use age_in_days for age calculations#1119
labkey-martyp wants to merge 3 commits intorelease26.3-SNAPSHOTfrom
26.3_fb_tnprc_migration

Conversation

@labkey-martyp
Copy link
Copy Markdown
Contributor

@labkey-martyp labkey-martyp commented Apr 22, 2026

Rationale

The new age_in_days is consistent across postgres and sql server and more in-line with age as measuring calendar days. A few fixes and cleanups associated with age calculations as well.

Related Pull Requests

Changes

  • Use age_in_days in calculated age at time columns
  • Use age_in_days in demographicsAge
  • Consistently check lastDayAtCenter
  • Fix ageClassAtTime which was showing current age class

Copy link
Copy Markdown
Contributor

@labkey-bpatel labkey-bpatel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Please review my comment.

" CAST(CASE WHEN x.birth IS NULL OR x.effDate IS NULL THEN NULL\n" +
" ELSE age_in_days(x.birth, x.effDate) END AS INTEGER) AS AgeAtTimeDays,\n" +
" CAST(CASE WHEN x.birth IS NULL OR x.effDate IS NULL THEN NULL\n" +
" ELSE CONVERT(age_in_months(x.birth, x.effDate), INTEGER) END AS INTEGER) AS AgeAtTimeMonths,\n" +
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AgeAtTimeYearsRounded, AgeAtTimeDays, and AgeAtTimeMonths - these were previously cast as float (even though it produced whole number values first), but now they are integer -- is this intentional?

" ELSE CAST(" + dateSql + " AS DATE) END AS effDate\n" +
" FROM \"" + schemaName + "\".\"" + queryName + "\" c\n" +
" LEFT JOIN \"" + ehrPath + "\".study.demographics d ON (d.Id = c." + idCol.getFieldKey().toSQLString() + ")\n" +
") x"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AgeClassAtTime semantics change looks like a great improvement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants