Re: [PHP-PEAR] 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