user3495551
user3495551

Reputation: 43

Unsure about mysql query

I was wondering if this was correct? I have a feeling that it isnt and would like to know the best way to fix it.

$query= mysql_num_rows(mysql_query("SELECT * FROM members WHERE email='$email'"));
while ($row = mysql_fetch_array($query)) {
    $firstname = $row['firstname'];
}

Thank you.

Upvotes: 0

Views: 56

Answers (5)

MrCode
MrCode

Reputation: 64526

You're assigning the number of rows to $query, whereas you should be assigning the return value of mysql_query() because mysql_fetch_array() requires a result identifier as its argument.

Other issues:

  • Usage of the deprecated MySQL library. Consider upgrading to PDO or MySQLi
  • The source of $email is not shown but there may be an SQL Injection vulnerability. Use a prepared statement in PDO or MySQLi to prevent this.
  • Check the return value before you try to fetch rows. If your query failed, you would be passing a boolean to mysql_fetch_array().

Refactored to show the proper logic (but still should not be used because it's deprecated):

$query= mysql_query("SELECT * FROM members WHERE email='" . mysql_real_escape_string($email) . "'");

if($query){ // check the return value
    while ($row = mysql_fetch_array($query)) {
        $firstname = $row['firstname'];
    }
}

MySQLi example using a prepared statement:

$db = new mysqli('localhost', 'user', 'pass', 'dbname');
if($stmt = $db->prepare('SELECT * FROM members WHERE email = ?')){
    $stmt->bind_param('s', $email);
    $stmt->execute();

    if($result = $stmt->get_result()){
        while ($row = $result->fetch_assoc()){
            $firstname = $row['firstname'];
        }
    }
}

Upvotes: 3

Pank
Pank

Reputation: 14258

MySQL:

$query= mysql_query("SELECT * FROM members WHERE email='$email'");
while ($row = mysql_fetch_array($query)) {
    $firstname = $row['firstname'];
}

MySQLi:

$mysqli = new mysqli("localhost", "root_user", "root_password", "database_name");

if ($mysqli->connect_errno) {
    printf("Connect failed: %s\n", $mysqli->connect_error);
    exit();
}

if ($result = $mysqli->query("SELECT * FROM members WHERE email ='$email'")) {        
    while ($row = $result->fetch_array(MYSQLI_ASSOC)){
      $firstname = $row['firstname'];
    }
    $result->close();
}

$mysqli->close();

Upvotes: 2

Skewled
Skewled

Reputation: 783

Please move away from using mysql and start using mysqli or PDO, as the mysql function is depreciated now.

As for your query, here is an example:

$dbc = mysqli_connect(DB_HOST, DB_USER, DB_PASSWORD, DB_NAME); // The Connection

$query = "SELECT * FROM members WHERE email ='$email'"; // MySQL Query
$data = mysqli_query($dbc, $query); // Perform the Query

 $row = mysqli_fetch_array($data); // Work with the data using $row

  $firstname = $row['firstname'];

To display output:

 while($row = mysqli_fetch_array($data)) {

  // do something to echo

 }

Upvotes: 0

Matt
Matt

Reputation: 81

$query= mysql_num_rows(mysql_query("SELECT * FROM members WHERE email='$email'"));

should be

$query=mysql_query("SELECT * FROM members WHERE email='$email'");

Upvotes: 0

B_CooperA
B_CooperA

Reputation: 659

Where do you need that mysql_num_rows? I highly doubt that this will work. I guess you can't combine mysql functions, but can't be sure since I haven't use mysql functions for years and neither should you since they're deprecated. Try using mysqli.

Upvotes: 0

Related Questions