Skip to main content
Fixed SQL
Source Link
200_success
  • 144.2k
  • 22
  • 188
  • 473

SQL

If you want a count, ask for a count. Don't ask the server to send all the data for all the rows, just to have the client count the rows. (A few hundred rows is a trivial amount of data for a database to handle, but still, you should follow good practices.)

SELECT SUM(*inactive_count) AS obj_countinactive_count
    FROM (
        SELECT COUNT(article_active) AS inactive_count
            FROM wcx_articles WHERE article_active = '0'
        UNION ALL
        SELECT COUNT(video_active)
            FROM wcx_videos WHERE video_active = '0'
        UNION ALL
        SELECT COUNT(app_active)
            FROM wcx_apps WHERE app_active = '0'
        UNION ALL
        SELECT COUNT(link_active)
            FROM wcx_links WHERE link_active = '0'
    ) AS wcs_objects;

Schema

If in fact all four of your tables are union-compatible with each other, then you should strongly consider merging them into one table with a wcx_type attribute that declares what type of object that row represents.

Are your …_active columns really strings? Those columns should be of the BOOLEAN (i.e., TINYINT(1)) type. The query should be adjusted accordingly: use …_active = 0 instead of …_active = '0'.

PHP

You reused the $query parameter to store the result of the query. Don't be stingy like that — it's confusing. Use a new variable to store the result.

You have a leak: mysqli_free_result( $query ); never gets called, because that statement is unreachable.

You are mixing the object-oriented and procedural styles of . You should either use the object-oriented style consistently…

public function num_rows($query) {
    if (!($result = $this->link->query($query))) {
        $this->log_db_errors($this->link->error, $result, 'Fatal');
        return $this->link->error;
    }
    $num_rows = $result->num_rows;
    $result->free();
    return $num_rows;
}

or use the procedural style consistently…

public function num_rows($query) {
    if (!($result = mysqli_query($this->link, $query))) {
        $this->log_db_errors(mysqli_error($this->link), $result, 'Fatal');
        return mysqli_error($this->link);
    }
    $num_rows = mysqli_num_rows($result);
    mysqli_free_result($result);
    return $num_rows;
}

SQL

If you want a count, ask for a count. Don't ask the server to send all the data for all the rows, just to have the client count the rows. (A few hundred rows is a trivial amount of data for a database to handle, but still, you should follow good practices.)

SELECT SUM(*) AS obj_count
    FROM (
        SELECT COUNT(article_active)
            FROM wcx_articles WHERE article_active = '0'
        UNION ALL
        SELECT COUNT(video_active)
            FROM wcx_videos WHERE video_active = '0'
        UNION ALL
        SELECT COUNT(app_active)
            FROM wcx_apps WHERE app_active = '0'
        UNION ALL
        SELECT COUNT(link_active)
            FROM wcx_links WHERE link_active = '0'
    ) AS wcs_objects;

Schema

If in fact all four of your tables are union-compatible with each other, then you should strongly consider merging them into one table with a wcx_type attribute that declares what type of object that row represents.

Are your …_active columns really strings? Those columns should be of the BOOLEAN (i.e., TINYINT(1)) type. The query should be adjusted accordingly: use …_active = 0 instead of …_active = '0'.

PHP

You reused the $query parameter to store the result of the query. Don't be stingy like that — it's confusing. Use a new variable to store the result.

You have a leak: mysqli_free_result( $query ); never gets called, because that statement is unreachable.

You are mixing the object-oriented and procedural styles of . You should either use the object-oriented style consistently…

public function num_rows($query) {
    if (!($result = $this->link->query($query))) {
        $this->log_db_errors($this->link->error, $result, 'Fatal');
        return $this->link->error;
    }
    $num_rows = $result->num_rows;
    $result->free();
    return $num_rows;
}

or use the procedural style consistently…

public function num_rows($query) {
    if (!($result = mysqli_query($this->link, $query))) {
        $this->log_db_errors(mysqli_error($this->link), $result, 'Fatal');
        return mysqli_error($this->link);
    }
    $num_rows = mysqli_num_rows($result);
    mysqli_free_result($result);
    return $num_rows;
}

