Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-26025

Various problems with range (top) queries in the sqlsrv native driver

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0.1
    • Fix Version/s: 2.0.2
    • Component/s: Database SQL/XMLDB
    • Labels:
    • Environment:
    • Database:
      Microsoft SQL
    • Difficulty:
      Difficult
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      I created a new instance of Moodle 2.0.1+ (Build: 20110112) on my Windows Server 2003 test machine. At the end of the setup I added Site News to my front page and got an error saying "The column 'name' was specified multiple times for 'q'". After turning the Moodle debugging option up to Developer level I see the following output:

      Warning: file_put_contents(D:\MoodleData2/cache/lang/en/core_error.php) [function.file-put-contents]: failed to open stream: Permission denied in D:\Moodle2\lib\moodlelib.php on line 5986

      Error reading from database

      More information about this error

      Debug info: SQLState: 42000<br>
      Error Code: 8156<br>
      Message: [Microsoft][SQL Server Native Client 10.0][SQL Server]The column 'name' was specified multiple times for 'q'.<br>

      SELECT , name, timemodified, usermodified, groupid, timestart, timeend, firstname, lastname, email, picture, imagealt FROM (SELECT TOP 9223372036854775806 ROW_NUMBER() OVER(ORDER BY modified DESC) AS sqlsrvrownumber, *, name, timemodified, usermodified, groupid, timestart, timeend, firstname, lastname, email, picture, imagealt FROM (SELECT p., d.name, d.timemodified, d.usermodified, d.groupid, d.timestart, d.timeend,
      u.firstname, u.lastname, u.email, u.picture, u.imagealt
      FROM mdl_forum_discussions d
      JOIN mdl_forum_posts p ON p.discussion = d.id
      JOIN mdl_user u ON p.userid = u.id

      WHERE d.forum = '1' AND p.parent = 0

      ) AS q) AS q WHERE q.sqlsrvrownumber > 0 AND q.sqlsrvrownumber <= 1
      [array (
      0 => '1',
      )]
      Stack trace:
      line 391 of \lib\dml\moodle_database.php: dml_read_exception thrown
      line 252 of \lib\dml\sqlsrv_native_moodle_database.php: call to moodle_database->query_end()
      line 363 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->query_end()
      line 761 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->do_query()
      line 905 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->get_recordset_sql()
      line 2596 of \mod\forum\lib.php: call to sqlsrv_native_moodle_database->get_records_sql()
      line 5084 of \mod\forum\lib.php: call to forum_get_discussions()
      line 175 of \index.php: call to forum_print_latest_discussions()

      I've attached the html output with the error.

        Gliffy Diagrams

        1. Error.htm
          151 kB
          Luis de Vasconcelos

          Activity

          Hide
          eclectics Gregor McNish added a comment -

          We have the same problem after a recent 2.01 update – I think the permission denied error is separate though; we get the sql error on existing Site News.

          There's a forum post on the issue http://moodle.org/mod/forum/discuss.php?d=166122

          It has other mssql issues in glossary and profile.

          Show
          eclectics Gregor McNish added a comment - We have the same problem after a recent 2.01 update – I think the permission denied error is separate though; we get the sql error on existing Site News. There's a forum post on the issue http://moodle.org/mod/forum/discuss.php?d=166122 It has other mssql issues in glossary and profile.
          Hide
          eclectics Gregor McNish added a comment -

          The profile issue has also been put in the tracker as MDL-25906

          Show
          eclectics Gregor McNish added a comment - The profile issue has also been put in the tracker as MDL-25906
          Hide
          eclectics Gregor McNish added a comment -

          I've noted this in MDL-25906 as well. Hope this is correct procedure?
          May have isolated this (though don't know the proper fix)
          In lib/dml/sqlsrv_native_moodle_database.php there's the function

          private function limit_to_top_n

          Replacing this function with it's previous incarnation from our older codebase restored functionality to site news, glossary, profile and logs.

          The function changed in the most recent release (CVS file v1.3 comment is weekly git snapshot 12/30/2010, but it doesn't reference the tracker??).

          Show
          eclectics Gregor McNish added a comment - I've noted this in MDL-25906 as well. Hope this is correct procedure? May have isolated this (though don't know the proper fix) In lib/dml/sqlsrv_native_moodle_database.php there's the function private function limit_to_top_n Replacing this function with it's previous incarnation from our older codebase restored functionality to site news, glossary, profile and logs. The function changed in the most recent release (CVS file v1.3 comment is weekly git snapshot 12/30/2010, but it doesn't reference the tracker??).
          Hide
          eclectics Gregor McNish added a comment -

          Looked around a bit more – probably the change is the result of MDL-25321?

          Show
          eclectics Gregor McNish added a comment - Looked around a bit more – probably the change is the result of MDL-25321 ?
          Hide
          libertymoodle Luis de Vasconcelos added a comment -

          I installed the official final 2.0.1 release from 25th December 2010 and I don't experience the problem on that version. Then I upgraded to MOODLE_20_WEEKLY and the problem reappears - so it looks like a regression in MOODLE_20_WEEKLY.

          Show
          libertymoodle Luis de Vasconcelos added a comment - I installed the official final 2.0.1 release from 25th December 2010 and I don't experience the problem on that version. Then I upgraded to MOODLE_20_WEEKLY and the problem reappears - so it looks like a regression in MOODLE_20_WEEKLY.
          Hide
          jeremy.bascom Jeremy Bascom added a comment -

          Thanks for tracking this down some. If I can help with testing fixes, let me know.

          Show
          jeremy.bascom Jeremy Bascom added a comment - Thanks for tracking this down some. If I can help with testing fixes, let me know.
          Hide
          libertymoodle Luis de Vasconcelos added a comment - - edited

          I'm changing the priority of this one as it really is a show-stopper for us...

          To test further I added a Glossary to a course and I get the same "Error reading from database" message:

          Debug info: SQLState: 42000<br>
          Error Code: 8156<br>
          Message: [Microsoft][SQL Server Native Client 10.0][SQL Server]The column 'glossarypivot' was specified multiple times for 'q'.<br>

          SELECT , glossarypivot FROM (SELECT TOP 9223372036854775806 ROW_NUMBER() OVER(ORDER BY concept) AS sqlsrvrownumber, *, glossarypivot FROM (SELECT ge., ge.concept AS glossarypivot FROM mdl_glossary_entries ge WHERE (ge.glossaryid = '1' or ge.sourceglossaryid = '1') AND
          (ge.approved <> 0 OR ge.userid = '2')
          ) AS q) AS q WHERE q.sqlsrvrownumber > 0 AND q.sqlsrvrownumber <= 10
          [array (
          0 => '1',
          1 => '1',
          2 => '2',
          )]
          Stack trace:
          line 391 of \lib\dml\moodle_database.php: dml_read_exception thrown
          line 252 of \lib\dml\sqlsrv_native_moodle_database.php: call to moodle_database->query_end()
          line 363 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->query_end()
          line 761 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->do_query()
          line 905 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->get_recordset_sql()
          line 265 of \mod\glossary\sql.php: call to sqlsrv_native_moodle_database->get_records_sql()
          line 371 of \mod\glossary\view.php: call to require()

          When I view a Students profile I get this error:

          Debug info: SQLState: 42000<br>
          Error Code: 8155<br>
          Message: [Microsoft][SQL Server Native Client 10.0][SQL Server]No column was specified for column 1 of 'q'.<br>
          SQLState: 42000<br>
          Error Code: 8155<br>
          Message: [Microsoft][SQL Server Native Client 10.0][SQL Server]No column was specified for column 2 of 'q'.<br>

          SELECT 1 FROM (SELECT TOP 9223372036854775806 ROW_NUMBER() OVER(ORDER BY 1) AS sqlsrvrownumber, 1 FROM (SELECT 1
          FROM mdl_role_assignments
          WHERE userid = '3' AND roleid IN (3)) AS q) AS q WHERE q.sqlsrvrownumber > 0 AND q.sqlsrvrownumber <= 1
          [array (
          0 => '3',
          )]
          Stack trace:
          line 391 of \lib\dml\moodle_database.php: dml_read_exception thrown
          line 252 of \lib\dml\sqlsrv_native_moodle_database.php: call to moodle_database->query_end()
          line 363 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->query_end()
          line 761 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->do_query()
          line 1553 of \lib\dml\moodle_database.php: call to sqlsrv_native_moodle_database->get_recordset_sql()
          line 825 of \lib\accesslib.php: call to moodle_database->record_exists_sql()
          line 3284 of \lib\navigationlib.php: call to has_coursecontact_role()
          line 3208 of \lib\navigationlib.php: call to settings_navigation->generate_user_settings()
          line 2583 of \lib\navigationlib.php: call to settings_navigation->load_user_settings()
          line 583 of \lib\pagelib.php: call to settings_navigation->initialise()
          line 599 of \lib\pagelib.php: call to moodle_page->magic_get_settingsnav()
          line 118 of \user\profile.php: call to moodle_page->__get()

          And when I go into the "Live logs from the past hour" in a course I get this "Error reading from database":

          Debug info: SQLState: 42000<br>
          Error Code: 8156<br>
          Message: [Microsoft][SQL Server Native Client 10.0][SQL Server]The column 'firstname' was specified multiple times for 'q'.<br>

          SELECT , firstname, lastname, picture FROM (SELECT TOP 9223372036854775806 ROW_NUMBER() OVER(ORDER BY time DESC) AS sqlsrvrownumber, *, firstname, lastname, picture FROM (SELECT l., u.firstname, u.lastname, u.picture
          FROM mdl_log l
          LEFT JOIN mdl_user u ON l.userid = u.id
          WHERE l.course = '2' AND l.time > '1295417679' AND l.time < '1295504079'
          ) AS q) AS q WHERE q.sqlsrvrownumber > 0 AND q.sqlsrvrownumber <= 500
          [array (
          0 => '2',
          1 => 1295417679,
          2 => 1295504079,
          )]
          Stack trace:
          line 391 of \lib\dml\moodle_database.php: dml_read_exception thrown
          line 252 of \lib\dml\sqlsrv_native_moodle_database.php: call to moodle_database->query_end()
          line 363 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->query_end()
          line 761 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->do_query()
          line 905 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->get_recordset_sql()
          line 1825 of \lib\datalib.php: call to sqlsrv_native_moodle_database->get_records_sql()
          line 303 of \course\lib.php: call to get_logs()
          line 315 of \course\lib.php: call to build_logs_array()
          line 43 of \course\report\log\live.php: call to print_log()

          Should the Moodle codebase be using the evil "SELECT *" syntax at all - especially in queries containing JOINS?

          Show
          libertymoodle Luis de Vasconcelos added a comment - - edited I'm changing the priority of this one as it really is a show-stopper for us... To test further I added a Glossary to a course and I get the same "Error reading from database" message: Debug info: SQLState: 42000<br> Error Code: 8156<br> Message: [Microsoft] [SQL Server Native Client 10.0] [SQL Server] The column 'glossarypivot' was specified multiple times for 'q'.<br> SELECT , glossarypivot FROM (SELECT TOP 9223372036854775806 ROW_NUMBER() OVER(ORDER BY concept) AS sqlsrvrownumber, *, glossarypivot FROM (SELECT ge. , ge.concept AS glossarypivot FROM mdl_glossary_entries ge WHERE (ge.glossaryid = '1' or ge.sourceglossaryid = '1') AND (ge.approved <> 0 OR ge.userid = '2') ) AS q) AS q WHERE q.sqlsrvrownumber > 0 AND q.sqlsrvrownumber <= 10 [array ( 0 => '1', 1 => '1', 2 => '2', )] Stack trace: line 391 of \lib\dml\moodle_database.php: dml_read_exception thrown line 252 of \lib\dml\sqlsrv_native_moodle_database.php: call to moodle_database->query_end() line 363 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->query_end() line 761 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->do_query() line 905 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->get_recordset_sql() line 265 of \mod\glossary\sql.php: call to sqlsrv_native_moodle_database->get_records_sql() line 371 of \mod\glossary\view.php: call to require() When I view a Students profile I get this error: Debug info: SQLState: 42000<br> Error Code: 8155<br> Message: [Microsoft] [SQL Server Native Client 10.0] [SQL Server] No column was specified for column 1 of 'q'.<br> SQLState: 42000<br> Error Code: 8155<br> Message: [Microsoft] [SQL Server Native Client 10.0] [SQL Server] No column was specified for column 2 of 'q'.<br> SELECT 1 FROM (SELECT TOP 9223372036854775806 ROW_NUMBER() OVER(ORDER BY 1) AS sqlsrvrownumber, 1 FROM (SELECT 1 FROM mdl_role_assignments WHERE userid = '3' AND roleid IN (3)) AS q) AS q WHERE q.sqlsrvrownumber > 0 AND q.sqlsrvrownumber <= 1 [array ( 0 => '3', )] Stack trace: line 391 of \lib\dml\moodle_database.php: dml_read_exception thrown line 252 of \lib\dml\sqlsrv_native_moodle_database.php: call to moodle_database->query_end() line 363 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->query_end() line 761 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->do_query() line 1553 of \lib\dml\moodle_database.php: call to sqlsrv_native_moodle_database->get_recordset_sql() line 825 of \lib\accesslib.php: call to moodle_database->record_exists_sql() line 3284 of \lib\navigationlib.php: call to has_coursecontact_role() line 3208 of \lib\navigationlib.php: call to settings_navigation->generate_user_settings() line 2583 of \lib\navigationlib.php: call to settings_navigation->load_user_settings() line 583 of \lib\pagelib.php: call to settings_navigation->initialise() line 599 of \lib\pagelib.php: call to moodle_page->magic_get_settingsnav() line 118 of \user\profile.php: call to moodle_page->__get() And when I go into the "Live logs from the past hour" in a course I get this "Error reading from database": Debug info: SQLState: 42000<br> Error Code: 8156<br> Message: [Microsoft] [SQL Server Native Client 10.0] [SQL Server] The column 'firstname' was specified multiple times for 'q'.<br> SELECT , firstname, lastname, picture FROM (SELECT TOP 9223372036854775806 ROW_NUMBER() OVER(ORDER BY time DESC) AS sqlsrvrownumber, *, firstname, lastname, picture FROM (SELECT l. , u.firstname, u.lastname, u.picture FROM mdl_log l LEFT JOIN mdl_user u ON l.userid = u.id WHERE l.course = '2' AND l.time > '1295417679' AND l.time < '1295504079' ) AS q) AS q WHERE q.sqlsrvrownumber > 0 AND q.sqlsrvrownumber <= 500 [array ( 0 => '2', 1 => 1295417679, 2 => 1295504079, )] Stack trace: line 391 of \lib\dml\moodle_database.php: dml_read_exception thrown line 252 of \lib\dml\sqlsrv_native_moodle_database.php: call to moodle_database->query_end() line 363 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->query_end() line 761 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->do_query() line 905 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->get_recordset_sql() line 1825 of \lib\datalib.php: call to sqlsrv_native_moodle_database->get_records_sql() line 303 of \course\lib.php: call to get_logs() line 315 of \course\lib.php: call to build_logs_array() line 43 of \course\report\log\live.php: call to print_log() Should the Moodle codebase be using the evil "SELECT *" syntax at all - especially in queries containing JOINS?
          Hide
          libertymoodle Luis de Vasconcelos added a comment -

          When I add a Lesson to a Course I also get the "Error reading from database":

          Debug info: SQLState: 42000<br>
          Error Code: 421<br>
          Message: [Microsoft][SQL Server Native Client 10.0][SQL Server]The ntext data type cannot be selected as DISTINCT because it is not comparable.<br>

          SELECT DISTINCT u.id, u.*
          FROM mdl_user u,
          mdl_lesson_attempts a
          WHERE a.lessonid = '1' and
          u.id = a.userid
          ORDER BY u.lastname
          [array (
          0 => '1',
          )]
          Stack trace:

          • line 391 of \lib\dml\moodle_database.php: dml_read_exception thrown
          • line 252 of \lib\dml\sqlsrv_native_moodle_database.php: call to moodle_database->query_end()
          • line 363 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->query_end()
          • line 761 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->do_query()
          • line 905 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->get_recordset_sql()
          • line 63 of \mod\lesson\report.php: call to sqlsrv_native_moodle_database->get_records_sql()
          Show
          libertymoodle Luis de Vasconcelos added a comment - When I add a Lesson to a Course I also get the "Error reading from database": Debug info: SQLState: 42000<br> Error Code: 421<br> Message: [Microsoft] [SQL Server Native Client 10.0] [SQL Server] The ntext data type cannot be selected as DISTINCT because it is not comparable.<br> SELECT DISTINCT u.id, u.* FROM mdl_user u, mdl_lesson_attempts a WHERE a.lessonid = '1' and u.id = a.userid ORDER BY u.lastname [array ( 0 => '1', )] Stack trace: line 391 of \lib\dml\moodle_database.php: dml_read_exception thrown line 252 of \lib\dml\sqlsrv_native_moodle_database.php: call to moodle_database->query_end() line 363 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->query_end() line 761 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->do_query() line 905 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->get_recordset_sql() line 63 of \mod\lesson\report.php: call to sqlsrv_native_moodle_database->get_records_sql()
          Hide
          libertymoodle Luis de Vasconcelos added a comment -

          I'm using:

          $CFG->dbtype = 'sqlsrv';
          $CFG->dblibrary = 'native';

          in my config.php file

          Show
          libertymoodle Luis de Vasconcelos added a comment - I'm using: $CFG->dbtype = 'sqlsrv'; $CFG->dblibrary = 'native'; in my config.php file
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Thanks for the report and details, guys.

          I guess this is a regression introduced recently when fixing MDL-25321 (the original function was also breaking a lot of queries). Exclusively using the sqlsrv driver (not the mssql one).

          So I'm moving this to the stable backlog and assigning it straight to Sam... with MAXIMUM priority.

          Ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Thanks for the report and details, guys. I guess this is a regression introduced recently when fixing MDL-25321 (the original function was also breaking a lot of queries). Exclusively using the sqlsrv driver (not the mssql one). So I'm moving this to the stable backlog and assigning it straight to Sam... with MAXIMUM priority. Ciao
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Hi guys,
          I've just spoken to Martin about this, unfortunately it has missed getting into this sprint and by the looks of it will be looked at in the next sprint.
          I've already done a bit of preliminary testing and have isolated two clear issues with the new code from the fantastic examples you have all posted:

          1. The first issue is because of duplicate column names when we nest the original queries to get the row number. This only happen if one of the fields in the original query was x.* in which case you would end up with * + additional comments, guaranteed to cause problems.
          2. The second problem is numeric constants that get used to check a fields existence in some cases.

          The solution to the second problem is pretty clear cut and easily dealt with by improving the regex that identifies constants to include purely numeric constants.
          The second issue is a little more complex unfortunately, the solution is certainly to remove column dependence during nesting. The reason we currently depend upon columns is because we generate a row number field which in Moodle's case would replace the id as the first column. I think it should be possible however to deal with this within our sqlsrv recordset wrapper which is used for all queries. Eloy I'd like to talk to you about this when I start work on this issue.

          Luis I haven't managed to replicate the issue that resulted in the message "Message: [Microsoft][SQL Server Native Client 10.0][SQL Server]The ntext data type cannot be selected as DISTINCT because it is not comparable.". I'll have a deeper look into this one when I commence work on this issue.

          Cheers
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Hi guys, I've just spoken to Martin about this, unfortunately it has missed getting into this sprint and by the looks of it will be looked at in the next sprint. I've already done a bit of preliminary testing and have isolated two clear issues with the new code from the fantastic examples you have all posted: The first issue is because of duplicate column names when we nest the original queries to get the row number. This only happen if one of the fields in the original query was x.* in which case you would end up with * + additional comments, guaranteed to cause problems. The second problem is numeric constants that get used to check a fields existence in some cases. The solution to the second problem is pretty clear cut and easily dealt with by improving the regex that identifies constants to include purely numeric constants. The second issue is a little more complex unfortunately, the solution is certainly to remove column dependence during nesting. The reason we currently depend upon columns is because we generate a row number field which in Moodle's case would replace the id as the first column. I think it should be possible however to deal with this within our sqlsrv recordset wrapper which is used for all queries. Eloy I'd like to talk to you about this when I start work on this issue. Luis I haven't managed to replicate the issue that resulted in the message "Message: [Microsoft] [SQL Server Native Client 10.0] [SQL Server] The ntext data type cannot be selected as DISTINCT because it is not comparable.". I'll have a deeper look into this one when I commence work on this issue. Cheers Sam
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Eloy if you have a moment could you please have a look at https://github.com/samhemelryk/moodle/compare/master...wip-MDL-26025 and let me know what you think?
          I've implement what I was thinking about above and added some unit tests to explicitly catch this sort of thing.

          Cheers
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Eloy if you have a moment could you please have a look at https://github.com/samhemelryk/moodle/compare/master...wip-MDL-26025 and let me know what you think? I've implement what I was thinking about above and added some unit tests to explicitly catch this sort of thing. Cheers Sam
          Hide
          samhemelryk Sam Hemelryk added a comment -

          I was talking to Eloy about this issue this morning, the following was our conversation so that it doesn't get forgotten:

          (09:02:14) samhemelryk: I have a purposed fix, but it involves altering the sqlsrv recordset class to remove the row number field when iterating a recordset.... so I'm not sure whether it may be crossing some boundaries or something
          (09:03:18) stronk7: Silly question, Sam. Does the sqlsrv driver has some seek() of similar to quickly advance the recordsets?
          (09:03:38) samhemelryk: Good question.... no clue
          (09:03:42) stronk7: I was thinking about that the other day. Instead of modifying the SQL...
          (09:04:13) samhemelryk: You think just generate a complete recordset and then speed to the records we are interested in
          (09:05:41) stronk7: yup, that's the way the MSSQL emulated the limit clause. It has a quick-scroll-forward: mssql_data_seek()
          (09:06:02) samhemelryk: Ahhh well that may be a much better option!
          (09:06:21) stronk7: yeah, but surely... it doesn't exist.
          (09:07:59) stronk7: uhm... have you take a look to the oracle impl? It also emulates limits with some subqueries and has been working ok so far... and the original $sql is always unmodified.
          (09:08:55) stronk7: Perhaps you could follow that approach, it's a matter of find the equivalent to rownum in mssql, surely some rank function will provide it
          (09:08:57) stronk7: uhm
          (09:09:33) stronk7: I really think the regexp on sql is a dead end and we always will find some query broken.
          (09:09:42) samhemelryk: Year I agree
          (09:11:18) stronk7: one more suggestion... take a look to adodb's sqlsrv implementation. Sure they also have limit emulation there (and MS did it, like ours, so surely will be faulty, but just in case)
          (09:11:40) samhemelryk: Ok cool thanks for the help
          (09:11:41) stronk7: Perhaps the Oracle alternative is the easier to follow
          (09:12:06) stronk7: (at least visually)

          Show
          samhemelryk Sam Hemelryk added a comment - I was talking to Eloy about this issue this morning, the following was our conversation so that it doesn't get forgotten: (09:02:14) samhemelryk: I have a purposed fix, but it involves altering the sqlsrv recordset class to remove the row number field when iterating a recordset.... so I'm not sure whether it may be crossing some boundaries or something (09:03:18) stronk7: Silly question, Sam. Does the sqlsrv driver has some seek() of similar to quickly advance the recordsets? (09:03:38) samhemelryk: Good question.... no clue (09:03:42) stronk7: I was thinking about that the other day. Instead of modifying the SQL... (09:04:13) samhemelryk: You think just generate a complete recordset and then speed to the records we are interested in (09:05:41) stronk7: yup, that's the way the MSSQL emulated the limit clause. It has a quick-scroll-forward: mssql_data_seek() (09:06:02) samhemelryk: Ahhh well that may be a much better option! (09:06:21) stronk7: yeah, but surely... it doesn't exist. (09:07:59) stronk7: uhm... have you take a look to the oracle impl? It also emulates limits with some subqueries and has been working ok so far... and the original $sql is always unmodified. (09:08:55) stronk7: Perhaps you could follow that approach, it's a matter of find the equivalent to rownum in mssql, surely some rank function will provide it (09:08:57) stronk7: uhm (09:09:33) stronk7: I really think the regexp on sql is a dead end and we always will find some query broken. (09:09:42) samhemelryk: Year I agree (09:11:18) stronk7: one more suggestion... take a look to adodb's sqlsrv implementation. Sure they also have limit emulation there (and MS did it, like ours, so surely will be faulty, but just in case) (09:11:40) samhemelryk: Ok cool thanks for the help (09:11:41) stronk7: Perhaps the Oracle alternative is the easier to follow (09:12:06) stronk7: (at least visually)
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          I think your iteration above @ github is better than current one, so +1 for it, note I haven't tested it.

          About the recommendation of the Oracle alternative... well, it's pretty much the same you are using, but cannot be implemented so easily mainly because MSSQL doesn't allow ORDER BY clauses in subselects and you need to put it separately in the rank function (row_number() or whatever it's called).

          Also, I've verified that current php driver doesn't offer scrollable (seek) result sets, so perhaps the only way we have it to continue improving the regexp alternative. I don't like it but...

          Ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - I think your iteration above @ github is better than current one, so +1 for it, note I haven't tested it. About the recommendation of the Oracle alternative... well, it's pretty much the same you are using, but cannot be implemented so easily mainly because MSSQL doesn't allow ORDER BY clauses in subselects and you need to put it separately in the rank function (row_number() or whatever it's called). Also, I've verified that current php driver doesn't offer scrollable (seek) result sets, so perhaps the only way we have it to continue improving the regexp alternative. I don't like it but... Ciao
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          One more little thing, in the recordset... why not simply, always:

          unset($row['sqlsrvrownumber']);

          does it throw anything if not set?

          Ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - One more little thing, in the recordset... why not simply, always: unset($row['sqlsrvrownumber']); does it throw anything if not set? Ciao
          Hide
          samhemelryk Sam Hemelryk added a comment -

          PULL-237 created

          Show
          samhemelryk Sam Hemelryk added a comment - PULL-237 created
          Hide
          mudrd8mz David Mudrak added a comment -

          Tested. Thanks for the fix Sam!

          Show
          mudrd8mz David Mudrak added a comment - Tested. Thanks for the fix Sam!
          Hide
          libertymoodle Luis de Vasconcelos added a comment -

          Excuse the stupid question, but now that this issue is closed and resolved when will the fix be available for me to download i.e. in Stable?

          Alternatively, how can I test this fix?

          Show
          libertymoodle Luis de Vasconcelos added a comment - Excuse the stupid question, but now that this issue is closed and resolved when will the fix be available for me to download i.e. in Stable? Alternatively, how can I test this fix?
          Hide
          mudrd8mz David Mudrak added a comment -

          Luis, the fix is now part of Moodle 2.0.1+ package that is built weekly after the testing is finished. You can download the most recent weekly build from http://download.moodle.org/ and it already contains the fix.

          Show
          mudrd8mz David Mudrak added a comment - Luis, the fix is now part of Moodle 2.0.1+ package that is built weekly after the testing is finished. You can download the most recent weekly build from http://download.moodle.org/ and it already contains the fix.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                21/Feb/11