Permalink
Browse files

feature(db): allows using parameterized queries in core DB functions

Elgg's DB functions: `get_data`, `get_data_rows`, `insert_data`, `delete_data`,
`update_data`, `execute_delayed_write_query`, and `execute_delayed_read_query`
now all accept a `$params` array.

Params can be a numeric array of values to correspond to positional (`?`)
placeholders in the query, or can be an associative array of values with keys
like `:foo` corresponding to named (e.g. `:foo`) placeholders.

The function `update_data` now has an argument to instruct it to return the
number of rows affected.

This softly deprecates `sanitize_*`, nudges devs toward parameterized queries,
and converts a few existing queries to params.
  • Loading branch information...
mrclay committed Mar 9, 2016
1 parent 63d95d4 commit a9e516826f8ee8bb763e12b3451aee96436ae985
@@ -39,36 +39,36 @@ public function __construct(ElggDb $db) {
/**
* {@inheritdoc}
*/
public function getData($query, $callback = '') {
return $this->db->getData($query, $callback);
public function getData($query, $callback = '', array $params = []) {
return $this->db->getData($query, $callback, $params);
}
/**
* {@inheritdoc}
*/
public function getDataRow($query, $callback = '') {
return $this->db->getDataRow($query, $callback);
public function getDataRow($query, $callback = '', array $params = []) {
return $this->db->getDataRow($query, $callback, $params);
}
/**
* {@inheritdoc}
*/
public function insertData($query) {
return $this->db->insertData($query);
public function insertData($query, array $params = []) {
return $this->db->insertData($query, $params);
}
/**
* {@inheritdoc}
*/
public function updateData($query, $getNumRows = false) {
return $this->db->updateData($query, $getNumRows);
public function updateData($query, $getNumRows = false, array $params = []) {
return $this->db->updateData($query, $getNumRows, $params);
}
/**
* {@inheritdoc}
*/
public function deleteData($query) {
return $this->db->deleteData($query);
public function deleteData($query, array $params = []) {
return $this->db->deleteData($query, $params);
}
/**
@@ -157,9 +157,9 @@ public function runSqlScript($scriptlocation) {
*
* @deprecated 2.1 This method will not be available on this class in 3.0
*/
public function registerDelayedQuery($query, $type, $handler = "") {
public function registerDelayedQuery($query, $type, $handler = "", array $params = []) {
elgg_deprecated_notice(__METHOD__ . " was deprecated and will be removed in 3.0", '2.1');
return $this->db->registerDelayedQuery($query, $type, $handler);
return $this->db->registerDelayedQuery($query, $type, $handler, $params);
}
/**
@@ -22,6 +22,7 @@ class Database {
const DELAYED_QUERY = 'q';
const DELAYED_TYPE = 't';
const DELAYED_HANDLER = 'h';
const DELAYED_PARAMS = 'p';
/**
* @var string $tablePrefix Prefix for database tables
@@ -193,15 +194,16 @@ public function connect($type = "readwrite") {
* argument to $callback. If no callback function is defined, the
* entire result set is returned as an array.
*
* @param mixed $query The query being passed.
* @param string $callback Optionally, the name of a function to call back to on each row
* @param string $query The query being passed.
* @param callable $callback Optionally, the name of a function to call back to on each row
* @param array $params Query params. E.g. [1, 'steve'] or [':id' => 1, ':name' => 'steve']
*
* @return array An array of database result objects or callback function results. If the query
* returned nothing, an empty array.
* @throws \DatabaseException
*/
public function getData($query, $callback = '') {
return $this->getResults($query, $callback, false);
public function getData($query, $callback = null, array $params = []) {
return $this->getResults($query, $callback, false, $params);
}
/**
@@ -211,28 +213,30 @@ public function getData($query, $callback = '') {
* matched. If a callback function $callback is specified, the row will be passed
* as the only argument to $callback.
*
* @param mixed $query The query to execute.
* @param string $callback A callback function
* @param string $query The query to execute.
* @param callable $callback A callback function to apply to the row
* @param array $params Query params. E.g. [1, 'steve'] or [':id' => 1, ':name' => 'steve']
*
* @return mixed A single database result object or the result of the callback function.
* @throws \DatabaseException
*/
public function getDataRow($query, $callback = '') {
return $this->getResults($query, $callback, true);
public function getDataRow($query, $callback = null, array $params = []) {
return $this->getResults($query, $callback, true, $params);
}
/**
* Insert a row into the database.
*
* @note Altering the DB invalidates all queries in the query cache.
*
* @param mixed $query The query to execute.
* @param string $query The query to execute.
* @param array $params Query params. E.g. [1, 'steve'] or [':id' => 1, ':name' => 'steve']
*
* @return int|false The database id of the inserted row if a AUTO_INCREMENT field is
* defined, 0 if not, and false on failure.
* @throws \DatabaseException
*/
public function insertData($query) {
public function insertData($query, array $params = []) {
if ($this->logger) {
$this->logger->info("DB query $query");
@@ -242,7 +246,7 @@ public function insertData($query) {
$this->invalidateQueryCache();
$this->executeQuery($query, $connection);
$this->executeQuery($query, $connection, $params);
return (int)$connection->lastInsertId();
}
@@ -251,22 +255,25 @@ public function insertData($query) {
*
* @note Altering the DB invalidates all queries in the query cache.
*
* @param string $query The query to run.
* @param bool $getNumRows Return the number of rows affected (default: false)
* @note WARNING! update_data() has the 2nd and 3rd arguments reversed.
*
* @param string $query The query to run.
* @param bool $get_num_rows Return the number of rows affected (default: false).
* @param array $params Query params. E.g. [1, 'steve'] or [':id' => 1, ':name' => 'steve']
*
* @return bool|int
* @throws \DatabaseException
*/
public function updateData($query, $getNumRows = false) {
public function updateData($query, $get_num_rows = false, array $params = []) {
if ($this->logger) {
$this->logger->info("DB query $query");
}
$this->invalidateQueryCache();
$stmt = $this->executeQuery($query, $this->getConnection('write'));
if ($getNumRows) {
$stmt = $this->executeQuery($query, $this->getConnection('write'), $params);
if ($get_num_rows) {
return $stmt->rowCount();
} else {
return true;
@@ -278,12 +285,13 @@ public function updateData($query, $getNumRows = false) {
*
* @note Altering the DB invalidates all queries in query cache.
*
* @param string $query The SQL query to run
* @param string $query The SQL query to run
* @param array $params Query params. E.g. [1, 'steve'] or [':id' => 1, ':name' => 'steve']
*
* @return int The number of affected rows
* @throws \DatabaseException
*/
public function deleteData($query) {
public function deleteData($query, array $params = []) {
if ($this->logger) {
$this->logger->info("DB query $query");
@@ -293,7 +301,7 @@ public function deleteData($query) {
$this->invalidateQueryCache();
$stmt = $this->executeQuery("$query", $connection);
$stmt = $this->executeQuery("$query", $connection, $params);
return (int)$stmt->rowCount();
}
@@ -334,17 +342,22 @@ public function fingerprintCallback($callback) {
* @param string $query The select query to execute
* @param string $callback An optional callback function to run on each row
* @param bool $single Return only a single result?
* @param array $params Query params. E.g. [1, 'steve'] or [':id' => 1, ':name' => 'steve']
*
* @return array An array of database result objects or callback function results. If the query
* returned nothing, an empty array.
* @throws \DatabaseException
*/
protected function getResults($query, $callback = null, $single = false) {
protected function getResults($query, $callback = null, $single = false, array $params = []) {
// Since we want to cache results of running the callback, we need to
// need to namespace the query with the callback and single result request.
// https://github.com/elgg/elgg/issues/4049
$query_id = (int)$single . $query . '|';
if ($params) {
$query_id .= serialize($params) . '|';
}
if ($callback) {
if (!is_callable($callback)) {
$inspector = new \Elgg\Debug\Inspector();
@@ -359,6 +372,7 @@ protected function getResults($query, $callback = null, $single = false) {
if ($this->queryCache) {
if (isset($this->queryCache[$hash])) {
if ($this->logger) {
// TODO add params in $query here
$this->logger->info("DB query $query results returned from cache (hash: $hash)");
}
return $this->queryCache[$hash];
@@ -367,7 +381,7 @@ protected function getResults($query, $callback = null, $single = false) {
$return = array();
$stmt = $this->executeQuery($query, $this->getConnection('read'));
$stmt = $this->executeQuery($query, $this->getConnection('read'), $params);
while ($row = $stmt->fetch()) {
if ($callback) {
$row = call_user_func($callback, $row);
@@ -385,6 +399,7 @@ protected function getResults($query, $callback = null, $single = false) {
if ($this->queryCache) {
$this->queryCache[$hash] = $return;
if ($this->logger) {
// TODO add params in $query here
$this->logger->info("DB query $query results cached (hash: $hash)");
}
}
@@ -400,32 +415,39 @@ protected function getResults($query, $callback = null, $single = false) {
*
* @param string $query The query
* @param Connection $connection The DB connection
* @param array $params Query params. E.g. [1, 'steve'] or [':id' => 1, ':name' => 'steve']
*
* @return Statement The result of the query
* @throws \DatabaseException
*/
protected function executeQuery($query, Connection $connection) {
protected function executeQuery($query, Connection $connection, array $params = []) {
if ($query == null) {
throw new \DatabaseException("Query cannot be null");
}
$this->queryCount++;
try {
if (!$this->timer) {
return $connection->query($query);
}
$timer_key = preg_replace('~\\s+~', ' ', trim($query));
if ($this->timer) {
$timer_key = preg_replace('~\\s+~', ' ', trim($query . '|' . serialize($params)));
$this->timer->begin(['SQL', $timer_key]);
$value = $connection->query($query);
$this->timer->end(['SQL', $timer_key]);
}
return $value;
try {
if ($params) {
$value = $connection->executeQuery($query, $params);
} else {
// faster
$value = $connection->query($query);
}
} catch (\Exception $e) {
throw new \DatabaseException($e->getMessage() . "\n\n QUERY: $query");
}
if ($this->timer) {
$this->timer->end(['SQL', $timer_key]);
}
return $value;
}
/**
@@ -494,25 +516,27 @@ public function runSqlScript($scriptlocation) {
/**
* Queue a query for execution upon shutdown.
*
* You can specify a handler function if you care about the result. This function will always
* You can specify a callback if you care about the result. This function will always
* be passed a \Doctrine\DBAL\Driver\Statement.
*
* @param string $query The query to execute
* @param string $type The query type ('read' or 'write')
* @param string $handler A callback function to pass the results array to
* @param string $query The query to execute
* @param string $type The query type ('read' or 'write')
* @param callable $callback A callback function to pass the results array to
* @param array $params Query params. E.g. [1, 'steve'] or [':id' => 1, ':name' => 'steve']
*
* @return boolean Whether registering was successful.
* @access private
*/
public function registerDelayedQuery($query, $type, $handler = "") {
public function registerDelayedQuery($query, $type, $callback = null, array $params = []) {
if ($type != 'read' && $type != 'write') {
return false;
}
$this->delayedQueries[] = [
self::DELAYED_QUERY => $query,
self::DELAYED_TYPE => $type,
self::DELAYED_HANDLER => $handler,
self::DELAYED_HANDLER => $callback,
self::DELAYED_PARAMS => $params,
];
return true;
@@ -532,9 +556,11 @@ public function executeDelayedQueries() {
$query = $set[self::DELAYED_QUERY];
$type = $set[self::DELAYED_TYPE];
$handler = $set[self::DELAYED_HANDLER];
$params = $set[self::DELAYED_PARAMS];
try {
$stmt = $this->executeQuery($query, $this->getConnection($type));
$stmt = $this->executeQuery($query, $this->getConnection($type), $params);
if (is_callable($handler)) {
call_user_func($handler, $stmt);
@@ -638,6 +664,7 @@ public function getTablePrefix() {
* @param int $value Value to sanitize
* @param bool $signed Whether negative values are allowed (default: true)
* @return int
* @deprecated Use query parameters where possible
*/
public function sanitizeInt($value, $signed = true) {
$value = (int) $value;
@@ -657,6 +684,7 @@ public function sanitizeInt($value, $signed = true) {
* @param string $value Value to escape
* @return string
* @throws \DatabaseException
* @deprecated Use query parameters where possible
*/
public function sanitizeString($value) {
$quoted = $this->getConnection('read')->quote($value);
Oops, something went wrong.

0 comments on commit a9e5168

Please sign in to comment.