r102523 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r102522‎ | r102523 | r102524 >
Date:15:50, 9 November 2011
Author:freakolowsky
Status:ok (Comments)
Tags:
Comment:
* phpunit fixes & optimizations for oracle backend
Modified paths:
  • /trunk/phase3/includes/db/DatabaseOracle.php (modified) (history)
  • /trunk/phase3/maintenance/oracle/archives/patch_rebuild_dupfunc.sql (modified) (history)
  • /trunk/phase3/maintenance/oracle/tables.sql (modified) (history)
  • /trunk/phase3/tests/phpunit/MediaWikiTestCase.php (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/oracle/archives/patch_rebuild_dupfunc.sql
@@ -10,7 +10,7 @@
1111 BEGIN
1212 BEGIN
1313 EXECUTE IMMEDIATE 'DROP TABLE ' || p_newprefix || p_tabname ||
14 - ' CASCADE CONSTRAINTS';
 14+ ' CASCADE CONSTRAINTS PURGE';
1515 EXCEPTION
1616 WHEN e_table_not_exist THEN
1717 NULL;
@@ -20,8 +20,9 @@
2121 END IF;
2222 IF (l_temporary) THEN
2323 EXECUTE IMMEDIATE 'CREATE GLOBAL TEMPORARY TABLE ' || p_newprefix ||
24 - p_tabname || ' AS SELECT * FROM ' || p_oldprefix ||
25 - p_tabname || ' WHERE ROWNUM = 0';
 24+ p_tabname ||
 25+ ' ON COMMIT PRESERVE ROWS AS SELECT * FROM ' ||
 26+ p_oldprefix || p_tabname || ' WHERE ROWNUM = 0';
2627 ELSE
2728 EXECUTE IMMEDIATE 'CREATE TABLE ' || p_newprefix || p_tabname ||
2829 ' AS SELECT * FROM ' || p_oldprefix || p_tabname ||
@@ -68,7 +69,8 @@
6970 FROM user_constraints uc
7071 WHERE table_name = p_oldprefix || p_tabname
7172 AND constraint_type = 'R') LOOP
72 - IF nvl(length(l_temp_ei_sql), 0) > 0 THEN
 73+ IF nvl(length(l_temp_ei_sql), 0) > 0 AND
 74+ INSTR(l_temp_ei_sql, 'PRIMARY KEY') = 0 THEN
7375 EXECUTE IMMEDIATE l_temp_ei_sql;
7476 END IF;
7577 END LOOP;
@@ -142,5 +144,6 @@
143145 END IF;
144146 END LOOP;
145147 END;
 148+
146149 /*$mw$*/
147150
Index: trunk/phase3/maintenance/oracle/tables.sql
@@ -781,7 +781,7 @@
782782 BEGIN
783783 BEGIN
784784 EXECUTE IMMEDIATE 'DROP TABLE ' || p_newprefix || p_tabname ||
785 - ' CASCADE CONSTRAINTS';
 785+ ' CASCADE CONSTRAINTS PURGE';
786786 EXCEPTION
787787 WHEN e_table_not_exist THEN
788788 NULL;
@@ -791,8 +791,9 @@
792792 END IF;
793793 IF (l_temporary) THEN
794794 EXECUTE IMMEDIATE 'CREATE GLOBAL TEMPORARY TABLE ' || p_newprefix ||
795 - p_tabname || ' AS SELECT * FROM ' || p_oldprefix ||
796 - p_tabname || ' WHERE ROWNUM = 0';
 795+ p_tabname ||
 796+ ' ON COMMIT PRESERVE ROWS AS SELECT * FROM ' ||
 797+ p_oldprefix || p_tabname || ' WHERE ROWNUM = 0';
797798 ELSE
798799 EXECUTE IMMEDIATE 'CREATE TABLE ' || p_newprefix || p_tabname ||
799800 ' AS SELECT * FROM ' || p_oldprefix || p_tabname ||
@@ -839,7 +840,8 @@
840841 FROM user_constraints uc
841842 WHERE table_name = p_oldprefix || p_tabname
842843 AND constraint_type = 'R') LOOP
843 - IF nvl(length(l_temp_ei_sql), 0) > 0 THEN
 844+ IF nvl(length(l_temp_ei_sql), 0) > 0 AND
 845+ INSTR(l_temp_ei_sql, 'PRIMARY KEY') = 0 THEN
844846 EXECUTE IMMEDIATE l_temp_ei_sql;
845847 END IF;
846848 END LOOP;
Index: trunk/phase3/tests/phpunit/MediaWikiTestCase.php
@@ -125,6 +125,22 @@
126126 $this->db->insert( 'user', array(
127127 'user_id' => 0,
128128 'user_name' => 'Anonymous' ) );
 129+
 130+ # Insert 0 page to prevent FK violations
 131+ # Blank page
 132+ $this->db->insert( 'page', array(
 133+ 'page_id' => 0,
 134+ 'page_namespace' => 0,
 135+ 'page_title' => ' ',
 136+ 'page_restrictions' => NULL,
 137+ 'page_counter' => 0,
 138+ 'page_is_redirect' => 0,
 139+ 'page_is_new' => 0,
 140+ 'page_random' => 0,
 141+ 'page_touched' => $this->db->timestamp(),
 142+ 'page_latest' => 0,
 143+ 'page_len' => 0 ) );
 144+
129145 }
130146 }
131147
@@ -134,9 +150,29 @@
135151 private function resetDB() {
136152 if( $this->db ) {
137153 foreach( $this->listTables() as $tbl ) {
138 - if( $tbl == 'interwiki' || $tbl == 'user' ) continue;
139 - $this->db->delete( $tbl, '*', __METHOD__ );
 154+ if( $tbl == 'interwiki' || $tbl == 'user' || $tbl == 'MWUSER' ) continue;
 155+ if ( $this->db->getType() == 'oracle' )
 156+ $this->db->query( 'TRUNCATE TABLE '.$this->db->tableName($tbl), __METHOD__ );
 157+ else
 158+ $this->db->delete( $tbl, '*', __METHOD__ );
140159 }
 160+
 161+ if ( $this->db->getType() == 'oracle' ) {
 162+ # Insert 0 page to prevent FK violations
 163+ # Blank page
 164+ $this->db->insert( 'page', array(
 165+ 'page_id' => 0,
 166+ 'page_namespace' => 0,
 167+ 'page_title' => ' ',
 168+ 'page_restrictions' => NULL,
 169+ 'page_counter' => 0,
 170+ 'page_is_redirect' => 0,
 171+ 'page_is_new' => 0,
 172+ 'page_random' => 0,
 173+ 'page_touched' => $this->db->timestamp(),
 174+ 'page_latest' => 0,
 175+ 'page_len' => 0 ) );
 176+ }
141177 }
142178 }
143179
Index: trunk/phase3/includes/db/DatabaseOracle.php
@@ -745,7 +745,7 @@
746746 }
747747
748748 function duplicateTableStructure( $oldName, $newName, $temporary = false, $fname = 'DatabaseOracle::duplicateTableStructure' ) {
749 - $temporary = $temporary ? 'TRUE' : 'FALSE';
 749+ $temporary = 'FALSE'; //$temporary ? 'TRUE' : 'FALSE';
750750
751751 $newName = strtoupper( $newName );
752752 $oldName = strtoupper( $oldName );
@@ -768,9 +768,9 @@
769769
770770 // dirty code ... i know
771771 $endArray = array();
772 - $endArray[] = $prefix.'MWUSER';
773 - $endArray[] = $prefix.'PAGE';
774 - $endArray[] = $prefix.'IMAGE';
 772+ $endArray[] = strtoupper($prefix.'MWUSER');
 773+ $endArray[] = strtoupper($prefix.'PAGE');
 774+ $endArray[] = strtoupper($prefix.'IMAGE');
775775 $fixedOrderTabs = $endArray;
776776 while (($row = $result->fetchRow()) !== false) {
777777 if (!in_array($row['table_name'], $fixedOrderTabs))

Sign-offs

UserFlagDate
Hasharinspected11:47, 5 December 2011

Comments

#Comment by Hashar (talk | contribs)   10:36, 6 December 2011

I am not able to review that since I have about 0 knowledge about Oracle. But I do trust freakolowsky to had made a fine commit :-)

#Comment by 😂 (talk | contribs)   23:49, 12 December 2011

We shouldn't do truncate only for Oracle. We could do the same for Mysql, but I left it off since we don't know if we have truncate permissions.

#Comment by Freakolowsky (talk | contribs)   05:30, 13 December 2011

About truncate ...

For MySQL prior to to 5.1.16 it only required delete privilege, so for those version it shouldn't be an issue. After that version it requires drop permissions, but since phpunit is a development tool mostly, i don't see a reason why not to use it as developers probably grant their own right on their test env.

There is also a way to do it automatic for <5.1.16 and with a cli parameter for greater versions...

but in any case ... TRUNCATE .... gooooood ....

Status & tagging log