Moodle

Entering an event tag from the block displays duplicate value error

Details

  • Type: Bug Bug
  • Status: Closed 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:

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

discovered while testing

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-39800 Course tags are being incorrectly weighted

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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

Vote (0)
Watch (5)

Dates

  • Created:
    Updated:
    Resolved: