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

mariadb returns wrong defaults from RDBMS metadata (plus caching issue)

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Major Major
    • 3.8.6, 3.9.3
    • 3.8.5, 3.9.2, 3.10, 4.0
    • Database SQL/XMLDB
    • MOODLE_310_STABLE, MOODLE_38_STABLE, MOODLE_39_STABLE, MOODLE_400_STABLE
    • MOODLE_38_STABLE, MOODLE_39_STABLE
    • Hide

      Note:

      • This needs to be verified in all branches to confirm that there aren't problems with any of them.

      Requirements:

      • mariadb version >= 10.2.7
      • ability to issue ALTER TABLE statements (via SQL OR any UI/Web tool).

      1) Defaults are now correctly fetched from DB.

      1. Install Moodle for the branch being tested in an MariaDB database (>= 10.2.7).
      2. Go to Admin -> Development -> XMLDB Editor
      3. Click on the "Check defaults" link that is on the top of the table. The press "Yes" in the confirmation screen. The report will run.
      4. Verify that the report shows "Wrong defaults found: 0" and "No inconsistent default values have been found, your DB does not need further actions."
      5. Using any sql client or tool, let's change some mdl_assign columns defaults:
        1. mdl_assign->course (set default to null)
          ALTER TABLE mdl_assign ALTER COLUMN course DROP DEFAULT
        2. mdl_assign->introformat (set default to 10)
          ALTER TABLE mdl_assign ALTER COLUMN introformat SET DEFAULT 10
      6. Run the "Check defaults" report again.
      7. Verify that the report shows "Wrong defaults found: 2" and "Some inconsistent defaults have been found in your DB".
      8. Verify that the list of errors is:
        1. Table: assign. Field: course, Expected '0' Actual -
        2. Table: assign. Field: introformat, Expected '0' Actual '10'
      9. Verify that 2 ALTER TABLE statements are suggested to fix the problems.
      10. Stay in (don't close, neither navigate to other page) that report page and copy the 2 ALTER TABLE statements.

      2) Reports always fetch fresh (non-cached) metadata from DB.

      1. Using any sql client or tool execute the 2 ALTER TABLE statements that you've just copied at the end of the previous test.
      2. Go to the report page (browser) that was left open and is showing the 2 errors from the end of the previous test.
      3. Hit "Refresh" (usually F5, maybe Command-R...) in your browser.
      4. Verify that the report shows "Wrong defaults found: 0" and "No inconsistent default values have been found, your DB does not need further actions."

      3) Verify that the new tests (now covering null defaults) work ok on all DBs

      1. Nothing to do here, beloved tester. CI will tell us.
      Show
      Note: This needs to be verified in all branches to confirm that there aren't problems with any of them. Requirements: mariadb version >= 10.2.7 ability to issue ALTER TABLE statements (via SQL OR any UI/Web tool). 1) Defaults are now correctly fetched from DB. Install Moodle for the branch being tested in an MariaDB database (>= 10.2.7). Go to Admin -> Development -> XMLDB Editor Click on the "Check defaults" link that is on the top of the table. The press "Yes" in the confirmation screen. The report will run. Verify that the report shows "Wrong defaults found: 0" and "No inconsistent default values have been found, your DB does not need further actions." Using any sql client or tool, let's change some mdl_assign columns defaults: mdl_assign->course (set default to null) ALTER TABLE mdl_assign ALTER COLUMN course DROP DEFAULT mdl_assign->introformat (set default to 10) ALTER TABLE mdl_assign ALTER COLUMN introformat SET DEFAULT 10 Run the "Check defaults" report again. Verify that the report shows "Wrong defaults found: 2" and "Some inconsistent defaults have been found in your DB". Verify that the list of errors is: Table: assign. Field: course, Expected '0' Actual - Table: assign. Field: introformat, Expected '0' Actual '10' Verify that 2 ALTER TABLE statements are suggested to fix the problems. Stay in (don't close, neither navigate to other page) that report page and copy the 2 ALTER TABLE statements. 2) Reports always fetch fresh (non-cached) metadata from DB. Using any sql client or tool execute the 2 ALTER TABLE statements that you've just copied at the end of the previous test. Go to the report page (browser) that was left open and is showing the 2 errors from the end of the previous test. Hit "Refresh" (usually F5, maybe Command-R...) in your browser. Verify that the report shows "Wrong defaults found: 0" and "No inconsistent default values have been found, your DB does not need further actions." 3) Verify that the new tests (now covering null defaults) work ok on all DBs Nothing to do here, beloved tester. CI will tell us.

      There are 2 issues to fix here (both were detected @ MDL-63252):

      A) mariadb driver is fetching wrong defaults from database metadata

      Has been confirmed that, for any mariadb version >= 10.2.7, columns having no default (aka, default = null), are interpreted (by metadata utilities) like having a default of '' (empty string).

      This is easily reproducible by the xmldb_editor check defaults reports and may impact also upgrade scripts changing defaults here and there.

      Tracing down the problem, it comes from MDL-59583, where the handling of defaults was changed, because of a change in mariadb 10.2.7 and up.

      Also, itt seems that the case for null defaults (aka, no default), escaped all the checks. It seems that, in lib/dml/tests/dml_test.php, the test_get_columns test is missing to test nulls.

      So this point is about to:

      1. Fix the metadata handling to correctly detect nulls as no-default.
      2. Complete existing tests to cover the fix.

      B) xmldb reports (check indexes, check defaults, check bigints...) show old metadata info

      All those reports, extending XMLDBCheckAction, use $DB->get_columns() to fetch metadata. Problem is that the information provided by that function is cached by MUC so, if anybody execute any ALTER statement to fix a default, an int... on next execution the report will continue showing the same information, leading to confusion.

      For sure, that will make the reports slightly slower, but that's not really important at all, accuracy must win.

      So this point is about to:

      1. Ensure that the reports always fetch fresh (non-cached) information from database.

      And that's all, ciao

      And thanks to syxton and smithrn for detecting the problems and reporting them.

        1. MDL-69973.jpg
          MDL-69973.jpg
          57 kB

            stronk7 Eloy Lafuente (stronk7)
            stronk7 Eloy Lafuente (stronk7)
            Carlos Escobedo Carlos Escobedo
            Sara Arjona (@sarjona) Sara Arjona (@sarjona)
            Anna Carissa Sadia Anna Carissa Sadia
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved:

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 7 hours, 5 minutes
                7h 5m

                  Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.