Uploaded image for project: '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

      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()

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

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

              Thanks for spotting that and providing a solution.

              Show
              salvetore Michael de Raadt added a comment - Thanks for spotting that and providing a solution.
              salvetore 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
              salvetore 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
              salvetore 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.
              salvetore Michael de Raadt made changes -
              Priority Minor [ 4 ] Major [ 3 ]
              Hide
              woolardfa@appstate.edu 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
              woolardfa@appstate.edu 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
              woolardfa@appstate.edu Fred Woolard added a comment - Or this? https://github.com/appalachianstate/moodle/compare/master...MDL-34072
              Hide
              danmarsden 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
              danmarsden 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
              woolardfa@appstate.edu Fred Woolard added a comment -

              Updated to remove alias altogether.

              Show
              woolardfa@appstate.edu Fred Woolard added a comment - Updated to remove alias altogether.
              Hide
              danmarsden 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
              danmarsden 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
              woolardfa@appstate.edu 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
              woolardfa@appstate.edu 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.
              woolardfa@appstate.edu Fred Woolard made changes -
              Testing Instructions Access the site registration page from the Settings block (Site Administration -> Registration)
              danmarsden Dan Marsden made changes -
              Assignee moodle.com [ moodle.com ] Dan Marsden [ danmarsden ]
              danmarsden Dan Marsden made changes -
              Peer reviewer danmarsden
              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
              danmarsden 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
              danmarsden 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!
              danmarsden Dan Marsden made changes -
              Status Open [ 1 ] Waiting for integration review [ 10010 ]
              Hide
              poltawski 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
              poltawski 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
              samhemelryk Sam Hemelryk made changes -
              Currently in integration Yes [ 10041 ]
              samhemelryk Sam Hemelryk made changes -
              Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
              Integrator samhemelryk
              Hide
              samhemelryk Sam Hemelryk added a comment -

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

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks guys. This has been integrated now. Merged to 23 and cherry-picked to master
              samhemelryk 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 ]
              timb Tim Barker made changes -
              Tester abgreeve
              abgreeve Adrian Greeve made changes -
              Tester abgreeve salvetore
              salvetore Michael de Raadt made changes -
              Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
              Hide
              salvetore 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
              salvetore Michael de Raadt added a comment - Test result: Success! Tested under Oracle on 2.3 and Master. Thanks for your efforts, Fred and Dan.
              salvetore Michael de Raadt made changes -
              Status Testing in progress [ 10011 ] Tested [ 10006 ]
              Hide
              poltawski 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
              poltawski 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}
              poltawski Dan Poltawski made changes -
              Status Tested [ 10006 ] Closed [ 6 ]
              Resolution Fixed [ 1 ]
              Currently in integration Yes [ 10041 ]
              Integration date 19/Jul/12
              Hide
              woolardfa@appstate.edu 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
              woolardfa@appstate.edu 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
              woolardfa@appstate.edu 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
              woolardfa@appstate.edu 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.
              danmarsden Dan Marsden made changes -
              Link This issue caused a regression MDL-34440 [ MDL-34440 ]
              fred Frédéric Massart made changes -
              Link This issue caused a regression MDL-34473 [ MDL-34473 ]
              Hide
              fred 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
              fred 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
              woolardfa@appstate.edu 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
              woolardfa@appstate.edu 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
              stronk7 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
              stronk7 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.
              stronk7 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:
                    Fix Release Date:
                    10/Sep/12