Backup / Restore

Discussion of general topics related to the new version and its place in the world. Don't discuss new features, report bugs, ask for support, et cetera. Don't use this to spam for other boards or attack those boards!
Forum rules
Discussion of general topics related to the new release and its place in the world. Don't discuss new features, report bugs, ask for support, et cetera. Don't use this to spam for other boards or attack those boards!
User avatar
b3rx
Registered User
Posts: 77
Joined: Mon Dec 27, 2004 5:11 am
Location: Cebu City, Ph
Contact:

Re: Backup / Restore

Post by b3rx » Thu Feb 15, 2007 2:42 am

is this in cvs now?
b3rx.com

"The only real mistake is the one from which we learn nothing..."

Image Image Image

User avatar
DavidMJ
Registered User
Posts: 932
Joined: Thu Jun 16, 2005 1:14 am
Location: Great Neck, NY

Re: Backup / Restore

Post by DavidMJ » Thu Feb 15, 2007 2:46 am

No, but my last post has an attachment to it if you would like to play around with it.
Freedom from fear

User avatar
b3rx
Registered User
Posts: 77
Joined: Mon Dec 27, 2004 5:11 am
Location: Cebu City, Ph
Contact:

Re: Backup / Restore

Post by b3rx » Thu Feb 15, 2007 3:22 am

thanks! but i was wondering if your update would be part of olympus?
b3rx.com

"The only real mistake is the one from which we learn nothing..."

Image Image Image

User avatar
DavidMJ
Registered User
Posts: 932
Joined: Thu Jun 16, 2005 1:14 am
Location: Great Neck, NY

Re: Backup / Restore

Post by DavidMJ » Thu Feb 15, 2007 3:25 am

More likely than not but it depends on how well tested it will be.
Freedom from fear

User avatar
b3rx
Registered User
Posts: 77
Joined: Mon Dec 27, 2004 5:11 am
Location: Cebu City, Ph
Contact:

Re: Backup / Restore

Post by b3rx » Thu Feb 15, 2007 3:28 am

thanks DavidMJ! phpBB rocks!

cheers!
b3rx.com

"The only real mistake is the one from which we learn nothing..."

Image Image Image

code reader
Registered User
Posts: 653
Joined: Wed Sep 21, 2005 3:01 pm

Re: Backup / Restore

Post by code reader » Thu Feb 15, 2007 6:10 am

thanks.
in my eyes, the "restore" part looks like a significant improvement. did not look into other parts.

there are some small issues that i want to comment on:
  • now, that the code is more streamlined, it would make sense to test for errors.
  • this piece of code:

    Code: Select all

    <?php
                             // SQLite gets improved performance when you shove all of these disk write queries at once :D
                            if ($db->sql_layer == 'sqlite')
                            {
                                $db->sql_query($data);
                            }
    is a leftover from the old way. $data has no value. sqlite should be handled like all the rest.
  • the "switch" part of the sql layer has some repetition, as all but firebird and postgres behave exactly the same, up to the delimiter. i would like to suggest this small modification that uses php "fallthrough":

    Code: Select all

    <?php
                                $delim = ";\n";
                                switch ($db->sql_layer)
                                {
                                    case 'mssql':
                                    case 'mssql_odbc':
                                    case 'oracle':
                                        $delim = $db->sql_layer == 'oracle' ? "/\n" : "GO\n";
                                    case 'mysql':
                                    case 'mysql4':
                                    case 'mysqli':
                                        while (!$eof($fp))
                                        {
                                            $result = $db->sql_query(fgetd($fp, $delim, $read, $seek, $eof));
                                            if (!$result)
                                            {
                                                // handle errors in some way. at a minimum do:
                                                $error_count++;
                                            }
                                        }
                                    break;
                                    case 'firebird':
                                      // same as now + handle errors
                                    case 'postgres':
                                      // ditto
                                }
    
    note the outline for error handling. of course, this is just a placeholder.
  • the firebird part doesn't look right. i am not familiar with this db layer, but it seems that the "set term" should also go to the db layer as a query. the "continue" seems to be out of place.
  • you set $close, but never call $close();
again, thank you for taking the comments and opinions seriously.
have a good one.

