Optimizing Laravel Eloquent multiple joins

Question

The following is working and readable SQL for Eloquent that I have created. The downside is that it looks more like a 1:1 with SQL and I don't think I'm using the full potential of Eloquent. I thought of using scope queries to shorten it, but for some reasons the selectRaw prevents it (due to multiple join variable scopes).

Data is scattered across three tables and modifying the tables is not an option.

Notes:

• $genreIdList is an array of IDs,
• $limit is the max number of results requested,
• $offset is the starting point of the pull,
• selectRaw is necessary as the selected columns are needed.

Here is the original SQL:

SELECT 
b.title, 
GROUP_CONCAT(DISTINCT(g.name) ORDER BY relevancy, g.name ASC SEPARATOR ' ') as genre_list, 
GROUP_CONCAT(DISTINCT CAST(relevancy as char) ORDER BY relevancy ASC SEPARATOR ',') as genre_relevancy, 
bg.book_id 
FROM book_genres bg 
INNER JOIN genre_book g ON bg.genre_id = g.id AND g.id IN (". (($genreIdList) ? $genreIdList : 0) . ") 
INNER JOIN book b ON bg.book_id = b.id
INNER JOIN licensors l ON b.licensor_id = l.id 
WHERE b.status = 'active' AND b.premium = 'yes' AND l.status = 'active' 
GROUP BY bg.book_id

This is the PHP/Eloquent version:

 $relatedBooks = Book::selectRaw("book.title,
 GROUP_CONCAT(DISTINCT(gb.name) ORDER BY bg.relevancy, gb.name ASC SEPARATOR ' ') as genre_list,
 GROUP_CONCAT(DISTINCT CAST(bg.relevancy as char) ORDER BY bg.relevancy ASC SEPARATOR ',') as genre_relevancy,
 bg.book_id ")
 ->join('book_genres AS bg', 'bg.book_id', '=', 'book.id')
 ->join('genre_book AS gb', 'gb.id', '=', 'bg.genre_id')
 ->join('licensors AS l', 'l.id', '=', 'book.licensor_id')
 ->whereIn('gb.id', $genreIdList)
 ->where('book.premium', '=', 'yes')
 ->where('book.status', '=', 'active')
 ->where('l.status', '=', 'active')
 ->take($limit)
 ->offset($offset)
 ->get(); 

• Gufran: It might not help you at all but groupBy('book.id') on Books table ? looks like the Books table has many books with same ID, which is not good in most cases. can you update the post with table structures and sample output you are expecting ?
• azngunit81: @Gufran fixed the groupby - I added the origin SQL (which had the groupby) to the question
• Phrancis: I removed SQL tag. Next time don't tag SQL if you don't want SQL code review, save us database programmers some time.
• azngunit81: @Phrancis It is related to SQL into PHP and it has to do with modeling and using Eloquent's functions to make SQL equivalent so the tag is valid. You just went straight to the sql part when I have stated that the purpose of the review is to make sure that it goes from SQL to Eloquent efficiently

Answer 1

SQL formatting

Your SQL script in my opinion could use some indentation and a few more line breaks.

Variable names

Table & column aliases are useful. But it is a good practice to use a variable name that at least says something about what the table contains.

Consistency

SQL is not case sensitive, but still it is good practice to use one style and stick to it.

Here is the same script that reads much more easily

SELECT 
 Bk.title, 
 GROUP_CONCAT
 (
 DISTINCT (Gnr.name) ORDER BY Gnr.relevancy, 
 Gnr.name ASC 
 SEPARATOR ' '
 ) AS genre_list, 
 GROUP_CONCAT
 (
 DISTINCT CAST(Gnr.relevancy AS CHAR) ORDER BY Gnr.relevancy ASC 
 SEPARATOR ','
 ) AS genre_relevancy, 
 BkGnr.book_id 
FROM book_genres BkGnr 
 INNER JOIN genre_book Gnr 
 ON BkGnr.genre_id = Gnr.id 
 AND Gnr.id IN (". (($genreIdList) ? $genreIdList : 0) . ") 
 INNER JOIN book Bk 
 ON BkGnr.book_id = Bk.id
 INNER JOIN licensors Lic 
 ON Bk.licensor_id = Lic.id 
WHERE Bk.status = 'active' 
 AND Bk.premium = 'yes' 
 AND Lic.status = 'active' 
GROUP BY BkGnr.book_id

SQL functions

This doesn't seem to make sense:

DISTINCT (Gnr.name)
DISTINCT CAST(Gnr.relevancy AS CHAR)

Why would there be duplicate genre name records... wouldn't that defeat the purpose of normalizing into a Genre type of table?

Also I don't think casting as CHAR is a good idea. Use VARCHAR instead. If you know about what the maximum character lenght should be, specify it so RDBMS doesn't have to guess, e.g., VARCHAR(50).

This is a bad idea.

AND Gnr.id IN (". (($genreIdList) ? $genreIdList : 0) . ") 

It is bad practice to concatenate PHP and SQL together. Instead, use prepared statements with PDO.

SQL procedure

This is the PHP/Eloquent version:

This whole section looks messy. You would gain clarity, repeatability and performance by using a SQL stored procedure. I'm going to assume your RDBMS is MySQL as is most common with PHP. Here is what you would execute once in MySQL to create it:

DROP PROCEDURE sp_genreIdList IF EXISTS;
DELIMITER | 
CREATE PROCEDURE sp_genreIdList (IN p_genreIdList VARCHAR) -- assuming you are passing a string type parameter to RDBMS
AS
BEGIN
SELECT 
 Bk.title, 
 GROUP_CONCAT
 (
 DISTINCT (Gnr.name) ORDER BY Gnr.relevancy, 
 Gnr.name ASC 
 SEPARATOR ' '
 ) AS genre_list, 
 GROUP_CONCAT
 (
 DISTINCT CAST(Gnr.relevancy AS CHAR) ORDER BY Gnr.relevancy ASC 
 SEPARATOR ','
 ) AS genre_relevancy, 
 BkGnr.book_id 
FROM book_genres BkGnr 
 INNER JOIN genre_book Gnr 
 ON BkGnr.genre_id = Gnr.id 
 AND Gnr.id IN (". (($genreIdList) ? $genreIdList : 0) . ") 
 INNER JOIN book Bk 
 ON BkGnr.book_id = Bk.id
 INNER JOIN licensors Lic 
 ON Bk.licensor_id = Lic.id 
WHERE Bk.status = 'active' 
 AND Bk.premium = 'yes' 
 AND Lic.status = 'active' 
GROUP BY BkGnr.book_id;
END|
DELIMITER ; --make sure you don't forget that line

Then from PHP you simply:

$sql = "CALL sp_genreIdList("$genreIdList");";

Or something similar. And you can do this from multiple scripts in multiple languages and you will still get consistent results from RDBMS.

• azngunit81: I would not be accepting your answer for the following reasons: 1) as stated the DB itself is FIXED - it was implemented poorly and I have already mentioned IN BOLD that table modifications or suggestion will not be needed. 2) you have provided ZERO Eloquent syntax and purely SQL - I have only provided SQL as a reference for translation as it didn't pertain to the solution that I wanted or needed. the Algorithm itself is fine as it suited the business logic. 3) syntax spacing are irrelevant for my needs as again SQL was only for referencing and nothing to do with the code.
• Phrancis: Suit yourself. PS: No modifications to any tables were suggested.