Page MenuHomePhabricator

SQL errors with corrupted table name aliases Database::select() generated queries
Closed, DeclinedPublic

Description

Author: cnit

Description:
My extension has the following PHP code:

function getIntervalResults( $offset, $limit ) {

		$result = array();
		$db = & wfGetDB( DB_SLAVE );
		$qp_users = $db->tableName( 'qp_users' );
		$qp_users_polls = $db->tableName( 'qp_users_polls' );
		$res = $db->select( "$qp_users_polls qup, $qp_users qu",
			array( 'qu.uid as uid', 'name as username', 'count(pid) as pidcount' ),
			'qu.uid=qup.uid',
			__METHOD__,
			array( 'GROUP BY' => 'qup.uid',
						'ORDER BY' => $this->order_by,
						'OFFSET' => intval( $offset ),
						'LIMIT' => intval( $limit ) )
		);
		while ( $row = $db->fetchObject( $res ) ) {
			$result[] = $row;
		}
		return $result;

}

It used to work with 1.15, works with 1.17 (right now), however with current trunk it produces the following broken query (copypasted from log):

SELECT /* qp_UsersList::getIntervalResults QuestPC */ qu.uid as uid,name as username,count(pid) as pidcount FROM wiki_wiki_qp_users_polls` qup, wiki_qp_users qu` WHERE qu.uid=qup.uid GROUP BY qup.uid ORDER BY count(pid) DESC, name ASC LIMIT 50

As you probably see, the FROM arguments are corrupted:
FROM wiki_wiki_qp_users_polls` qup, wiki_qp_users qu`

especially wiki_wiki_qp_users_polls`

which causes:

(SQL запрос скрыт)

произошёл из функции «qp_UsersList::getIntervalResults». База данных возвратила ошибку «1064: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ' qup, wiki_qp_users` qu` WHERE qu.uid=qup.uid GROUP BY qup.uid ORDER BY coun' at line 1 (127.0.0.1)».


Version: 1.20.x
Severity: normal

Details

Reference
bz31534

Event Timeline

bzimport raised the priority of this task from to Low.Nov 21 2014, 11:58 PM
bzimport set Reference to bz31534.
bzimport added a subscriber: Unknown Object (MLST).

Try using some somewhat up to date code? ie:

function getIntervalResults( $offset, $limit ) {

$result = array();
$db = wfGetDB( DB_SLAVE );
$res = $db->select(
    array( 'qp_users_polls', 'qu' => 'qp_users' ),
    array( 'qu.uid as uid', 'name as username', 'count(pid) as pidcount' ),
    'qu.uid=qup.uid',
    __METHOD__,
    array( 'GROUP BY' => 'qup.uid',
        'ORDER BY' => $this->order_by,
        'OFFSET' => $offset,
        'LIMIT' => $limit
    )
);
foreach( $res as $row ) {
    $result[] = $row;
}
return $result;

}

questpc wrote:

May I ask you to explain what's wrong with my code? I decided to convert manually built SQL strings with db->execute() to complex selects. The original source code fragment is:

		$qp_users_polls = self::$db->tableName( 'qp_users_polls' );
		$qp_users = self::$db->tableName( 'qp_users' );
		$query = "SELECT qup.uid AS uid, name AS username, short_interpretation, long_interpretation, structured_interpretation " .
				"FROM $qp_users_polls qup " .
				"INNER JOIN $qp_users qu ON qup.uid = qu.uid " .
				"WHERE pid = " . intval( $this->pid ) . " " .
				"LIMIT " . intval( $offset ) . ", " . intval( $limit );
		$res = self::$db->query( $query, __METHOD__ );
		$result = array();
		while ( $row = self::$db->fetchObject( $res ) ) {
			$interpResult = new qp_InterpResult();
			$interpResult->short = $row->short_interpretation;
			$interpResult->long = $row->long_interpretation;
			$interpResult->structured = $row->structured_interpretation;
			$result[intval( $row->uid )] = array(
				'username' => $row->username,
				'interpretation' => $interpResult
			);
		}

I've converted it to:

		$res = self::$db->select(
			array( 'qu' => 'qp_users', 'qup' => 'qp_users_polls' ),
			array( 'qup.uid AS uid', 'name AS username', 'short_interpretation', 'long_interpretation', 'structured_interpretation' ),
			/* WHERE */ 'pid = ' . intval( $this->pid ),
			__METHOD__,
			array( 'OFFSET' => $offset, 'LIMIT' => $limit ),
			/* JOIN */ array(
				'qu' => array( 'INNER JOIN', 'qup.uid = qu.uid' )
			)
		);
		$result = array();
		foreach ( $res as $row ) {
			$interpResult = new qp_InterpResult();
			$interpResult->short = $row->short_interpretation;
			$interpResult->long = $row->long_interpretation;
			$interpResult->structured = $row->structured_interpretation;
			$result[intval( $row->uid )] = array(
				'username' => $row->username,
				'interpretation' => $interpResult
			);
		}

which produces the following invalid query (in 1.17, my primary target for development):

Обнаружена ошибка синтаксиса запроса к базе данных. Это может означать ошибку в программном обеспечении. Последний запрос к базе данных:

SELECT qup.uid AS uid,name AS username,short_interpretation,long_interpretation,structured_interpretation FROM `wiki_qp_users_polls` 'qup' INNER JOIN `wiki_qp_users` 'qu' ON ((qup.uid = qu.uid)) WHERE pid = 7 LIMIT 20

произошёл из функции «qp_PollStore::pollVotersPager». База данных возвратила ошибку «1064: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ''qup' INNER JOIN wiki_qp_users 'qu' ON ((qup.uid = qu.uid)) WHERE pid = 7 LI' at line 1 (127.0.0.1)».

They Database class built query is identical to manually built query, except the Database class wraps table aliases into single quotes. Original query works.

Also, there are two more questions:

  1. Is it safe to do not intval() 'LIMIT' and 'OFFSET' options values?
  2. What if $res returned by query is not iterable object, how bad foreach() will fail? Isn't Database::fetchObject() more error-prone?

questpc wrote:

The first error (in 1.19) disappeared when I changed my code following by Reedy recommendations (tweaking a bit). The last error disappeared when I've updated my farm's code to 1.17 release from some earlier SVN snapshot.

The first error is probably valid, however manifests itself only when using old and discouraging coding style. I don't know whether should I close this error or not, leaving it up to you.