Backup / Restore

[b3rx]

is this in cvs now?

[DavidMJ]

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

[b3rx]

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

[DavidMJ]

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

[b3rx]

thanks DavidMJ! phpBB rocks!

cheers!

[code reader]

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.

[DavidMJ]

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.

[code reader]

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".

[DavidMJ]

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.

[Handyman]

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.