Moodle
  1. Moodle
  2. MDL-31806

Course tags block causes errors on oracle

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2, 2.3
    • Fix Version/s: 2.1.5, 2.2.2
    • Component/s: Tags
    • Labels:
    • Environment:
      Linux
    • Database:
      Oracle
    • Testing Instructions:
      Hide

      Steps to reproduce:
      1) Go to Admin > Plugins > Blocks > Tags and enable 'show course tags'
      2) Navigate to course home page
      4) Add tag block to course
      5) Refresh the page

      Expected result:
      Tag block displayed and no errors

      Actual result:
      DB Error ORA-00918 - column ambigiously defined

      Show
      Steps to reproduce: 1) Go to Admin > Plugins > Blocks > Tags and enable 'show course tags' 2) Navigate to course home page 4) Add tag block to course 5) Refresh the page Expected result: Tag block displayed and no errors Actual result: DB Error ORA-00918 - column ambigiously defined
    • Workaround:
      Hide

      Issue is to be in the following query

      File => /tag/coursetaglib.php
      Function => coursetag_get_all_tags()

      Query=>

      SELECT id, name, id, tagtype, rawname, f.timemodified, flag, count
      FROM e_tag t,
      (SELECT tagid, MAX(timemodified) as timemodified, COUNT(id) as count
      FROM e_tag_instance WHERE tagid NOT IN
      (SELECT tagid FROM e_tag_instance ti, e_course c
      WHERE c.visible = 0
      AND ti.itemtype = 'course'
      AND ti.itemid = c.id)
      GROUP BY tagid) f
      WHERE t.id = f.tagid
      ORDER BY count DESC, name ASC

      Oracle does not allow same name in select query (SELECT id, name, id) so place an alias for one of the id say

      SELECT id as t_id, name, id, tagtype, rawname, f.timemodified, flag, count
      FROM e_tag t,
      (SELECT tagid, MAX(timemodified) as timemodified, COUNT(id) as count
      FROM e_tag_instance WHERE tagid NOT IN
      (SELECT tagid FROM e_tag_instance ti, e_course c
      WHERE c.visible = 0
      AND ti.itemtype = 'course'
      AND ti.itemid = c.id)
      GROUP BY tagid) f
      WHERE t.id = f.tagid
      ORDER BY count DESC, name ASC

      Show
      Issue is to be in the following query File => /tag/coursetaglib.php Function => coursetag_get_all_tags() Query=> SELECT id, name, id, tagtype, rawname, f.timemodified, flag, count FROM e_tag t, (SELECT tagid, MAX(timemodified) as timemodified, COUNT(id) as count FROM e_tag_instance WHERE tagid NOT IN (SELECT tagid FROM e_tag_instance ti, e_course c WHERE c.visible = 0 AND ti.itemtype = 'course' AND ti.itemid = c.id) GROUP BY tagid) f WHERE t.id = f.tagid ORDER BY count DESC, name ASC Oracle does not allow same name in select query (SELECT id, name, id) so place an alias for one of the id say SELECT id as t_id, name, id, tagtype, rawname, f.timemodified, flag, count FROM e_tag t, (SELECT tagid, MAX(timemodified) as timemodified, COUNT(id) as count FROM e_tag_instance WHERE tagid NOT IN (SELECT tagid FROM e_tag_instance ti, e_course c WHERE c.visible = 0 AND ti.itemtype = 'course' AND ti.itemid = c.id) GROUP BY tagid) f WHERE t.id = f.tagid ORDER BY count DESC, name ASC
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-31806-master
    • Rank:
      38428

      Description

      When a teacher tries to click "Enrolled users" link under course administration then the page displays "Error reading from database" if a tag block is added with course tags enabled.

      • line 125 of /tag/coursetagslib.php: call to oci_native_moodle_database->get_records_sql()
      • line 115 of /blocks/tags/block_tags.php: call to coursetag_get_all_tags()
      • line 280 of /blocks/moodleblock.class.php: call to block_tags->get_content()
      • line 232 of /blocks/moodleblock.class.php: call to block_base->formatted_contents()
      • line 926 of /lib/blocklib.php: call to block_base->get_content_for_output()
      • line 978 of /lib/blocklib.php: call to block_manager->create_block_contents()
      • line 349 of /lib/blocklib.php: call to block_manager->ensure_content_created()

      SELECT id, name, id, tagtype, rawname, f.timemodified, flag, count
      FROM e_tag t,
      (SELECT tagid, MAX(timemodified) as timemodified, COUNT(id) as count
      FROM e_tag_instance WHERE tagid NOT IN
      (SELECT tagid FROM e_tag_instance ti, e_course c
      WHERE c.visible = 0
      AND ti.itemtype = 'course'
      AND ti.itemid = c.id)
      GROUP BY tagid) f
      WHERE t.id = f.tagid
      ORDER BY count DESC, name ASC

        Activity

        Hide
        Charles Fulton added a comment -

        I don't have access to an Oracle DB so I can't test this fix, but removing the second id from the query string should fix it and not have any negative downstream effects.

        Show
        Charles Fulton added a comment - I don't have access to an Oracle DB so I can't test this fix, but removing the second id from the query string should fix it and not have any negative downstream effects.
        Hide
        Dan Poltawski added a comment -

        Looks good. I also verified this problem and the fix on oracle.

        Show
        Dan Poltawski added a comment - Looks good. I also verified this problem and the fix on oracle.
        Hide
        Dan Poltawski added a comment -

        Charles: could you create branches for MOODLE_21_STABLE and MOODLE_22_STABLE for this fix?

        It'd also be great if you could do the commit message in our 'house style' mentioning component:
        MDL-31806 tags: remove second occurrence of id in query string

        Then i'll get this submitted for integration - thanks!

        Show
        Dan Poltawski added a comment - Charles: could you create branches for MOODLE_21_STABLE and MOODLE_22_STABLE for this fix? It'd also be great if you could do the commit message in our 'house style' mentioning component: MDL-31806 tags: remove second occurrence of id in query string Then i'll get this submitted for integration - thanks!
        Hide
        Charles Fulton added a comment -

        Amended original branch and created additional branches. Thanks for the quick review!

        Show
        Charles Fulton added a comment - Amended original branch and created additional branches. Thanks for the quick review!
        Hide
        Dan Poltawski added a comment -

        Thanks for the patch!

        Show
        Dan Poltawski added a comment - Thanks for the patch!
        Hide
        Eloy Lafuente (stronk7) 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
        Eloy Lafuente (stronk7) 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
        Sam Hemelryk added a comment -

        Thanks Charles this has been integrated now

        Show
        Sam Hemelryk added a comment - Thanks Charles this has been integrated now
        Hide
        Rajesh Taneja added a comment -

        No problem found on mysql, will request Dan to test this on oracle.

        Thanks for working on this Charles.

        Show
        Rajesh Taneja added a comment - No problem found on mysql, will request Dan to test this on oracle. Thanks for working on this Charles.
        Hide
        Dan Poltawski added a comment -

        Passing test

        Show
        Dan Poltawski added a comment - Passing test
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!).

        icao_reverse('arreis olik rebemevon afla letoh ognat');
        

        Closing, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!). icao_reverse('arreis olik rebemevon afla letoh ognat'); Closing, ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: