r100280 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r100279‎ | r100280 | r100281 >
Date:21:21, 19 October 2011
Author:aaron
Status:ok (Comments)
Tags:
Comment:
Code cleanups & fixed strange comma
Modified paths:
  • /trunk/phase3/includes/Revision.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Revision.php
——@@ -297,7 +297,7 @@
298298 'rev_text_id',
299299 'rev_timestamp',
300300 'rev_comment',
301 - 'rev_user_text,'.
 301+ 'rev_user_text',
302302 'rev_user',
303303 'rev_minor_edit',
304304 'rev_deleted',
——@@ -397,8 +397,9 @@
398398 $this->mTitle = null; # Load on demand if needed
399399 $this->mCurrent = false;
400400 # If we still have no len_size, see it we have the text to figure it out
401 - if ( !$this->mSize )
 401+ if ( !$this->mSize ) {
402402 $this->mSize = is_null( $this->mText ) ? null : strlen( $this->mText );
 403+ }
403404 } else {
404405 throw new MWException( 'Revision constructor passed invalid row format.' );
405406 }

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r20291add Revision::selectFields(): returns a list of fields that should be SELECTe...river15:29, 9 March 2007

Comments

#Comment by Krinkle (talk | contribs)   02:05, 27 December 2011

Wow, it's been in there since r20291 !

#Comment by Krinkle (talk | contribs)   02:07, 27 December 2011

Weird, one would expect that to raise an error of some kind by either PHP (for the loose period) or MySQL for the invalid column name.

#Comment by Reedy (talk | contribs)   02:13, 27 December 2011

Well, no, you're using 'rev_user_text,rev_user' which you're imploding to a string... so it'll get ", " before it, and the same after (if there are more elements

Certainly, a "bug" that doesn't actually change anything. Sneaky

#Comment by Krinkle (talk | contribs)   02:19, 27 December 2011

Ah, right, I didn't read it like that 

array(
 			'rev_comment',
			'rev_user_text,'.
 			'rev_user',
 			'rev_minor_edit',
);

is just 

array(
 			'rev_comment',
			'rev_user_text,'.'rev_user',
 			'rev_minor_edit',
);

so two columns in one array item.

This error would've been caught if column names were quoted or back-ticked in the resulting SQL query

#Comment by Reedy (talk | contribs)   02:33, 27 December 2011

Or if we deleted one of the two columns.

It isn't blatantly obvious, though it is rather sneaky

#Comment by Reedy (talk | contribs)   02:07, 27 December 2011

Yay for valid sytnax....

Status & tagging log