christillman / encounterpro_os

EncounterPro-OS Electronic Health Record Software
GNU Affero General Public License v3.0
0 stars 0 forks source link

SQL errors in Patient Canceled display scripts #40

Open christillman opened 9 months ago

christillman commented 9 months ago

Contact Details

toff.tillman@gmail.com

What happened?

Hi Ciru,

I was looking through the clinic error logs and noticed a couple of instances where errors were generated because of incorrect SQL in a display script query. Script Id 1017 is the one I noticed in the log, you received 3 errors for it on 2023-11-21 around 16:55 to 16:57. Script id 1000969 has a similar erroneous query.

1017 Patient Canceled Prescription Drugs Report 1000969 Patient Canceled Treatments

The query for 1017 looks like

SELECT distinct treatment_description,begin_date,created.user_short_name AS By_,end_date,cancelled.user_short_name AS By_ 
FROM p_treatment_item p WITH (NOLOCK)   
LEFT OUTER JOIN p_treatment_progress pp WITH (NOLOCK)  
    ON p.treatment_id = pp.treatment_id  
INNER JOIN c_user cancelled WITH (NOLOCK)  
    ON pp.created_by = cancelled.user_id 
INNER JOIN c_user created WITH  (NOLOCK)  
    ON p.created_by = created.user_id    
WHERE  p.treatment_type = 'MEDICATION'  
AND progress_type in ('CANCELLED','Cancelled')  
AND (('end_date 00:00:00' > 'begin_date 23:59:59') OR (created.created_by <> cancelled.created_by))    
AND p.cpr_id = '%cpr_id%'    
order by p.begin_date Desc  

I'm not sure what caused the SQL processing error; did you perhaps fix the syntax?

The similar script 1000969 has

SELECT distinct   pti.treatment_description  ,
    CONVERT(varchar,pti.begin_date,120) as begin_date  ,
    created.user_short_name as 'Created_By'  ,
    CONVERT(varchar,pti.end_date,120) as end_date  ,
    cancelled.user_short_name as 'Cancelled_By'  
FROM p_treatment_item pti WITH (NOLOCK)   
LEFT OUTER JOIN p_treatment_progress ptp WITH (NOLOCK)  ON pti.treatment_id = ptp.treatment_id  
INNER JOIN c_user cancelled WITH (NOLOCK)  ON pti.completed_by = cancelled.user_id  
INNER JOIN c_user created WITH (NOLOCK)  ON pti.created_by = created.user_id    
WHERE  ptp.progress_type in ('CANCELLED','Cancelled')  
AND (('end_date 00:00:00' > 'begin_date 23:59:59') OR (pti.created_by <> pti.completed_by))  
AND pti.cpr_id = '15'    
order by pti.begin_date Desc

This one errors because the begin_date used for ordering is not in the select clause. (You could use the CONVERT expression to order by instead).

But what bothers me about both of these is the inclusion of

'end_date 00:00:00' > 'begin_date 23:59:59'

This will always be true, because the e in "end" comes after the b in "begin". It is not comparing any dates like it seems to want to, it is just comparing those two strings. Do you think they meant something like "not cancelled on the same day"? If that's what they mean it should be

datediff(day, begin_date, end_date) > 0

Another part doesn't work as intended either, in 1017

created.created_by <> cancelled.created_by

This compares the creator of the created user to the creator of the cancelling user. Nobody cares who created the users.

The other one 1000969 has

pti.created_by <> pti.completed_by

This is a little better, saying a different user cancelled it than the one who created it. But it's also a little problematic because many of the created_by and completed_by fields are either #SYSTEM or NULL. The NULL values will be excluded by the clause as written. One has to be a little careful with negatives in SQL because of this; you always figure NULL will be excluded if you use =, but don't think about it being excluded when you use <>. Usually you mean that the values are different, and if you consider NULL to be different than a real value, well it doesn't show up in the list so that's unexpected.

I think my suggestion for both of these would be

IsNull(created.user_id,'#SYSTEM') <> IsNull(cancelled.user_id,'#SYSTEM')

This would match records that were created by one real user and cancelled by another real user.

How did it happen?

Checking out the error logs

Expectations

Errors should be prevented

Urgency and Importance

Not urgent I suspect

Version

7.2.1.5

Relevant log output

Error processing query.  display_script_id=1017, display_command_id=27664, query= (as above)

Code of Conduct

christillman commented 4 months ago

Fixed in next release 7.2.2.0 per suggestions.

MCIR commented 1 month ago

Hi Chris,

Are you able to tell where in the application these scripts are used? They don't look familiar to me. Could they be running in the background of a certain process or are a subscript?.

MCIR commented 1 month ago

Hi Chris, I located where the script '1017' Patient Canceled Prescription Drugs Report' is applied in the application.

A video illustrating the steps to get to the page in the application is linked (see link to video below).

After loading the script, the application freezes and requires a restart. Additionally, two error messages appear before the script loads.

https://1drv.ms/v/s!AuBDbs81Gi8WgaUdHYFi-a4ocdLMqA?e=cT2pqT

Will keep trying to find where the other script 'Canceled Treatments' is used in the application.

christillman commented 1 month ago

That's very useful info, thanks. Was the video done with version 7.2.2.0, or an earlier version?

On Sat, Jul 20, 2024 at 4:47 AM Madeline Wanjiru @.***> wrote:

Hi Chris, I located where the script '1017' Patient Canceled Prescription Drugs Report' is applied in the application.

A video illustrating the steps to get to the page in the application is linked (see link to video below).

After loading the script, the application freezes and requires a restart. Additionally, two error messages appear before the script loads.

https://1drv.ms/v/s!AuBDbs81Gi8WgaUdHYFi-a4ocdLMqA?e=cT2pqT

Will keep trying to find where the other script 'Canceled Treatments' is used in the application.

— Reply to this email directly, view it on GitHub https://github.com/christillman/encounterpro_os/issues/40#issuecomment-2239623482, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGEGTU2N6RO5GS62XYLS7ELZNE7JLAVCNFSM6AAAAABANQUMZOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZZGYZDGNBYGI . You are receiving this because you were assigned.Message ID: @.***>

-- Chris Tillman Developer

christillman commented 1 month ago

I found the source of the problem, Ciru. I made a change in 7.2.1.3 to substitute IsNull for COALESCE because COALESCE was causing "not prepared" errors with the new MSOLEDBSQL driver. I didn't realize that using IsNull to read the sql scripts from the database would truncate them to 256 characters. When the truncated SQL is attempted to be executed, it is causing the application crash. I will get this fixed in the 7.2.3 by avoiding both COALESCE and IsNull, and instead using the more basic equivalent CASE statements.

christillman commented 1 month ago

Here are display scripts that would have been affected by this bug.

select s.display_script_id, s.description, c.display_command, a.attribute, cast(long_value as varchar(8000)) from c_Display_Script_Cmd_Attribute a join c_Display_Script_Command c on a.display_command_id = c.display_command_id join c_Display_Script s on s.display_script_id = c.display_script_id where long_value is not null and datalength(long_value) > 255 and c.status = 'OK' and s.status = 'OK' order by s.description