Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-59323

XMLDB: Check defaults does not work correctly with char fields

    XMLWordPrintable

Details

    • Bug
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • 3.2.2
    • 3.4
    • Database SQL/XMLDB
    • PostgreSQL
    • MOODLE_32_STABLE
    • MOODLE_34_STABLE
    • MDL-59323-master
    • Hide

      1 On a fresh Moodle install (for example one generated for Behat testing), go to admin / development / xmldb and run the 'Check defaults' script

      EXPECTED: There should be no incorrect defaults

      2 Make changes to a database table so that it has an incorrect default. These commands work in Postgres, but different variants are needed for other database(s):

      – default where previous default was blank
      ALTER TABLE mdl_course_categories ALTER COLUMN name SET DEFAULT 'Kermit';
      – default where previously no default
      ALTER TABLE mdl_course_categories ALTER COLUMN idnumber SET DEFAULT '001';
      – no default where previously default
      ALTER TABLE mdl_course_categories ALTER COLUMN path DROP DEFAULT;
      – different default compared to previous default
      ALTER TABLE mdl_course_categories ALTER COLUMN visible SET DEFAULT 42;

      3 Purge all caches

      4 Rerun the check defaults

      EXPECTED: There should now be 4 incorrect defaults shown:

      • Table: course_categories. Field: name, Expected '' Actual 'Kermit'
      • Table: course_categories. Field: idnumber, Expected - Actual '001'
      • Table: course_categories. Field: visible, Expected '1' Actual '42'
      • Table: course_categories. Field: path, Expected '' Actual -

      NOTE: The SQL commands below are not complete ('path' is not included), I guess this is a separate bug but I didn't want to change this bit because the logic used to generate those is also used for actually doing things not just this check script!

       

      Show
      1 On a fresh Moodle install (for example one generated for Behat testing), go to admin / development / xmldb and run the 'Check defaults' script EXPECTED: There should be no incorrect defaults 2 Make changes to a database table so that it has an incorrect default. These commands work in Postgres, but different variants are needed for other database(s): – default where previous default was blank ALTER TABLE mdl_course_categories ALTER COLUMN name SET DEFAULT 'Kermit'; – default where previously no default ALTER TABLE mdl_course_categories ALTER COLUMN idnumber SET DEFAULT '001'; – no default where previously default ALTER TABLE mdl_course_categories ALTER COLUMN path DROP DEFAULT; – different default compared to previous default ALTER TABLE mdl_course_categories ALTER COLUMN visible SET DEFAULT 42; 3 Purge all caches 4 Rerun the check defaults EXPECTED: There should now be 4 incorrect defaults shown: Table: course_categories. Field: name, Expected '' Actual 'Kermit' Table: course_categories. Field: idnumber, Expected - Actual '001' Table: course_categories. Field: visible, Expected '1' Actual '42' Table: course_categories. Field: path, Expected '' Actual - NOTE: The SQL commands below are not complete ('path' is not included), I guess this is a separate bug but I didn't want to change this bit because the logic used to generate those is also used for actually doing things not just this check script!  

    Description

      When a NOT NULL char field is specified in XMLDB as not having a default (there is no DEFAULT in the xml), it actually is set to have default empty string, at least in Postgres. See MDL-6218.

      The 'Check defaults' button in xmldb does not detect inconsistencies that arise as a result. For example, it is possible that a new installation would create the field (with default '') but an upgrade would create it with no default. Because check defaults treats 'no default' an 'empty default' interchangeably, it does not spot these differences.

      I propose enhancing the check defaults feature so that it includes this detail in the check. This would help developers like me spot these problems, as we do use 'check defaults' as a step in our development checklist.

      (I don't have a lot of time to work on this but hopefully it might be good enough... )

      Edited to add : When a NOT NULL char ...

       

       

       

      Attachments

        Issue Links

          Activity

            People

              quen Sam Marshall
              quen Sam Marshall
              Andrew Lyons Andrew Lyons
              Dan Poltawski Dan Poltawski
              Adrian Greeve Adrian Greeve
              David Woloszyn, Huong Nguyen, Jake Dallimore, Meirza, Michael Hawkins, Raquel Ortega, Safat Shahin, Stevani Andolo
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:
                13/Nov/17