You still haven't answered my question as to the purpose of GetCell. If you're using it to fetch fields from the DB, one at a time, don't. The performance will be terrible. Don't use multiple queries when one will do. If you're on free hosting, your site will likely be suspended for high resource usage.
Originally Posted by
ryanmm
PHP Code:
function GetCell($dbh,$ID,$field)
{
$query= $dbh->prepare("SELECT :$field FROM * WHERE PermUserID = :$ID)
$query = $dbh->query($queryString);
[...]
Did I use the prepare right?
Not really.
- Only scalar values can be parameterized. As stated previously:
Originally Posted by
misson
Don't interpolate values into queries. [...] The exception is when something other than a scalar value, such as table names and lists (typically, the right argument to the
IN operator), varies, since prepared statement parameters only support scalar values.
Column names aren't scalar values, and thus can't be parameterized. Only the argument to the PermUserID=? expression can be parameterized.
- SELECT [...] FROM * isn't valid, as * isn't a valid table name. You must select from a table, view or subselect. (If you feel inclined to learn about subselects, learn about joins first.)
- You're still interpolating values into your statement, which potentially opens it up to SQL injection (this is one of those security issues a framework should take care of). Prepared statements are a little like function calls in that both abstract out some portion of a block as a variable, and take a value as an argument that becomes the value of the variable.
- You need to call PDOStatement::execute, not PDO::query.
PHP Code:
// $table and $field need to be sanitized first
// preparing is like defining a function
$query = $dbh->prepare("SELECT $field FROM $table WHERE PermUserID=:id")
// execute is like calling a function. leading colon is optional on keys
$query->execute(array(':id' => $ID));
// or
$query->execute(array('id' => $ID));
As it is, your code doesn't make sense, since you overwrite $query one line after storing the prepared statement in it.
PHP Code:
$x = 1;
$x = 2; # old value of $x is discarded, so previous statement has no effect
Originally Posted by
ryanmm
PHP Code:
foreach ($query as $query2)
{
$queryReturn[$i] = $query2;
$i++;
}
There's no need to iterate over the result within the function. If you truly want an array, use PDOStatement::fetchAll. However, this will use up more memory and be less performant than simply using the result directly. SQL queries can return before the full result is available. Since you typically only need one row at a time, you can handle them as they arrive. The PDO drivers will store only the results that are currently available, discarding them as they are used. Calling PDOStatement::fetchAll will both block until the entire result is available and slurp all of it into memory, both wasteful acts. Better to simply return the result. Calling code can then iterate over it with foreach, the same as if it were an array.
- Returning an array in some circumstances and a scalar in others will only cause problems. Simply return an array in all cases.
- It won't cause problems, but you don't need to use an array index when appending to an array. If you leave out the index in a bracket expression ([]) when assigning, you'll assign to one past the last element.
PHP Code:
foreach ($items as $item) {
$results[] = $item;
}
- You're missing out on one of the primary benefits of prepared statements: they only need to be parsed once, which makes subsequent queries faster. Cache prepared statements so you don't have to recreate them.
Here's an illustrative example of how GetCell could work:
PHP Code:
<?php
/* Takes an SQL identifier and ensures it's properly quoted. If the argument isn't a valid ID, throws a PDOException.
*/
function sanitize_SQL_identifier($name) {
// matches (e.g.): tbl, `tbl`, `db`.`tbl`, `db.tbl`, `db.`tbl
if (! preg_match('/^(?:`?([^`.]*)`?\.)?`?([^`]*)`?$/', $name, $parts)) {
throw new PDOException("'$name' isn't a valid SQL identifier");
}
if (empty($parts[1])) {
return "`$parts[2]`";
} else {
return "`$parts[1]`.`$parts[2]`";
}
}
/* GetCell: Get a single field from a database.
Arguments:
$db: a PDO object
$table: a table identifier
$field: a column identifier. In particular, it cannot be an expression, such as an aggregate
$id: PermUserID of row to fetch.
Returns a PDOStatement, which can be iterated over.
Throws a PDOException if (e.g.) $table or $field aren't valid SQL identifiers.
*/
function GetCell($db, $table, $field, $id) {
// $queries is a prepared statement cache.
static $queries = array();
$field = sanitize_SQL_identifier($field);
$table = sanitize_SQL_identifier($table);
$stmt = "SELECT $field FROM $table WHERE PermUserID=:id";
if (! isset($queries[$stmt])) {
$queries[$stmt] = $db->prepare($stmt);
}
$queries[$stmt]->execute(array(':id' => $id));
return $queries[$stmt];
}
However, I still wouldn't use it.
Originally Posted by
ryanmm
I dont understand those colons.
They mark named parameters. You can also use '?', which are positional parameters.
The two types of parameters are analogous to positional and keyword arguments to functions. PHP doesn't really have the latter; instead, you'd use an associative array. Python, for example, has them.
Code:
def dostuff(i,j,k):
return i*j-i*k
# positional
dostuff(1, 3, 2)
# keyword
dostuff(j=3,k=2,i=1)
PHP Code:
function dostuff($kwargs) {
return $kwargs['i']*$kwargs['j'] - $kwargs['i']*$kwargs['k'];
}
dostuff(array('j' => 3, 'k' => 2, 'i' => 1));
If there's only one parameter in a prepared statement, I tend to use a positional parameter; if more than one, named parameters. Named parameters let you change their order in either the statement or the call without having to change any other code.
And what do the double colons mean in PDO::prepare? I've never seen double colons used in any PDO code.
They mean prepare is a method of PDO, rather than a plain function not attached to a class. The double colon "::" is the scope resolution operator. You most often see them when used with static fields, or when calling a method on a parent:
PHP Code:
class A {
static $n=42;
public $foo;
function __construct($foo) {
$this->foo=$foo;
}
function bam() {
return "bug-AWWK";
}
}
class B extends A {
static $n=23;
public $bar;
function __construct($foo, $bar) {
// $this in parent::__construct is still this object, not an A.
parent::__construct($foo);
$this->bar=$bar;
echo self::$n, "\n", // 23
parent::$n, "\n"; // 42
echo self::bam(), "\n"; // calls A::bam()
}
}
$b = new B('foo', 'bar');
If you really wish to continue to develop the infrastructure yourself, you're going to need to read the manual and try a few tutorials. However, be prepared to spend some time getting it right, otherwise you'll wind up in big trouble later down the line (such as when someone hacks your site and steals your customer base). Just as with manufacturing, costs associated with fixing problems rise exponentially the later you get in the development cycle.
LinkBack URL
About LinkBacks
Reply With Quote
