5
\$\begingroup\$

This code correctly gathers diagnosis records for a patient to show distinct records based on the AMA ICD9 code, diagnosis description, and date of posting. The output is distinct on (ICD code, cdesc, tposted). The most recent date of usage, tposted, is assigned to each unique combination of (ICD9 code, cdesc).

Additionally, each record has flags as to whether it is a chronic condition, chronic, and/or part of the past medical history, PMH. A problem/diagnosis is considered current/active, if it has reoccurred since its last resolution, has never been resolved, is chronic, or has occurred on the date of patient encounter, tencounter. The code also includes the fact that people often have more then one name associated with their account and that the tables are at least 2NF with "recid" as foreign key reference to other tables.

My concerns are as follows:

  1. The CTE's defined before the LOOP statement all apply to each of the select statements of the union (good). However, within the LOOP...END LOOP section, I am having to rewrite the "person" and "alldx" CTE for each associated SELECT statement. Is there any way to define these CTE's once, ideally in the first section and have enough permanence to be used in the LOOP section and avoid defining the same CTE multiple times?
  2. Would it be faster to create multiple views and then perhaps do left joins or Updates to set the flags on each record as opposed to doing a loop on the output records?
  3. Is there a better way to do this? How could this code be improved for speed?
  CREATE OR REPLACE FUNCTION test (patient_recid integer, tencounter timestamp without time zone)
      RETURNS SETOF view_dx AS

$BODY$ 

#variable_conflict use_column

DECLARE
    r view_dx%rowtype;

BEGIN

-- will loop through all the records produced by the unions and assign tposted, pmh, chronic, etc...
FOR r IN 

With person AS (
    select p.chart_recid as recid
    from patients p
    where p.recid = patient_recid
)
, alldx AS (
    select d.tposted, d.treated, d.resolved, d.recid as dx_recid, d.pmh, d.icd9_recid, i.code, i.cdesc, i.chronic
    from dx d
    join icd9 i on (d.icd9_recid = i.recid)
    join person p on (d.chart_recid = p.recid)
    where d.tposted::date <= tencounter::date
)
, dx_at_encounter AS (      -- get all diagnosis at time of encounter
    select * from alldx a
    where a.tposted::date = tencounter::date
)
, dx_resolved AS (              -- get most recent date of every resolved problem.
    select b.* from alldx b
    join (
        select a.code, a.cdesc , max(tposted) as tposted
        from alldx a
        where a.resolved = true 
        group by code,cdesc) j
    on (b.code = j.code and b.cdesc = j.cdesc and b.tposted = j.tposted)    
)
, never_resolved AS (       -- get all problems that have never been resolved before time of encounter.
                -- "not exists" is applied to each select output row AFTER the output row b.* is formed.
    select b.* from alldx b
    where not exists 
        (select 1 
         from dx_resolved d
         where b.code = d.code and b.cdesc = d.cdesc) 
)
, recurrent AS (        -- get all recurrent problems. (Problems that are now current after being resolved).
    select  b.* 
    from alldx b
    join dx_resolved r on (b.cdesc = r.cdesc and b.tposted::date > r.tposted::date )
    where (b.resolved is null or b.resolved = false)  

)
, chronic_dx AS (
    select b.*
    from alldx b
    where b.chronic = true
)

-- all diagnosis at time of encounter
select  a.code, 
    a.cdesc
from dx_at_encounter a

union 
-- all recurrent problems
select 
    a.code, 
    a.cdesc
from recurrent a

union

-- all problems that have never been resolved
select 
    a.code, 
    a.cdesc
from never_resolved a

union

--all chonic problems
select 
    a.code, 
    a.cdesc
from chronic_dx a

-- LOOP goes to END LOOP which returns back to LOOP to process each of the result records from the unions.
LOOP
    r.tposted = (       -- get most recent useage of a diagnosis.
            With person AS (
                select p.chart_recid as recid
                from patients p
                where p.recid = patient_recid
            )
            , alldx AS (
                select d.tposted, d.treated, d.resolved, d.recid as dx_recid, d.pmh, d.icd9_recid, i.code, i.cdesc, i.chronic
                from dx d
                join icd9 i on (d.icd9_recid = i.recid)
                join person p on (d.chart_recid = p.recid)
                where d.tposted::date <= tencounter::date
            )
            select max(b.tposted)
            from alldx b
            where r.cicd9 = b.code and r.cdesc = b.cdesc);

    r.treated = (   
                With person AS (
                select p.chart_recid as recid
                from patients p
                where p.recid = patient_recid
                )
                , alldx AS (
                    select d.tposted, d.treated, d.resolved, d.recid as dx_recid, d.pmh, d.icd9_recid, i.code, i.cdesc, i.chronic
                    from dx d
                    join icd9 i on (d.icd9_recid = i.recid)
                    join person p on (d.chart_recid = p.recid)
                    where d.tposted::date <= tencounter::date
                )
                select b.treated from alldx b
                where b.tposted = r.tposted and b.code = r.cicd9 and b.cdesc = r.cdesc);

    r.resolved =  (     
                With person AS (
                select p.chart_recid as recid
                from patients p
                where p.recid = patient_recid
                )
                , alldx AS (
                    select d.tposted, d.treated, d.resolved, d.recid as dx_recid, d.pmh, d.icd9_recid, i.code, i.cdesc, i.chronic
                    from dx d
                    join icd9 i on (d.icd9_recid = i.recid)
                    join person p on (d.chart_recid = p.recid)
                    where d.tposted::date <= tencounter::date
                )
                select b.resolved from alldx b
                where b.tposted = r.tposted and b.code = r.cicd9 and b.cdesc = r.cdesc);


    r.pmh = (
            With person AS (
                select p.chart_recid as recid
                from patients p
                where p.recid = patient_recid
            )
            , alldx AS (
                select d.tposted, d.treated, d.resolved, d.recid as dx_recid, d.pmh, d.icd9_recid, i.code, i.cdesc, i.chronic
                from dx d
                join icd9 i on (d.icd9_recid = i.recid)
                join person p on (d.chart_recid = p.recid)
                where d.tposted::date <= tencounter::date
            )
            select distinct true 
            from alldx b
            where
            b.pmh = true and 
            b.code = r.cicd9 and 
            b.cdesc = r.cdesc ); 

    r.chronic = (
                With person AS (
                select p.chart_recid as recid
                from patients p
                where p.recid = patient_recid
            )
            , alldx AS (
                select d.tposted, d.treated, d.resolved, d.recid as dx_recid, d.pmh, d.icd9_recid, i.code, i.cdesc, i.chronic
                from dx d
                join icd9 i on (d.icd9_recid = i.recid)
                join person p on (d.chart_recid = p.recid)
                where d.tposted::date <= tencounter::date
            )
            select distinct true 
            from alldx b
            where
            b.chronic = true and 
            b.code = r.cicd9 and 
            b.cdesc = r.cdesc); 

     RETURN NEXT r; -- return current row of SELECT

END LOOP;

RETURN;

END

$BODY$
  LANGUAGE plpgsql VOLATILE
  COST 100
  ROWS 1000;
