Moodle
  1. Moodle
  2. MDL-29765

Review all uses of DB->sql_empty() and take rid of the unnecessary ones

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Duplicate
    • Affects Version/s: 2.0.5, 2.1.2, 2.2
    • Fix Version/s: None
    • Component/s: Database SQL/XMLDB
    • Labels:
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Rank:
      19281

      Description

      Before 2.x we needed to use the $DB->sql_empty() helper method in a lot of places (basically each time that one '' (empty string) was searched/used.

      But under 2.x a lot of those uses are not necessary anymore when the '' is used in any param (each moodle_database driver will handle it as necessary as part of the binding process).

      This reduces the use of the helper function exclusively to the places where '' is used within one SQL statement. The rest (params in general) should be changed to ''.

      This issue is about to perform that cleanup and test all the changes under Oracle (the only one needing the special handling of '') and another DB (mysql|postgres|mssql) to verify that everything continues working ok.

      Surely master only. Ciao

        Issue Links

          Activity

          Hide
          Eloy Lafuente (stronk7) added a comment -

          See http://tracker.moodle.org/browse/MDL-30761?focusedCommentId=134661&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-134661

          Under Oracle we are applying the dirty hack only to insert/update/set_field() params, where $tablename is well known.

          So this issue should also include the task of deciding if we go to any of the 2a) or 2b) routes there, aka:

          a) extend support to all the 1-table get_recordxxxx() operations (this will leave out get_recordxxxx_sql() ones, but seems easier and safer, although incomplete.
          b) support it everywhere, so each time we get one === '' in param, it's converted always to 1-whitespace, without introspecting into metadata at all, I think this is more risky, but if covered properly by tests and proved to work, can be done too.

          And of course, there is option c) => Leave things as they are now (only applying the dirty-hack automatically on insert/update/set_field).

          Once one of the 3 alternatives above is picked (and implemented with tests) forever, we can review current uses.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - See http://tracker.moodle.org/browse/MDL-30761?focusedCommentId=134661&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-134661 Under Oracle we are applying the dirty hack only to insert/update/set_field() params, where $tablename is well known. So this issue should also include the task of deciding if we go to any of the 2a) or 2b) routes there, aka: a) extend support to all the 1-table get_recordxxxx() operations (this will leave out get_recordxxxx_sql() ones, but seems easier and safer, although incomplete. b) support it everywhere, so each time we get one === '' in param, it's converted always to 1-whitespace, without introspecting into metadata at all, I think this is more risky, but if covered properly by tests and proved to work, can be done too. And of course, there is option c) => Leave things as they are now (only applying the dirty-hack automatically on insert/update/set_field). Once one of the 3 alternatives above is picked (and implemented with tests) forever, we can review current uses. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Closing as duplicate. Active work is happening @ MDL-37742.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Closing as duplicate. Active work is happening @ MDL-37742 . Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: