Remove raw SQL from DataList class

[Kevinn1109]

Module version(s) affected

>=5.2.0

Description

From 5.2 onwards, the DataList class contains a raw MySQL query to update the eagerLoadedList:

silverstripe-framework/src/ORM/DataList.php

Lines 1364 to 1373 in cb351be

	$joinRows = DB::query(
	'SELECT * FROM "' . $joinTable
	// Only get joins relevant for the parent list
	. '" WHERE "' . $parentIDField . '" IN (' . implode(',', $parentIDs) . ')'
	// Exclude any children that got filtered out
	. ' AND ' . $childIDField . ' IN (' . $fetchedIDsAsString . ')'
	// Respect sort order of fetched items
	. ' ORDER BY FIELD(' . $childIDField . ', ' . $fetchedIDsAsString . ')'
	);
	}

This query uses the 'FIELD' keyword, which is exclusive to only MySQL and breaks support for any other possible SQL engine. Personally I'm of the opinion that the ORM should be universal for all possible database engines, which is now lost. I'm not sure if this point of view is shared with the SilverStripe team, so I'd like to hear about that before putting in any time fixing it myself.

How to reproduce

FIELD() is exclusive to MySQL and fails in for example Microsoft SQL server.

Possible Solution

One workaround without giving up the ordering is by using PHP rather than SQL to do the ordering. This might come with a minor performance penalty, but that's a little price to pay for universal database engine support.

PRs

• FIX Use ORM functions to perform eager loading instead of raw query #11509

[GuySartorelli]

Thank you for reporting this.
In future please check the relevant boxes in the checklist in the template, don't just remove them. It's not clear for example if you looked to see if a similar issue exists, because you deleted the checkbox.

If you can think of an appropriate way to replace the instance of the FIELD() function in the SQL query without introducing additional queries or filtering in PHP, I'd be happy to look into that option.
Sorting these in PHP rather than using the SQL query to sort it for us isn't really an option.

[Kevinn1109]

I've created a potential workaround to replace the hardcoded query, using the built-in SQLSelect. Because the ordering in the original SQL query is derived from the ManyManyList query, I thought it made sense to copy its OrderBy clause and join the child table to allow using it.

It gets the job done, but one thing I'm not particularly proud of is the rather hacky way to obtain the join table name. It should in theory always be correct, but I would have preferred a more direct approach.