Errors with Oracle

[s-cenni]

With Oracle I get this error.

Exception encountered in event observer 'enrol_attributes_plugin::process_login': Error reading from database

    line 271 of /lib/dml/oci_native_moodle_database.php: call to moodle_database->query_end()
    line 1140 of /lib/dml/oci_native_moodle_database.php: call to oci_native_moodle_database->query_end()
    line 291 of /enrol/attributes/lib.php: call to oci_native_moodle_database->get_records_sql()
    line 218 of /enrol/attributes/lib.php: call to enrol_attributes_plugin::process_enrolments()
    line ? of unknownfile: call to enrol_attributes_plugin::process_login()
    line 155 of /lib/classes/event/manager.php: call to call_user_func()
    line 75 of /lib/classes/event/manager.php: call to core\event\manager::process_buffers()
    line 804 of /lib/classes/event/base.php: call to core\event\manager::dispatch()
    line 4309 of /lib/moodlelib.php: call to core\event\base->trigger()
    line 195 of /login/index.php: call to complete_user_login()

in this query $select .= ' RIGHT JOIN '.$CFG->prefix.'user_info_data d'.$join_id.' ON d'.$join_id.'.userid = u.id'; $where .= ' (d'.$join_id.'.fieldid = '.$customkey.' AND ( d'.$join_id.'.data = \''.$rule->value.'\' OR d'.$join_id.'.data LIKE \'%;'.$rule->value.'\' OR d'.$join_id.'.data LIKE \''.$rule->value.';%\' OR d'.$join_id.'.data LIKE \'%;'.$rule->value.';%\' ))';

There are 2 problems: 1) data is a CLOB that needs to be manage with $DB->sql_like function https://docs.moodle.org/dev/Data_manipulation_API#SQL_compatibility_functions 2) Oracle doens't like the where true condition

I replaced $where = 'true'; with $where = '1 = 1'; and change the function arraysyntax_tosql in this way

public static function arraysyntax_tosql($arraysyntax) { // TODO : protected
        $select = '';
        $where = '1 = 1';
        static $join_id = 0;
        $params = array();
        $customuserfields = $arraysyntax['customuserfields'];

        foreach ($arraysyntax['rules'] as $rule) {
            // first just check if we have a value 'ANY' to enroll all people :
            if (isset($rule->value) && $rule->value == 'ANY') {
                return array(
                    'select' => '',
                    'where'  => '1 = 1'
                );
            }
        }

        foreach ($arraysyntax['rules'] as $rule) {
            if (isset($rule->cond_op)) {
                $where .= ' '.strtoupper($rule->cond_op).' ';
            }
            else {
                $where .= ' AND ';
            }
            if (isset($rule->rules)) {
                $sub_arraysyntax = array(
                    'customuserfields'  => $customuserfields,
                    'rules'             => $rule->rules
                );
                $sub_sql = self::arraysyntax_tosql($sub_arraysyntax);
                $select .= ' '.$sub_sql['select'].' ';
                $where .= ' ( '.$sub_sql['where'].' ) ';
                $params = array_merge($params,$sub_sql['params']);
            }
            else {
                if ($customkey = array_search($rule->param, $customuserfields, true)) {
                    // custom user field actually exists
                    $join_id++;
                    global $DB;
                    $data = 'd'.$join_id.'.data';
                    $select .= ' RIGHT JOIN {user_info_data} d'.$join_id.' ON d'.$join_id.'.userid = u.id';
                    $where .= ' (d'.$join_id.'.fieldid = ? AND ('.$DB->sql_compare_text($data).' = '.$DB->sql_compare_text('?').
                            ' OR '.$DB->sql_like($DB->sql_compare_text($data),'?').' OR '
                            .$DB->sql_like($DB->sql_compare_text($data),'?').' OR '.$DB->sql_like($DB->sql_compare_text($data),'?').'))';
                    array_push($params, $customkey,$rule->value,'%;'.$rule->value,$rule->value.';%','%;'.$rule->value.';%');
                }
            }
        }

        $where = preg_replace('/^true AND/', '', $where);
        $where = preg_replace('/^true OR/', '', $where);
        $where = preg_replace('/^true/', '', $where);

        return array(
            'select' => $select,
            'where' => $where,
            'params' => $params
        );
    }

and changing $users = $DB->get_records_sql($select . $arraysql['select'] . $where . $arraysql['where']); in $users = $DB->get_records_sql($select . $arraysql['select'] . $where . $arraysql['where'],$arraysql['params']); (in lib.php) and $debug_users = $DB->get_records_sql($debug_sqlquery); in $debug_users = $DB->get_records_sql($debug_sqlquery,$debug_arraysql['params']);

If you want I can send all my change code with a Pull Request

Bye Sara

[scara]

Hi @s-cenni, nice to read you again working on solving Moodle vs Oracle issues :+1:! Please, double check your code since e.g. $where = preg_replace('/^true AND/', '', $where); should be improved with your new true strategy too: while your version will still work it doesn't act as the original code which removes the initial true condition. Besides there's no need to call preg_replace() 3 times: for performance reasons it could be just $where = preg_replace('/^true( AND| OR)?/', '', $where); (not tested). I'd suggest to propose your code changed to adress what written above.

IMHO the true value should be a constant (here, a local variable too), properly commented with an example for being cross-compatible with the DBs supported by Moodle, which should be more maintainable and let others to change e.g. this constant value w/o knowing anything of the logics around that but the reason why for this particular value.

HTH, Matteo

[ndunand]

Hi @s-cenni , Hi @scara ,

Thanks for spotting this Sara, and thanks for pointing this out Matteo. I'd love to see this into a PR so that I can spot out the differences more easily, and we can discuss it further.

[s-cenni]

Hi @scara, hi @ndunand , I send my PR (https://github.com/ndunand/moodle-enrol_attributes/pull/13)

Thanks Matteo for your great suggestions! (Yes, I keep going with my battle! Using Oracle always gives me a lot of work & fun!! ;) )

Temporarily I left the '1 = 1' condition in just one line because in the other parts 'where true' is replaced by the regexs. I didn't changed it because I'm not sure how to write an always true condition in SQL :(. Maybe it would be easier to change PHP code to create the correct query without this condition...

[scara]

Hi @s-cenni, same in MSSQL :smirk:. Besides, mine was just an hint.