User avatar
DavidMJ
Registered User
Posts: 932
Joined: Thu Jun 16, 2005 1:14 am
Location: Great Neck, NY

Re: Backup / Restore

Post by DavidMJ » Thu Feb 15, 2007 12:25 pm

If you don't know what you are talking about, refrain from making comments ;) Firebird does not allow us to activate SET TERM over PHP. We instead interpret it to have the same results. SQLite stays the way it is because SQLite is for pretty darn small boards. They cannot get very big without serious performance issues and thus the size of the backup can't get huge. Anyway, SQLite can't be chunked properly as it does not provide any way to escape newlines. Firebird and PostgreSQL are handled pretty totally differently, you can't just group them together! Firebird switches it's system of chunks every once and a while, PostgreSQL has to handle subexpressions... The file is closed implicitly by PHP, I just forgot to stick in the function call.
Freedom from fear

code reader
Registered User
Posts: 653
Joined: Wed Sep 21, 2005 3:01 pm

Re: Backup / Restore

Post by code reader » Thu Feb 15, 2007 2:42 pm

DavidMJ wrote: If you don't know what you are talking about, refrain from making comments ;) Firebird does not allow us to activate SET TERM over PHP. We instead interpret it to have the same results. SQLite stays the way it is because SQLite is for pretty darn small boards. They cannot get very big without serious performance issues and thus the size of the backup can't get huge. Anyway, SQLite can't be chunked properly as it does not provide any way to escape newlines. Firebird and PostgreSQL are handled pretty totally differently, you can't just group them together! Firebird switches it's system of chunks every once and a while, PostgreSQL has to handle subexpressions... The file is closed implicitly by PHP, I just forgot to stick in the function call.
some comments:
  • i see your point regarding firebird.
  • if you'll read your own code you'll find that you feed sqlite an uninitialized variable ($data) which is a leftover from the old line $data = file_get_contents()
    which is not in the code any more, so the sqlite lines i referred to are clearly and simply a bug. that's what i said in my previous post, but i think you missed that point.
  • i did not say or write that firebird and postgres should be "grouped together". for those two, i suggest to handle them, and i quote: "// same as now + handle errors"
    where "now" clearly refers to the code you posted in the attachment.
    since they are not handled the same "now", you shouldn't interpret my suggestion as "group them together".
    the little piece of code i posted just prevented replicating the basic loop for the "regular" databases.
    in your code the same loop is replicated 3 times, where the only difference is the delimiter.
    having the basic loop once instead of thrice makes return-value checking and error-handling, which after all were the main point of my comment, easier.
  • in understand that you "just forgot to stick in the function call". please understand that i "just reminded you to stick in the function call".
i do appreciate your willingness to listen and to change the code for the better.
however, i do not appreciate your disrespect and snide comments. i don't think they were called for. we each make mistakes, yourself no less than anyone else, and none of us expects to be treated rudely.
appending a ;) to the end of a condescending comment hardly "makes up for it".

User avatar
DavidMJ
Registered User
Posts: 932
Joined: Thu Jun 16, 2005 1:14 am
Location: Great Neck, NY

Re: Backup / Restore

Post by DavidMJ » Thu Feb 15, 2007 3:24 pm

It was not meant to be condescending, it was meant to be informative. If you do not know something about xyz, don't try to make a suggestion regarding xyz.
Freedom from fear

User avatar
Handyman
Registered User
Posts: 522
Joined: Thu Feb 03, 2005 5:09 am
Location: Where no man has gone before!
Contact:

Re: Backup / Restore

Post by Handyman » Thu Feb 15, 2007 10:27 pm

DavidMJ, I saw you marked the bug fixed and updated the CVS.
I'll be testing it soon… with some large database backups all run local and I'll let you know how it performs.
It should be fine on a Mac and FireFox, but it was just Windows with IE that was a problem.

Thanks for the update.
My phpBB3 Mods || My Mod Queue
Search Engine Friendly (SEO) URLs || Profile link on Avatar and/or Username || AJAX Chat
Display Posts Anywhere || CashMod || AJAX Quick Edit || AJAX Quick Reply

Image

Post Reply