r107376 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r107375‎ | r107376 | r107377 >
Date:12:29, 27 December 2011
Author:maxsem
Status:resolved (Comments)
Tags:core 
Comment:
Added support for stored procedures/functions to MySQL:
* Refactored DatabaseBase::sourceStream(), made it possible for descendant classes to alter its behaviour w/o having to redo it completely like Oracle does.
* MySQL class now supports specifying DELIMITER.
* Thrown away the mess of catering for double semicolon. If it's a problem, fix your .sql files!
* Haven't actually touched Oracle.
* Tests!
Modified paths:
  • /trunk/phase3/includes/db/Database.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseMysql.php (modified) (history)
  • /trunk/phase3/includes/db/DatabasePostgres.php (modified) (history)
  • /trunk/phase3/tests/phpunit/data/db/mysql (added) (history)
  • /trunk/phase3/tests/phpunit/data/db/mysql/functions.sql (added) (history)
  • /trunk/phase3/tests/phpunit/data/db/postgres (added) (history)
  • /trunk/phase3/tests/phpunit/data/db/postgres/functions.sql (added) (history)
  • /trunk/phase3/tests/phpunit/includes/db/DatabaseTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/includes/db/DatabaseTest.php
@@ -2,14 +2,22 @@
33
44 /**
55 * @group Database
 6+ * @group DatabaseBase
67 */
78 class DatabaseTest extends MediaWikiTestCase {
8 - var $db;
 9+ var $db, $functionTest = false;
910
1011 function setUp() {
11 - $this->db = wfGetDB( DB_SLAVE );
 12+ $this->db = wfGetDB( DB_MASTER );
1213 }
1314
 15+ function tearDown() {
 16+ if ( $this->functionTest ) {
 17+ $this->dropFunctions();
 18+ $this->functionTest = false;
 19+ }
 20+ }
 21+
1422 function testAddQuotesNull() {
1523 $check = "NULL";
1624 if ( $this->db->getType() === 'sqlite' || $this->db->getType() === 'oracle' ) {
@@ -90,6 +98,23 @@
9199 $sql );
92100 }
93101
 102+ function testStoredFunctions() {
 103+ if ( !in_array( wfGetDB( DB_MASTER )->getType(), array( 'mysql', 'postgres' ) ) ) {
 104+ $this->markTestSkipped( 'MySQL or Postgres required' );
 105+ }
 106+ global $IP;
 107+ $this->dropFunctions();
 108+ $this->functionTest = true;
 109+ $this->assertTrue( $this->db->sourceFile( "$IP/tests/phpunit/data/db/{$this->db->getType()}/functions.sql" ) );
 110+ $res = $this->db->query( 'SELECT mw_test_function() AS test', __METHOD__ );
 111+ $this->assertEquals( 42, $res->fetchObject()->test );
 112+ }
 113+
 114+ private function dropFunctions() {
 115+ $this->db->query( 'DROP FUNCTION IF EXISTS mw_test_function'
 116+ . ( $this->db->getType() == 'postgres' ? '()' : '' )
 117+ );
 118+ }
94119 }
95120
96121
Index: trunk/phase3/tests/phpunit/data/db/mysql/functions.sql
@@ -0,0 +1,12 @@
 2+-- MySQL test file for DatabaseTest::testStoredFunctions()
 3+
 4+DELIMITER //
 5+
 6+CREATE FUNCTION mw_test_function()
 7+RETURNS int DETERMINISTIC
 8+BEGIN
 9+ SET @foo = 21;
 10+ RETURN @foo * 2;
 11+END//
 12+
 13+DELIMITER //
Property changes on: trunk/phase3/tests/phpunit/data/db/mysql/functions.sql
___________________________________________________________________
Added: svn:eol-style
114 + native
Index: trunk/phase3/tests/phpunit/data/db/postgres/functions.sql
@@ -0,0 +1,12 @@
 2+-- Postgres test file for DatabaseTest::testStoredFunctions()
 3+
 4+CREATE FUNCTION mw_test_function()
 5+RETURNS INTEGER
 6+LANGUAGE plpgsql AS
 7+$mw$
 8+DECLARE foo INTEGER;
 9+BEGIN
 10+ foo := 21;
 11+ RETURN foo * 2;
 12+END
 13+$mw$;
Property changes on: trunk/phase3/tests/phpunit/data/db/postgres/functions.sql
___________________________________________________________________
Added: svn:eol-style
114 + native
Index: trunk/phase3/includes/db/DatabaseMysql.php
@@ -670,6 +670,15 @@
671671 }
672672 }
673673
 674+ protected function streamStatementEnd( &$sql, &$newLine ) {
 675+ if ( strtoupper( substr( $newLine, 0, 9 ) ) == 'DELIMITER' ) {
 676+ preg_match( '/^DELIMITER\s+(\S+)/' , $newLine, $m );
 677+ $this->delimiter = $m[1];
 678+ $newLine = '';
 679+ }
 680+ return parent::streamStatementEnd( $sql, $newLine );
 681+ }
 682+
674683 /**
675684 * @param $lockName string
676685 * @param $method string
Index: trunk/phase3/includes/db/DatabasePostgres.php
@@ -1050,4 +1050,17 @@
10511051 public function getSearchEngine() {
10521052 return 'SearchPostgres';
10531053 }
 1054+
 1055+ protected function streamStatementEnd( &$sql, &$newLine ) {
 1056+ # Allow dollar quoting for function declarations
 1057+ if ( substr( $newLine, 0, 4 ) == '$mw$' ) {
 1058+ if ( $this->delimiter ) {
 1059+ $this->delimiter = false;
 1060+ }
 1061+ else {
 1062+ $this->delimiter = ';';
 1063+ }
 1064+ }
 1065+ return parent::streamStatementEnd( $sql, $newLine );
 1066+ }
10541067 } // end DatabasePostgres class
Index: trunk/phase3/includes/db/Database.php
@@ -228,6 +228,8 @@
229229
230230 protected $htmlErrors;
231231
 232+ protected $delimiter = ';';
 233+
232234 # ------------------------------------------------------------------------------
233235 # Accessors
234236 # ------------------------------------------------------------------------------
@@ -3156,19 +3158,17 @@
31573159 function sourceStream( $fp, $lineCallback = false, $resultCallback = false,
31583160 $fname = 'DatabaseBase::sourceStream' )
31593161 {
3160 - $cmd = "";
 3162+ $cmd = '';
31613163 $done = false;
3162 - $dollarquote = false;
31633164
3164 - while ( ! feof( $fp ) ) {
 3165+ while ( !feof( $fp ) ) {
31653166 if ( $lineCallback ) {
31663167 call_user_func( $lineCallback );
31673168 }
31683169
31693170 $line = trim( fgets( $fp ) );
3170 - $sl = strlen( $line ) - 1;
31713171
3172 - if ( $sl < 0 ) {
 3172+ if ( $line == '' ) {
31733173 continue;
31743174 }
31753175
@@ -3176,31 +3176,15 @@
31773177 continue;
31783178 }
31793179
3180 - # # Allow dollar quoting for function declarations
3181 - if ( substr( $line, 0, 4 ) == '$mw$' ) {
3182 - if ( $dollarquote ) {
3183 - $dollarquote = false;
3184 - $done = true;
3185 - }
3186 - else {
3187 - $dollarquote = true;
3188 - }
3189 - }
3190 - elseif ( !$dollarquote ) {
3191 - if ( ';' == $line[$sl] && ( $sl < 2 || ';' != $line[$sl - 1] ) ) {
3192 - $done = true;
3193 - $line = substr( $line, 0, $sl );
3194 - }
3195 - }
3196 -
31973180 if ( $cmd != '' ) {
31983181 $cmd .= ' ';
31993182 }
32003183
 3184+ $done = $this->streamStatementEnd( $cmd, $line );
 3185+
32013186 $cmd .= "$line\n";
32023187
3203 - if ( $done ) {
3204 - $cmd = str_replace( ';;', ";", $cmd );
 3188+ if ( $done || feof( $fp ) ) {
32053189 $cmd = $this->replaceVars( $cmd );
32063190 $res = $this->query( $cmd, $fname );
32073191
@@ -3222,6 +3206,24 @@
32233207 }
32243208
32253209 /**
 3210+ * Called by sourceStream() to check if we've reached a statement end
 3211+ *
 3212+ * @param $sql String: SQL assembled so far
 3213+ * @param $newLine String: New line about to be added to $sql
 3214+ * @returns Bool: Whether $newLine contains end of the statement
 3215+ */
 3216+ protected function streamStatementEnd( &$sql, &$newLine ) {
 3217+ if ( $this->delimiter ) {
 3218+ $prev = $newLine;
 3219+ $newLine = preg_replace( '/' . preg_quote( $this->delimiter, '/' ) . '$/', '', $newLine );
 3220+ if ( $newLine != $prev ) {
 3221+ return true;
 3222+ }
 3223+ }
 3224+ return false;
 3225+ }
 3226+
 3227+ /**
32263228 * Database independent variable replacement. Replaces a set of variables
32273229 * in an SQL statement with their contents as given by $this->getSchemaVars().
32283230 *

Follow-up revisions

RevisionCommit summaryAuthorDate
r107994Follow-up r107376: disable test by default, causes failures in some configura...maxsem08:38, 4 January 2012
r108603Reverts MySQL stored procedure support...hashar09:46, 11 January 2012
r108671Revert r108603, which was itself a revert of r107376, r107994. Before conside...maxsem20:19, 11 January 2012

Comments

#Comment by Aaron Schulz (talk | contribs)   19:54, 3 January 2012
1) DatabaseTest::testStoredFunctions
DBQueryError: A database error has occurred.  Did you forget to run maintenance/update.php after upg
rading?  See: [https://www.mediawiki.org/wiki/Manual:Upgrading#Run_the_update_script https://www.mediawiki.org/wiki/Manual:Upgrading#Run_the_update_script]
Query:
 CREATE FUNCTION mw_test_function()
 RETURNS int DETERMINISTIC
 BEGIN
 SET @foo = 21;
 RETURN @foo * 2;
 END

Function: DatabaseBase::sourceFile( D:\www\MW_trunk\phase3/tests/phpunit/data/db/mysql/functions.sql
 )
Error: 1419 You do not have the SUPER privilege and binary logging is enabled (you *might* want to u
se the less safe log_bin_trust_function_creators variable) (localhost)
#Comment by MaxSem (talk | contribs)   08:39, 4 January 2012

Disabled in r107994.

Status & tagging log