SQL

If you want a count, ask for a count. Don't ask the server to send all the data for all the rows, just to have the client count the rows. (A few hundred rows is a trivial amount of data for a database to handle, but still, you should follow good practices.)

SELECT SUM(inactive_count) AS inactive_count
    FROM (
        SELECT COUNT(article_active) AS inactive_count
            FROM wcx_articles WHERE article_active = '0'
        UNION ALL
        SELECT COUNT(video_active)
            FROM wcx_videos WHERE video_active = '0'
        UNION ALL
        SELECT COUNT(app_active)
            FROM wcx_apps WHERE app_active = '0'
        UNION ALL
        SELECT COUNT(link_active)
            FROM wcx_links WHERE link_active = '0'
    ) AS wcs_objects;

Schema

If in fact all four of your tables are union-compatible with each other, then you should strongly consider merging them into one table with a wcx_type attribute that declares what type of object that row represents.

Are your …_active columns really strings? Those columns should be of the BOOLEAN (i.e., TINYINT(1)) type. The query should be adjusted accordingly: use …_active = 0 instead of …_active = '0'.

PHP

You reused the $query parameter to store the result of the query. Don't be stingy like that — it's confusing. Use a new variable to store the result.

You have a leak: mysqli_free_result( $query ); never gets called, because that statement is unreachable.

You are mixing the object-oriented and procedural styles of . You should either use the object-oriented style consistently…

public function num_rows($query) {
    if (!($result = $this->link->query($query))) {
        $this->log_db_errors($this->link->error, $result, 'Fatal');
        return $this->link->error;
    }
    $num_rows = $result->num_rows;
    $result->free();
    return $num_rows;
}

or use the procedural style consistently…

public function num_rows($query) {
    if (!($result = mysqli_query($this->link, $query))) {
        $this->log_db_errors(mysqli_error($this->link), $result, 'Fatal');
        return mysqli_error($this->link);
    }
    $num_rows = mysqli_num_rows($result);
    mysqli_free_result($result);
    return $num_rows;
}
Added OOP vs. procedural implementations
Source Link
200_success
  • 144.2k
  • 22
  • 188
  • 473

SQL

If you want a count, ask for a count. Don't ask the server to send all the data for all the rows, just to have the client count the rows. (A few hundred rows is a trivial amount of data for a database to handle, but still, you should follow good practices.)

SELECT SUM(*) AS obj_count
    FROM (
        SELECT COUNT(article_active)
            FROM wcx_articles WHERE article_active = '0'
        UNION ALL
        SELECT COUNT(video_active)
            FROM wcx_videos WHERE video_active = '0'
        UNION ALL
        SELECT COUNT(app_active)
            FROM wcx_apps WHERE app_active = '0'
        UNION ALL
        SELECT COUNT(link_active)
            FROM wcx_links WHERE link_active = '0'
    ) AS wcs_objects;

Schema

If in fact all four of your tables are union-compatible with each other, then you should strongly consider merging them into one table with a wcx_type attribute that declares what type of object that row represents.

Are your …_active columns really strings? Those columns should be of the BOOLEAN (i.e., TINYINT(1)) type. The query should be adjusted accordingly: use …_active = 0 instead of …_active = '0'.

PHP

You reused the $query parameter to store the result of the query. Don't be stingy like that — it's confusing. Use a new variable to store the result.

You have a leak: mysqli_free_result( $query ); never gets called, because that statement is unreachable.

You are mixing the object-oriented and procedural styles of . Be consistent. You should either use the object-oriented style consistently…

public function num_rows($query) {
    if (!($result = $this->link->query($query))) {
        $this->log_db_errors($this->link->error, $result, 'Fatal');
        return $this->link->error;
    }
    $num_rows = $result->num_rows;
    $result->free();
    return $num_rows;
}

or use the procedural style consistently…

public function num_rows($query) {
    if (!($result = mysqli_query($this->link, $query))) {
        $this->log_db_errors(mysqli_error($this->link), $result, 'Fatal');
        return mysqli_error($this->link);
    }
    $num_rows = mysqli_num_rows($result);
    mysqli_free_result($result);
    return $num_rows;
}

SQL

If you want a count, ask for a count. Don't ask the server to send all the data for all the rows, just to have the client count the rows. (A few hundred rows is a trivial amount of data for a database to handle, but still, you should follow good practices.)

SELECT SUM(*) AS obj_count
    FROM (
        SELECT COUNT(article_active)
            FROM wcx_articles WHERE article_active = '0'
        UNION ALL
        SELECT COUNT(video_active)
            FROM wcx_videos WHERE video_active = '0'
        UNION ALL
        SELECT COUNT(app_active)
            FROM wcx_apps WHERE app_active = '0'
        UNION ALL
        SELECT COUNT(link_active)
            FROM wcx_links WHERE link_active = '0'
    ) AS wcs_objects;

Schema

If in fact all four of your tables are union-compatible with each other, then you should strongly consider merging them into one table with a wcx_type attribute that declares what type of object that row represents.

Are your …_active columns really strings? Those columns should be of the BOOLEAN (i.e., TINYINT(1)) type. The query should be adjusted accordingly: use …_active = 0 instead of …_active = '0'.

PHP

You reused the $query parameter to store the result of the query. Don't be stingy like that — it's confusing. Use a new variable to store the result.

You are mixing the object-oriented and procedural styles of . Be consistent.

SQL

If you want a count, ask for a count. Don't ask the server to send all the data for all the rows, just to have the client count the rows. (A few hundred rows is a trivial amount of data for a database to handle, but still, you should follow good practices.)

SELECT SUM(*) AS obj_count
    FROM (
        SELECT COUNT(article_active)
            FROM wcx_articles WHERE article_active = '0'
        UNION ALL
        SELECT COUNT(video_active)
            FROM wcx_videos WHERE video_active = '0'
        UNION ALL
        SELECT COUNT(app_active)
            FROM wcx_apps WHERE app_active = '0'
        UNION ALL
        SELECT COUNT(link_active)
            FROM wcx_links WHERE link_active = '0'
    ) AS wcs_objects;

Schema

If in fact all four of your tables are union-compatible with each other, then you should strongly consider merging them into one table with a wcx_type attribute that declares what type of object that row represents.

Are your …_active columns really strings? Those columns should be of the BOOLEAN (i.e., TINYINT(1)) type. The query should be adjusted accordingly: use …_active = 0 instead of …_active = '0'.

PHP

You reused the $query parameter to store the result of the query. Don't be stingy like that — it's confusing. Use a new variable to store the result.

You have a leak: mysqli_free_result( $query ); never gets called, because that statement is unreachable.

You are mixing the object-oriented and procedural styles of . You should either use the object-oriented style consistently…

public function num_rows($query) {
    if (!($result = $this->link->query($query))) {
        $this->log_db_errors($this->link->error, $result, 'Fatal');
        return $this->link->error;
    }
    $num_rows = $result->num_rows;
    $result->free();
    return $num_rows;
}

or use the procedural style consistently…

public function num_rows($query) {
    if (!($result = mysqli_query($this->link, $query))) {
        $this->log_db_errors(mysqli_error($this->link), $result, 'Fatal');
        return mysqli_error($this->link);
    }
    $num_rows = mysqli_num_rows($result);
    mysqli_free_result($result);
    return $num_rows;
}
added syntax coloration
Source Link
Marc-Andre
  • 6.7k
  • 5
  • 38
  • 65

SQL

If you want a count, ask for a count. Don't ask the server to send all the data for all the rows, just to have the client count the rows. (A few hundred rows is a trivial amount of data for a database to handle, but still, you should follow good practices.)

