Moodle
  1. Moodle
  2. MDL-41339

Entering an event tag from the block displays duplicate value error

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.4.7, 2.5.3, 2.6
    • Component/s: Tags
    • Labels:
    • Rank:
      52324

      Description

      If you enable tags for courses, add the tags block, and then enter in the same tag with different users, you get the following error:

      Did you remember to make the first column something unique in your call to get_records? Duplicate value '4' found in column 'id'.

      line 771 of /lib/dml/pgsql_native_moodle_database.php: call to debugging()
      line 336 of /tag/lib.php: call to pgsql_native_moodle_database->get_records_sql()
      line 405 of /tag/lib.php: call to tag_get_tags()
      line 291 of /tag/coursetagslib.php: call to tag_get_tags_ids()
      line 51 of /tag/coursetags_add.php: call to coursetag_store_keywords()

        Issue Links

          Activity

          Hide
          Jenny Gray added a comment -

          This error occurs from the tag_get_tags() function who's SQL returns tg.id, tg.tagtype, tg.name, tg.rawname, tg.flag, ti.ordering where tg is the tag table and ti is tag_instance.

          It is possible for there to be multiple entries in ti with the same data but for different user accounts - as Adrian noted, this is the correct data when more than one person uses the same tag term for the same thing (course/blog post/whatever).

          The error is thrown because the first column requested contains the tag id, which is rightly not unique.

          The solution is, I think, to add ti.id to be the first column returned. It won't be used by any of the myriad functions which call tag_get_tags but it won't get in their way either.

          I considered alternatives of stripping out ti.ordering and making the result set distinct on tag id but tag_get_tag_ids uses ordering, so this doesn't seem appropriate. None of the current columns contain unique values and so the ordering of the returned columns cannot be altered to fix the bug.

          Show
          Jenny Gray added a comment - This error occurs from the tag_get_tags() function who's SQL returns tg.id, tg.tagtype, tg.name, tg.rawname, tg.flag, ti.ordering where tg is the tag table and ti is tag_instance. It is possible for there to be multiple entries in ti with the same data but for different user accounts - as Adrian noted, this is the correct data when more than one person uses the same tag term for the same thing (course/blog post/whatever). The error is thrown because the first column requested contains the tag id, which is rightly not unique. The solution is, I think, to add ti.id to be the first column returned. It won't be used by any of the myriad functions which call tag_get_tags but it won't get in their way either. I considered alternatives of stripping out ti.ordering and making the result set distinct on tag id but tag_get_tag_ids uses ordering, so this doesn't seem appropriate. None of the current columns contain unique values and so the ordering of the returned columns cannot be altered to fix the bug.
          Hide
          Adrian Greeve added a comment -

          Hello Jenny,

          I like your solution. Just a couple of things before this heads to integration.

          Could you please add some testing instructions, and also add branches for 2.5 and 2.4?

          Thank you.

          Show
          Adrian Greeve added a comment - Hello Jenny, I like your solution. Just a couple of things before this heads to integration. Could you please add some testing instructions, and also add branches for 2.5 and 2.4? Thank you.
          Hide
          Jenny Gray added a comment -

          Rebased to current master. Created 2.4 and 2.5 branches and added testing instructions.

          Show
          Jenny Gray added a comment - Rebased to current master. Created 2.4 and 2.5 branches and added testing instructions.
          Hide
          Jenny Gray added a comment -

          Ready for integration I think, if you're happy Adrian?

          Show
          Jenny Gray added a comment - Ready for integration I think, if you're happy Adrian?
          Hide
          Adrian Greeve added a comment -

          Yes, of course. I think this is ready.

          Show
          Adrian Greeve added a comment - Yes, of course. I think this is ready.
          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
          Damyon Wiese added a comment -

          Thanks Jenny. I have integrated this to 24, 25 and master.

          Note: I added one extra commit to change the SQL to use 'AS' instead of as.

          See point 2 in the general section here:

          http://docs.moodle.org/dev/SQL_coding_style

          Show
          Damyon Wiese added a comment - Thanks Jenny. I have integrated this to 24, 25 and master. Note: I added one extra commit to change the SQL to use 'AS' instead of as. See point 2 in the general section here: http://docs.moodle.org/dev/SQL_coding_style
          Hide
          David Monllaó added a comment -

          It passes; I can add again the same tag without any warning. Tested in 24, 25 and master.

          Show
          David Monllaó added a comment - It passes; I can add again the same tag without any warning. Tested in 24, 25 and master.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          "Aequam memento rebus in arduis servare mentem"

          Many thanks for your hard work, this is now part of "Moodle, the LMS". Closing!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - "Aequam memento rebus in arduis servare mentem" Many thanks for your hard work, this is now part of "Moodle, the LMS". Closing! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: