Page MenuHomePhabricator

since 3b702fe, list fields can cause sql to break
Closed, ResolvedPublic

Description

Problem introduced by 3b702fe (Changed default sort to be on more than one field)

Prerequisite: one of the sort fields (not the last one) in the query passed to the database object is a list field. Basically setOrderBy() calculated mOrderByStr as

	table.normalField1, table.normalField2, table.normalField3, table.listField, table.normalField4

now, handleVirtualFields() is applied, which replaces table.listField, in

	table.normalField1, table.normalField2, table.normalField3, table.listField, table.normalField4

resulting in

	table.normalField1, table.normalField2, table.normalField3,table.listField__full table.normalField4

which obviously causes the mysql to fail due to the missing comma after table.listField__full

For a solution, I suggest to introduce

				if ( isset( $matches[2] ) && ($matches[2] == ',') ) {
					$replacement .= ',';
				}

into \CargoSQLQuery::handleVirtualFields() after replacement calculation, before preg_replace is executed on line 960ff.

Note: I did neither check (or validate) \CargoSQLQuery::handleVirtualCoordinateFields nor \CargoSQLQuery::handleHierarchyFields for the occurance of this problem.

Event Timeline

Change 416702 had a related patch set uploaded (by Oetterer; owner: Oetterer):
[mediawiki/extensions/Cargo@master] Fix list fields can cause sql to break (ref T189019)

https://gerrit.wikimedia.org/r/416702

Change 416702 merged by jenkins-bot:
[mediawiki/extensions/Cargo@master] Fix list fields can cause sql to break (ref T189019)

https://gerrit.wikimedia.org/r/416702

@Oetterer - sorry for the extremely long delay on this. I just checked in a change to the code to replace $mOrderByStr (a string) with $mOrderBy (an array). So maybe this takes care of any potential problems with comma handling?

Yaron_Koren claimed this task.

Setting to "Resolved" - feel free to re-open if this is still an issue.