r89483 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r89482‎ | r89483 | r89484 >
Date:22:07, 4 June 2011
Author:maxsem
Status:reverted (Comments)
Tags:
Comment:
SQLite-specific dropTable(): works a bit faster because it doesn't need a separate table existence check and does not rely on subtly broken tableExists() which I will fix a bit later
Modified paths:
  • /trunk/phase3/includes/db/DatabaseSqlite.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/db/DatabaseSqlite.php
——@@ -714,6 +714,11 @@
715715 return parent::buildLike( $params ) . "ESCAPE '\' ";
716716 }
717717
 718+ public function dropTable( $tableName, $fName = 'DatabaseSqlite::dropTable' ) {
 719+ $sql = 'DROP TABLE IF EXISTS ' . $this->tableName( $tableName );
 720+ return $this->query( $sql, $fName );
 721+ }
 722+
718723 /**
719724 * @return string
720725 */

Follow-up revisions

RevisionCommit summaryAuthorDate
r96212rv r89483 per CRmaxsem20:14, 3 September 2011

Comments

#Comment by Aaron Schulz (talk | contribs)   23:26, 4 August 2011

So there is no change of anyone caring if the table never existed? Why not just make the tableExists() fix?

#Comment by Aaron Schulz (talk | contribs)   16:57, 6 August 2011
  • change/chance
#Comment by Catrope (talk | contribs)   09:23, 20 August 2011

Marking fixme per Aaron's comments.

#Comment by MaxSem (talk | contribs)   19:15, 27 August 2011

I couldn't reproduce reliably this problem with with tableExists() on temprary tables that caused unit tests to crash - apparently, it was due to lack of DB object persistence. I'll revert this revision if there's someone who thinks that having this override harmful - Aaron, note that IF EXISTS simply repeats in SQL what DatgabaseBase is doing in PHP.

#Comment by Aaron Schulz (talk | contribs)   06:13, 4 September 2011

Right, but I was thinking of the return value of dropTable.

Status & tagging log