Inserting 1 line into my SQL database

Question

This is for adding code into my table really easily, however it will be called 2000+ times per second, so I need to know if this is the most efficient code to add a row to the database.

<?php
$site = $_GET['url'];
$con=mysqli_connect("localhost","","","rocket_newsites");
// Check connection
if (mysqli_connect_errno()) {
  echo "Failed to connect to MySQL: " . mysqli_connect_error();
}

$result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`sites` (`id`, `url`) VALUES (NULL, '$site');");


mysqli_close($con);
exit();
?>

Answer 1

Looking at your code, there are several things that immediately I can tell are issues. Before we even get to performance, there are basic security measures that need to be taken first.

SQL Injection

Like you've seen in the comments, your code is highly susceptible to SQL injection. But, what is it exactly? By definition, it is the

insertion or "injection" of a SQL query via the input data from the client to the application.

Now how is your code affected by this? Well imagine we visit a URL like

http://rocket.com/newsite?url=google.com

That's all fine and dandy. If your connection works, "google.com" will be entered into your database.

However, imagine we visited a URL such as:

http://rocket.com/newsite?url=');

_{Note: You can't do this straight from the URL bar. I used PHP's get_file_contents() to test this.}

Now we have a problem. Why is this such a big deal? Well let's take a look at the query when this happens:

INSERT INTO `rocket_newsites`.`sites` (`id`, `url`) VALUES (NULL, '');', '');

Even StackExchange's syntax highlighter knows this is wrong!

Well now you have a syntax error! Your string was interpreted and we were left with some left-over characters (;', '');). Now the attacker knows you're vulnerable to injection.

Contrary to popular belief, having something such as ?url=');DROP`sites`; will rarely actually delete your table. This is because MySQLi's regular query function does not handle multiple queries in one string. It will however, produce a syntax error. So neither way is a good one!

I was asked in chat:

how can you have an answer about sql injection without the "bobby tables" xkcd comic?

And frankly, I agree. It demonstrates the dangers of SQL injection quite simply and shortly!

Source (CC BY-NC 2.5)

How can you solve this? Well I shouldn't have to do all the work!

Improper Error Handling

This is one many people forget, especially as beginners!

Let's take a look at your error handling:

echo "Failed to connect to MySQL: " . mysqli_connect_error();

Why is this bad? Because even if there's an error connecting, you continue to go through with the query. This means an error will be thrown to the attacker, which gives him incredibly important information. Now anyone with a little bit of intrusion knowledge could brute-force your database credentials.

Give only the information the users needs to get it right the next time. Anything more can compromise the security of your application. This is why forms never tell you if it's your username or your password that is wrong ;)

Be aware that there is also no error handling for your query. Anything that goes wrong, just goes wrong!

Got the security issues out of the way, now for what you asked for!

Performance

Here are some tips for optimizing your queries.

• If you don't need the id column, don't have it. It's slowing you down by inserting NULL. And if you don't need it, then maybe you shouldn't be using a RDBMS. Perhaps look into NoSQL.
• Don't start a new connection on every page, especially if each user is not unique. Utilize sessions and caching.
• You might be able to use transactions, depending on the project.
• Look into stored procedures.
• Ask yourself why this is being called so often and see if you can change the way the application is designed to better suit such intense querying.
• Look into other other server-side languages and their profiled statistics. Perhaps PHP really isn't the most optimized language for the job!

Answer 2

First and foremost, do what @AlexL suggested. Once that is done, here are my tips on improving SQL performance (and PHP readability as a side benefit).

INSERT INTO ... (NULL)

As duly pointed out on another answer, there is no point to doing this. If the id column is not set to AUTO_INCREMENT just do this:

ALTER TABLE rocket_newsites.sites MODIFY id INT AUTO_INCREMENT PRIMARY KEY;

Stored Procedure

The way you have it now, when you pass the query to mysql each time it has to figure out the best execution plan anew, as it is being passed ad-lib and MySQL will not cache it in your case.

According to the manual

Queries must be exactly the same (byte for byte) to be seen as identical.

By using a stored procedure instead, you store the execution plan along with it, making its response time significantly faster after the first time it is called.

Creating the procedure

This will only need to be done once, you can either do it in your MySQL client (preferred), or pass the whole block via PHP connection.

DELIMITER $$
    DROP PROCEDURE IF EXISTS sp_insert_site$$
CREATE PROCEDURE sp_insert_site (IN p_url)
BEGIN
INSERT INTO rocket_newsites.sites (url)
VALUES (p_url);
END$$
DELIMITER ;

Then, this line:

$result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`sites` (`id`, `url`) VALUES (NULL, '$site');");

Would become, simply:

$result = mysqli_query($con,"CALL sp_insert_site('$site');");

But again, Fix Your PHP Security Issues First!

Answer 3

As many have already suggested, the way you execute the query is vulnerable to SQL injection attacks. Even if the code will "only live for one day", you shouldn't do it the dirty way, when it's ridiculously easy to do so much better, for example:

<?php
$site = $_GET['url'];
try {
    $db = new PDO('mysql:host=localhost;dbname=rocket_newsites;charset=utf8', 'username', 'password');
    $stmt = $db->prepare("INSERT INTO rocket_newsites.sites (url) VALUES (:url)");
    $stmt->bindValue(':url', $site, PDO::PARAM_STR);
    $stmt->execute();
} catch(PDOException $ex) {
    echo "An Error occured!";
}
?>

It's trivially easy to transform your original script into this form, and now this is safe from 1st order SQL injection attacks. (See this post for more details.)

I also removed the id column from the parameter list, because you were setting it to NULL, so it was unnecessary in the first place.

As for the performance, if this script will be called 1000+ times per second, that will be very efficient, but it's hard to do something about that within this script. It's caller that should change its behavior. Change the caller to buffer the requests, and send multiple URLs at once so you can change this script to use bulk inserting.

Answer 4

opening and closing 2000 connections every second is not good for performance, having some sort of connection pool that opens/closes connections as needed would be much less overhead

I would advise looking at mysql_pconnect()