Is this an inefficient way to build a query result for a view

[eluser]someoneinomaha[/eluser]
Hello.

I'm working on a band calendar that has the following structure:

show information (date, description, etc)
venue information (a show has one venue)
band information (a show can have multiple bands)

The method I'm using right now is to pull back all of the records within a certain date range - left joining the bands. This results is duplicate rows because there can be many bands to a show.

So what I've opted to do is return the rows from the query and build an array. That way in the view, I can loop through the array without removing duplicates. I think this will work just fine, but I'm wondering if this is a really inefficient way to do it.

My model method looks like this:

Code:

function get_music_shows($start_date, $end_date, $status_id)
    {
       $return_value = array();
       $sql = 'SELECT s.*, v.id as venue_id, v.name as venue_name, b.id as band_id, b.name as band_name
               FROM shows s
               INNER JOIN venues v on s.venue_id = v.id
               LEFT JOIN bands_shows bs on s.id = bs.show_id
               LEFT JOIN bands b on bs.band_id = b.id
               WHERE (s.show_date >= ? AND s.show_date <= ?) AND (s.showtype_id = 1) AND (s.status_id = ?)
               ORDER BY s.show_date';
       $query = $this->db->query($sql, array($start_date, $end_date, $status_id));
       $result = $query->result();
       $show_id = 0;
       foreach($result as $row)
       {
           if ($show_id != $row->id)
           {
               $show_id = $row->show_id;
               $row_value = array('show_id' => $row->id, 'venue_id' => $row->venue_id, 'cost' => $row->cost, 'ages' => $row->ages,
                                  'time' => $row->time, 'details' => $row->details, 'show_date' => $row->show_date,
                                  'venue_name' => $row->venue_name, 'venue_id' => $row->venue_id);
               $return_value[$show_id] = $row_value;
           }
           $return_value[$show_id]['bands'][] = array('band_id' => $row->band_id, 'band' => $row->band_name);
       }
       return $return_value;
    }

If anyone has feedback on this, I'd really appreciate it.

Thanks for your time.

[eluser]web_developer[/eluser]
why are you not returning result in array format?

return $query->result_array();

No need to run for loop

[eluser]mddd[/eluser]
You could use the GROUP_CONCAT function of MySQL to return 1 row for each show, and get a single field containing all the band names (and another field if you want the band ids too).

Something like

Code:

SELECT s.*, v.id, v.name, GROUP_CONCAT(b.name) band_names, GROUP_CONCAT(b.id) band_ids
FROM shows s LEFT JOIN venues v LEFT JOIN bands b
GROUP BY s.id

(this is not correct code but a simplification to show the idea)

Now you won't have to loop through the results to get the bands for each show. Just use explode() to get the list of bands from $row['band_names'].

[eluser]sophistry[/eluser]
GROUP_CONCAT is clever but has limitation: 1024 char max (default can be increased). so, if your band names are really long or you have lots of bands your GROUP_CONCAT string will be cut off and there will be no error.

caveat emptor.

[eluser]someoneinomaha[/eluser]
Thanks for the idea and for the warning on the limitation.

Another thing that isn't clear to me here is how I would match the band with the id using the GROUP_CONCAT() function.

Can I take it from your suggestions that what I originally posted is a pretty inefficient way to accomplish what I'm trying to do?

I was thinking that there are really only two ways to do this:

1) Do a join and build the array - eliminating the duplicate rows.
2) Do a query for each show to get the bands.

I figured that hitting the database for each show was not a great idea, so I went the other direction.

I have read the result_array() documentation, but I thought that would just give me an array that would be identical to the database results, which isn't getting me further to my goal (I don't think).

Thanks again for the helpful replies!

[eluser]rogierb[/eluser]
Hi,

There is a third option.

Why create an array if you have all the info inside the result object?
Just loop through it and only display the things you want from a record.

If there is a unique show+venue part, echo it, otherwise echo the band info

Maybe less clean but a lot faster.

[eluser]mddd[/eluser]
I wouldn't say it is super inefficient. You will always have to do some processing in php to build what you want.
I totally agree that firing a query for each show IS inefficient. So that leaves you with combining things in mysql (using group functions) or checking things out in php like you are doing. I don't think the option you made is bad. Processing information in php is going to be much faster than using a lot of queries, on most servers.

[eluser]someoneinomaha[/eluser]
rogierb -

I had thought about that as well. My thought for creating the array in the model was that the view would be so much less complex that way.

But if I am taking a significant performance hit this way, maybe it does make sense to run all of the logic through the view.

Does anyone else have an opinion on this?

[eluser]mddd[/eluser]
The logic inside the view won't be very different from the logic you have in the model now. The main difference would that you are not copying the information from the database result to a new array. So you will could maybe save some memory there. But I would keep the logic in the model. It is more transparent. If you make database changes, you don't want that to have a direct impact on what goes on in the view. Better make things nice in the model and use the view just for outputting it all.