Just trying to create a simple PDO query function

ryanmm

Member
Messages
38
Reaction score
0
Points
6
Here's my function:
PHP:
function GetCell($funcConnect,$funcSQL)
{
	$funcResult = $funcConnect->query($funcSQL);
return $funcResult;
}

here's how i'm calling it:
PHP:
<?php echo GetCell($UsersConnect,"SELECT age FROM cpanelname_Users WHERE PermUserID='$CurUserID'"); ?>)


Here How I constructed the dsn
PHP:
$host = 'localhost';
$port = 3306;
$UserDatabase = 'cpanelname_Users';
$LiveCompDatabase = 'cpanelname_LiveComps';
$username = 'cpanelname_Blue';
$password = '********';

// Construct the DSN
$dsnUser = "mysql:host=$host;port=$port;dbname=$UserDatabase";

// Construct the DSN
$dsnLiveComps = "mysql:host=$host;port=$port;dbname=$LiveCompDatabase";


//Create the connection to the USERS TABLE and to the LIVECOMPS TABLE
$LiveCompConnect = new PDO($dsnLiveComps, $username, $password);
$UsersConnect  = new PDO($dsnUser, $username, $password);

however when i call it, it returns nothing. I tried using Var_dump to see what it would return and it returned "bool(false)"

Any ideas?
 
Last edited:

misson

Community Paragon
Community Support
Messages
2,572
Reaction score
72
Points
48
Here's my function:
PHP:
function GetCell($funcConnect,$funcSQL)
{
	$funcResult = $funcConnect->query($funcSQL);
return $funcResult;
}
What is the purpose of GetCell? As it is, it doesn't seem to gain you much. What you should be doing is hiding DB access behind a data access layer (DAL) to isolate persistent storage from the rest of the app. See the Active Record ([2], [3]) and the Data Mapper ([2], [3]) patterns for examples.


here's how i'm calling it:
PHP:
<?php echo GetCell($UsersConnect,"SELECT age FROM cpanelname_Users WHERE PermUserID='$CurUserID'"); ?>)
Don't interpolate values into queries. PDO::query is intended for static queries. If a query has a parameter, use PDO::prepare to create a prepared statement. 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. In such cases, the part that varies rarely comes from user input.

PHP:
//Create the connection to the USERS TABLE and to the LIVECOMPS TABLE
$LiveCompConnect = new PDO($dsnLiveComps, $username, $password);
$UsersConnect  = new PDO($dsnUser, $username, $password);
There's no need for two database connections; it's a waste of resources. If you're using multiple databases, simply prefix table names with database names in statements.

Rather than using global DB connections, create a class to manage DB connections. For a (very) simple example, see LocalDB.

however when i call it, it returns nothing. I tried using Var_dump to see what it would return and it returned "bool(false)"
It's returning FALSE, not nothing. PDO::query and PDO::prepare return FALSE on failure. My best guess as to why is that you're selecting from `cpanelname_Users`, which is a database, not a table. You can use PDO::errorInfo to get an error message. If you do this, the DB error message should only be displayed for admin users, otherwise you're disclosing too much information. Another option is to use exceptions by setting the error mode to PDO::ERRMODE_EXCEPTION. This lets you handle all PDO errors in a section of code with a single catch block, rather than handling each potential point of failure separately.

However you choose to do it, you need to add error handling to your script.
 

ryanmm

Member
Messages
38
Reaction score
0
Points
6
Thank you very much for the thorough response.

I think it might take 20 hours or so to study everything you suggested, who knows how much longer to learn. Understand, I admire you for suggesting it, if I was a programmer in training, aiming to become a lifetime career programmer, this would be the only sound advice to suggest at all. However I am not trying to become a career programmer. I am really more of an entrepreneur with a decent amount of programming experience who is trying to just finish one site for one specific idea and simply doesn't trust allowing other people(professional programmers) in on the idea at this point. I will probably study what you have wrote and try to understand it. In the meantime, is there anyway to remedy my function as it is? I'm just trying to turn an SQL select statement into a PDO function
 
Last edited:

misson

Community Paragon
Community Support
Messages
2,572
Reaction score
72
Points
48
In the meantime, is there anyway to remedy my function as it is?
The error handling. As I said, my best guess is that you're using a database name where you should be using a table name, but there isn't enough information. Use error handling to find out the exact problem.

As for just getting a site up-and-running, it'd be best to use a PHP framework. It will free you from having to develop the infrastructure. You won't have to worry about interfacing with a DB or security or any of the many lower level aspects. It will take a little time to learn the framework API, but will be faster, more secure, more stable and more flexible overall.
 

callumacrae

not alex mac
Community Support
Messages
5,257
Reaction score
97
Points
48
Stop being a hydrogenPHP fanboi, it hasn't even been released yet :D

Codeigniter ftw

~Callum
 

ryanmm

