Re: [Gnumed-devel] jl-gui et alii - v_emr_tree

[Karsten Hilbert]

On Tue, Aug 20, 2013 at 09:43:26AM +0200, Jerzy Luszawski wrote:

> > I've got a few questions on this view:
> > 
> > CREATE OR REPLACE VIEW clin.v_emr_tree AS
> > (
> >     SELECT DISTINCT
> >             enc.pk_patient,
> >             epi.pk_health_issue AS pk_issue,
> >             epi.health_issue AS issue_description,
> >             epi.pk_episode,
> >             epi.description AS episode_description,
> >             enc.pk_encounter,
> >             enc.assessment_of_encounter AS encounter_description,
> >             enc.started AS encounter_started,
> >             enc.l10n_type AS encounter_l10n_type
> >     FROM
> >             clin.v_pat_encounters enc
> >                     JOIN clin.clin_narrative cn ON enc.pk_encounter = 
> > cn.fk_encounter
> >                             JOIN clin.v_pat_episodes epi ON cn.fk_episode = 
> > epi.pk_episode
> >     ORDER BY
> >             enc.pk_patient,
> >             epi.pk_health_issue,
> >             epi.health_issue,
> >             epi.pk_episode,
> >             epi.description,
> >             enc.pk_encounter,
> >             enc.assessment_of_encounter,
> >             enc.started,
> >             enc.l10n_type
> > ) UNION
> >     SELECT
> >             v_pat_episodes.pk_patient,
> >             v_pat_episodes.pk_health_issue AS pk_issue,
> >             v_pat_episodes.health_issue AS issue_description,
> >             v_pat_episodes.pk_episode,
> >             v_pat_episodes.description AS episode_description,
> >             NULL::unknown AS pk_encounter,
> >             NULL::unknown AS encounter_description,
> >             NULL::unknown AS encounter_started,
> >             NULL::unknown AS encounter_l10n_type
> >     FROM clin.v_pat_episodes
> > ORDER BY 1, 2, 4, 6
> > ;
> > 
> > 
> > 1) Do you intend to apply the "ORDER BY 1,2,4,6" to the
> >    entire view or to the second UNION sub query ? (if
> >    it is the second sub-query the 6 seems unhelpful
> >    as it is always NULL)
> It refers to whole UNION, by specification of PostgreSQL, AFAIR. That's
> why it requires column numbers instead of names.

Only if the column names of the UNION sub queries do not
return identical column names (which are then transposed to
the outer query) -- which they do in your view.

> It seems that the
> first ORDER BY is superfluous, though. 

I agree.

I would also suggest to take into account the following points:

- sorting rows retrieved from the view can easily be done
  by the query retrieving them

- if the view is pre-sorted it will waste a full sort in case
  someone retrieves rows from the view but wants to sort them
  differently

- should one later decide to want them rows sorted differently
  a change to the view inside the database is needed (unless
  one wants to suffer the double-sort penalty) rather then
  just some random SQL statement somewhere in client code

> > 2) Do you see problems with rewriting the view to
> >    directly look at the base tables rather than
> >    views thereof ? (that way this view will be more
> >    robust when other views are modified)
> AFAIK Postgres does good job by optimizing the views by itself.
> Moreover it would probably require more nested SELECTS, exactly as the
> base views do. I see no benefits (except getting independence from
> other views, but in exchange for readability).

I see, yes, readability suffers a bit when looking directly
at the base tables. I did not mean using per-column nested
SELECTs, however (which I agree are not the best solution),
but rather joining against the base tables.

> > 4) What is the reason for choosing assessment to be the
> >    description of the encounter ?  (this will severely limit
> >    the generic applicability of this view)
> Isn't assessment_of_encounter just this - short description?

I see, well, yes, that's right :-)

So perhaps I can suggest renaming encounter_description to
encounter_assessment to make it very clear where this column
takes its data from ?

> At least I use it this way. And I need it to be directly visible and
> short, I mean without any additional dates, statistics or whatever
> makes more elaborated "description". 

Sure, no problem, whichever way you need it.

> > 5) I think I don't fully understand which expected duplicity
> >    you are protecting against with the sub-query global DISTINCT
> >    on the first UNION sub-query ?
> It is not obvious, I know :) I need *only encounters with at least one
> narrative*, so I join with clin.clin_narrative, although I do not use
> any of its fields. There can be many narratives... The alternative is
> to explicitly look for existing narratives by SELECT in the WHERE
> clause.

Maybe rephrasing helps me better understand: You need rows
from clin.clin_narrative (but only the encounter and episode
parts thereof) and expand upon those with data from
clin.encounter, clin.episode, and, by extension,
clin.health_issue. Is that correct ?

I would assume this query also expresses the desire:

SELECT DISTINCT ON (c_narr.fk_encounter)
        c_enc.fk_patient as pk_patient,
        c_epi.fk_health_issue as pk_issue,
        c_hi.description as issue_description,
        c_epi.pk as pk_episode,
        c_epi.description as episode_description,
        c_narr.fk_encounter as pk_encounter,
        c_enc.assessment_of_encounter as encounter_description,
        c_enc.started as encounter_started,
        _(c_enc_t.description) as encounter_l10n_type
FROM
        clin.clin_narrative c_narr
                LEFT JOIN clin.encounter c_enc on c_narr.fk_encounter = c_enc.pk
                        LEFT JOIN clin.episode c_epi on c_narr.fk_episode = 
c_epi.pk
                                LEFT JOIN clin.health_issue 
c_epi.fk_health_issue = c_hi.pk
                                        LEFT JOIN clin.encounter_type c_enc_t 
on c_enc.fk_type = c_enc_type.pk
;

Is that right ?

(I think we don't want an INNER JOIN on clin.health_issue
because there can be episodes w/o issues. We also don't need
the more costly INNER JOIN from clin_narrative to
clin.encounter or clin.episode because each are garantueed
to be equivalent to a LEFT JOIN by design - AFAICT LEFT
JOINS are less costly as PG only has to look at one side of
the JOIN. Same with clin.encounter -> clin.encounter_type.)

Am I mistaken ?

> The general purpose of this wiew is to create pure hierarchical tree of
> health_issue->episode->encounter. Additional with no multiple links
> between encounter and episodes.
> It resolves the problem with encounters not linked directly to episodes.
> Quoting our earlier discussion:
> ----
> > And encounter is not linked directly to episode, only
> > through narratives.  
> 
> Sure, that's quite required. Encounters are multi-problem in
> most if not all cases (although many specialities choose to
> ignore all but one problem most of the time).
> ----
> I deal with one problem at a time and need the frontend (at least my
> plugins) to support this workflow. But I understand that others'
> workflow can be different from my narrow specialisation.

Don't worry. You are fully entitled to your workflow. I just
want to make sure we fully understand what we are going to
put into GNUmed.

Karsten
-- 
GPG key ID E4071346 @ gpg-keyserver.de
E167 67FD A291 2BEA 73BD  4537 78B9 A9F9 E407 1346