center-for-learning-management / moodle-local_edusupport

Plugin to manage a moodle based helpdesk
GNU General Public License v3.0
4 stars 3 forks source link

LIMIT 0,1 gibt fatal error mit postgres #50

Closed dasistwas closed 2 years ago

dasistwas commented 2 years ago

https://github.com/center-for-learning-management/moodle-local_edusupport/blob/acdc61ff2068f46df142cf92fed7b3fe6ed5ff9e/classes/lib.php#L442

Zeile 442 kann man ersatzlos streichen, da get_record_sql ohnehin nur einen Eintrag liefert.


Fehler
Fehler beim Lesen der Datenbank
Debug info: ERROR: LIMIT #,# syntax is not supported
LINE 3: ... WHERE userid = $1 AND courseid = $2 LIMIT 0,1
^
HINT: Use separate LIMIT and OFFSET clauses.
SELECT id,userid
FROM mdl_local_edusupport_supporters
WHERE userid = $1 AND courseid = $2 LIMIT 0,1
[array (
0 => '2',
1 => 1,
)]
Error code: dmlreadexception
Stack trace:

    line 486 of /lib/dml/moodle_database.php: dml_read_exception thrown
    line 329 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end()
    line 920 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end()
    line 1671 of /lib/dml/moodle_database.php: call to pgsql_native_moodle_database->get_records_sql()
    line 443 of /local/edusupport/classes/lib.php: call to moodle_database->get_record_sql()
    line 107 of /local/edusupport/lib.php: call to local_edusupport\lib::is_supportteam()
    line 1527 of /lib/navigationlib.php: call to local_edusupport_extend_navigation()
    line 1490 of /lib/navigationlib.php: call to global_navigation->load_local_plugin_navigation()
    line 3503 of /lib/navigationlib.php: call to global_navigation->initialise()
    line 3548 of /lib/navigationlib.php: call to navbar->has_items()
    line 4536 of /lib/outputrenderers.php: call to navbar->get_items()
    line 52 of /theme/moove/layout/frontpage.php: call to core_renderer->region_main_settings_menu()
    line 1374 of /lib/outputrenderers.php: call to include()
    line 1304 of /lib/outputrenderers.php: call to core_renderer->render_page_layout()
    line 100 of /index.php: call to core_renderer->header()
shreo1992-beep commented 2 years ago

i have solved this issue with postgress when i was installing the plugin i have changed the value of $sql to be like this

$sql .= " LIMIT 0 OFFSET 1";

since that postgress doesn't understand this syntax LIMIT 0,1

dasistwas commented 2 years ago

You can just delete the line. It is not necessary.

rschrenk commented 2 years ago

In the current version, yes. But it was designed that way to provide a few features in future, like course based support levels or responsibilities.

Therefore we should correct the statement to be compatible with PostreSQL instead of deleting the line.

dasistwas commented 2 years ago

The "get_record_sql" method only gets a single entry per definition. If that is going to be changed, it would be better to use "get_records_select" in order to stay compatible with any DB supported by Moodle that would be preferable.

https://docs.moodle.org/dev/Data_manipulation_API#get_records_select

All what you have now in the sql-Statement can be managed by the API call

$DB->get_records_select($table, $select, array $params=null, $sort='', $fields='*', $limitfrom=0, $limitnum=0)

You can even set a limit if you want to have only one result for now and later expand to multiple results.

But I am not fully aware of the future changes so I might be wrong as well.