Moodle
  1. Moodle
  2. MDL-34072

Site registration fails on Oracle

    Details

    • Database:
      Oracle
    • Testing Instructions:
      Hide

      While using a site running under Oracle
      Access the site registration page from the Settings block (Site Administration -> Registration)

      Show
      While using a site running under Oracle Access the site registration page from the Settings block (Site Administration -> Registration)
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Rank:
      42377

      Description

      Bad SQL in course/lib.php functions average_number_of_participants() and average_number_of_course_modules() will cause the site registration to fail on Oracle (error below).

      Suggest to remove the 'as' preceding subquery alias 'total'

      Without the 'as' it will run correctly on MySQL, PostgreSQL, and Oracle.

      Debug info: ORA-00933: SQL command not properly ended
      SELECT COUNT(*) FROM (
      SELECT DISTINCT ue.userid, e.courseid
      FROM p_user_enrolments ue, p_enrol e, p_course c
      WHERE ue.enrolid = e.id
      AND e.courseid <> :o_siteid
      AND c.id = e.courseid
      AND c.visible = 1) as total
      [array (
      'o_siteid' => '1',
      )]
      Error code: dmlreadexception
      Stack trace:
      
          line 407 of /lib/dml/moodle_database.php: dml_read_exception thrown
          line 284 of /lib/dml/oci_native_moodle_database.php: call to moodle_database->query_end()
          line 1111 of /lib/dml/oci_native_moodle_database.php: call to oci_native_moodle_database->query_end()
          line 1337 of /lib/dml/moodle_database.php: call to oci_native_moodle_database->get_records_sql()
          line 1045 of /lib/dml/oci_native_moodle_database.php: call to moodle_database->get_record_sql()
          line 1410 of /lib/dml/moodle_database.php: call to oci_native_moodle_database->get_record_sql()
          line 1581 of /lib/dml/moodle_database.php: call to moodle_database->get_field_sql()
          line 4038 of /course/lib.php: call to moodle_database->count_records_sql()
          line 373 of /admin/registration/forms.php: call to average_number_of_participants()
          line 173 of /lib/formslib.php: call to site_registration_form->definition()
          line 61 of /admin/registration/register.php: call to moodleform->moodleform()
      

        Issue Links

          Activity

          Fred Woolard created issue -
          Fred Woolard made changes -
          Field Original Value New Value
          Database Oracle [ 10011 ]
          Show
          Fred Woolard added a comment - https://github.com/appalachianstate/moodle/tree/MDL-34072
          Fred Woolard made changes -
          Hide
          Michael de Raadt added a comment -

          Thanks for spotting that and providing a solution.

          Show
          Michael de Raadt added a comment - Thanks for spotting that and providing a solution.
          Michael de Raadt made changes -
          Fix Version/s STABLE backlog [ 10463 ]
          Description Bad SQL in course/lib.php functions average_number_of_participants() and average_number_of_course_modules() will cause the site registration to fail on Oracle (error below).

          Suggest to remove the 'as' preceding subquery alias 'total'

          Without the 'as' it will run correctly on MySQL, PostgreSQL, and Oracle.


          Debug info: ORA-00933: SQL command not properly ended
          SELECT COUNT(*) FROM (
          SELECT DISTINCT ue.userid, e.courseid
          FROM p_user_enrolments ue, p_enrol e, p_course c
          WHERE ue.enrolid = e.id
          AND e.courseid <> :o_siteid
          AND c.id = e.courseid
          AND c.visible = 1) as total
          [array (
          'o_siteid' => '1',
          )]
          Error code: dmlreadexception
          Stack trace:

              line 407 of /lib/dml/moodle_database.php: dml_read_exception thrown
              line 284 of /lib/dml/oci_native_moodle_database.php: call to moodle_database->query_end()
              line 1111 of /lib/dml/oci_native_moodle_database.php: call to oci_native_moodle_database->query_end()
              line 1337 of /lib/dml/moodle_database.php: call to oci_native_moodle_database->get_records_sql()
              line 1045 of /lib/dml/oci_native_moodle_database.php: call to moodle_database->get_record_sql()
              line 1410 of /lib/dml/moodle_database.php: call to oci_native_moodle_database->get_record_sql()
              line 1581 of /lib/dml/moodle_database.php: call to moodle_database->get_field_sql()
              line 4038 of /course/lib.php: call to moodle_database->count_records_sql()
              line 373 of /admin/registration/forms.php: call to average_number_of_participants()
              line 173 of /lib/formslib.php: call to site_registration_form->definition()
              line 61 of /admin/registration/register.php: call to moodleform->moodleform()

          Bad SQL in course/lib.php functions average_number_of_participants() and average_number_of_course_modules() will cause the site registration to fail on Oracle (error below).

          Suggest to remove the 'as' preceding subquery alias 'total'

          Without the 'as' it will run correctly on MySQL, PostgreSQL, and Oracle.

          {noformat}
          Debug info: ORA-00933: SQL command not properly ended
          SELECT COUNT(*) FROM (
          SELECT DISTINCT ue.userid, e.courseid
          FROM p_user_enrolments ue, p_enrol e, p_course c
          WHERE ue.enrolid = e.id
          AND e.courseid <> :o_siteid
          AND c.id = e.courseid
          AND c.visible = 1) as total
          [array (
          'o_siteid' => '1',
          )]
          Error code: dmlreadexception
          Stack trace:

              line 407 of /lib/dml/moodle_database.php: dml_read_exception thrown
              line 284 of /lib/dml/oci_native_moodle_database.php: call to moodle_database->query_end()
              line 1111 of /lib/dml/oci_native_moodle_database.php: call to oci_native_moodle_database->query_end()
              line 1337 of /lib/dml/moodle_database.php: call to oci_native_moodle_database->get_records_sql()
              line 1045 of /lib/dml/oci_native_moodle_database.php: call to moodle_database->get_record_sql()
              line 1410 of /lib/dml/moodle_database.php: call to oci_native_moodle_database->get_record_sql()
              line 1581 of /lib/dml/moodle_database.php: call to moodle_database->get_field_sql()
              line 4038 of /course/lib.php: call to moodle_database->count_records_sql()
              line 373 of /admin/registration/forms.php: call to average_number_of_participants()
              line 173 of /lib/formslib.php: call to site_registration_form->definition()
              line 61 of /admin/registration/register.php: call to moodleform->moodleform()
          {noformat}
          Labels patch triaged
          Component/s Administration [ 10050 ]
          Component/s Database SQL/XMLDB [ 10131 ]
          Component/s Course [ 10057 ]
          Hide
          Michael de Raadt added a comment -

          The problem is on line 4036 of /course/lib.php

          This alias is not needed at all, so the "total" could be removed as well as the "as".

          Fred: could you please provided a link to your diff in Git? It will be easier to work with your patch that way.

          Show
          Michael de Raadt added a comment - The problem is on line 4036 of /course/lib.php This alias is not needed at all, so the "total" could be removed as well as the "as". Fred: could you please provided a link to your diff in Git? It will be easier to work with your patch that way.
          Michael de Raadt made changes -
          Priority Minor [ 4 ] Major [ 3 ]
          Hide
          Fred Woolard added a comment -

          Apologies, I'm still learning much about git/github. Is this what you are requesting?
          https://github.com/appalachianstate/moodle/commit/22129059a6e16f7cc4c1a6e86ad7b4648117534b

          Show
          Fred Woolard added a comment - Apologies, I'm still learning much about git/github. Is this what you are requesting? https://github.com/appalachianstate/moodle/commit/22129059a6e16f7cc4c1a6e86ad7b4648117534b
          Show
          Fred Woolard added a comment - Or this? https://github.com/appalachianstate/moodle/compare/master...MDL-34072
          Hide
          Dan Marsden added a comment -

          Hi Fred - that's the right way to link to the git diff - but as Michael mentions, - the alias isn't needed so you should probably remove the "total" text as well. - if you update your diff someone can probably push this through.

          Show
          Dan Marsden added a comment - Hi Fred - that's the right way to link to the git diff - but as Michael mentions, - the alias isn't needed so you should probably remove the "total" text as well. - if you update your diff someone can probably push this through.
          Hide
          Fred Woolard added a comment -

          Updated to remove alias altogether.

          Show
          Fred Woolard added a comment - Updated to remove alias altogether.
          Hide
          Dan Marsden added a comment -

          looks pretty good to me - only thing I'd suggest is to rebase your 2 commits into a single commit to make the integrators life easier - they would probably do this themselves but it's possible they would reject it until it was rebased.

          All that is needed after rebasing is to add some testing instructions to this issue so that the QA team can test that the fix is correct. I haven't looked - does this appear on the registration page before or after you try to submit for registration?

          Show
          Dan Marsden added a comment - looks pretty good to me - only thing I'd suggest is to rebase your 2 commits into a single commit to make the integrators life easier - they would probably do this themselves but it's possible they would reject it until it was rebased. All that is needed after rebasing is to add some testing instructions to this issue so that the QA team can test that the fix is correct. I haven't looked - does this appear on the registration page before or after you try to submit for registration?
          Hide
          Fred Woolard added a comment -

          Commits merged using rebase (thanks for that handy tip).

          Those two methods only accessed by the registration form & registration_manager when fetching site metrics. Easiest test is to access site registration page, that'll do it. Will add that to the ticket in appropriate field.

          Show
          Fred Woolard added a comment - Commits merged using rebase (thanks for that handy tip). Those two methods only accessed by the registration form & registration_manager when fetching site metrics. Easiest test is to access site registration page, that'll do it. Will add that to the ticket in appropriate field.
          Fred Woolard made changes -
          Testing Instructions Access the site registration page from the Settings block (Site Administration -> Registration)
          Dan Marsden made changes -
          Assignee moodle.com [ moodle.com ] Dan Marsden [ danmarsden ]
          Dan Marsden made changes -
          Peer reviewer danmarsden
          Dan Marsden made changes -
          Testing Instructions Access the site registration page from the Settings block (Site Administration -> Registration) While using a site running under Oracle
          Access the site registration page from the Settings block (Site Administration -> Registration)
          Hide
          Dan Marsden added a comment -

          looks good to me - Thanks Fred, pushing this up for integration.

          NOTE TO INTEGRATOR: please cherry-pick onto 2.3Stable and master - thanks!

          Show
          Dan Marsden added a comment - looks good to me - Thanks Fred, pushing this up for integration. NOTE TO INTEGRATOR: please cherry-pick onto 2.3Stable and master - thanks!
          Dan Marsden made changes -
          Status Open [ 1 ] Waiting for integration review [ 10010 ]
          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
          Sam Hemelryk made changes -
          Currently in integration Yes [ 10041 ]
          Sam Hemelryk made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Integrator samhemelryk
          Hide
          Sam Hemelryk added a comment -

          Thanks guys. This has been integrated now.
          Merged to 23 and cherry-picked to master

          Show
          Sam Hemelryk added a comment - Thanks guys. This has been integrated now. Merged to 23 and cherry-picked to master
          Sam Hemelryk made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Fix Version/s 2.3.2 [ 12353 ]
          Fix Version/s STABLE backlog [ 10463 ]
          Tim Barker made changes -
          Tester abgreeve
          Adrian Greeve made changes -
          Tester abgreeve salvetore
          Michael de Raadt made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Hide
          Michael de Raadt added a comment -

          Test result: Success!

          Tested under Oracle on 2.3 and Master.

          Thanks for your efforts, Fred and Dan.

          Show
          Michael de Raadt added a comment - Test result: Success! Tested under Oracle on 2.3 and Master. Thanks for your efforts, Fred and Dan.
          Michael de Raadt made changes -
          Status Testing in progress [ 10011 ] Tested [ 10006 ]
          Hide
          Dan Poltawski added a comment -

          *Notice*: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26

          Congratulations

          {tracker.user.name}

          !

          You've made into Moodle

          {tracker.fixversion-1}

          +

          I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world.

          cheers!

          {tracker.friendlyintegrator}
          Show
          Dan Poltawski added a comment - * Notice *: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26 Congratulations {tracker.user.name} ! You've made into Moodle {tracker.fixversion-1} + I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world. cheers! {tracker.friendlyintegrator}
          Dan Poltawski made changes -
          Status Tested [ 10006 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Currently in integration Yes [ 10041 ]
          Integration date 19/Jul/12
          Hide
          Fred Woolard added a comment -

          Apparently, the alias is still needed for the native/mysqli connections. The updated query will work with older mysql connection.

          Show
          Fred Woolard added a comment - Apparently, the alias is still needed for the native/mysqli connections. The updated query will work with older mysql connection.
          Hide
          Fred Woolard added a comment - - edited

          Updated the pull branch to put the 'total' alias back into the queries.

          This form of the query I have tested successfully against Oracle, PostgreSQL, MySQL old standard, and MySQL with mysqli connection.

          Show
          Fred Woolard added a comment - - edited Updated the pull branch to put the 'total' alias back into the queries. This form of the query I have tested successfully against Oracle, PostgreSQL, MySQL old standard, and MySQL with mysqli connection.
          Dan Marsden made changes -
          Link This issue caused a regression MDL-34440 [ MDL-34440 ]
          Frédéric Massart made changes -
          Link This issue caused a regression MDL-34473 [ MDL-34473 ]
          Hide
          Frédéric Massart added a comment -

          MDL-34440 will fix the issue reported by Fred Woolard.
          Fred, could you create a new branch including your fix, and post it on MDL-34440?

          Cheers!

          Show
          Frédéric Massart added a comment - MDL-34440 will fix the issue reported by Fred Woolard. Fred, could you create a new branch including your fix, and post it on MDL-34440 ? Cheers!
          Hide
          Fred Woolard added a comment - - edited

          Done. I do not have edit privilege on the MDL-34440 ticket, so added diff link in comment.

          Show
          Fred Woolard added a comment - - edited Done. I do not have edit privilege on the MDL-34440 ticket, so added diff link in comment.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Note: I'm backporting the final solution (take rid of the AS keyword) @ MDL-30514. Not sure why both MDL-34072 and MDL-34440 were not backported... but the problem remains there.

          Show
          Eloy Lafuente (stronk7) added a comment - Note: I'm backporting the final solution (take rid of the AS keyword) @ MDL-30514 . Not sure why both MDL-34072 and MDL-34440 were not backported... but the problem remains there.
          Eloy Lafuente (stronk7) made changes -
          Link This issue has a non-specific relationship to MDL-30514 [ MDL-30514 ]

            People

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

              Dates

              • Created:
                Updated:
                Resolved: