Moodle
  1. Moodle
  2. MDL-29566

Incorrect handling of float columns metadata and associated tests

    Details

    • Database:
      MySQL
    • Testing Instructions:
      Hide

      This requires the 5 DBs installed (mysql, postgres, mssql, sqlsrv, oracle) and testing it under 21_STABLE could be enough (patch is 100% the same in 20 and master).

      1) Using MySQL (skip the report against other DBs), go to "Site administration > Development > XMLDB editor > Check defaults and run the report in a brand new site (results may be different if the site is one upgrade from previous versions).
      2) TEST: It returns "Wrong defaults found: 0" (before the patch it was detecting false wrong defaults.

      3) Run DB functional tests against the 5 databases.
      4) TEST: No failure / exception related with test_get_columns() happens (ignore any other failure).

      Show
      This requires the 5 DBs installed (mysql, postgres, mssql, sqlsrv, oracle) and testing it under 21_STABLE could be enough (patch is 100% the same in 20 and master). 1) Using MySQL (skip the report against other DBs), go to "Site administration > Development > XMLDB editor > Check defaults and run the report in a brand new site (results may be different if the site is one upgrade from previous versions). 2) TEST: It returns "Wrong defaults found: 0" (before the patch it was detecting false wrong defaults. 3) Run DB functional tests against the 5 databases. 4) TEST: No failure / exception related with test_get_columns() happens (ignore any other failure).
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      19433

      Description

      Playing with other modifications in the XMLDB Editor, I detected that the builtin "Check defaults" report was showing persistently some errors in 4 float columns. And that happened in brand new installed sites and only under MySQL.

      After investigating a bit these was detected:

      • float columns without length & decimal specs (allowed) aren't handled by mysql->get_columns() method. It is currently expecting always the length and decimal specs to exist. So fails badly for float columns not having them.
      • float columns and some bits of decimal columns aren't covered by unit tests.

      So here we go trying to fix both problems.

        Issue Links

          Activity

          Hide
          Eloy Lafuente (stronk7) added a comment -

          Adding attachment showing the (wrong) results under MySQL happening due to this bug.

          Show
          Eloy Lafuente (stronk7) added a comment - Adding attachment showing the (wrong) results under MySQL happening due to this bug.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Sending to integration. Patch is 100% the same for all branches but I've supplied already all them for commodity (and because I had cherry-picked here for testing).

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Sending to integration. Patch is 100% the same for all branches but I've supplied already all them for commodity (and because I had cherry-picked here for testing). Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Note to integrators: Due to modifications around same lines, to avoid conflicts, this must be integrated BEFORE MDL-29567 . TIA!

          Show
          Eloy Lafuente (stronk7) added a comment - Note to integrators: Due to modifications around same lines, to avoid conflicts, this must be integrated BEFORE MDL-29567 . TIA!
          Hide
          Sam Hemelryk added a comment -

          Hi Eloy - this has been integrated now - will look at MDL-29567 shortly.
          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Eloy - this has been integrated now - will look at MDL-29567 shortly. Cheers Sam
          Hide
          Aparup Banerjee added a comment -

          yep works as expected

          Show
          Aparup Banerjee added a comment - yep works as expected
          Hide
          Eloy Lafuente (stronk7) added a comment -

          git repositories have been updated with your awesome changes, thanks! Closing.

          Show
          Eloy Lafuente (stronk7) added a comment - git repositories have been updated with your awesome changes, thanks! Closing.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: