-1

I am trying to add a new column to a table with a set date as the column head and a tinyint(1) as the datatype

function addAttendance($date) {
    include('connection.php');
    $column_name = strtolower($date);
    if(!preg_match('/[^A-Za-z0-9.#\\-$]/', $column_name)){
        if(!empty($column_name)) {
            $st = $db->prepare("DESCRIBE attendance");
            $st->execute();
            $st = $st->fetchAll(PDO::FETCH_COLUMN);
            $compare = $st;

            foreach($compare as $key) {
                if($key === $column_name) {
                        die('Project name already exists. Please select a different name.');
                }
            }

            $st = $db->prepare("ALTER TABLE attendance ADD $column_name BOOLEAN");
            $st->execute();

        } else { die('Project name is empty.');}     
    } else { die('Project name can only contain letters and numbers.');}
}
random_user_name
  • 25,694
  • 7
  • 76
  • 115

2 Answers2

3

You haven't described what isn't working in this code, but I'll offer a guess that $date contains some formatting of the current date, like '2014-05-28'. Your regular expression filtering would permit that string. Then you use that as a column name:

$column_name = strtolower($date);

When you go to add the column you generate what appears to be an arithmetic expression as a column name, which will result in an error:

mysql> ALTER TABLE attendance ADD COLUMN 2014-05-28 BOOLEAN;
ERROR 1064 (42000): You have an error in your SQL syntax; 
check the manual that corresponds to your MySQL server version for the right syntax to use
near '2014-05-28 BOOLEAN' at line 1

If you delimit the column name in back-ticks, you can actually use that as a column name:

mysql> ALTER TABLE attendance ADD COLUMN `2014-05-28` BOOLEAN;
Query OK, 0 rows affected (0.11 sec)

However, just because SQL permits such strange column names, doesn't mean it's a good idea. Using that column will be inconvenient at best, and you should probably rethink this idea.


The other thing wrong with your code is that you don't check for errors from prepare() or execute(). You may have enabled PDO to throw exceptions on error, but I'm assuming you haven't. That would mean that you should expect prepare and execute to return false if they find an error.

And of course calling $st->execute() is a fatal error if $st doesn't have that method.

So you should check for errors:

if (($st = $db->prepare("ALTER TABLE attendance ADD $column_name BOOLEAN")) === false) {
    die(print_r($db->errorInfo(), true));
}
if ($st->execute() === false) {
    die(print_r($st->errorInfo(), true));
}
Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
0

PDO::prepare really isn't meant to be used that way. It is meant as a way of preparing a statement for execution. (The first comment here explains in much more depth). You are looking for PDO::exec. Here is probably the most important excerpt:

To those wondering why adding quotes to around a placeholder is wrong, and why you can't use placeholders for table or column names:

There is a common misconception about how the placeholders in prepared statements work: they are not simply substituted in as (escaped) strings, and the resulting SQL executed. Instead, a DBMS asked to "prepare" a statement comes up with a complete query plan for how it would execute that query, including which tables and indexes it would use, which will be the same regardless of how you fill in the placeholders.

The plan for "SELECT name FROM my_table WHERE id = :value" will be the same whatever you substitute for ":value", but the seemingly similar "SELECT name FROM :table WHERE id = :value" cannot be planned, because the DBMS has no idea what table you're actually going to select from.

It should be noted that while a column name containing only letters and numbers (which is currently enforced by your preg_match) will be safe, it is best practice to still quote an input (such as with PDO::quote). I would also suggest that you look into proper naming rules for columns, because that regular expression might not be the most accurate.

Community
  • 1
  • 1
cwallenpoole
  • 79,954
  • 26
  • 128
  • 166
  • PDO::quote() won't do the right type of quoting for column identifiers. – Bill Karwin May 29 '14 at 02:00
  • @BillKarwin You're right, but again, there is a RegEx protecting it, at least in theory. That thought has more to do with, "What if they decide to remove the RegEx at some point?" Sure, they'll still be up a creek, but not quite as bad as if they didn't have anything to begin with. – cwallenpoole May 29 '14 at 02:03