Member
Messages
38
Reaction score
0
Points
6
PHP:
function GetCell($dbh,$ID,$field) 
{	
	$query= $dbh->prepare("SELECT :$field FROM * WHERE PermUserID = :$ID)
	$query = $dbh->query($queryString);
	$i = 0;

  	foreach ($query as $query2) 
	{
		$queryReturn[$i] = $query2;
		$i++;
	}

  	if($i > 1) 
           {
		return $queryReturn;
	}
	else
	{
		return $queryReturn[0];
	}
}

Did I use the prepare right? I dont understand those colons. And what do the double colons mean in PDO::prepare? I've never seen double colons used in any PDO code.
 
Last edited:

misson

Community Paragon
Community Support
Messages
2,572
Reaction score
72
Points
48
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.

PHP:
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:
    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:
    // $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:
    $x = 1;
    $x = 2; # old value of $x is discarded, so previous statement has no effect
  • PHP:
        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:
    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:
<?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.

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:
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:
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.
 
Last edited:

ryanmm

Member
Messages
38
Reaction score
0
Points
6
Wow, thank you again for taking so much time to answer me. I do really appreciate it.

I started looking into codeigniter. It looks intersting, not too difficult really. The only thing is I know I will have to create my own custom functions for a few things, as they are not really common at all.

You say I should just do one large query per page and then assign variables rather than do a new query each time it is needed. Would I have to worry about that with code igniter?

I suppose it just kind of sucks cuz im already about halfway through this project and now I have to figure out where to put everything I've already coded, (like the custom functions i mentioned before) and how to make it all work together again. But I suppose it will help as there are still some things I haven't done yet that I need to do like creating a user register page and the standard "send email to users email and have them click a link to verify" things like that

---------- Post added at 07:00 PM ---------- Previous post was at 05:08 PM ----------

They way my page works, not all of the info I get from the database may be needed. Thats why I'm doing selecting one at a time. It's basically if/then... if this database cell == X, then do this, after you do that, check if another cell == Y, etc. I actually thought because of this that I would be saving resources. Would it still be faster to just do one query at the beginning for all the if/then branches?
 

misson

Community Paragon
Community Support
Messages
2,572
Reaction score
72
Points
48
You say I should just do one large query per page and then assign variables rather than do a new query each time it is needed.
Note that this advice comes from what I can only guess is the purpose of GetCell. The reason I keep asking what the purpose is (you should still answer, in a little more detail and at a higher level) is because what you're trying to use it for might be a bad idea. As the sig says, my questions aren't rhetorical.

It's not one query per page. I can't think of a succinct way of putting it, but "one query per modeled object" (or collection of modeled objects) comes close, where "modeled object" is whatever the data in the database models. Basically, any time you need to get data from the database, do it once rather than in a loop.

Don't:
PHP:
<?php

$ids = array(...);

// example 1: fetch a column from DB, one row at a time
$query = $db->prepare('SELECT field FROM table WHERE id=?');
foreach ($ids as $id) {
    $query->execute(array($id));
    ...
}

// example 2: get each table cell value separately
?>
<table>
  <thead>...</thead>
  <tbody>
    <?php foreach ($ids as $id): ?>
      <tr>
        <?php foreach ($fields as $field): ?>
          <td><?php GetCell($db, $table, $field, $id); ?></td>
        <?php endforeach; ?>
      </tr>
    <?php endforeach; ?>
  </tbody>
</table>

Do:
PHP:
<?php

// example 1: fetch a column from DB all at once, matching some condition
$query = $db->prepare('SELECT field FROM table WHERE ...');
if ($query->execute(array(...))) {
    foreach ($query as $idx => $field) {
        ...
    }
}

// example 2: get all the data to appear in the table in one query
$query = $db->prepare('SELECT foo, bar, baz FROM table WHERE ...');
if ($query->execute(array(...))):
  ?>
  <table>
    <thead>...</thead>
    <tbody>
      <?php foreach ($query as $item): ?>
        <tr>
          <?php foreach ($item as $field => $value): ?>
            <td><?php echo $value; ?></td>
          <?php endforeach; ?>
        </tr>
      <?php endforeach; ?>
    </tbody>
  </table>
  <?php
endif;

// or, better yet, separate data handling from display (the "view"):
class Table { // this class would actually be provided by the framework
    public $data;
    function display() {
        if ($this->data) { 
          ?>
          <table>
            <thead>...</thead>
            <tbody>
              <?php foreach ($this->data as $item): ?>
                <tr>
                  <?php foreach ($item as $field => $value): ?>
                    <td><?php echo $value; ?></td>
                  <?php endforeach; ?>
                </tr>
              <?php endforeach; ?>
            </tbody>
          </table>
          <?php
        }
    }
}

// the "controller", though it also performs a modeling task (namely, DB access)
$query = $db->prepare('SELECT foo, bar, baz FROM table WHERE ...');
if ($query->execute(array(...))) {
    $tableView = new TableView;
    $tableView->data = $query;
    $tableView->show(); // outputs a table
}
The last is a step towards the Model-View-Controller pattern pattern (MVC; [2], [3]), though Model-View might be a better description of the sample code (there is an implicit "controller" of sorts, as noted in the code, but that scarcely counts).

Would I have to worry about that with code igniter?
Rely on the framework for DB access as much as possible. Even if you need to use a custom query, see if the framework can be used. It might be able to construct the query, or execute it and process the results. Then you don't have to worry about any of it.

Would it still be faster to just do one query at the beginning for all the if/then branches?
Probably not, but it depends on the nature of the queries and the data relationships. Some of the processing might be best performed in the query statement, for example. Looking at the general case, if a row matching a query can be looked up in an index, separate queries may not be so bad. If the DBMS has to scan the table, then multiple queries will definitely perform worse.

If the other fields are from the same row and don't contain large amounts of data (they don't contain, for example, BLOBs), I'd fetch them in a single query, though only the fields you need. SELECT * (if you're tempted) should only be used in very specific circumstances.

These topics require familiarity with the relational model to properly understand them. Given your current focus, it's probably better to pick a framework and learn how to use it.
 
Last edited:

ryanmm

Member
Messages
38
Reaction score
0
Points
6
Looks like my post dispeared or something. apologies if i double post.

I use GetCell to display info on the page, and also to run if/then statements.

so for example:

<title><PHP? echo GetCell('username args') . 'at my site'; ?> <title>

but I'm also sending in prompts to the user dynamically, something like:

if(GetCell(ThisUsersPostNum) >1000)
{
//display a the SUPER USER php in a div
echo SUPER_USERS.php;
}

So for example, if I have more variables in SUPER_USERS.php, I might not want to query them right on page load because they may not be needed anyway

(file contents of SUPER_USERS.php)
<html>
Your a great SUPERUSER!!!!, you've got <?PHP echo GetCell(CreditsNum); ?> credits!
<html>
 
Last edited:

misson

Community Paragon
Community Support
Messages
2,572
Reaction score
72
Points
48
Looks like my post dispeared or something. apologies if i double post.
No worries. The forum misbehaves from time-to-time.

As for the getCell usage example, it may cut down on the amount of data transferred for non-super users, but it increases the cost of retrieving data for super users, as each statement that is prepared and each query executed has overhead. Avoiding overhead is one of the primary reasons not to query each column separately. Realistically, the difference between the cost per page of the various options is negligible. However, it might be a big difference in the cost for page requests in a given period if your site experiences heavy traffic. If there are 5-10 fields you are fetching, doing it separately means overhead costs 5-10 times more.

If the size of the additional fields are small, fetch them all in the same query. You're not saving anything by leaving a few numbers out of the query results. If some of the fields are large, you could organize columns in sets, such as "every user" and "super user" sets. Fetch the columns in the "every user" set for every user, and fetch those from the "super user". This will drastically reduce the number of queries.

One nice aspect of getCell is it lets you do either of the above without changing the code that calls getCell. It can fetch more columns than requested, storing them in a cache. When a field is requested, first check whether it's in the cache: if so, return it; if not, fetch it (and other columns from its column set) and store those in the cache. Since different data classes might have different storage requirements and column sets, it might be easier to switch to OOP and create classes to handle fetching data to/from the DB. This is a step towards the data mapper pattern (see links in my earlier post).

PHP:
// Warning: untested and performs no sanitization on table or column names
class FieldFetcher {
    public $db, $table;
    protected $columnSets, $cache, $queries;
    
    function __construct($db=null, $table=null, $columnSets=array()) {
        $this->db = $db;
        $this->table=$table;
        foreach ($columnSets as $set) {
            $this->addColumnSet($set);
        }
    }
    
    /* A very simple interface. Doesn't support named sets, or adding columns
       to an existing set.
     */
    function addColumnSet($colSet) {
        sort($colSet);
        foreach ($colSet as $col) {
            $this->columnSets[$col] = $set;
        }
    }
    
    function fetch($id, $field) {
        $colSet = $this->columnSetString($field);
        if (! isset($this->cache[$id][$colSet]))) {
            $query = $this->query($colSet);
            $this->cache[$id][$colSet] =& $query->execute(array($id));
        }
        return $this->cache[$id][$colSet][$field];
    }
    
    // protected methods
    protected function columnSetString($field) {
        return implode(',', $this->columnSets[$field]);
    }
    
    protected function query($colSet) {
        if (!isset($this->queries[$colSet])) {
            $this->queries[$colSet] = $db->prepare("SELECT $colSet FROM {$this->table} WHERE id=?");
            $this->queries[$colSet]->setFetchMode(PDO::FETCH_ASSOC);
        }
        return $this->queries[$colSet];
    }
    
}

Usage:

PHP:
<?php
$userFields = new FieldFetcher($db, 'mydb.users');
$userFields->addColumnSet(array('id', 'name', ...));
$userFields->addColumnSet(array('CreditsNum', ...));
...
?>You're a great SUPERUSER!!!!, you've got <?php echo $userFields->fetch('CreditsNum'); ?> credits!

Still, I'd personally use a framework in a situation such as yours.
 
Last edited:
Top