Moodle
  1. Moodle
  2. MDL-32392

Consider deleting protected and unused is_min_version() in some DB drivers

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Database SQL/XMLDB
    • Labels:
    • Rank:
      39250

      Description

      While working on MDL-32368 for postgres, it was detected that some drivers have the is_min_version() method, defined as protected, and never used at all along codebase.

      grep -r is_min_version *
      
      lib/dml/mssql_native_moodle_database.php:    protected function is_min_version($version) {
      lib/dml/oci_native_moodle_database.php:    protected function is_min_version($version) {
      lib/dml/pgsql_native_moodle_database.php:    protected function is_min_version($version) {
      lib/dml/sqlsrv_native_moodle_database.php:    protected function is_min_version($version) {
      

      So this is about to:

      1) delete the method from all drivers.
      2) change it to private in the pgsql one, as far as MDL-32368 is using it.

      Ciao

        Issue Links

          Activity

          Hide
          Petr Škoda added a comment -

          thanks for the report

          Show
          Petr Škoda added a comment - thanks for the report
          Hide
          Dan Poltawski added a comment -

          Thanks, that has been integrated now

          Show
          Dan Poltawski added a comment - Thanks, that has been integrated now
          Hide
          Michael de Raadt added a comment -

          I'll test this with Oracle shortly.

          Show
          Michael de Raadt added a comment - I'll test this with Oracle shortly.
          Hide
          Rajesh Taneja added a comment -

          Couldn't test this on oracle, requested Michael to test this and will pass this after his comments.

          Show
          Rajesh Taneja added a comment - Couldn't test this on oracle, requested Michael to test this and will pass this after his comments.
          Hide
          Michael de Raadt added a comment -

          Results of the Oracle test included a couple of failures, but I don't think either is relevant to this issue.

          Time: 02:37:41, Memory: 143.75Mb
          
          There were 2 failures:
          
          1) dml_testcase::test_sql_concat
          ANSI behaviour: Concatenating NULL must return NULL - But in Oracle :-(. [%s]
          Failed asserting that '123.45test' is null.
          
          D:\xampp\htdocs\moodle_testing_oracle\lib\dml\tests\dml_test.php:3586
          D:\xampp\php\phpunit:46
          
          2) moodlesimplepie_testcase::test_getfeed
          Failed to load the sample RSS file. Please check your proxy settings in Moodle.
          %s
          Failed asserting that 'cURL Error: Operation timed out after 2012 milliseconds w
          ith 30241 out of 32188 bytes received' is null.
          
          D:\xampp\htdocs\moodle_testing_oracle\lib\tests\rsslib_test.php:56
          D:\xampp\htdocs\moodle_testing_oracle\lib\phpunit\lib.php:1058
          D:\xampp\php\phpunit:46
          
          FAILURES!
          Tests: 1070, Assertions: 20535, Failures: 2.
          
          Show
          Michael de Raadt added a comment - Results of the Oracle test included a couple of failures, but I don't think either is relevant to this issue. Time: 02:37:41, Memory: 143.75Mb There were 2 failures: 1) dml_testcase::test_sql_concat ANSI behaviour: Concatenating NULL must return NULL - But in Oracle :-(. [%s] Failed asserting that '123.45test' is null. D:\xampp\htdocs\moodle_testing_oracle\lib\dml\tests\dml_test.php:3586 D:\xampp\php\phpunit:46 2) moodlesimplepie_testcase::test_getfeed Failed to load the sample RSS file. Please check your proxy settings in Moodle. %s Failed asserting that 'cURL Error: Operation timed out after 2012 milliseconds w ith 30241 out of 32188 bytes received' is null. D:\xampp\htdocs\moodle_testing_oracle\lib\tests\rsslib_test.php:56 D:\xampp\htdocs\moodle_testing_oracle\lib\phpunit\lib.php:1058 D:\xampp\php\phpunit:46 FAILURES! Tests: 1070, Assertions: 20535, Failures: 2.
          Hide
          Rajesh Taneja added a comment -

          Thanks Michael,

          Reported failures doesn't seem to be related to removing/modifying is_min_version(), hence passing this test.

          Thanks everyone for working on this

          Show
          Rajesh Taneja added a comment - Thanks Michael, Reported failures doesn't seem to be related to removing/modifying is_min_version(), hence passing this test. Thanks everyone for working on this
          Hide
          Dan Poltawski added a comment -

          Bonza mate!

          Your changes have made it into the Moodle release! Its time to celebrate! Put a shrimp on the barbie and grab a stubby.

          Hooroo

          Show
          Dan Poltawski added a comment - Bonza mate! Your changes have made it into the Moodle release! Its time to celebrate! Put a shrimp on the barbie and grab a stubby. Hooroo

            People

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

              Dates

              • Created:
                Updated:
                Resolved: