Moodle
  1. Moodle
  2. MDL-12542

backup_todb not called on restore on date before it is inserted into mdl_user

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9
    • Fix Version/s: 1.7.4, 1.8.4, 1.9
    • Component/s: Backup
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_17_STABLE, MOODLE_18_STABLE, MOODLE_19_STABLE
    • Rank:
      29776

      Description

      In old Moodles, some of the columns are nullable. Therefore moodle sometimes ends up trying to insert $@NULL@$ into the mdl_user table on restore, which really does not work because some of the colums are VARCHAR(2).

      So we need to be calling backup_todb on everything.

      However, then you hit the problem that in a clean install of 1.9, all the columns are NOT NULL, so trying to insert NULLs gives errors, so we also need to replace NULL with ''.

        Activity

        Hide
        Eloy Lafuente (stronk7) added a comment -

        Hi Tim,

        just saw your commit related to this, great! Anyway can you consider if, instead of:

        + $user->$field = backup_todb($user->$field);
        + if (is_null($user->$field))

        { + $user->$field = ''; + }

        we could use:

        + $user->$field = backup_todb($user->$field);
        + if (is_null($user->$field))

        { + unset($user->$field); + }

        that way, DB defaults will be in effect and IMO that's better than forcing empties (that can cause problems in some DB no understading empties (a.k.a. Oracle).

        For your consideration, thanks and ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Hi Tim, just saw your commit related to this, great! Anyway can you consider if, instead of: + $user->$field = backup_todb($user->$field); + if (is_null($user->$field)) { + $user->$field = ''; + } we could use: + $user->$field = backup_todb($user->$field); + if (is_null($user->$field)) { + unset($user->$field); + } that way, DB defaults will be in effect and IMO that's better than forcing empties (that can cause problems in some DB no understading empties (a.k.a. Oracle). For your consideration, thanks and ciao
        Hide
        Tim Hunt added a comment -

        I think that sounds like a good idea. But I am bored of this issue. I only fixed it because it was preventing me from fixing another bug. If you want to make the improvement and test it, please be my guest!

        Show
        Tim Hunt added a comment - I think that sounds like a good idea. But I am bored of this issue. I only fixed it because it was preventing me from fixing another bug. If you want to make the improvement and test it, please be my guest!

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: