Help "validating" query



  • Hi all. I'm wrote a query but it's returning an empty result set on the live database and I'm not sure if it's because I wrote the query wrong or if there just aren't any kids who meet the spec. Can you take a look at it and tell me if it does what it's supposed to do?

    What it's supposed to do:

    Some appointments are "mandatory", in the sense that a patient needs to have an "assessment" once per year. The query is meant to find all patients who:

    • had an assessment (or an "intake" appointment) more than 10 months ago (approx)
    • are not scheduled for an assessment in the next 50 days (approx)

    What the query looks like:

      select patient.patunique
      from ( select appt.patunique
                   , appt.date
                from appt
                where appt.reason = 'Annual'
           ) next
      left join patient on next.patunique = patient.patunique
      inner join ( select appt.patunique
                        , max(appt.date) as date
                     from appt 
                    where appt.reason = 'Annual'
                       or appt.reason = 'Intake'
                 group by appt.patunique
                 ) last on last.patunique = patient.patunique
     where last.date < CURDATE() - 300
       and ( next.date is null 
          or next.date > CURDATE() + 50 )
    order by last.date asc 
    

    Does this look sane?


  • Discourse touched me in a no-no place

    @Captain What happens if the left join is changed to a right join?



  • @loopback0 Nothing LOL



  • @Captain said in Help "validating" query:

    Does this look sane?

    Not really.

    The last subquery is finding the latest scheduled appointment. There's no way next.date can ever be greater than last.date, but anyone who's ever had an annual appointment will have an entry in next and so will not be in your result set. This query should pick up people who've had an intake appointment but not an annual appointment, if you change the left join to a right join.

    Still, it looks overcomplicated to me. Doesn't the last subquery by itself pretty much give you what you want (with the condition on last.date, which you could make into a having clause?) What's wrong with just this:

    select appt.patunique
    from appt 
    where appt.reason = 'Annual' or appt.reason = 'Intake' 
    group by appt.patunique
    having max(appt.date) < CURDATE() - 300
    order by max(appt.date) asc
    

    Granted, this won't pick up the case where someone has an appointment too far ahead. I don't know if you need to (it looked like your original code was trying to). It also doesn't cover the case where someone's last appointment was 8 years ago and you probably don't need to worry about them any more, but your original code didn't care about this either.

    A more sophisticated version that addresses the first issue might be something like this:

    select last.patunique
    from (
         select appt.patunique, max(appt.date) as date
         from appt 
         where appt.reason = 'Annual' or appt.reason = 'Intake' 
         and appt.date < CURDATE()
         group by appt.patunique
         having max(appt.date) < CURDATE() - 300
      ) last
    where not exists (
         select 1 from appt
         where patunique = last.patunique
         and reason = 'Annual'
         and date between CURDATE() and CURDATE() + 50
         )
    order by last.date
    

    Notice that we're restricting the last subquery to appointments in the past now, so last.date is now the last time they actually had an appointment and doesn't include any future appointments. Then we explicitly check for appointments in the next 50 days.

    You could do the "where not exists" clause as an outer join to a subquery and check for it being null, but I think "where not exists" expresses the intent better.



  • @Scarlet_Manuka said in Help "validating" query:

    Thanks for your help. I don't get a lot of practice writing SQL anymore. I guess I could have tried harder before asking, but it "looked" right to someone in my position/skill-level. I'd be pretty okay in a few days if I kept at it.



  • @Captain No probs, SQL is one of the main things I do so I'm happy to help where I can.


  • :belt_onion:

    @Captain said in Help "validating" query:

    left join

    should be a right join.

    or you won't get anyone that has no future appointment


  • :belt_onion:

    @darkmatter also, depending on the db type, you could do:

    select distinct appt.patunique
    from appt 
    where (appt.reason = 'Annual' or appt.reason = 'Intake')
       and appt.date < CURDATE() - 300
    minus
    select distinct appt.patunique 
    from appt
    where reason = 'Annual'
       and date between CURDATE() and CURDATE() + 50
    

    *edits - made way simplerer.


  • Discourse touched me in a no-no place

    @darkmatter said in Help "validating" query:

    should be a right join.

    👍🏼


  • :belt_onion:

    @loopback0 you :hanzo:'d me on that i see.



  • @Captain Maybe it's just me, but it feels like some of the where clauses should be moved to their respective subqueries for performance reasons.



  • @darkmatter said in Help "validating" query:

    @darkmatter also, depending on the db type, you could do:

    select distinct appt.patunique
    from appt 
    where (appt.reason = 'Annual' or appt.reason = 'Intake')
       and appt.date < CURDATE() - 300
    minus
    select distinct appt.patunique 
    from appt
    where reason = 'Annual'
       and date between CURDATE() and CURDATE() + 50
    

    *edits - made way simplerer.

    That returns false positives - someone who had an annual appointment 380 days ago and another 20 days ago will be returned by your query.


Log in to reply