0

Disclaimer: I am aware that I have in my code deprecated mysql functions. That is on my todo list.

I have a MySql select giving me seasons for diferent items in a house booking system.

Kind of:

Low season: 2010-01-01, 2010-03-01, 100 //meaning start,end,price

This comes in my first sql:

while($season_row=mysql_fetch_assoc($season_res)){
    $seasonsArray[$season_row['id_item']][] = array(
        $season_row['season_start'], 
        $season_row['season_end'],
        $season_row['daily_price']
    );
}

The dates are defined here (arriving to the function as YYYY-mm-dd):

function seasonPrice($from,$to,$requested_item){

    $start = round(strtotime($from)/86400)*86400; // like 2008-01-01
    $end = round(strtotime($to)/86400)*86400;     // to 2015-01-01

    $formattedStart = date('Y-m-d', $start);
    $formattedEnd   = date('Y-m-d', $end);

Now I need to loop between 2 dates, between the items of the $seasonsArray and then check the price of that item in that specific day.

I did this with:

foreach($seasonsArray as $item=>$value){            
    for( $thisDay = $start; $thisDay < $end; $thisDay = $thisDay + 86400){

        foreach($value as $innerValue){     
            $season_start = roundToSeconds($innerValue[0]);
            $season_end = roundToSeconds($innerValue[1]);
            if($thisDay >= $season_start && $thisDay <= $season_end) {  
                $foundPrice[] = round($innerValue[2]);
            }
        }

        $thisSerie[] = array($thisDay * 1000, isset($foundPrice) ? $foundPrice[0] : 0);

        // security check to avoid double assigned seasons to same day
        if(count($foundPrice) > 1){ die('There is double bookings in item: '.$item);}   

        unset($foundPrice);
    }
    $seasonPrices[] = array(
        'data'=> $thisSerie,
        'label'=> 'House ID: '.$item, 
    );  
}

But I get: Fatal error: Allowed memory size of 100663296 bytes exhausted

Any suggestion on where my code can be improved to not need so much memory? Or is there a bug and I don't see it?

Rikard
  • 7,485
  • 11
  • 55
  • 92
  • what are you doing with the seasons array? maybe you can already do that with the sql query or at least part of that to simplify it, 3 nested for look like something bad to do – arieljuod Jan 02 '14 at 23:13
  • @arieljuod the seasons array is just to collect data from the sql query. Each house item has many seasons, like 4/5 per year with a start a end and a price for day during that period. Agree with 3loops = no good, but don't see a better way... – Rikard Jan 02 '14 at 23:18
  • can you post an example of the data you get from the database and the data you want at the end of the loops? it's not clea what you want to do with those loops, maybe an example can help. i'm guessing that yo can simplify the loop doing a better query but i don't understand what you want at the end – arieljuod Jan 02 '14 at 23:25
  • @arieljuod, sure. Let me knows if this helps: http://jsfiddle.net/beNUS/show/ - and thank you for looking at this. – Rikard Jan 02 '14 at 23:38
  • the unclear part is what you want to do with that data, that loop is inside seasonPrice function? what does seasonPrice return? what do you expect as output with that input? – arieljuod Jan 04 '14 at 01:33

2 Answers2

2

I'd generate a range of days and join against your seasons table, and use a single query to get the desired resulset, e.g.:

SELECT dates.Date,
       coalesce(s.price, 0) AS price
FROM
  (SELECT a.Date
   FROM
     ( SELECT curdate() - INTERVAL (a.a + (10 * b.a) + (100 * c.a)) DAY AS Date, '0' AS price
      FROM
        (SELECT 0 AS a
         UNION ALL SELECT 1
         UNION ALL SELECT 2
         UNION ALL SELECT 3
         UNION ALL SELECT 4
         UNION ALL SELECT 5
         UNION ALL SELECT 6
         UNION ALL SELECT 7
         UNION ALL SELECT 8
         UNION ALL SELECT 9) AS a
      CROSS JOIN
        (SELECT 0 AS a
         UNION ALL SELECT 1
         UNION ALL SELECT 2
         UNION ALL SELECT 3
         UNION ALL SELECT 4
         UNION ALL SELECT 5
         UNION ALL SELECT 6
         UNION ALL SELECT 7
         UNION ALL SELECT 8
         UNION ALL SELECT 9) AS b
      CROSS JOIN
        (SELECT 0 AS a
         UNION ALL SELECT 1
         UNION ALL SELECT 2
         UNION ALL SELECT 3
         UNION ALL SELECT 4
         UNION ALL SELECT 5
         UNION ALL SELECT 6
         UNION ALL SELECT 7
         UNION ALL SELECT 8
         UNION ALL SELECT 9) AS c) a
   WHERE a.Date BETWEEN '$from' AND '$to'
   ORDER BY a.Date) dates
LEFT JOIN seasons s ON dates.Date BETWEEN s.start AND s.END

The complicated inner query avoids the creation of a temp table (taken from generate days from date range) and works for up to 1000 days, but creating a temp table would be fine.

Community
  • 1
  • 1
JRL
  • 76,767
  • 18
  • 98
  • 146
  • Thanks for looking at this! I've been looking at your answer and trying to understand it :) I feel your answer will upgrade my knowledge when I fully understand it. If you care to add some somments on the code great. Anyway, I'm really happy to have this suggestion to study and to learn with. Thanks! – Rikard Jan 04 '14 at 10:32
0

It looks to be like you are never leaving the for loop. Where do $start and $end get set. Verify their values by printing them out.

As far as optimization, there is no need to be looping through each day. Skip the for loop that counts days and use $season_start and $season_end to calculate the day in the second foreach loop.

In fact, there is a bug right now unless $session_start and $session_end are always more than a day apart, because sometime the event will happen in between the 24 hour periods you are looping by.

Damien Black
  • 5,579
  • 18
  • 24
  • Thanks for looking at this. I don't see how I can skip the day-loop. The seasons I get from my sql have only day start and end, like 3/4 month each season. So 4/5 per year with a start a end and a price for day during that period. – Rikard Jan 02 '14 at 23:19