ALTER FUNCTION test(integer, timestamp without time zone)
  OWNER TO postgres;
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Good job on your first question \$\endgroup\$ – SirPython Nov 6 '15 at 1:22
  • 3
    \$\begingroup\$ Be careful with heavy use of CTEs in PostgreSQL. We have a wart there. blog.2ndquadrant.com/postgresql-ctes-are-optimization-fences (I can't offer much input on the question at hand without a sqlfiddle and sample data) \$\endgroup\$ – Craig Ringer Nov 6 '15 at 5:32
  • \$\begingroup\$ @CraigRinger Very good article. As a novice myself, I really do appreciate the clarity CTE's bring to the table--even if they are slower. Thanks! \$\endgroup\$ – Alan Wayne Nov 6 '15 at 5:52
  • \$\begingroup\$ I like them syntactically, and for that reason it really annoys me that PostgreSQL has baked in this optimizer limitation as a feature :( \$\endgroup\$ – Craig Ringer Nov 6 '15 at 6:00
  • 2
    \$\begingroup\$ Please note Craig's other point as well: it's difficult or impossible for us to review a complex query like this without knowing your database schema and seeing some sample inputs and outputs. \$\endgroup\$ – 200_success Nov 6 '15 at 7:07
5
\$\begingroup\$

As far as I can tell, CTE's defined before the LOOP do not transfer to the LOOP itself. However, a temporary table can be defined in the BEGIN block which is available in the LOOP block. The following solution runs 50 times faster then my original code. Anybody have a better approach?

CREATE OR REPLACE FUNCTION test2 (patient_recid integer, tencounter timestamp without time zone)
  RETURNS SETOF view_dx AS
$BODY$ 

#variable_conflict use_column



DECLARE
    r view_dx%rowtype;


BEGIN
    -- create table can only be created in the BEGIN block
    Create temp table all_dx ON COMMIT DROP AS
    With person AS (
        select p.chart_recid as recid
        from patients p
        where p.recid = patient_recid
        )
       , alldx AS (
        select d.tposted, d.treated, d.resolved, d.recid as dx_recid, d.pmh, d.icd9_recid, i.code, i.cdesc, i.chronic
        from dx d
        join icd9 i on (d.icd9_recid = i.recid)
        join person p on (d.chart_recid = p.recid)
        where d.tposted::date <= tencounter::date
        )
      select * from alldx order by tposted desc;    

-- will loop through all the records produced by the unions and assign tposted, pmh, chronic, etc...
FOR r IN 

With 
dx_at_encounter AS (        -- get all diagnosis at time of encounter
    select code, cdesc from all_dx a
    where a.tposted::date = tencounter::date
)
, dx_resolved AS (              -- get most recent date of every resolved problem.
    select b.* from all_dx b
    join (
        select a.code, a.cdesc , max(tposted) as tposted
        from all_dx a
        where a.resolved = true 
        group by code,cdesc) j
    on (b.code = j.code and b.cdesc = j.cdesc and b.tposted = j.tposted)    
)
, never_resolved AS (       -- get all problems that have never been resolved before time of encounter.
                -- "not exists" is applied to each select output row AFTER the output row b.* is formed.
    select b.code, b.cdesc from all_dx b
    where not exists 
        (select 1 
         from dx_resolved d
         where b.code = d.code and b.cdesc = d.cdesc) 
)
, recurrent AS (        -- get all recurrent problems. (Problems that are now current after being resolved).
    select  b.code, b.cdesc
    from all_dx b
    join dx_resolved r on (b.cdesc = r.cdesc and b.tposted::date > r.tposted::date )
    where (b.resolved is null or b.resolved = false)  

)
, chronic_dx AS (
    select b.code, b.cdesc
    from all_dx b
    where b.chronic = true
)

-- all diagnosis at time of encounter
select  a.code, 
    a.cdesc
from dx_at_encounter a

union 
-- all recurrent problems
select 
    a.code, 
    a.cdesc
from recurrent a

union

-- all problems that have never been resolved
select 
    a.code, 
    a.cdesc
from never_resolved a

union

--all chonic problems
select 
    a.code, 
    a.cdesc
from chronic_dx a

-- LOOP goes to END LOOP which returns back to LOOP to process each of the result records from the unions.
LOOP
    r.tposted = (       -- get most recent useage of a diagnosis.
            select max(b.tposted)
            from all_dx b
            where r.cicd9 = b.code and r.cdesc = b.cdesc);

    r.treated = (   
            select b.treated from all_dx b
            where b.tposted = r.tposted and b.code = r.cicd9 and b.cdesc = r.cdesc);

    r.resolved =  (     
            select b.resolved from all_dx b
                where b.tposted = r.tposted and b.code = r.cicd9 and b.cdesc = r.cdesc);

    r.pmh = (
            select distinct true 
            from all_dx b
            where
            b.pmh = true and 
            b.code = r.cicd9 and 
            b.cdesc = r.cdesc ); 

    r.chronic = (
            select distinct true 
            from all_dx b
            where
            b.chronic = true and 
            b.code = r.cicd9 and 
            b.cdesc = r.cdesc); 

     RETURN NEXT r; -- return current row of SELECT

END LOOP;

RETURN;

END

$BODY$
  LANGUAGE plpgsql VOLATILE
  COST 100
  ROWS 1000;
ALTER FUNCTION test2(integer, timestamp without time zone)
  OWNER TO postgres;
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.