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

          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