Moodle
  1. Moodle
  2. MDL-26458

[Microsoft][SQL Server Native Client 10.0][SQL Server]Invalid column name 'name'

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0.1
    • Fix Version/s: 2.0.3
    • Component/s: Database SQL/XMLDB
    • Labels:
    • Database:
      Microsoft SQL
    • Difficulty:
      Moderate
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      16172

      Description

      Debug info: SQLState: 42S22<br>
      Error Code: 207<br>
      Message: [Microsoft][SQL Server Native Client 10.0][SQL Server]Invalid column name 'name'.<br>

      SELECT q.* FROM (SELECT TOP 9223372036854775806 ROW_NUMBER() OVER(ORDER BY name, concept) AS sqlsrvrownumber, q.* FROM ( SELECT gec.id AS cid, ge.*, gec.entryid, gc.name AS glossarypivot FROM mdl_glossary_entries ge,
      mdl_glossary_entries_categories gec,
      mdl_glossary_categories gc WHERE (ge.glossaryid = '1' OR ge.sourceglossaryid = '1') AND
      ge.id = gec.entryid AND gc.id = gec.categoryid AND
      (ge.approved <> 0 OR ge.userid = '4') ) AS q) AS q WHERE q.sqlsrvrownumber > 0 AND q.sqlsrvrownumber <= 10
      [array (
      0 => '1',
      1 => '1',
      2 => '4',
      )]
      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 899 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->get_recordset_sql()
      line 276 of \mod\glossary\sql.php: call to sqlsrv_native_moodle_database->get_records_sql()
      line 371 of \mod\glossary\view.php: call to require()

        Issue Links

          Activity

          Hide
          Eloy Lafuente (stronk7) added a comment -

          Uhm...

          once more, the complex thing of rewriting queries on-the-fly. Perhaps we should investigate alternatives...

          ... and here they are:

          1) Always SELECT TOP $limitfrom + $limitnum and then, in PHP, iterate over the first $limitfrom ones. Note that, in Moodle we don't use to specify BIG $limitfrom values, just "a few". Obviously, biggest numbers will cause slowest responses, but I think this happen under ALL databases, no matter where the scroll happens:

          https://github.com/stronk7/moodle/compare/master...MDL-26458_sqlsrv_simple_limit

          2) Always SELECT TOP $limitfrom + $limitnum and then, declare the cursor as scrollable (yes, yay, since version 1.1 the driver has scrollable cursors!!) and go straight to $limitfrom (in the DB):

          https://github.com/stronk7/moodle/compare/master...MDL-26458_sqlsrv_simple_limit_take2

          Both kill completely the need of regexps (but the TOP one, already being used by the mssql driver and, afaik, working ok) and both are passing all current tests.

          TODO:

          • Add some tests about requesting records out from the existing range. See how the FIVE database drivers behave. Report it and discuss to get a common behavior in all them. I'm not sure but I think we are missing these tests.
          • Add some more test about to perform updates and deletes to the records being iterated by one resultset in Moodle. See how the 5 databases do it to see if there are problems / differences.
          • MSSQL specific, compare the speed of the current sqlsrv driver with the new ones proposed above, with big recordsets, complex queries and $limitfrom values from 0 to, say, 100.000

          I hope this will drive us to a good end! Looks ok!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Uhm... once more, the complex thing of rewriting queries on-the-fly. Perhaps we should investigate alternatives... ... and here they are: 1) Always SELECT TOP $limitfrom + $limitnum and then, in PHP, iterate over the first $limitfrom ones. Note that, in Moodle we don't use to specify BIG $limitfrom values, just "a few". Obviously, biggest numbers will cause slowest responses, but I think this happen under ALL databases, no matter where the scroll happens: https://github.com/stronk7/moodle/compare/master...MDL-26458_sqlsrv_simple_limit 2) Always SELECT TOP $limitfrom + $limitnum and then, declare the cursor as scrollable (yes, yay, since version 1.1 the driver has scrollable cursors!!) and go straight to $limitfrom (in the DB): https://github.com/stronk7/moodle/compare/master...MDL-26458_sqlsrv_simple_limit_take2 Both kill completely the need of regexps (but the TOP one, already being used by the mssql driver and, afaik, working ok) and both are passing all current tests. TODO: Add some tests about requesting records out from the existing range. See how the FIVE database drivers behave. Report it and discuss to get a common behavior in all them. I'm not sure but I think we are missing these tests. Add some more test about to perform updates and deletes to the records being iterated by one resultset in Moodle. See how the 5 databases do it to see if there are problems / differences. MSSQL specific, compare the speed of the current sqlsrv driver with the new ones proposed above, with big recordsets, complex queries and $limitfrom values from 0 to, say, 100.000 I hope this will drive us to a good end! Looks ok! Ciao
          Hide
          Aparup Banerjee added a comment -

          i've added the unit test @ ,
          so now we've moved on from :

          Message: [Microsoft][SQL Server Native Client 10.0][SQL Server]Invalid column name 'course'.

          SELECT q.* FROM (SELECT TOP 9223372036854775806 ROW_NUMBER() OVER(ORDER BY course, nametext) AS sqlsrvrownumber, q.* FROM (SELECT t2.id, t1.course AS cid, t2.nametext FROM mdl_unit_table t1, mdl_unit_table2 t2
          WHERE t2.course=t1.course ) AS q) AS q WHERE q.sqlsrvrownumber > 2 AND q.sqlsrvrownumber <= 3

          to (with Eloy's scrollable fix) @ https://github.com/nebgor/moodle/compare/mistress...MDL-26458 :

          SQLState: 42000
          Error Code: 306

          Message: [Microsoft][SQL Server Native Client 10.0][SQL Server]The text, ntext, and image data types cannot be compared or sorted, except when using IS NULL or LIKE operator.

          SQLState: 42000
          Error Code: 16945

          Message: [Microsoft][SQL Server Native Client 10.0][SQL Server]The cursor was not declared.

          SELECT TOP 3 t2.id, t1.course AS cid, t2.nametext FROM mdl_unit_table t1, mdl_unit_table2 t2
          WHERE t2.course=t1.course ORDER BY t1.course, t2.nametext
          [array (
          )]
          line 252 of \lib\dml\sqlsrv_native_moodle_database.php: call to moodle_database->query_end()
          line 368 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->query_end()
          line 770 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->do_query()
          line 804 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->get_recordset_sql()
          line 1087 of \lib\dml\simpletest\testdml.php: call to sqlsrv_native_moodle_database->get_records_sql()
          line ... of ...
          line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run()
          line ... of ...
          line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run()

          i'm going to try the other patch once i can figure out how to play around with these commits in git! cherry-pick i suppose?

          Show
          Aparup Banerjee added a comment - i've added the unit test @ , so now we've moved on from : Message: [Microsoft] [SQL Server Native Client 10.0] [SQL Server] Invalid column name 'course'. SELECT q.* FROM (SELECT TOP 9223372036854775806 ROW_NUMBER() OVER(ORDER BY course, nametext) AS sqlsrvrownumber, q.* FROM (SELECT t2.id, t1.course AS cid, t2.nametext FROM mdl_unit_table t1, mdl_unit_table2 t2 WHERE t2.course=t1.course ) AS q) AS q WHERE q.sqlsrvrownumber > 2 AND q.sqlsrvrownumber <= 3 to (with Eloy's scrollable fix) @ https://github.com/nebgor/moodle/compare/mistress...MDL-26458 : SQLState: 42000 Error Code: 306 Message: [Microsoft] [SQL Server Native Client 10.0] [SQL Server] The text, ntext, and image data types cannot be compared or sorted, except when using IS NULL or LIKE operator. SQLState: 42000 Error Code: 16945 Message: [Microsoft] [SQL Server Native Client 10.0] [SQL Server] The cursor was not declared. SELECT TOP 3 t2.id, t1.course AS cid, t2.nametext FROM mdl_unit_table t1, mdl_unit_table2 t2 WHERE t2.course=t1.course ORDER BY t1.course, t2.nametext [array ( )] line 252 of \lib\dml\sqlsrv_native_moodle_database.php: call to moodle_database->query_end() line 368 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->query_end() line 770 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->do_query() line 804 of \lib\dml\sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->get_recordset_sql() line 1087 of \lib\dml\simpletest\testdml.php: call to sqlsrv_native_moodle_database->get_records_sql() line ... of ... line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run() line ... of ... line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run() i'm going to try the other patch once i can figure out how to play around with these commits in git! cherry-pick i suppose?
          Hide
          Aparup Banerjee added a comment -

          the scrollable cursor works!
          passing unit tests updated at https://github.com/nebgor/moodle/compare/mistress...MDL-26458
          next up todo 1.2 (5 DB tests):

          mssql (sqlsrv drv) : Pass @ C:\server\workspace\mssql\lib\dml\simpletest\testdml.php line 1089
          mssql(mssql drv) : Pass @ mssql/moodle/lib/dml/simpletest/testdml.php line 1089
          mysql : Pass @ mysql/moodle/lib/dml/simpletest/testdml.php line 1089
          postgres : Pass @ pg/moodle/lib/dml/simpletest/testdml.php line 1089
          oracle : Pass @ C:\server\workspace\oracle\lib\dml\simpletest\testdml.php line 1089 , (the very next test fails )

          now to add more tests...

          Show
          Aparup Banerjee added a comment - the scrollable cursor works! passing unit tests updated at https://github.com/nebgor/moodle/compare/mistress...MDL-26458 next up todo 1.2 (5 DB tests): mssql (sqlsrv drv) : Pass @ C:\server\workspace\mssql\lib\dml\simpletest\testdml.php line 1089 mssql(mssql drv) : Pass @ mssql/moodle/lib/dml/simpletest/testdml.php line 1089 mysql : Pass @ mysql/moodle/lib/dml/simpletest/testdml.php line 1089 postgres : Pass @ pg/moodle/lib/dml/simpletest/testdml.php line 1089 oracle : Pass @ C:\server\workspace\oracle\lib\dml\simpletest\testdml.php line 1089 , (the very next test fails ) now to add more tests...
          Hide
          Aparup Banerjee added a comment -

          ok, just had a lil Eloy time, turns out the update/delete tests are about doing that within an open recordset to test against DB locking (and any problems etc)

          Show
          Aparup Banerjee added a comment - ok, just had a lil Eloy time, turns out the update/delete tests are about doing that within an open recordset to test against DB locking (and any problems etc)
          Hide
          Aparup Banerjee added a comment -

          updated tests with updating db within open recordset... @ https://github.com/nebgor/moodle/compare/mistress...MDL-26458

          they seem to fail across the board with a variety of results.

          -----------------------------
          MSSQL (sqlsvr)
          -----------------------------
          Fail: lib/dml/simpletest/testdml.php / ► dml_test / ► test_update_record
          Equal expectation fails at character 0 with [100] and [42] at [C:\server\workspace\mssql\lib\dml\simpletest\testdml.php line 2078]
          line ... of ...
          line 2078 of \lib\dml\simpletest\testdml.php: call to UnitTestCase->assertEqual()
          line ... of ...
          line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run()
          line ... of ...
          line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run()

          -------------------------------
          MYSQL
          -------------------------------
          Fail: lib/dml/simpletest/testdml.php / ► dml_test / ► test_update_record
          Equal expectation fails at character 0 with [100] and [42] at [/home/aparup/mcode/m20/mysql/moodle/lib/dml/simpletest/testdml.php line 2078]
          --------------------------------

          -------------------------------
          POSTGRESQL
          -------------------------------
          Fatal error: Maximum execution time of 60 seconds exceeded in /home/aparup/mcode/m20/pg/moodle/lib/dml/pgsql_native_moodle_recordset.php on line 0 Call Stack: 0.0010 694304 1.

          {main}

          () /home/aparup/mcode/m20/pg/moodle/admin/report/unittest/index.php:0 3.5146 77343080 2. autogroup_test_coverage->run() /home/aparup/mcode/m20/pg/moodle/admin/report/unittest/index.php:100 3.5146 77346288 3. AutoGroupTest->run() /home/aparup/mcode/m20/pg/moodle/lib/simpletestcoveragelib.php:144 3.5147 77346568 4. TestSuite->run() /home/aparup/mcode/m20/pg/moodle/admin/report/unittest/ex_simple_test.php:37 3.5183 77405272 5. TestSuite->run() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/test_case.php:620 3.5185 77424128 6. SimpleTestCase->run() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/test_case.php:617 23.3154 80890520 7. SimpleExceptionTrappingInvoker->invoke() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/test_case.php:148 23.3155 80891464 8. SimpleInvokerDecorator->invoke() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/exceptions.php:43 23.3155 80891560 9. SimpleErrorTrappingInvoker->invoke() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/invoker.php:137 23.3155 80891856 10. SimpleInvokerDecorator->invoke() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/errors.php:53 23.3155 80891936 11. SimpleInvoker->invoke() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/invoker.php:137 23.3156 80892184 12. dml_test->test_update_record() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/invoker.php:72 87.6350 81265288 13. pgsql_native_moodle_recordset->next() /home/aparup/mcode/m20/pg/moodle/lib/dml/pgsql_native_moodle_recordset.php:0

          -----------------
          ORACLE 10g
          -----------------
          Fail: lib/dml/simpletest/testdml.php / ► dml_test / ► test_update_record
          Equal expectation fails at character 0 with [100] and [42] at [C:\server\workspace\oracle\lib\dml\simpletest\testdml.php line 2078]
          line ... of ...
          line 2078 of \lib\dml\simpletest\testdml.php: call to UnitTestCase->assertEqual()
          line ... of ...
          line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run()
          line ... of ...
          line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run()

          ------------------
          MSSQL (mssql driver)
          -------------------

          Exception: lib/dml/simpletest/testdml.php / ► dml_test / ► test_update_record
          Unexpected PHP error [mssql_data_seek(): Bad row offset] severity [E_WARNING] in [/home/aparup/mcode/m20/mssql/moodle/lib/dml/mssql_native_moodle_database.php line 710]

          Show
          Aparup Banerjee added a comment - updated tests with updating db within open recordset... @ https://github.com/nebgor/moodle/compare/mistress...MDL-26458 they seem to fail across the board with a variety of results. ----------------------------- MSSQL (sqlsvr) ----------------------------- Fail: lib/dml/simpletest/testdml.php / ► dml_test / ► test_update_record Equal expectation fails at character 0 with [100] and [42] at [C:\server\workspace\mssql\lib\dml\simpletest\testdml.php line 2078] line ... of ... line 2078 of \lib\dml\simpletest\testdml.php: call to UnitTestCase->assertEqual() line ... of ... line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run() line ... of ... line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run() ------------------------------- MYSQL ------------------------------- Fail: lib/dml/simpletest/testdml.php / ► dml_test / ► test_update_record Equal expectation fails at character 0 with [100] and [42] at [/home/aparup/mcode/m20/mysql/moodle/lib/dml/simpletest/testdml.php line 2078] -------------------------------- ------------------------------- POSTGRESQL ------------------------------- Fatal error: Maximum execution time of 60 seconds exceeded in /home/aparup/mcode/m20/pg/moodle/lib/dml/pgsql_native_moodle_recordset.php on line 0 Call Stack: 0.0010 694304 1. {main} () /home/aparup/mcode/m20/pg/moodle/admin/report/unittest/index.php:0 3.5146 77343080 2. autogroup_test_coverage->run() /home/aparup/mcode/m20/pg/moodle/admin/report/unittest/index.php:100 3.5146 77346288 3. AutoGroupTest->run() /home/aparup/mcode/m20/pg/moodle/lib/simpletestcoveragelib.php:144 3.5147 77346568 4. TestSuite->run() /home/aparup/mcode/m20/pg/moodle/admin/report/unittest/ex_simple_test.php:37 3.5183 77405272 5. TestSuite->run() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/test_case.php:620 3.5185 77424128 6. SimpleTestCase->run() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/test_case.php:617 23.3154 80890520 7. SimpleExceptionTrappingInvoker->invoke() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/test_case.php:148 23.3155 80891464 8. SimpleInvokerDecorator->invoke() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/exceptions.php:43 23.3155 80891560 9. SimpleErrorTrappingInvoker->invoke() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/invoker.php:137 23.3155 80891856 10. SimpleInvokerDecorator->invoke() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/errors.php:53 23.3155 80891936 11. SimpleInvoker->invoke() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/invoker.php:137 23.3156 80892184 12. dml_test->test_update_record() /home/aparup/mcode/m20/pg/moodle/lib/simpletestlib/invoker.php:72 87.6350 81265288 13. pgsql_native_moodle_recordset->next() /home/aparup/mcode/m20/pg/moodle/lib/dml/pgsql_native_moodle_recordset.php:0 ----------------- ORACLE 10g ----------------- Fail: lib/dml/simpletest/testdml.php / ► dml_test / ► test_update_record Equal expectation fails at character 0 with [100] and [42] at [C:\server\workspace\oracle\lib\dml\simpletest\testdml.php line 2078] line ... of ... line 2078 of \lib\dml\simpletest\testdml.php: call to UnitTestCase->assertEqual() line ... of ... line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run() line ... of ... line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run() ------------------ MSSQL (mssql driver) ------------------- Exception: lib/dml/simpletest/testdml.php / ► dml_test / ► test_update_record Unexpected PHP error [mssql_data_seek(): Bad row offset] severity [E_WARNING] in [/home/aparup/mcode/m20/mssql/moodle/lib/dml/mssql_native_moodle_database.php line 710]
          Hide
          Aparup Banerjee added a comment -

          hm i get the above results even without any of Eloy's fixes @ https://github.com/nebgor/moodle/compare/mistress...MDL-26458_noscroll.

          applying https://github.com/stronk7/moodle/compare/master...MDL-26458_sqlsrv_simple_limit to my branch now and testing..

          Show
          Aparup Banerjee added a comment - hm i get the above results even without any of Eloy's fixes @ https://github.com/nebgor/moodle/compare/mistress...MDL-26458_noscroll . applying https://github.com/stronk7/moodle/compare/master...MDL-26458_sqlsrv_simple_limit to my branch now and testing..
          Hide
          Aparup Banerjee added a comment -

          bah, the updating records within iteration of recordset still getting all the same test results as above with https://github.com/nebgor/moodle/tree/MDL-26458_noscroll_sqlsrvtest.

          Eloy can you comment on the updating unit tests? do they make sense? Maybe they should be failing and we can leave them as expecting to fail?

          as for the new test with the non-scrolling fix ( same fix as in mssql drv):
          Method: fetches TOP ($limitfrom + $limitnum)
          PASS : C:\server\workspace\mssql\lib\dml\simpletest\testdml.php line 1089 (sqlsrv)

          rest are all passing too.
          Method: fetches TOP ($limitfrom + $limitnum)
          PASS : home/aparup/mcode/m20/mssql/moodle/lib/dml/simpletest/testdml.php line 1089

          PASS : /mysql/moodle/lib/dml/simpletest/testdml.php line 1089

          Method: fetches LIMIT $limitnum OFFSET $limitfrom via sql
          PASS : /pg/moodle/lib/dml/simpletest/testdml.php line 1089

          Method: emulates limits by adding rownum field to full resultset and SQL operations on rownum field. ( like in http://blog.lishman.com/2008/03/rownum.html)
          PASS : C:\server\workspace\oracle\lib\dml\simpletest\testdml.php line 1089
          --------------------

          Show
          Aparup Banerjee added a comment - bah, the updating records within iteration of recordset still getting all the same test results as above with https://github.com/nebgor/moodle/tree/MDL-26458_noscroll_sqlsrvtest . Eloy can you comment on the updating unit tests? do they make sense? Maybe they should be failing and we can leave them as expecting to fail? as for the new test with the non-scrolling fix ( same fix as in mssql drv): Method: fetches TOP ($limitfrom + $limitnum) PASS : C:\server\workspace\mssql\lib\dml\simpletest\testdml.php line 1089 (sqlsrv) rest are all passing too. Method: fetches TOP ($limitfrom + $limitnum) PASS : home/aparup/mcode/m20/mssql/moodle/lib/dml/simpletest/testdml.php line 1089 PASS : /mysql/moodle/lib/dml/simpletest/testdml.php line 1089 Method: fetches LIMIT $limitnum OFFSET $limitfrom via sql PASS : /pg/moodle/lib/dml/simpletest/testdml.php line 1089 Method: emulates limits by adding rownum field to full resultset and SQL operations on rownum field. ( like in http://blog.lishman.com/2008/03/rownum.html ) PASS : C:\server\workspace\oracle\lib\dml\simpletest\testdml.php line 1089 --------------------
          Hide
          Aparup Banerjee added a comment -

          updated https://github.com/nebgor/moodle/compare/mistress...MDL-26458 with a deletion test too.. that fails too .. Curses!

          Show
          Aparup Banerjee added a comment - updated https://github.com/nebgor/moodle/compare/mistress...MDL-26458 with a deletion test too.. that fails too .. Curses!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hehe, Aparup.

          Or I'm wrong or your update/delete locking tests are completely wrong as far as they don't return any record, haha, hence it fails or iterate til infinite and friends. I've put some print_object() calls within the $rs for loop and they never get printed!

          As far as that test_update_record() is already too long and it's easy to lost the track of the updated and inserted records I'd suggest you to create one new:

          function test_recordset_locks_update()

          And within it:

          • Create one simple table 2 columns (id, course)
          • Create, say, 5 records, with course = 1..5
          • Get recordset, skipping 2, fetching 2
          • Within the loop, update the record (set_field() is enough) with course = 10
          • Within the loop, test that not record_exists() with the original course
          • Out from the loop check that you've 2 records with course = 10
          • Destroy test table structures as usual

          function test_recordset_locks_delete()

          • Create one simple table 2 columns (id, course)
          • Create, say, 5 records, with course = 1..5
          • Get recordset, skipping 2, fetching 2
          • Within the loop, delete the record (by course)
          • Within the loop, test that not record_exists() with the original course
          • Out from the loop check that you've 3 records now
          • Destroy test table structures as usual

          That way both tests will be independent, each one with its own well-known test data and won't affect other tests.

          Finally here it's a little patch to the limit tests with order clauses, doing some more tests, just in case you want to apply it.

          index 9a2adf4..071c930 100755
          --- a/lib/dml/simpletest/testdml.php
          +++ b/lib/dml/simpletest/testdml.php
          @@ -1082,11 +1082,12 @@ class dml_test extends UnitTestCase {
                   $this->assertEqual($inskey2, end($records)->id);
           
                   // test 2 tables with aliases and limits with order bys
          -        $sql = "SELECT t2.id, t1.course AS cid, t2.nametext FROM {{$tablename}} t1, {{$tablename2}} t2
          +        $sql = "SELECT t1.id, t1.course AS cid, t2.nametext FROM {{$tablename}} t1, {{$tablename2}} t2
                       WHERE t2.course=t1.course ORDER BY t1.course, ". $DB->sql_compare_text('t2.nametext');
          -        $records = $DB->get_records_sql($sql, null, 2, 1); // Skip courses 3 and 5, get 4
          -        $this->assertEqual(1, count($records));
          -        $this->assertEqual('4', end($records)->cid);
          +        $records = $DB->get_records_sql($sql, null, 2, 2); // Skip two course (cid) = 3, get 4 and 5
          +        $this->assertEqual(2, count($records));
          +        $this->assertEqual('4', reset($records)->cid);
          +        $this->assertEqual('5', end($records)->cid);
          

          Ciao

          PS: Also, we are missing the out-of-range limit tests I commented above, but don't try them yet, let's sort these (limit with order and update/delete locks within recordsets) before and, once working ok, we'll investigate them.

          Show
          Eloy Lafuente (stronk7) added a comment - Hehe, Aparup. Or I'm wrong or your update/delete locking tests are completely wrong as far as they don't return any record, haha, hence it fails or iterate til infinite and friends. I've put some print_object() calls within the $rs for loop and they never get printed! As far as that test_update_record() is already too long and it's easy to lost the track of the updated and inserted records I'd suggest you to create one new: function test_recordset_locks_update() And within it: Create one simple table 2 columns (id, course) Create, say, 5 records, with course = 1..5 Get recordset, skipping 2, fetching 2 Within the loop, update the record (set_field() is enough) with course = 10 Within the loop, test that not record_exists() with the original course Out from the loop check that you've 2 records with course = 10 Destroy test table structures as usual function test_recordset_locks_delete() Create one simple table 2 columns (id, course) Create, say, 5 records, with course = 1..5 Get recordset, skipping 2, fetching 2 Within the loop, delete the record (by course) Within the loop, test that not record_exists() with the original course Out from the loop check that you've 3 records now Destroy test table structures as usual That way both tests will be independent, each one with its own well-known test data and won't affect other tests. Finally here it's a little patch to the limit tests with order clauses, doing some more tests, just in case you want to apply it. index 9a2adf4..071c930 100755 --- a/lib/dml/simpletest/testdml.php +++ b/lib/dml/simpletest/testdml.php @@ -1082,11 +1082,12 @@ class dml_test extends UnitTestCase { $ this ->assertEqual($inskey2, end($records)->id); // test 2 tables with aliases and limits with order bys - $sql = "SELECT t2.id, t1.course AS cid, t2.nametext FROM {{$tablename}} t1, {{$tablename2}} t2 + $sql = "SELECT t1.id, t1.course AS cid, t2.nametext FROM {{$tablename}} t1, {{$tablename2}} t2 WHERE t2.course=t1.course ORDER BY t1.course, ". $DB->sql_compare_text('t2.nametext'); - $records = $DB->get_records_sql($sql, null , 2, 1); // Skip courses 3 and 5, get 4 - $ this ->assertEqual(1, count($records)); - $ this ->assertEqual('4', end($records)->cid); + $records = $DB->get_records_sql($sql, null , 2, 2); // Skip two course (cid) = 3, get 4 and 5 + $ this ->assertEqual(2, count($records)); + $ this ->assertEqual('4', reset($records)->cid); + $ this ->assertEqual('5', end($records)->cid); Ciao PS: Also, we are missing the out-of-range limit tests I commented above, but don't try them yet, let's sort these (limit with order and update/delete locks within recordsets) before and, once working ok, we'll investigate them.
          Hide
          Aparup Banerjee added a comment -

          Hi Eloy,
          i've updated https://github.com/nebgor/moodle/compare/mistress...MDL-26458 , please have a look

          it seems the above are all sorted out, will have to look around for any other unit tests that are failing which might relate to this change. (too bad no 'you have NEW failed tests')

          also gambled with limit out of range unit tests:
          Equal expectation fails because [Integer: 0] differs from [Integer: 2] by 2 at [/home/aparup/mcode/m20/pg/moodle/lib/dml/simpletest/testdml.php line 1101]
          Equal expectation fails because [Integer: 0] differs from [Integer: 4] by 4 at [/home/aparup/mcode/m20/pg/moodle/lib/dml/simpletest/testdml.php line 1106]

          Show
          Aparup Banerjee added a comment - Hi Eloy, i've updated https://github.com/nebgor/moodle/compare/mistress...MDL-26458 , please have a look it seems the above are all sorted out, will have to look around for any other unit tests that are failing which might relate to this change. (too bad no 'you have NEW failed tests') also gambled with limit out of range unit tests: Equal expectation fails because [Integer: 0] differs from [Integer: 2] by 2 at [/home/aparup/mcode/m20/pg/moodle/lib/dml/simpletest/testdml.php line 1101] Equal expectation fails because [Integer: 0] differs from [Integer: 4] by 4 at [/home/aparup/mcode/m20/pg/moodle/lib/dml/simpletest/testdml.php line 1106]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi,

          3 little things:

          1) in test_recordset_locks_delete() it seems that you aren't deleting . And the final count is also wrong (must verify that there are 4 remaining records in the table)

          2) I would take out any reference to "(limits with sqlsrv)" in the comment. They are for all the DBs.

          3) could you join all the simpletest commits into just one and rebase the branch so we'll have only 2 commits? the sqlsrv one and the simpletests one.

          Show
          Eloy Lafuente (stronk7) added a comment - Hi, 3 little things: 1) in test_recordset_locks_delete() it seems that you aren't deleting . And the final count is also wrong (must verify that there are 4 remaining records in the table) 2) I would take out any reference to "(limits with sqlsrv)" in the comment. They are for all the DBs. 3) could you join all the simpletest commits into just one and rebase the branch so we'll have only 2 commits? the sqlsrv one and the simpletests one.
          Hide
          Aparup Banerjee added a comment -

          thanks Eloy, i have a feeling i've lost a commit with the change to locks_delete() in one of these 5 DB installs but i've just committed it again anyway.

          I've updated the branch https://github.com/nebgor/moodle/compare/mistress...MDL-26458

          created the branch https://github.com/nebgor/moodle/compare/mistress...MDL-26458_PULL for pull

          Show
          Aparup Banerjee added a comment - thanks Eloy, i have a feeling i've lost a commit with the change to locks_delete() in one of these 5 DB installs but i've just committed it again anyway. I've updated the branch https://github.com/nebgor/moodle/compare/mistress...MDL-26458 created the branch https://github.com/nebgor/moodle/compare/mistress...MDL-26458_PULL for pull
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Cool, this is looking near-near perfect!

          Have you run de tests under all (5) DBs? What are you getting?

          If finally the only tests not passing are the "out of range" ones (apart from the collation ones, those doesn't matter), perhaps I would suggest to:

          1) take the "out of range" tests apart from pull-request (it's another problem, far less critical and we can try to handle it in another MDL issue).
          2) final minor tweaks to the patch (keep out some "Destroy" comment, perhaps comment the 2 new methods with standard phpdocs
          3) Review sqlsrv related documentation in MoodleDocs to check/confirm/add the need to be using at least version 1.1 of the driver (it sounds to me that it's already said somewhere, just to be sure).

          With that... and perhaps testing some installation/upgrade/overall use I think this is ok for a pull request as far as problems with the current sqlsrv driver are important.

          Good work! Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Cool, this is looking near-near perfect! Have you run de tests under all (5) DBs? What are you getting? If finally the only tests not passing are the "out of range" ones (apart from the collation ones, those doesn't matter), perhaps I would suggest to: 1) take the "out of range" tests apart from pull-request (it's another problem, far less critical and we can try to handle it in another MDL issue). 2) final minor tweaks to the patch (keep out some "Destroy" comment, perhaps comment the 2 new methods with standard phpdocs 3) Review sqlsrv related documentation in MoodleDocs to check/confirm/add the need to be using at least version 1.1 of the driver (it sounds to me that it's already said somewhere, just to be sure). With that... and perhaps testing some installation/upgrade/overall use I think this is ok for a pull request as far as problems with the current sqlsrv driver are important. Good work! Ciao
          Hide
          Aparup Banerjee added a comment -

          getting failures for some out of range tests,

          • Equal expectation fails because [Integer: 0] differs from [Integer: 2] by 2 at [/home/aparup/mcode/m20/mysql/moodle/lib/dml/simpletest/testdml.php line 1100]
          • Equal expectation fails because [Integer: 0] differs from [Integer: 4] by 4 at [/home/aparup/mcode/m20/pg/moodle/lib/dml/simpletest/testdml.php line 1104]

          but these are tests which only show more issues ( to be created after push to upstream?), thus will include them in the pull.

          http://docs.moodle.org/en/Using_the_Microsoft_SQL_Server_Driver_for_PHP#Installation_overview was already pointing to the SQLSRV driver v1.1 download location but i've also specified it as a minimum now. (apparently versions older than v1.1 aren't downloadable from the MS site)

          [SQLSRV]
          fresh installation ran fine.
          ran a restoration of a course which ran fine. (attaching your mbz, which i find very useful and fun)
          general usage: seems fine except for the below .

          • other problems seen that might be unrelated (and may or maynot need issues created, just recording here for later):
          • @ mod/quiz/report.php?id=17&mode=statistics
          • /mod/wiki/prettyview.php?pageid=1 (link to printer friendly view has broken img)

          in the midst of installing a 1.9 site on mssql... :-/

          Show
          Aparup Banerjee added a comment - getting failures for some out of range tests, Equal expectation fails because [Integer: 0] differs from [Integer: 2] by 2 at [/home/aparup/mcode/m20/mysql/moodle/lib/dml/simpletest/testdml.php line 1100] Equal expectation fails because [Integer: 0] differs from [Integer: 4] by 4 at [/home/aparup/mcode/m20/pg/moodle/lib/dml/simpletest/testdml.php line 1104] but these are tests which only show more issues ( to be created after push to upstream?), thus will include them in the pull. http://docs.moodle.org/en/Using_the_Microsoft_SQL_Server_Driver_for_PHP#Installation_overview was already pointing to the SQLSRV driver v1.1 download location but i've also specified it as a minimum now. (apparently versions older than v1.1 aren't downloadable from the MS site) [SQLSRV] fresh installation ran fine. ran a restoration of a course which ran fine. (attaching your mbz, which i find very useful and fun) general usage: seems fine except for the below . other problems seen that might be unrelated (and may or maynot need issues created, just recording here for later): @ mod/quiz/report.php?id=17&mode=statistics /mod/wiki/prettyview.php?pageid=1 (link to printer friendly view has broken img) in the midst of installing a 1.9 site on mssql... :-/
          Hide
          Aparup Banerjee added a comment -

          useful restore file

          Show
          Aparup Banerjee added a comment - useful restore file
          Hide
          Aparup Banerjee added a comment -

          ok upgrading from 1.9 (freetds) to a 2.0 (sqlsrv) with checkout from https://github.com/nebgor/moodle/compare/mistress...MDL-26458_PULL ran fine - i almost couldn't believe it.

          i've created a pull request PULL-381 for this now.

          Good luck testing!

          ps: Eloy. no significant variance on performance info, perhaps since i didn't test with 100,000 records
          performance info (cached) before: 0.870923 secs, DB reads/writes: 176/1
          performance info (cached) after: 0.881858 secs, DB reads/writes: 176/1

          Show
          Aparup Banerjee added a comment - ok upgrading from 1.9 (freetds) to a 2.0 (sqlsrv) with checkout from https://github.com/nebgor/moodle/compare/mistress...MDL-26458_PULL ran fine - i almost couldn't believe it. i've created a pull request PULL-381 for this now. Good luck testing! ps: Eloy. no significant variance on performance info, perhaps since i didn't test with 100,000 records performance info (cached) before: 0.870923 secs, DB reads/writes: 176/1 performance info (cached) after: 0.881858 secs, DB reads/writes: 176/1
          Hide
          Aparup Banerjee added a comment -

          pull created this should be fixed , thanks Eloy!

          Show
          Aparup Banerjee added a comment - pull created this should be fixed , thanks Eloy!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Great Aparup, 2 tiny details:

          1) I have seen two tests of this type:

          $this->assertTrue(!$DB->record_exists($tablename, array('course' => $cid)));
          

          Perhaps it's more readable to do, instead:

          $this->assertFalse($DB->record_exists($tablename, array('course' => $cid)));
          

          2) Have you created the new MDL issue about out of range recordsets? If so, plz, link to it from here. I think it will be "Major" as most but better to have it created from now.

          Good work, thanks and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Great Aparup, 2 tiny details: 1) I have seen two tests of this type: $ this ->assertTrue(!$DB->record_exists($tablename, array('course' => $cid))); Perhaps it's more readable to do, instead: $ this ->assertFalse($DB->record_exists($tablename, array('course' => $cid))); 2) Have you created the new MDL issue about out of range recordsets? If so, plz, link to it from here. I think it will be "Major" as most but better to have it created from now. Good work, thanks and ciao
          Hide
          Aparup Banerjee added a comment -

          out of range issue created @ MDL-26688
          from some strange experience (i remember Petr's voice) so i didn't trust assertFalse and preferred assertTrue but now that i see its used everywhere in this context, its been changed. Thanks Eloy

          Show
          Aparup Banerjee added a comment - out of range issue created @ MDL-26688 from some strange experience (i remember Petr's voice) so i didn't trust assertFalse and preferred assertTrue but now that i see its used everywhere in this context, its been changed. Thanks Eloy
          Hide
          Helen Foster added a comment -

          Thanks everyone.

          Show
          Helen Foster added a comment - Thanks everyone.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: