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

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

    Details

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

      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

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              skodak Petr Skoda added a comment -

              thanks for the report

              Show
              skodak Petr Skoda added a comment - thanks for the report
              Hide
              poltawski Dan Poltawski added a comment -

              Thanks, that has been integrated now

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

              I'll test this with Oracle shortly.

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

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

              Show
              rajeshtaneja Rajesh Taneja added a comment - Couldn't test this on oracle, requested Michael to test this and will pass this after his comments.
              Hide
              salvetore 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
              salvetore 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
              rajeshtaneja 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
              rajeshtaneja 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
              poltawski 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
              poltawski 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:
                    Fix Release Date:
                    25/Jun/12