Details

    • Type: Sub-task Sub-task
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 1.8
    • Fix Version/s: 2.0.10
    • Component/s: Database SQL/XMLDB
    • Labels:
      None
    • Environment:
      Tested on current MOODLE_18_STABLE, Postgres 8.1.4
    • Database:
      PostgreSQL
    • Affected Branches:
      MOODLE_18_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      397

      Description

      The following code is suggested by the xmldb editor to change a field type:

      /// Changing sign of field id on table ouwiki_links to unsigned
      $table = new XMLDBTable('ouwiki_links');
      $field = new XMLDBField('id');
      $field->setAttributes(XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, XMLDB_NOTNULL, XMLDB_SEQUENCE, null, null, null, null);

      /// Launch change of sign for field id
      $result = $result && change_field_unsigned($table, $field);

      When you do this on Postgres, keys related to that field (in this case the primary key) are lost. This is an unexpected side-effect.

      Here is another example that exhibits the same problem:

      /// Changing nullability of field userid on table ouwiki_comments to null
      $table = new XMLDBTable('ouwiki_comments');
      $field = new XMLDBField('userid');
      $field->setAttributes(XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, null, null, null, null, null, 'xhtml');

      /// Launch change of nullability for field userid
      $result = $result && change_field_notnull($table, $field);

      In this case, a foreign key (and its corresponding index) was lost.

      There are two possible answers to this:

      1) (Ideally) This is not expected behaviour, the keys/constraints should be preserved [in other words this should generate an ALTER TABLE type command rather than dropping the whole field and making a new one].

      OR

      2) This is expected behaviour because the keys need to be marked on the field before calling the function.

      In the case of #2, the code necessary to set up the keys so that they are retained should be included when you ask the xmldb editor to generate the php code for you. Also the fact that all keys etc. will be lost unless you specifically add them should be mentioned in the function phpdoc comments.

        Issue Links

          Activity

          Hide
          Eloy Lafuente (stronk7) added a comment -

          aha, this is due, if I'm not wrong, to the need to create a new field under PG (old versions) because it didn't support some changes.

          Anyway, with Moodle 2.0, we are raising min. DB reqs. so I'm going to run a complete battery test to see if all operations are supported without needing the "tmp field" approach. If we are "tmp field"-free, then I'll change code in generators to be free and keys/indexes won't be affected anymore.

          Alternatively, if we still need the "tmp field" approach for any operation in any DB... then I'll add the corresponding create/drop index/key php code as necessary (perhaps within some if (dbfamily == xx) to keep the non-affected DB working in an optimal way.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - aha, this is due, if I'm not wrong, to the need to create a new field under PG (old versions) because it didn't support some changes. Anyway, with Moodle 2.0, we are raising min. DB reqs. so I'm going to run a complete battery test to see if all operations are supported without needing the "tmp field" approach. If we are "tmp field"-free, then I'll change code in generators to be free and keys/indexes won't be affected anymore. Alternatively, if we still need the "tmp field" approach for any operation in any DB... then I'll add the corresponding create/drop index/key php code as necessary (perhaps within some if (dbfamily == xx) to keep the non-affected DB working in an optimal way. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue.

          For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

          Show
          Eloy Lafuente (stronk7) added a comment - This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue. For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

            People

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

              Dates

              • Created:
                Updated: