Last modified: 2013-10-23 18:17:23 UTC
In Database.php Select() and SelectRow() accept parameter options as string as well as array but doesn't handle strings well SelectRow() accepts $options as string and does $options['LIMIT'] = 1; and then later selectSQLText() does $options = (array)$options; which is strange. String type should not be supported or handled with care.
public function selectRow( $table, $vars, $conds, $fname = 'DatabaseBase::selectRow', $options = array(), $join_conds = array() ) { $options = (array)$options;
*** This bug has been marked as a duplicate of bug 36568 ***
(In reply to comment #2) > > *** This bug has been marked as a duplicate of bug 36568 *** I don't think its the same bug. The problem I reported is that string shouldn't be accepted as options
Strings are accepted as $options since many versions, doing the opposite would create backward compatibility problems with extensions using that for no benefit at all. The cast to array simply does the following: if ( is_string( $options ) ) { $options = array( $options ); } I don't see where there is a problem with this.
I saw your comment on Gerrit, the point of this is not to allow such constructions, since you cannot pass array( 'ORDER BY `smw` ' ) as $options. This is only intended for already-defined options that do not require a value, see $noKeyOptions in DatabaseBase::makeSelectOptions() (currently lines 1219 to 1262 of includes/db/Database.php on master).
I think its better to depreciate strings, as only no key options when passed as strings are turned into SQL, for ex: options = 'DISTINCT' will be processed into SQL but options = 'ORDER BY `smw`' wouldn't be processed at all. However, the extension code has a wrong notion of this, I identified this only when I saw my output wasn't ORDERed. May be a better solution is to just allow arrays, for options like DISTINCT one could pass array('DISTICT' => true)
isn't array('ORDER BY', 'smw') going to work?
er, array('ORDER BY' => 'smw')
(In reply to comment #8) > er, array('ORDER BY' => 'smw') (In reply to comment #7) > isn't array('ORDER BY', 'smw') going to work? It will, but the docs @param $options string|array give a wrong notion that 'ORDER BY `smw`' will work too.
