Page 1 of 3

Best practice - database mysql queries

Posted: Tue Jul 25, 2006 3:17 pm
by tdnxxx444
Weirdan | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]


Lets say I have a messaging section of my site.  In this site, there are various SELECT queries:

- Query #1 SELECTS all the message body details, and whoever wrote the message, based on message_id
i.e. 
[syntax="sql"]
SELECT message.body, message.from_account_id 
FROM message, account
WHERE message.message_id = $message_id 
AND account.account_id 
- Query #2 SELECTS just the message body, based on message_id
i.e.

Code: Select all

SELECT message.body
FROM message
WHERE message.message_id = $message_id 
- Query #3 SELECTS all messages from a certain account_id
i.e.

Code: Select all

SELECT message.body, message.from_account_id 
FROM message, account
WHERE message.from_account_id = $account_id
The queries above are just examples I thought off the top of my head.
Now my question is, is there a way to generalize or make all these queries generic, so that I don't have to keep on writing queries over and over, or do I have to write these different queries as they are all on a different case by case basis?


Weirdan | Please use[/syntax]

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]

Posted: Tue Jul 25, 2006 4:08 pm
by jamiel
All have to be pretty Standard queries unless you use an Object Persistance Layer like Propel. Database Abstraction layers will make your Queries cleaner and provide easy access to do data input checking and prepare statements.

Posted: Tue Jul 25, 2006 7:51 pm
by kbrown3074
What about a function call that send the dbase,table, and generic fields.? In that function use the $_GET to pull the vars and stuff those into the select statements that you need.

Re: Best practice - database mysql queries

Posted: Tue Jul 25, 2006 8:14 pm
by Christopher
tdnxxx444 wrote:Now my question is, is there a way to generalize or make all these queries generic, so that I don't have to keep on writing queries over and over, or do I have to write these different queries as they are all on a different case by case basis?
Well there are a lot of questions hidden in there and you are covering some important ground in application design as well. At the bottom of it are the subjects of reuse and layering and abstraction. One big question that comes to mind is how and if you are separating your domain layer code from your presentation layer code? Do you have Transaction Scripts for everything then you will have SQL scattered all over your application? Or do you have centralized your SQL into in your domain layer? That will make a difference in suggestions of possible solutions ]

Posted: Wed Jul 26, 2006 3:06 am
by onion2k
For the record, if you're going by "best practise", you should be doing:

Code: Select all

SELECT @message_id := $message_id;
SELECT message.body FROM message WHERE message.message_id = @message_id;
Variable substitution in a PHP SQL statement is dodgy from a security point of view. Parameterised SQL is the correct way to do things. It makes a SQL injection attack utterly impossible.

Noone does though because it's extra work. :roll:

Posted: Wed Jul 26, 2006 10:53 am
by Yossarian
Noone does though because it's extra work.
Thats why we should be using PDO in PHP5 (or PDO for PHP4 in phpclasses.org).

Its also a great way of starting to think about seperating your sql from the rest of your app, I mean its not an MVC type layer issue, but binding variables and their preparation does make you start thinking about seperating your sql.

@onion2k - totally agree.

Posted: Wed Jul 26, 2006 5:44 pm
by Ambush Commander
SQL should all be in one place. PHP developers may not write good SQL, and Database Administrators may not write good PHP, so don't mix the two together.

Re: Best practice - database mysql queries

Posted: Tue Aug 08, 2006 12:22 am
by tdnxxx444
arborint wrote: Well there are a lot of questions hidden in there and you are covering some important ground in application design as well. At the bottom of it are the subjects of reuse and layering and abstraction. One big question that comes to mind is how and if you are separating your domain layer code from your presentation layer code? Do you have Transaction Scripts for everything then you will have SQL scattered all over your application? Or do you have centralized your SQL into in your domain layer? That will make a difference in suggestions of possible solutions ]
I am planning to use a templating engine to separate my domain layer code from my presentation layer, but right now that has been implemented yet. Currently my SQL is scattered all over my application but I want to centralize it into the domain layer.

Posted: Tue Aug 08, 2006 12:50 am
by RobertGonzalez
Sounds hacky, but on one project I was working, I actually created a series of SQL statements and put them into an array. When I needed the query, I called it using the the array value and parsed the vars into it (not unlike prepared statements, but much less secure and certainly not as advanced).

Code: Select all

<?php
$prep_sql = array();
$prep_sql['get_user_name'] = 'SELECT username FROM ' . USERTABLE . ' WHERE userid = ' . $userid;
$prep_sql['get_page_data'] = 'SELECT page_title, page_content FROM ' . CONTENTTABLE . ' WHERE pageid = ' . $pageid;

// Call one, after setting var values of course...
if (!$result = mysql_query($prep_sql['get_user_name']))
{
    die("...");
}

// ...
?>
This is not exactly how I did it, but the process was similar. Again, not the cleanest way, but it was effective for that particular project.

Posted: Tue Aug 08, 2006 6:13 am
by Ollie Saunders
Now my question is, is there a way to generalize or make all these queries generic
You can't make the queries themselves generic, otherwise they would all be the same ;) ah ha *cough* but you can make there output the same or, if you like to sound clever, polymorphic. This means that they output with the same names so as to be as similar as possible.
Database Abstraction layers will make your Queries cleaner and provide easy access to do data input checking and prepare statements.
And slow down the application, and prevent you from making database specific optimizations. But that's another story.
Parameterised SQL is the correct way to do things. It makes a SQL injection attack utterly impossible.

Code: Select all

SELECT @message_id := $message_id; 
There is a PHP variable in that SQL isn't there? Do you agree that a PHP variable is present? Right, then you can still inject SQL, although I think it does improve things.
SQL should all be in one place. PHP developers may not write good SQL, and Database Administrators may not write good PHP, so don't mix the two together.
This is a very good point I totally agree with.

Here's how I am doing it, applogies for the quantity of code, I think it is necessary though:

Code: Select all

<?php
class TillingDatabaseAccesss
{
    /**
     * The database connection
     *
	 * @var Mysqli
	 */
	protected $_db;
	/**
	 * Number of queries executed via exec()
	 *
	 * @var int
	 */
	public $queriesExecuted = 0;
	/**
	 * Whether a transaction is currently open
	 *
	 * @var bool
	 */
	private $_transaction = false;
	/**
	 * @var MagicTable
	 */
	public $setting;				// instance of MagicTable for the 'Setting' table
	/**
	 * Create link to database and instaniate $this->setting
	 *
	 * @throws SystemException
	 */
	public function __construct()
	{
		$this->_db = new Mysqli(Tilling::$databaseHost, 'xxx', 'xxx', 'xxx');
		if (!$this->_db instanceof Mysqli) {
			throw new SystemException('Failed to connect to database');
		} else {
			$this->setting = new MagicTable($this->_db, 'Setting', 'handle', 'value');
		}
		OF_Field::registerDatabase($this);
	}
	/**
	 * Close link to database
	 */
	public function __destruct()
	{
		if ($this->_db instanceof Mysqli) $this->_db->close();
	}
	/**
	 * Execute a query and throw an exception if it fails.
	 *
	 * @param string $q
	 * @param string $name the name of the function calling exec(); use __FUNCTION__
	 * @return mysqli_result
	 */
	protected function exec($q, $name, $write = false)
	{
		$status = $this->_db->query($q);
		$this->queriesExecuted++;
		if (!$status) {
			$name.= ' generated the following error: '.$this->_db->error;
			$errNo = $this->_db->errno;
			if ($this->_transaction) {
			    $this->rollback();
			}
			throw new QueryException($name, $errNo, $q);
		} else {
		    return $status;
		}
		if ($write) {
		    // may want to save this data to another database
		}
	}
	/**
	 * Commits a query. Get any meta data about the last query that you need
	 * before calling this.
	 */
	protected function commit()
	{
		$this->_db->commit();
		$this->_transaction = false;
	}
	/**
	 * Rollsback an unsucessful query
	 */
	protected function rollback()
	{
		$this->_db->rollback();
		$this->_transaction = false;
	}
	/**
	 * Begins a transaction
	 */
	protected function begin()
	{
		$this->_transaction = true;
	    $this->_db->query('BEGIN');
	}
	/**
	 * Execute a query and return the first column of the first row
	 *
	 * @param string $q
	 * @param string $name
	 * @return mixed
	 */
	protected function execSingleValue($q, $name)
	{
		$result = $this->exec($q,$name);
		$this->commit();
		$row = $result->fetch_row();
		return $row[0];
	}
	public function escape($str)
	{
	    return $this->_db->escape_string($str);
	}
	/**
	 * This method overloading function is used to preprocess all input to a
	 * query with escape string.
	 *
	 * The user of the class will be able to call a private function such _getSetting()
	 * via call with getSetting() and have the input to _getSetting escaped for him.
	 *
	 * You can of course bypass __call() by calling the function _getSetting() directly
	 * but only when it is defined as public.
	 */
	public function __call($method, $arg)
	{
		foreach ($arg as $id => $value) {
			if (ctype_digit($value)) {
				$arg[$id] = (int)$value;
			} else if (is_string($value)) {
				$arg[$id] = $this->_db->escape_string($value);
			}
		}
		$method = '_'.$method;
		if (method_exists($this, $method)) {
		    return call_user_func_array(array($this, $method), $arg);
		} else {
		    throw new Exception('Tried to call non-existant method ' . __CLASS__ . '::' . $method.'()');
		}
	}
}
Then I've got another class, which is massive but here are pieces

Code: Select all

require_once 'TillingDatabaseAccess.php';
class TillingDatabase extends TillingDatabaseAccesss
{
// these three are polymorphic 

	/**
	 * Get site title and id
	 *
	 * @param int $limit
	 * @return Mysqli_result
	 */
	protected function _getRecentSites($limit)
	{
		$q = 'SELECT siteId AS id,title FROM Site ORDER BY id DESC LIMIT '.$limit;
		return $this->exec($q, __FUNCTION__);
	}
	/**
	 * Get client name and id
	 *
	 * @param int $limit
	 * @return Mysqli_result
	 */
	protected function _getRecentClients($limit)
	{
		// 'name AS title' so that these three recent functions are uniform
		$q = 'SELECT clientId AS id,name AS title FROM Client ORDER BY id DESC LIMIT '.$limit;
		return $this->exec($q, __FUNCTION__);
	}
	/**
	 * Get report title and id
	 *
	 * @param int $limit
	 * @return Mysqli_result
	 */
	protected function _getRecentReports($limit)
	{
		$q = 'SELECT reportId AS id,title FROM Report ORDER BY id DESC LIMIT '.$limit;
		return $this->exec($q, __FUNCTION__);
	}

// this is polymorphic, one delete query for all tables! 
	/**
	 * Delete specific rows from any table
	 *
	 * @param string $tableName
	 * @param string $rows
	 */
	protected function _deleteFrom($tableName, $rows)
	{
		$rows = implode(',', $rows);
		$id = strtolower($tableName[0]) . substr($tableName, 1) . 'Id';
	    $q = 'DELETE FROM ' . $tableName . ' WHERE ' . $id . ' IN (' . $rows . ')';
		$this->begin();
			$this->exec($q, __FUNCTION__);
			$affected = $this->_db->affected_rows;
		$this->commit();
		return $affected;
	}
...loads more

Posted: Thu Aug 24, 2006 3:41 am
by Case-
This is a nice example, thanks!
Starting to build my own Database class :)

Posted: Thu Aug 24, 2006 3:54 am
by Ollie Saunders
I'd be interested to know if you think you could better it. Post back.

Posted: Thu Aug 24, 2006 4:18 am
by matthijs
For the record, if you're going by "best practise", you should be doing:
SQL:
SELECT @message_id := $message_id;
SELECT message.body FROM message WHERE message.message_id = @message_id;

Variable substitution in a PHP SQL statement is dodgy from a security point of view. Parameterised SQL is the correct way to do things. It makes a SQL injection attack utterly impossible.
I would like to read/know more about this. Any good resources somewere? Or specific terms I can search for?

Posted: Thu Aug 24, 2006 4:29 am
by Ollie Saunders
You can use it if you like but its not total protection. It is a false sense of security. You should still escape properly.
To answer your question you should google for SQL Injection Attacks. http://en.wikibooks.org/wiki/Programmin ... _Injection

Posted: Thu Aug 24, 2006 6:37 am
by sike
ole wrote:You can use it if you like but its not total protection. It is a false sense of security. You should still escape properly.
To answer your question you should google for SQL Injection Attacks. http://en.wikibooks.org/wiki/Programmin ... _Injection
if you are talking aboput prepared statements (seems so) you are wrong. escaping / quoting is handled by the dba. see http://de.php.net/pdo#pdo.prepared-statements