Re: DB_mysql Bugs?

Vinai Kopp wrote: > > Hi, > > I've started to change a couple of scripts to make use of the PEAR DB > classes. > > I've bumped into the following Bugs(/Features I don't understand). > Please be patient with me :) > I'm using the current cvs revisions: > > A.) In the File "DB/mysql.php": > > 1.) In the function connect() > > $connect_function = $persistent ? "mysql_pconnect" : > "mysql_connect"; > > if ($dbhost && $user && $pw) { > $conn = $connect_function($dbhost, $user, $pw); > } elseif ($dbhost && $user) { > $conn = $connect_function($dbhost, $user); > } elseif ($dbhost) { > $conn = $connect_function($dbhost); > } else { > $conn = false; > } > > There seem a couple of @'s missing before the calls to $connect($dbhost, > $user). Are they left out on purpose? If so, please enlighten me why. I > suggest to add an @ before every call to $connect_function. > > 2.) In the function fetchRow() > if (!$row) { > $errno = mysql_errno($this->connection); > > if (!$errno) { > return $this->raiseError('end of results'); > } > > return $this->mysqlRaiseError($errno); > } > Why raise an error "end of results" instead of returning NULL? In the > comments before the function declaration it says that would be the > return value "if there is no more data". The pgsql DB class does seems > to return NULL if there are no more results. I don't think it should be > considered an error if there is no more data to be returned from a > select query. I suggest to return NULL instead. > > B.) In the File "DB.php": > > 1.) This one is close to the preceding one. The way I see it a bug was > introduced in Revision 1.44 of DB.php. > The function isError now returns true if NULL is passed as an argument. > That way it's not possible to do > if( DB::isError( $var ) ) print $var->getMessage(); > anymore without doing another is_object or something first. Also, it > would make it impossible to return NULL if no more data can be fetched > from a query in fetchRow and similar functions. I suggest to change it > back. > > Does it make sense to change the above statements how I suggested? > > Since I will be working a lot with the PEAR mysql and postgresql > classes, what are the policies for granting a cvs account, and what is > the procedure to request one? This was in fact changed by Chuck a couple of weeks ago. I missed the diff, but I would like to see some discussion on subtile API changes like this _before_ implementing them. Chuck, care to comment? :-) - Stig