Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0
    • Component/s: Database SQL/XMLDB
    • Labels:
      None
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Gliffy Diagrams

      1. 20060619-ddl.patch
        2 kB
        Andrei Bautu
      2. 200806200337-datalib.php.patch
        0.8 kB
        Andrei Bautu
      3. 200806202038-dml.patch
        3 kB
        Andrei Bautu
      4. 200806202038-dml.patch
        3 kB
        Andrei Bautu
      5. 200806301816-database_column_info.php.patch
        0.7 kB
        Andrei Bautu
      6. 200807311208-MDL-15320.patch
        1 kB
        Andrei Bautu
      7. 200808141240-database_manager.php.patch
        2 kB
        Andrei Bautu

        Activity

        Hide
        abautu Andrei Bautu added a comment -

        remove new line characters at the end of the sql_generator class files

        Show
        abautu Andrei Bautu added a comment - remove new line characters at the end of the sql_generator class files
        Hide
        skodak Petr Skoda added a comment -

        What is wrong with these single newlines? We have them all over the place. It would be imho problem only if there were two newline chars after the ?>, right?

        Show
        skodak Petr Skoda added a comment - What is wrong with these single newlines? We have them all over the place. It would be imho problem only if there were two newline chars after the ?>, right?
        Hide
        abautu Andrei Bautu added a comment -

        If you have any character outside php tags and php's output buffering if off, then sending http headers will not work.

        Show
        abautu Andrei Bautu added a comment - If you have any character outside php tags and php's output buffering if off, then sending http headers will not work.
        Hide
        abautu Andrei Bautu added a comment -

        added support for loading pdo subclasses. it could be merge with the if from adodb.

        Show
        abautu Andrei Bautu added a comment - added support for loading pdo subclasses. it could be merge with the if from adodb.
        Hide
        skodak Petr Skoda added a comment - - edited

        One unix newline is fine after the ?>, no need to remove it - the proof is moodle code itself because it has it in many files.
        see: http://us.php.net/manual/en/faq.using.php#faq.using.newlines

        Show
        skodak Petr Skoda added a comment - - edited One unix newline is fine after the ?>, no need to remove it - the proof is moodle code itself because it has it in many files. see: http://us.php.net/manual/en/faq.using.php#faq.using.newlines
        Hide
        skodak Petr Skoda added a comment -

        I have changed the datalib code to accept any $CFG->dblibrary

        Show
        skodak Petr Skoda added a comment - I have changed the datalib code to accept any $CFG->dblibrary
        Hide
        abautu Andrei Bautu added a comment -
        • fixed dblibrary (instead of library)
        • put code for storing db params into moodle_database contructor; subclasses can call it later
        • change TRUNCATE TABLE to DELETE FROM (which is not mysql specific)
        Show
        abautu Andrei Bautu added a comment - fixed dblibrary (instead of library) put code for storing db params into moodle_database contructor; subclasses can call it later change TRUNCATE TABLE to DELETE FROM (which is not mysql specific)
        Hide
        abautu Andrei Bautu added a comment -

        sorry about the tabs.

        Show
        abautu Andrei Bautu added a comment - sorry about the tabs.
        Hide
        skodak Petr Skoda added a comment -

        1/ Fixed the cfg->library typo - thanks!

        2/ I was thinking about refactoring of connect() and config saving function, in the end we might not need to store
        $cfg->dbhost = $this->dbhost;
        $cfg->dbname = $this->dbname;
        $cfg->dbuser = $this->dbuser;
        $cfg->dbpass = $this->dbpass;
        at all

        3/ TRUNCATE is optimal there, just override it in sqlite driver

        Show
        skodak Petr Skoda added a comment - 1/ Fixed the cfg->library typo - thanks! 2/ I was thinking about refactoring of connect() and config saving function, in the end we might not need to store $cfg->dbhost = $this->dbhost; $cfg->dbname = $this->dbname; $cfg->dbuser = $this->dbuser; $cfg->dbpass = $this->dbpass; at all 3/ TRUNCATE is optimal there, just override it in sqlite driver
        Hide
        abautu Andrei Bautu added a comment -

        2. caching that information may prove useful and doesn't take that much space. however, we could move it to pdo_moodle_database, as adodb_moodle_database does it's own caching.
        3. truncate may be optimal, but is it standard SQL? I think moodle_database should be as general as posible and subclasses should handle particularities. however, if you decide to use delete, there's an error there. I'll provide a new patch later on.

        Show
        abautu Andrei Bautu added a comment - 2. caching that information may prove useful and doesn't take that much space. however, we could move it to pdo_moodle_database, as adodb_moodle_database does it's own caching. 3. truncate may be optimal, but is it standard SQL? I think moodle_database should be as general as posible and subclasses should handle particularities. however, if you decide to use delete, there's an error there. I'll provide a new patch later on.
        Hide
        skodak Petr Skoda added a comment -

        2. I am going to rewrite this, the caching is not needed at all there because we do not support database reconnecting
        3. this is not about standards, this is about databases we officially support - all of them allow TRUNCATE; in fact we never care about standards as long as it works in all supported databases
        If you removed it from moodle_database we would have to add it into all drivers except sqlite, that would be far bellow optional.

        Show
        skodak Petr Skoda added a comment - 2. I am going to rewrite this, the caching is not needed at all there because we do not support database reconnecting 3. this is not about standards, this is about databases we officially support - all of them allow TRUNCATE; in fact we never care about standards as long as it works in all supported databases If you removed it from moodle_database we would have to add it into all drivers except sqlite, that would be far bellow optional.
        Hide
        abautu Andrei Bautu added a comment -

        2. caching this information may be used for other purposes: find out the curently used database to show that information to admins or create a new connection with same parameters (or just a part of them).
        X. if you have time, check http://tracker.moodle.org/secure/ManageAttachments.jspa?id=26733
        Thanks.

        Show
        abautu Andrei Bautu added a comment - 2. caching this information may be used for other purposes: find out the curently used database to show that information to admins or create a new connection with same parameters (or just a part of them). X. if you have time, check http://tracker.moodle.org/secure/ManageAttachments.jspa?id=26733 Thanks.
        Hide
        abautu Andrei Bautu added a comment -

        Columns cache should be cleared after ddl statements.

        Show
        abautu Andrei Bautu added a comment - Columns cache should be cleared after ddl statements.
        Hide
        skodak Petr Skoda added a comment -

        this should be all in cvs I hope, closing for now
        big thanks!!

        Show
        skodak Petr Skoda added a comment - this should be all in cvs I hope, closing for now big thanks!!

          People

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

            Dates

            • Created:
              Updated:
              Resolved:
              Fix Release Date:
              24/Nov/10