r87143 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r87142‎ | r87143 | r87144 >
Date:22:27, 29 April 2011
Author:overlordq
Status:ok (Comments)
Tags:todo 
Comment:
Followup to r85907, correctly quote table names.
Followup to r87129, add handling of arrayed GROUP BY/ORDER BY options to match core class so that this will indeed work.
Modified paths:
  • /trunk/phase3/includes/db/DatabasePostgres.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/db/DatabasePostgres.php
@@ -39,7 +39,7 @@
4040 AND attname=%s;
4141 SQL;
4242
43 - $table = $db->tableName( $table );
 43+ $table = $db->tableName( $table, false );
4444 $res = $db->query(
4545 sprintf( $q,
4646 $db->addQuotes( $wgDBmwschema ),
@@ -819,7 +819,7 @@
820820 if ( !$schema ) {
821821 $schema = $wgDBmwschema;
822822 }
823 - $table = $this->tableName( $table );
 823+ $table = $this->tableName( $table, false );
824824 $etable = $this->addQuotes( $table );
825825 $eschema = $this->addQuotes( $schema );
826826 $SQL = "SELECT 1 FROM pg_catalog.pg_class c, pg_catalog.pg_namespace n "
@@ -1002,13 +1002,21 @@
10031003 }
10041004
10051005 if ( isset( $options['GROUP BY'] ) ) {
1006 - $preLimitTail .= ' GROUP BY ' . $options['GROUP BY'];
 1006+ $gb = is_array( $options['GROUP BY'] )
 1007+ ? implode( ',', $options['GROUP BY'] )
 1008+ : $options['GROUP BY'];
 1009+ $preLimitTail .= " GROUP BY {$gb}";
10071010 }
 1011+
10081012 if ( isset( $options['HAVING'] ) ) {
10091013 $preLimitTail .= " HAVING {$options['HAVING']}";
10101014 }
 1015+
10111016 if ( isset( $options['ORDER BY'] ) ) {
1012 - $preLimitTail .= ' ORDER BY ' . $options['ORDER BY'];
 1017+ $ob = is_array( $options['ORDER BY'] )
 1018+ ? implode( ',', $options['ORDER BY'] )
 1019+ : $options['ORDER BY'];
 1020+ $preLimitTail .= " ORDER BY {$ob}";
10131021 }
10141022
10151023 //if ( isset( $options['LIMIT'] ) ) {

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r85907Follow up r85888: Add the parameter to DatabasePostgres.php and DatabaseOracl...platonides18:59, 12 April 2011
r87129* (bug 21196) Article::getContributors() no longer fail on PostgreSQL...ialex18:27, 29 April 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   00:40, 8 June 2011

Rather than calling $db->tableName( $foo, false ) with an opaque parameter and then calling $db->addQuotes() explicitly... consider simply calling $db->tableName( $foo ) and not double-escaping it. :) This will make things simpler and less confusing for future maintainers.

#Comment by Brion VIBBER (talk | contribs)   00:42, 8 June 2011

Hmm.. actually it makes sense, as identifier and *string* escaping may be different here, and you're tossing them into queries.

Proper thing to do is probably to have a clearer constant to use on the second parameter to tableName, as the 'false' is totally unclear. :) Marking OK with a todo.

Status & tagging log