Moodle
  1. Moodle
  2. MDL-35961

Strange SQL in mod/data/view.php table data_content is joined twice in the main SQL requests

    Details

    • Database:
      MySQL
    • Testing Instructions:
      Hide

      Warning! This needs to be tested on all databases.

      1. Create a database activity.
      2. Add fields and save the template, or alternatively use the preset and import some entries.
        • (Please note with the csv file that it contains 500 entries. This may take some time to import. Feel free to remove records to make this process faster)
      3. Do a few simple searches on the database.
        Check to make sure that there are no errors displayed.
        Ideally you would check to make sure that the right number of entries are returned.
      Show
      Warning! This needs to be tested on all databases. Create a database activity. Add fields and save the template, or alternatively use the preset and import some entries. (Please note with the csv file that it contains 500 entries. This may take some time to import. Feel free to remove records to make this process faster) Do a few simple searches on the database. Check to make sure that there are no errors displayed. Ideally you would check to make sure that the right number of entries are returned.
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-35961-master
    • Rank:
      44720

      Description

      Back in Moodle 1.9.x, we noticed that the list display of a database with several thousand of entries was unbearably slow (more than one minute)

      We noticed that in the two main SQL queries, the table data_content was joined TWICE as below :

      $tables = '

      {data_content} c,{data_records} r, {data_content}

      cs,

      {user}

      u ';

      We fixed it by removing the second alias cs in two places ...and got back to less than one second of processing

      By peeking to code of the latest Moodle 2.3.2, we noticed that the table data_content is still joined twice, which looks quite surprising ? We do not have yet huge database activities in Moodle 2.x but it seems that somebody should have a look to these queries.

      I am attaching the patch I have applied to mod/data/view.php for Moodle 2.3.2

      Cheers

        Issue Links

          Activity

          Hide
          Patrick Pollet added a comment -

          git diff for Moodle 2.3.2

          Show
          Patrick Pollet added a comment - git diff for Moodle 2.3.2
          Hide
          Adrian Greeve added a comment -

          I've checked out the code and I can verify that the sql statement has a double up of data_content and a join that goes with it. I've looked at the patch and tested it on mysql, mssql, and postgres.
          I'll write up some testing instructions and then submit it for peer review.

          Show
          Adrian Greeve added a comment - I've checked out the code and I can verify that the sql statement has a double up of data_content and a join that goes with it. I've looked at the patch and tested it on mysql, mssql, and postgres. I'll write up some testing instructions and then submit it for peer review.
          Hide
          Frédéric Massart added a comment -

          The patch looks good Adrian. Feel free to push it forward. Cheers!

          Show
          Frédéric Massart added a comment - The patch looks good Adrian. Feel free to push it forward. Cheers!
          Hide
          Adrian Greeve added a comment -

          To the integrators: Master only please. The double up of the alias and join doesn't really cause any problems as far as I can tell. This is really just an improvement of the sql query.

          Show
          Adrian Greeve added a comment - To the integrators: Master only please. The double up of the alias and join doesn't really cause any problems as far as I can tell. This is really just an improvement of the sql query.
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Adrian Greeve added a comment -

          Rebased and ready to go.

          Show
          Adrian Greeve added a comment - Rebased and ready to go.
          Hide
          Dan Poltawski added a comment -

          Thanks Adrian and Patrick, i've integrated this to 2.4 only.

          Theoretically this could be back-ported, but I think its best to be cautious with this.

          Show
          Dan Poltawski added a comment - Thanks Adrian and Patrick, i've integrated this to 2.4 only. Theoretically this could be back-ported, but I think its best to be cautious with this.
          Hide
          Michael de Raadt added a comment -

          Test result: Success!

          Tested in master on MS SQL, MySQL, Oracle and PostgreSQL.

          Show
          Michael de Raadt added a comment - Test result: Success! Tested in master on MS SQL, MySQL, Oracle and PostgreSQL.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Amazed. Inspired. Grateful. That’s how your generosity makes me feel.

          (not really)

          Closing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Amazed. Inspired. Grateful. That’s how your generosity makes me feel. (not really) Closing, thanks!
          Hide
          Howard Miller added a comment -

          Also a problem in 2.2. It appears to cause a big performance issue, so may I suggest that backporting this is a priority!

          Show
          Howard Miller added a comment - Also a problem in 2.2. It appears to cause a big performance issue, so may I suggest that backporting this is a priority!
          Hide
          Davo Smith added a comment -

          Adding an extra request to backport this to 2.2 as I've got massive performance issues with a site as a result of this problem. When no search is being performed MySQL copies the entire data_content table to a temporary table and then ignores it when producing the results. Removing the extra 'cs' table reduces the query execution time from several minutes to a fraction of a second.

          Show
          Davo Smith added a comment - Adding an extra request to backport this to 2.2 as I've got massive performance issues with a site as a result of this problem. When no search is being performed MySQL copies the entire data_content table to a temporary table and then ignores it when producing the results. Removing the extra 'cs' table reduces the query execution time from several minutes to a fraction of a second.
          Hide
          Dan Poltawski added a comment -

          Hi Davo,

          This was done in MDL-36668

          Show
          Dan Poltawski added a comment - Hi Davo, This was done in MDL-36668

            People

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

              Dates

              • Created:
                Updated:
                Resolved: