Moodle
  1. Moodle
  2. MDL-29516 DB layer improvements 2.3 META
  3. MDL-32029

Function get_tables of DML does not support empty prefix for external databases

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.3
    • Component/s: Database SQL/XMLDB
    • Labels:
    • Database:
      Any
    • Testing Instructions:
      Hide

      For developers:
      1/ try functional DB tests for all 4 drivers (except mysqli)
      2/ try to connect to external database and fetch list tables:
      Try to connect to an external DB with empty prefix:

      $prefix = '';
      
      if (!$handler = moodle_database::get_driver_instance($type, $library, true)) {
        throw new dml_exception('dbdriverproblem', "Unknown driver $library/$db_record->type");
      }
      try {
        $handler->connect($host, $username, $password, $name, $prefix, $options);
      } catch (moodle_exception $e) {
        die();
      }
      
      print_r($handler->get_tables(false));
      
      Show
      For developers: 1/ try functional DB tests for all 4 drivers (except mysqli) 2/ try to connect to external database and fetch list tables: Try to connect to an external DB with empty prefix: $prefix = ''; if (!$handler = moodle_database::get_driver_instance($type, $library, true )) { throw new dml_exception('dbdriverproblem', "Unknown driver $library/$db_record->type" ); } try { $handler->connect($host, $username, $password, $name, $prefix, $options); } catch (moodle_exception $e) { die(); } print_r($handler->get_tables( false ));
    • Workaround:
      Hide
      //if (strpos($tablename, $this->prefix) !== 0) {
      // continue;
      //}
      

      for:

      if (!empty($this->prefix) && strpos($tablename, $this->prefix) !== 0) {
      [...]
      
      Show
      // if (strpos($tablename, $ this ->prefix) !== 0) { // continue ; //} for: if (!empty($ this ->prefix) && strpos($tablename, $ this ->prefix) !== 0) { [...]
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w13_MDL-32029_m23_extdb
    • Rank:
      38703

      Description

      In all the functions get_tables in lib/dml/*_database.php is a function named get_tables that returns the name of all the databases.

      If you want to connecto to an external table and want to know the name of the tables in the structure and this external database does not have a prefix, the function returns nothing but lots of warnings like this:

      Warning: strpos() [function.strpos]: Empty delimiter in /moodle/lib/dml/pgsql_native_moodle_database.php on line 294
      

        Activity

        Hide
        Petr Škoda added a comment -

        Yes, for now only Moodle tables are supported in DML/DDL library (table prefix is mandatory there). Support for external tables is not implemented yet, sorry.

        Show
        Petr Škoda added a comment - Yes, for now only Moodle tables are supported in DML/DDL library (table prefix is mandatory there). Support for external tables is not implemented yet, sorry.
        Hide
        Pau Ferrer Ocaña (crazyserver) added a comment -

        We're working with a function like the one in the testing instructions and it's working really fine! the only thing we found is that small bug!

        Show
        Pau Ferrer Ocaña (crazyserver) added a comment - We're working with a function like the one in the testing instructions and it's working really fine! the only thing we found is that small bug!
        Hide
        Pau Ferrer Ocaña (crazyserver) added a comment -

        Only affects, and have the same solution:
        mssql_native_moodle_database.php
        oci_native_moodle_database.php
        pgsql_native_moodle_database.php
        sqlsrv_native_moodle_database.php

        The others are already solved

        Show
        Pau Ferrer Ocaña (crazyserver) added a comment - Only affects, and have the same solution: mssql_native_moodle_database.php oci_native_moodle_database.php pgsql_native_moodle_database.php sqlsrv_native_moodle_database.php The others are already solved
        Hide
        Petr Škoda added a comment -

        thanks for the report

        Show
        Petr Škoda added a comment - thanks for the report
        Hide
        Sam Hemelryk added a comment -

        The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        TIA and ciao

        Show
        Sam Hemelryk added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
        Hide
        Petr Škoda added a comment -

        rebased for integration

        Show
        Petr Škoda added a comment - rebased for integration
        Hide
        Sam Hemelryk added a comment -

        Thanks Petr - this has been integrated now

        Show
        Sam Hemelryk added a comment - Thanks Petr - this has been integrated now
        Hide
        Rajesh Taneja added a comment -

        MSSQL error

        Show
        Rajesh Taneja added a comment - MSSQL error
        Hide
        Rajesh Taneja added a comment -

        I was getting errors (because of my installation issues).
        Requested Eloy to test it and all pass

        mysql: 2/2 test cases complete: 1197 passes, 0 fails and 0 exceptions.
        postgres: 2/2 test cases complete: 1197 passes, 0 fails and 0 exceptions.
        mssql: 2/2 test cases complete: 1193 passes, 4 fails and 3 exceptions.
        oracle: 2/2 test cases complete: 1196 passes, 1 fails and 0 exceptions
        

        Thanks for all the help Eloy and thanks for fixing this Petr

        Show
        Rajesh Taneja added a comment - I was getting errors (because of my installation issues). Requested Eloy to test it and all pass mysql: 2/2 test cases complete: 1197 passes, 0 fails and 0 exceptions. postgres: 2/2 test cases complete: 1197 passes, 0 fails and 0 exceptions. mssql: 2/2 test cases complete: 1193 passes, 4 fails and 3 exceptions. oracle: 2/2 test cases complete: 1196 passes, 1 fails and 0 exceptions Thanks for all the help Eloy and thanks for fixing this Petr
        Hide
        Eloy Lafuente (stronk7) added a comment -

        And this has landed upstream, finally! Yay!

        תודה רבה && شكرا جزيلا



        Closing, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - And this has landed upstream, finally! Yay! תודה רבה && شكرا جزيلا Closing, ciao
        Hide
        Pau Ferrer Ocaña (crazyserver) added a comment -

        Thank you all!

        Show
        Pau Ferrer Ocaña (crazyserver) added a comment - Thank you all!

          People

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

            Dates

            • Created:
              Updated:
              Resolved: