r66682 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r66681‎ \| r66682 \| r66683 >
Date:	06:53, 20 May 2010
Author:	jeroendedauw
Status:	deferred
Tags:
Comment:	Code improvements
Modified paths:	/trunk/extensions/SemanticMediaWiki/includes/storage/SMW_SQLHelpers.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SemanticMediaWiki/includes/storage/SMW_SQLHelpers.php
—	—	@@ -55,8 +55,6 @@
56	56	* The function returns an array that includes all columns that have been
57	57	* changed. For each such column, the array contains an entry
58	58	* columnname => action, where action is one of 'up', 'new', or 'del'
59		~~- * If the table was already fine or was created completely anew, an empty~~
60		~~- * array is returned (assuming that both cases require no action).~~
61	59	*
62	60	* If progress reports during this operation are desired, then the parameter $reportTo should
63	61	* be given an object that has a method reportProgress(string) for doing so.
—	—	@@ -75,32 +73,13 @@
76	74	global $wgDBname, $wgDBtype, $wgDBTableOptions;
77	75
78	76	$tableName = $db->tableName( $tableName );
79		~~- $fname = 'SMWSQLHelpers::setupTable';~~
80	77
81		~~- SMWSQLHelpers::reportProgress( "Setting up table $tableName ...\n", $reportTo );~~
	78	+ self::reportProgress( "Setting up table $tableName ...\n", $reportTo );
82	79
83	80	if ( $db->tableExists( $tableName ) === false ) { // create new table
84		~~- $sql = 'CREATE TABLE ' . ( $wgDBtype == 'postgres' ? '' : "`$wgDBname`." ) . $tableName . ' (';~~
85		~~- $first = true;~~
86		-
87		~~- foreach ( $fields as $name => $type ) {~~
88		~~- if ( $first ) {~~
89		~~- $first = false;~~
90		~~- } else {~~
91		~~- $sql .= ',';~~
92		~~- }~~
93		-
94		~~- $sql .= $name . ' ' . $type;~~
95		~~- }~~
96		-
97		~~- $sql .= ') ' . ( $wgDBtype == 'postgres' ? '' : $wgDBTableOptions );~~
98		~~- $db->query( $sql, $fname );~~
99		-
100		~~- SMWSQLHelpers::reportProgress( " ... new table created\n", $reportTo );~~
101		-
102		~~- return array();~~
	81	+ $this->createTable( $tableName, $fields, $db, $reportTo );
103	82	} else { // check table signature
104		~~- SMWSQLHelpers::reportProgress( " ... table exists already, checking structure ...\n", $reportTo );~~
	83	+ self::reportProgress( " ... table exists already, checking structure ...\n", $reportTo );
105	84
106	85	if ( $wgDBtype == 'postgres' ) { // postgresql
107	86	// use the data dictionary in postgresql to get an output comparable to DESCRIBE
—	—	@@ -128,7 +107,7 @@
129	108	$sql = 'DESCRIBE ' . $tableName;
130	109	}
131	110
132		~~- $res = $db->query( $sql, $fname );~~
	111	+ $res = $db->query( $sql, __METHOD__ );
133	112	$curfields = array();
134	113	$result = array();
135	114
—	—	@@ -173,7 +152,7 @@
174	153	if ( !array_key_exists( $name, $curfields ) ) {
175	154	SMWSQLHelpers::reportProgress( " ... creating column $name ... ", $reportTo );
176	155
177		~~- $db->query( "ALTER TABLE $tableName ADD \"" . $name . "\" $type", $fname );~~
	156	+ $db->query( "ALTER TABLE $tableName ADD \"" . $name . "\" $type", __METHOD__ );
178	157	$result[$name] = 'new';
179	158
180	159	SMWSQLHelpers::reportProgress( "done \n", $reportTo );
—	—	@@ -189,11 +168,11 @@
190	169	$typeold = ( $notnullposold > 0 ) ? substr( $curfields[$name], 0, $notnullposold ):$curfields[$name];
191	170
192	171	if ( $typeold != $type ) {
193		~~- $db->query( "ALTER TABLE \"" . $tableName . "\" ALTER COLUMN \"" . $name . "\" TYPE " . $type, $fname );~~
	172	+ $db->query( "ALTER TABLE \"" . $tableName . "\" ALTER COLUMN \"" . $name . "\" TYPE " . $type, __METHOD__ );
194	173	}
195	174
196	175	if ( $notnullposold != $notnullposnew ) {
197		~~- $db->query( "ALTER TABLE \"" . $tableName . "\" ALTER COLUMN \"" . $name . "\" " . ( $notnullposnew > 0 ? 'SET':'DROP' ) . " NOT NULL", $fname );~~
	176	+ $db->query( "ALTER TABLE \"" . $tableName . "\" ALTER COLUMN \"" . $name . "\" " . ( $notnullposnew > 0 ? 'SET':'DROP' ) . " NOT NULL", __METHOD__ );
198	177	}
199	178
200	179	$result[$name] = 'up';
—	—	@@ -209,7 +188,7 @@
210	189	if ( $value !== false ) {
211	190	SMWSQLHelpers::reportProgress( " ... deleting obsolete column $name ... ", $reportTo );
212	191
213		~~- $db->query( "ALTER TABLE \"" . $tableName . "\" DROP COLUMN \"" . $name . "\"", $fname );~~
	192	+ $db->query( "ALTER TABLE \"" . $tableName . "\" DROP COLUMN \"" . $name . "\"", __METHOD__ );
214	193	$result[$name] = 'del';
215	194
216	195	SMWSQLHelpers::reportProgress( "done.\n", $reportTo );
—	—	@@ -222,14 +201,14 @@
223	202	if ( !array_key_exists( $name, $curfields ) ) {
224	203	SMWSQLHelpers::reportProgress( " ... creating column $name ... ", $reportTo );
225	204
226		~~- $db->query( "ALTER TABLE $tableName ADD `$name` $type $position", $fname );~~
	205	+ $db->query( "ALTER TABLE $tableName ADD `$name` $type $position", __METHOD__ );
227	206	$result[$name] = 'new';
228	207
229	208	SMWSQLHelpers::reportProgress( "done \n", $reportTo );
230	209	} elseif ( $curfields[$name] != $type ) {
231	210	SMWSQLHelpers::reportProgress( " ... changing type of column $name from '$curfields[$name]' to '$type' ... ", $reportTo );
232	211
233		~~- $db->query( "ALTER TABLE $tableName CHANGE `$name` `$name` $type $position", $fname );~~
	212	+ $db->query( "ALTER TABLE $tableName CHANGE `$name` `$name` $type $position", __METHOD__ );
234	213	$result[$name] = 'up';
235	214	$curfields[$name] = false;
236	215
—	—	@@ -245,7 +224,7 @@
246	225	foreach ( $curfields as $name => $value ) {
247	226	if ( $value !== false ) { // not encountered yet --> delete
248	227	SMWSQLHelpers::reportProgress( " ... deleting obsolete column $name ... ", $reportTo );
249		~~- $db->query( "ALTER TABLE $tableName DROP COLUMN `$name`", $fname );~~
	228	+ $db->query( "ALTER TABLE $tableName DROP COLUMN `$name`", __METHOD__ );
250	229	$result[$name] = 'del';
251	230	SMWSQLHelpers::reportProgress( "done.\n", $reportTo );
252	231	}
—	—	@@ -257,6 +236,29 @@
258	237	return $result;
259	238	}
260	239	}
	240	+
	241	+ private static function createTable( $tableName, array $fields, DatabaseBase $db, $reportTo ) {
	242	+ global $wgDBtype, $wgDBTableOptions, $wgDBname;
	243	+
	244	+ $sql = 'CREATE TABLE ' . ( $wgDBtype == 'postgres' ? '' : "`$wgDBname`." ) . $tableName . ' (';
	245	+
	246	+ $fieldSql = array();
	247	+
	248	+ foreach ( $fields as $fieldName => $fieldType ) {
	249	+ $fieldSql[] = "$fieldName $fieldType";
	250	+ }
	251	+
	252	+ $sql .= implode( ',', $fieldSql ) . ') ';
	253	+ if ( $wgDBtype != 'postgres' ) $sql .= $wgDBTableOptions;
	254	+
	255	+ $db->query( $sql, __METHOD__ );
	256	+
	257	+ self::reportProgress( " ... new table created\n", $reportTo );
	258	+ }
	259	+
	260	+ private static function updateTable() {
	261	+
	262	+ }
261	263
262	264	/**
263	265	* Make sure that each of the column descriptions in the given array is indexed by one index
—	—	@@ -270,7 +272,6 @@
271	273	global $wgDBtype, $verbose;
272	274
273	275	$tableName = $db->tableName( $tableName );
274		~~- $fname = 'SMWSQLHelpers::setupIndex';~~
275	276
276	277	if ( $wgDBtype == 'postgres' ) { // postgresql
277	278	$sql = "SELECT i.relname AS indexname,"
—	—	@@ -284,7 +285,7 @@
285	286	. " WHERE c.relkind = 'r'::\"char\" AND i.relkind = 'i'::\"char\""
286	287	. " AND c.relname = '" . $tableName . "'"
287	288	. " AND NOT pg_get_indexdef(i.oid) ~ '^CREATE UNIQUE INDEX'";
288		~~- $res = $db->query( $sql, $fname );~~
	289	+ $res = $db->query( $sql, __METHOD__ );
289	290
290	291	if ( !$res ) {
291	292	return false;
—	—	@@ -297,17 +298,17 @@
298	299	if ( array_key_exists( $row->indexcolumns, $columns ) ) {
299	300	$columns[$row->indexcolumns] = false;
300	301	} else {
301		~~- $db->query( 'DROP INDEX IF EXISTS ' . $row->indexname, $fname );~~
	302	+ $db->query( 'DROP INDEX IF EXISTS ' . $row->indexname, __METHOD__ );
302	303	}
303	304	}
304	305
305	306	foreach ( $columns as $key => $column ) { // Ddd the remaining indexes.
306	307	if ( $column != false ) {
307		~~- $db->query( "CREATE INDEX " . $tableName . "_index" . $key . " ON " . $tableName . " USING btree(" . $column . ")", $fname );~~
	308	+ $db->query( "CREATE INDEX " . $tableName . "_index" . $key . " ON " . $tableName . " USING btree(" . $column . ")", __METHOD__ );
308	309	}
309	310	}
310	311	} else { // MySQL
311		~~- $res = $db->query( 'SHOW INDEX FROM ' . $tableName , $fname );~~
	312	+ $res = $db->query( 'SHOW INDEX FROM ' . $tableName , __METHOD__ );
312	313
313	314	if ( !$res ) {
314	315	return false;
—	—	@@ -327,13 +328,13 @@
328	329	if ( $id !== false ) {
329	330	$columns[$id] = false;
330	331	} else { // Duplicate or unrequired index.
331		~~- $db->query( 'DROP INDEX ' . $key . ' ON ' . $tableName, $fname );~~
	332	+ $db->query( 'DROP INDEX ' . $key . ' ON ' . $tableName, __METHOD__ );
332	333	}
333	334	}
334	335
335	336	foreach ( $columns as $key => $column ) { // Ddd the remaining indexes.
336	337	if ( $column != false ) {
337		~~- $db->query( "ALTER TABLE $tableName ADD INDEX ( $column )", $fname );~~
	338	+ $db->query( "ALTER TABLE $tableName ADD INDEX ( $column )", __METHOD__ );
338	339	}
339	340	}
340	341	}

Status & tagging log

10:31, 20 May 2010 Reedy (talk | contribs) changed the status of r66682 [removed: new added: deferred]