SELECT SUM(*) AS obj_count
    FROM (
        SELECT COUNT(article_active)
            FROM wcx_articles WHERE article_active = '0'
        UNION ALL
        SELECT COUNT(video_active)
            FROM wcx_videos WHERE video_active = '0'
        UNION ALL
        SELECT COUNT(app_active)
            FROM wcx_apps WHERE app_active = '0'
        UNION ALL
        SELECT COUNT(link_active)
            FROM wcx_links WHERE link_active = '0'
    ) AS wcs_objects;
SELECT SUM(*) AS obj_count
    FROM (
        SELECT COUNT(article_active)
            FROM wcx_articles WHERE article_active = '0'
        UNION ALL
        SELECT COUNT(video_active)
            FROM wcx_videos WHERE video_active = '0'
        UNION ALL
        SELECT COUNT(app_active)
            FROM wcx_apps WHERE app_active = '0'
        UNION ALL
        SELECT COUNT(link_active)
            FROM wcx_links WHERE link_active = '0'
    ) AS wcs_objects;

Schema

If in fact all four of your tables are union-compatible with each other, then you should strongly consider merging them into one table with a wcx_type attribute that declares what type of object that row represents.

Are your …_active columns really strings? Those columns should be of the BOOLEAN (i.e., TINYINT(1)) type. The query should be adjusted accordingly: use …_active = 0 instead of …_active = '0'.

PHP

You reused the $query parameter to store the result of the query. Don't be stingy like that — it's confusing. Use a new variable to store the result.

You are mixing the object-oriented and procedural styles of . Be consistent.

SQL

If you want a count, ask for a count. Don't ask the server to send all the data for all the rows, just to have the client count the rows. (A few hundred rows is a trivial amount of data for a database to handle, but still, you should follow good practices.)

SELECT SUM(*) AS obj_count
    FROM (
        SELECT COUNT(article_active)
            FROM wcx_articles WHERE article_active = '0'
        UNION ALL
        SELECT COUNT(video_active)
            FROM wcx_videos WHERE video_active = '0'
        UNION ALL
        SELECT COUNT(app_active)
            FROM wcx_apps WHERE app_active = '0'
        UNION ALL
        SELECT COUNT(link_active)
            FROM wcx_links WHERE link_active = '0'
    ) AS wcs_objects;

Schema

If in fact all four of your tables are union-compatible with each other, then you should strongly consider merging them into one table with a wcx_type attribute that declares what type of object that row represents.

Are your …_active columns really strings? Those columns should be of the BOOLEAN (i.e., TINYINT(1)) type. The query should be adjusted accordingly: use …_active = 0 instead of …_active = '0'.

PHP

You reused the $query parameter to store the result of the query. Don't be stingy like that — it's confusing. Use a new variable to store the result.

You are mixing the object-oriented and procedural styles of . Be consistent.

SQL

If you want a count, ask for a count. Don't ask the server to send all the data for all the rows, just to have the client count the rows. (A few hundred rows is a trivial amount of data for a database to handle, but still, you should follow good practices.)

SELECT SUM(*) AS obj_count
    FROM (
        SELECT COUNT(article_active)
            FROM wcx_articles WHERE article_active = '0'
        UNION ALL
        SELECT COUNT(video_active)
            FROM wcx_videos WHERE video_active = '0'
        UNION ALL
        SELECT COUNT(app_active)
            FROM wcx_apps WHERE app_active = '0'
        UNION ALL
        SELECT COUNT(link_active)
            FROM wcx_links WHERE link_active = '0'
    ) AS wcs_objects;

Schema

If in fact all four of your tables are union-compatible with each other, then you should strongly consider merging them into one table with a wcx_type attribute that declares what type of object that row represents.

Are your …_active columns really strings? Those columns should be of the BOOLEAN (i.e., TINYINT(1)) type. The query should be adjusted accordingly: use …_active = 0 instead of …_active = '0'.

PHP

You reused the $query parameter to store the result of the query. Don't be stingy like that — it's confusing. Use a new variable to store the result.

You are mixing the object-oriented and procedural styles of . Be consistent.

edited body
Source Link
200_success
  • 144.2k
  • 22
  • 188
  • 473
Loading
Source Link
200_success
  • 144.2k
  • 22
  • 188
  • 473
Loading