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

SQL Coding Style - Indentation

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Open
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.9
    • Fix Version/s: None
    • Component/s: Policy
    • Labels:
    • Affected Branches:
      MOODLE_29_STABLE

      Description

      We have an indentation style for SQL (who knew!?). The current style is at https://docs.moodle.org/dev/SQL_coding_style and has existed since 2013.

      Personally, I disagree with the current implementation which simply shows an image with a red line at the gap between FROM and the table name.

      $sql = "SELECT a.id, a.value, a.another, b.id AS bid,
                     c.id AS cid, c.value AS yetanother
                FROM {tablea} a
                JOIN {tableb} b ON b.refa = a.id
           LEFT JOIN {tablec} c ON c.refc = b.id
               WHERE a.value = :avalue
                     AND b.value = :bvalue
                     AND c.value <> :cvalue";
      

      I would propose something where:

      1. all keywords from the same context are left-aligned together;
      2. anything under that context are indented by the usual four spaces;
      3. where there are multiple fields in the select, they are split onto separate lines in a sensible grouping;
      4. where there are multiple conditions in the WHERE (or the ON, HAVING, GROUP BY, ORDER BY, etc.) they are grouped accordingly and clearly; and
      5. where there is a join condition (e.g. ON) which has been broken onto multiple lines, it should be indented an additional 4 characters.

      $sql = "
      SELECT
          a.id, a.value, a.another,
          b.id AS bid,
          c.id AS cid, c.value AS yetanother
      FROM {tablea} a
      JOIN {tableb} ON b.refa = a.id
      LEFT JOIN {tablec} ON c.refc = b.id
      WHERE
          a.value = :avalue
      AND
          b.value = :bvalue
      AND
          c.value <> :cvalue
      ";
      

      In addition:

      1. where there are subqueries, these should start on a new line and follow the rest of the rules as standard.

      In the following, more complex, example:

      SELECT
          $uniquefield,
          otheruser.id, otheruser.username,
          message.id as mid, message.notification,
          contact.id as contactlistid, contact.blocked
      FROM {message_read} message
      JOIN (
          SELECT
              MAX(id) AS messageid,
              matchedmessage.useridto,
              matchedmessage.useridfrom
          FROM {message_read} matchedmessage
          INNER JOIN (
              SELECT
                  MAX(recentmessages.timecreated) timecreated,
                  recentmessages.useridfrom,
                  recentmessages.useridto
              FROM {message_read} recentmessages
              WHERE (recentmessages.useridfrom = 4 OR recentmessages.useridto = 4)
              GROUP BY recentmessages.useridfrom, recentmessages.useridto
          ) recent
              ON
                  matchedmessage.useridto     = recent.useridto
              AND
                  matchedmessage.useridfrom   = recent.useridfrom
              AND
                  matchedmessage.timecreated  = recent.timecreated
          GROUP BY
              matchedmessage.useridto, matchedmessage.useridfrom
      ) messagesubset ON messagesubset.messageid = message.id
      JOIN {user} otheruser
          ON
              (message.useridfrom = 4 AND message.useridto     = otheruser.id)
          OR
              (message.useridto   = 4 AND message.useridfrom   = otheruser.id)
      LEFT JOIN {message_contacts} contact ON contact.userid  = 4 AND contact.userid = otheruser.id
      WHERE
          otheruser.deleted = 0
      AND
          message.notification = 0
      ORDER BY message.timecreated DESC
      

      I'd also propose that we recommend the use of the PHP HEREDOC:

          $sql = <<<EOF
      SELECT
          $uniquefield,
          otheruser.id, otheruser.username,
          message.id as mid, message.notification,
          contact.id as contactlistid, contact.blocked
      FROM {message_read} message
      JOIN (
          SELECT
              MAX(id) AS messageid,
              matchedmessage.useridto,
              matchedmessage.useridfrom
          FROM {message_read} matchedmessage
          INNER JOIN (
              SELECT
                  MAX(recentmessages.timecreated) timecreated,
                  recentmessages.useridfrom,
                  recentmessages.useridto
              FROM {message_read} recentmessages
              WHERE (recentmessages.useridfrom = 4 OR recentmessages.useridto = 4)
              GROUP BY recentmessages.useridfrom, recentmessages.useridto
          ) recent
              ON
                  matchedmessage.useridto     = recent.useridto
              AND
                  matchedmessage.useridfrom   = recent.useridfrom
              AND
                  matchedmessage.timecreated  = recent.timecreated
          GROUP BY
              matchedmessage.useridto, matchedmessage.useridfrom
      ) messagesubset ON messagesubset.messageid = message.id
      JOIN {user} otheruser
          ON
              (message.useridfrom = 4 AND message.useridto     = otheruser.id)
          OR
              (message.useridto   = 4 AND message.useridfrom   = otheruser.id)
      LEFT JOIN {message_contacts} contact ON contact.userid  = 4 AND contact.userid = otheruser.id
      WHERE
          otheruser.deleted = 0
      AND
          message.notification = 0
      ORDER BY message.timecreated DESC
      EOF;
      

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              Unassigned
              Reporter:
              dobedobedoh Andrew Nicols
              Participants:
              Component watchers:
              Adrian Greeve, Andrew Nicols, Eloy Lafuente (stronk7), Juan Leyva, Jun Pataleta, Sander Bangma
              Votes:
              1 Vote for this issue
              Watchers:
              10 Start watching this issue

                Dates

                Created:
                Updated: