search

catalyst / moodle-tool_lockstats

Moodle cron / task API lock statistics admin tool
https://moodle.org/plugins/tool_lockstats
6 stars 13 forks source link

Potential bug: Comparisons of text column conditions are not allowed when deleting a lock record #76

Closed kristian-94 closed 4 years ago

kristian-94 commented 5 years ago

https://github.com/catalyst/moodle-tool_lockstats/blob/master/db/upgrade.php#L186 We have changed this DB field 'customdata' to text. However, during a log_unlock function, this can cause an error when trying to delete records since text isn't allowed in the where clause:


15:31:38 [error] 212#212: *401 FastCGI sent in stderr: "PHP message: Default exception handler: Comparisons of text column conditions are not allowed. Please use sql_compare_text() in your query. 

Debug: #012Error code: textconditionsnotallowed#012* 
line 691 of /lib/dml/moodle_database.php: dml_exception thrown#012* 
line 1937 of /lib/dml/moodle_database.php: call to moodle_database->where_clause()#012* 
line 336 of /admin/tool/lockstats/classes/proxy_lock_factory.php: call to moodle_database->delete_records()#012* 
line 191 of /admin/tool/lockstats/classes/proxy_lock_factory.php: call to tool_lockstats\proxy_lock_factory->log_unlock()#012* 
line 102 of /lib/classes/lock/lock.php: call to tool_lockstats\proxy_lock_factory->release_lock()#012* 
line 189 of /theme/styles.php: call to core\lock\lock->release()"

This area of the plugin needs to be rewritten to handle deletion when the field is a text type, or we need to change the DB field to a larger varchar type instead.

https://github.com/catalyst/moodle-tool_lockstats/blob/master/classes/proxy_lock_factory.php#L336

kristian-94 commented 5 years ago

We also should be checking for this in unit tests

kristian-94 commented 5 years ago

Update:

It turns out that this bug is not caused by the customdata field having type text, but rather the resourcekey field. Looking through the plugin history, I can't see any way that the resourcekey field could end up with type text through normal use of the plugin since it is never set to text before or after any upgrade. This bug may be a local issue/bug in this case, although it has happened across different environments with different versions of the plugin that this resourcekey field has been changed to a text type. Further investigation into the cause of that is ongoing

brendanheywood commented 4 years ago

so @kristian-94 is this a close as 'nothing to do' ?

kristian-94 commented 4 years ago

Yes, let's close it. Looks like the plugin's code is fine , this issue is caused by something else.