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

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

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.8.5, 3.9.2, 3.10, 4.0
    • Fix Version/s: 3.8.6, 3.9.3
    • Component/s: Database SQL/XMLDB
    • Labels:
    • Testing Instructions:
      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.
    • Affected Branches:
      MOODLE_310_STABLE, MOODLE_38_STABLE, MOODLE_39_STABLE, MOODLE_400_STABLE
    • Fixed Branches:
      MOODLE_38_STABLE, MOODLE_39_STABLE
    • Pull 3.8 Branch:
    • Pull 3.9 Branch:
    • Pull 3.10 Branch:
      MDL-69973_310
    • Pull Master Branch:

      Description

      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 Matthew Davidson and Ryan Smith for detecting the problems and reporting them.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              stronk7 Eloy Lafuente (stronk7)
              Reporter:
              stronk7 Eloy Lafuente (stronk7)
              Peer reviewer:
              Carlos Escobedo
              Integrator:
              Sara Arjona (@sarjona)
              Tester:
              Anna Carissa Sadia
              Participants:
              Component watchers:
              Andrew Nicols, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Fix Release Date:
                9/Nov/20

                  Time Tracking

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