r61636 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r61635‎ | r61636 | r61637 >
Date:14:58, 28 January 2010
Author:freakolowsky
Status:ok
Tags:
Comment:
Fixed as per Tim's comments on r61179:
* made insertOneRow and getSequenceData private
* renamed fieldInfo to fieldInfoMulti and made it private (for internal use on Oracle only)
* wrapped fieldInfoMulti in fieldInfo with error if $table parameter is an array
* reverted tableName function to only support a non-array parameter
* wrapped all tableName calls with array parameter with array_map with callback
* wrapped fieldInfo into textFieldSize function to avoid reimplementation of functionality
* fixed unnessesary multiple calls to tableName with same parameter inside a single procedure
Modified paths:
  • /trunk/phase3/includes/db/DatabaseOracle.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/db/DatabaseOracle.php
@@ -454,11 +454,12 @@
455455 return $retVal;
456456 }
457457
458 - function insertOneRow( $table, $row, $fname ) {
 458+ private function insertOneRow( $table, $row, $fname ) {
459459 global $wgLang;
460460
 461+ $table = $this->tableName( $table );
461462 // "INSERT INTO tables (a, b, c)"
462 - $sql = "INSERT INTO " . $this->tableName( $table ) . " (" . join( ',', array_keys( $row ) ) . ')';
 463+ $sql = "INSERT INTO " . $table . " (" . join( ',', array_keys( $row ) ) . ')';
463464 $sql .= " VALUES (";
464465
465466 // for each value, append ":key"
@@ -476,7 +477,7 @@
477478
478479 $stmt = oci_parse( $this->mConn, $sql );
479480 foreach ( $row as $col => &$val ) {
480 - $col_info = $this->fieldInfo( $this->tableName( $table ), $col );
 481+ $col_info = $this->fieldInfoMulti( $table, $col );
481482 $col_type = $col_info != false ? $col_info->type() : 'CONSTANT';
482483
483484 if ( $val === null ) {
@@ -585,13 +586,6 @@
586587 }
587588
588589 function tableName( $name ) {
589 - if (is_array($name)) {
590 - foreach($name as &$single_name) {
591 - $single_name = $this->tableName($single_name);
592 - }
593 - return $name;
594 - }
595 -
596590 global $wgSharedDB, $wgSharedPrefix, $wgSharedTables;
597591 /*
598592 Replace reserved words with better ones
@@ -663,7 +657,7 @@
664658 /**
665659 * Return sequence_name if table has a sequence
666660 */
667 - function getSequenceData( $table ) {
 661+ private function getSequenceData( $table ) {
668662 if ( $this->sequenceData == null ) {
669663 $result = $this->query( "SELECT lower(us.sequence_name), lower(utc.table_name), lower(utc.column_name) from user_sequences us, user_tab_columns utc where us.sequence_name = utc.table_name||'_'||utc.column_name||'_SEQ'" );
670664
@@ -741,18 +735,12 @@
742736 # Returns the size of a text field, or -1 for "unlimited"
743737 function textFieldSize( $table, $field ) {
744738 $table = $this->tableName( $table );
745 - $sql = "SELECT t.typname as ftype,a.atttypmod as size
746 - FROM pg_class c, pg_attribute a, pg_type t
747 - WHERE relname='$table' AND a.attrelid=c.oid AND
748 - a.atttypid=t.oid and a.attname='$field'";
749 - $res = $this->query( $sql );
750 - $row = $this->fetchObject( $res );
751 - if ( $row->ftype == "varchar" ) {
 739+ $fieldInfoData = $this->fieldInfo( $table, $field);
 740+ if ( $fieldInfoData->type == "varchar" ) {
752741 $size = $row->size - 4;
753742 } else {
754743 $size = $row->size;
755744 }
756 - $this->freeResult( $res );
757745 return $size;
758746 }
759747
@@ -834,14 +822,15 @@
835823 }
836824
837825 /**
838 - * Query whether a given column exists in the mediawiki schema
839 - * based on prebuilt table to simulate MySQL field info and keep query speed minimal
 826+ * Function translates mysql_fetch_field() functionality on ORACLE.
 827+ * Caching is present for reducing query time.
 828+ * For internal calls. Use fieldInfo for normal usage.
 829+ * Returns false if the field doesn't exist
 830+ *
 831+ * @param Array $table
 832+ * @param String $field
840833 */
841 - function fieldExists( $table, $field, $fname = 'DatabaseOracle::fieldExists' ) {
842 - return (bool)$this->fieldInfo( $table, $field, $fname );
843 - }
844 -
845 - function fieldInfo( $table, $field ) {
 834+ private function fieldInfoMulti( $table, $field ) {
846835 $tableWhere = '';
847836 $field = strtoupper($field);
848837 if (is_array($table)) {
@@ -885,6 +874,17 @@
886875 }
887876 }
888877
 878+ function fieldInfo( $table, $field ) {
 879+ if ( is_array( $table ) ) {
 880+ throw new DBUnexpectedError( $this, 'Database::fieldInfo called with table array!' );
 881+ }
 882+ return $this->fieldInfoMulti ($table, $field);
 883+ }
 884+
 885+ function fieldExists( $table, $field, $fname = 'DatabaseOracle::fieldExists' ) {
 886+ return (bool)$this->fieldInfo( $table, $field, $fname );
 887+ }
 888+
889889 function begin( $fname = '' ) {
890890 $this->mTrxLevel = 1;
891891 }
@@ -1021,10 +1021,14 @@
10221022 function selectRow( $table, $vars, $conds, $fname = 'DatabaseOracle::selectRow', $options = array(), $join_conds = array() ) {
10231023 global $wgLang;
10241024
 1025+ if (is_array($table)) {
 1026+ $table = array_map( array( &$this, 'tableName' ), $table );
 1027+ }
 1028+
10251029 $conds2 = array();
10261030 $conds = ($conds != null && !is_array($conds)) ? array($conds) : $conds;
10271031 foreach ( $conds as $col => $val ) {
1028 - $col_info = $this->fieldInfo( $this->tableName( $table ), $col );
 1032+ $col_info = $this->fieldInfoMulti( $table, $col );
10291033 $col_type = $col_info != false ? $col_info->type() : 'CONSTANT';
10301034 if ( $col_type == 'CLOB' ) {
10311035 $conds2['TO_CHAR(' . $col . ')'] = $wgLang->checkTitleEncoding( $val );
@@ -1035,14 +1039,6 @@
10361040 }
10371041 }
10381042
1039 - if ( is_array( $table ) ) {
1040 - foreach ( $table as $tab ) {
1041 - $tab = $this->tableName( $tab );
1042 - }
1043 - } else {
1044 - $table = $this->tableName( $table );
1045 - }
1046 -
10471043 return parent::selectRow( $table, $vars, $conds2, $fname, $options, $join_conds );
10481044 }
10491045
@@ -1092,11 +1088,15 @@
10931089 public function delete( $table, $conds, $fname = 'DatabaseOracle::delete' ) {
10941090 global $wgLang;
10951091
 1092+ if (is_array($table)) {
 1093+ $table = array_map( array( &$this, 'tableName' ), $table );
 1094+ }
 1095+
10961096 if ( $wgLang != null ) {
10971097 $conds2 = array();
10981098 $conds = ($conds != null && !is_array($conds)) ? array($conds) : $conds;
10991099 foreach ( $conds as $col => $val ) {
1100 - $col_info = $this->fieldInfo( $this->tableName( $table ), $col );
 1100+ $col_info = $this->fieldInfoMulti( $table, $col );
11011101 $col_type = $col_info != false ? $col_info->type() : 'CONSTANT';
11021102 if ( $col_type == 'CLOB' ) {
11031103 $conds2['TO_CHAR(' . $col . ')'] = $wgLang->checkTitleEncoding( $val );

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r61179Fixed error suppressing suggested in r58559 by Tim. Also fixed some warnings ...freakolowsky20:42, 17 January 2010

Status & tagging log