Uploaded image for project: '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:
          • Pull Master Diff URL:

            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.

              Gliffy Diagrams

                Attachments

                  Issue Links

                  has been marked as being related by

                  Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-29567 Fix problem with negative defaults under PostgreSQL and associated tests

                  • Minor - Minor loss of function where workaround is possible
                  • Closed

                    Activity

                    Ascending order - Click to sort in descending order
                    Hide
                    Permalink
                    stronk7 Eloy Lafuente (stronk7) added a comment -

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

                    Show
                    stronk7 Eloy Lafuente (stronk7) added a comment - Adding attachment showing the (wrong) results under MySQL happening due to this bug.
                    Hide
                    Permalink
                    stronk7 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
                    stronk7 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
                    Permalink
                    stronk7 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
                    stronk7 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
                    Permalink
                    samhemelryk Sam Hemelryk added a comment -

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

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

                    yep works as expected

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

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

                    Show
                    stronk7 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:
                          Fix Release Date:
                          10/Oct/11