0

This function gives me an infinite loop

function getCats($parent,$level){
    // retrieve all children of $parent
    $result = "";
    $query = "SELECT title,parent_id from t_cats where parent_id = '$parent'";

    if($rs = C_DB::fetchRecordset($query)){
        while($row = C_DB::fetchRow($rs)){
            $result .= str_repeat($parent,$level).$row['title']."\n";
            getCats($row['parent_id'],$level+1);
        }
    }

    return $result;
} 

here is my db table

CREATE TABLE  `db`.`t_cats` (
  `ID` int(10) unsigned NOT NULL auto_increment,
  `datasource_id` int(10) unsigned default '0',
  `version` char(10) character set latin1 default 'edit',
  `status` char(10) character set latin1 default 'new',
  `modified_date` datetime default NULL,
  `modified_by` int(10) unsigned default '0',
  `title` char(255) character set latin1 default NULL,
  `parent_id` int(11) default NULL,
  PRIMARY KEY  (`ID`),
  KEY `idx_datasource_id` (`datasource_id`)
) ENGINE=MyISAM AUTO_INCREMENT=50 DEFAULT CHARSET=utf8;

I just want to be able to get my list of categories recursive.

But what am i doing wrong?


EDIT:

function getCats($parent,$level){
                // retrieve all children of $parent
                $result ="";
                $query = "SELECT title,parent_id from t_cats where parent_id = '$parent'";
                if($rs = C_DB::fetchRecordset($query)){
                    while($row = C_DB::fetchRow($rs)){
                        $result.= str_repeat($parent,$level).$row['title']."\n";
                        getCats($row['id'],$level + 1   );
                    }
                }

                return $result;
    } 
sanders
  • 10,794
  • 27
  • 85
  • 127

6 Answers6

5

This line looks wrong:

getCats($row['parent_id'],$level+1);

You should be calling it with the current child ID - at the moment you're calling it with the same ID over and over. Something like this (you need to select the id from your table):

getCats($row['id'], $level + 1);

Edit: you need to update your query to select id:

$query = "SELECT id, title, parent_id from t_cats where parent_id = '$parent' AND id != parent_id";

I've also added a bit to stop it getting into a loop if an item is its own parent.

Greg
  • 316,276
  • 54
  • 369
  • 333
  • well: this: getCats($row['parent_id'],$level++); also ends up in an infinite loop. – sanders Aug 19 '09 at 15:14
  • This is the correct answer. Apologies for my quick-draw response and not fully reading the code. – chsh Aug 19 '09 at 15:14
  • sorry but still infinite loop. – sanders Aug 19 '09 at 15:16
  • the `id != parent_id` is good, but actually, this should somehow be controlled upon insertion ... – back2dos Aug 19 '09 at 15:52
  • Thanks for the reaction I have decided to try the modified preorder tree traversel model. This method seems to be better for the database and less heavy for the application. Thanks anyway! – sanders Aug 21 '09 at 08:00
2

I found this SitePoint article on "Storing Hierarchical Data in a Database" very helpful. It's all PHP examples, and it will improve the performance of what you're trying to do dramatically.

lo_fye
  • 6,790
  • 4
  • 33
  • 49
1

Maybe one of the items in the db has itself as parent?

Simon Groenewolt
  • 10,607
  • 1
  • 36
  • 64
0

I don't know C_DB, but I'd bet that the $rs returned by fetchrecordset is a reference, which means that every invocation of getCats is using the same $rs. Exactly what it will do then is unpredictable, depending on how fetchRow is implemented.

If you want to do this (and recursive closures are a pain in SQL, I know), you should open a new connection inside getCats. and be using a separate connection for each access.

Colin Fine
  • 3,334
  • 1
  • 20
  • 32
0

correct answer provided by greg ...

2 side notes:

  1. in the case of infinite loops, track recursion depth (you can conveniently use $level here) or overall invocation count (if you are lazy, since this is a oneliner accessing a global counter), and terminate recursion with an exception, when it reaches a maximum value (in general, 10 is already enough to see the problem, but of course that may vary) ... and then get some debug output ... for example $query or something like "calling getCats($parent,$level)" ... would've shown you the problem in no time in this case ... :)

  2. you should minimize the amount of queries ... traversing a tree like that is quite inefficient ... especially, if the database is on another machine ...

greetz

back2dos

back2dos
  • 15,588
  • 34
  • 50
-1

Erm shouldnt it be:

$query = "SELECT title,parent_id from t_cats where id = '$parent'";

And not:

$query = "SELECT title,parent_id from t_cats where parent_id = '$parent'";
Owen
  • 6,992
  • 7
  • 44
  • 77
  • Doesn't make sense to me. If your retrieving the parent id then querying the row based upon it it will obviously be recursive. i.e. if I do select * From a where Id = 1 then retrieve that id back and continually call select * From a where Id = 1 ill get the same row every time. This seems to be whats happening, just with parent id. Im tired so maybe im missing something. – Owen Aug 19 '09 at 15:51