From 90c676019652341836edd7bf71dfc70979341810 Mon Sep 17 00:00:00 2001 From: Timothy Warren Date: Thu, 9 Aug 2012 16:15:36 +0000 Subject: [PATCH] Parsed join conditions fixes #2 --- classes/db_pdo.php | 34 +++--- classes/query_builder.php | 192 +++++++++++++++++++--------------- classes/query_parser.php | 61 +++++++++-- tests/core/db_qp_test.php | 32 ++++-- tests/db_files/FB_TEST_DB.FDB | Bin 802816 -> 802816 bytes 5 files changed, 204 insertions(+), 115 deletions(-) diff --git a/classes/db_pdo.php b/classes/db_pdo.php index bbdb2df..eca348a 100644 --- a/classes/db_pdo.php +++ b/classes/db_pdo.php @@ -29,21 +29,21 @@ abstract class DB_PDO extends PDO { * @var mixed */ protected $statement; - + /** * Character to escape identifiers * * @var string */ protected $escape_char = '"'; - + /** * Reference to sql sub class * * @var Object */ public $sql; - + /** * Reference to util sub class * @@ -62,11 +62,11 @@ abstract class DB_PDO extends PDO { public function __construct($dsn, $username=NULL, $password=NULL, $driver_options=array()) { parent::__construct($dsn, $username, $password, $driver_options); - + // Load the sql class for the driver $class = get_class($this)."_sql"; $this->sql = new $class(); - + // Load the util class for the driver $class = get_class($this)."_util"; $this->util = new $class($this); @@ -210,7 +210,7 @@ abstract class DB_PDO extends PDO { { return array_map(array($this, 'quote_ident'), $ident); } - + // If the string is already quoted, return the string if (($pos = strpos($ident, $this->escape_char)) !== FALSE && $pos === 0) { @@ -223,9 +223,9 @@ abstract class DB_PDO extends PDO { // Return the re-compiled string return implode('.', array_map(array($this, '_quote'), $hiers)); } - + // -------------------------------------------------------------------------- - + /** * Helper method for quote_ident * @@ -238,7 +238,7 @@ abstract class DB_PDO extends PDO { { return $str; } - + return "{$this->escape_char}{$str}{$this->escape_char}"; } @@ -366,9 +366,9 @@ abstract class DB_PDO extends PDO { { return $this->driver_query($this->sql->system_table_list()); } - + // -------------------------------------------------------------------------- - + /** * Retrieve column information for the current database table * @@ -379,9 +379,9 @@ abstract class DB_PDO extends PDO { { return $this->driver_query($this->sql->column_list($table), FALSE); } - + // -------------------------------------------------------------------------- - + /** * Retrieve list of data types for the database * @@ -408,7 +408,7 @@ abstract class DB_PDO extends PDO { { return FALSE; } - + // Return predefined data if (is_array($sql)) { @@ -422,7 +422,7 @@ abstract class DB_PDO extends PDO { return ($filtered_index) ? db_filter($all, 0) : $all; } - + // -------------------------------------------------------------------------- /** @@ -432,8 +432,8 @@ abstract class DB_PDO extends PDO { */ public function num_rows() { - return isset($this->statement) && is_object($this->statement) - ? $this->statement->rowCount() + return isset($this->statement) && is_object($this->statement) + ? $this->statement->rowCount() : FALSE; } diff --git a/classes/query_builder.php b/classes/query_builder.php index 480bccc..1e8a910 100644 --- a/classes/query_builder.php +++ b/classes/query_builder.php @@ -52,35 +52,35 @@ class Query_Builder { * @var string */ private $select_string; - + /** * Compiled 'from' clause * * @var string - */ + */ private $from_string; - + /** * Compiled arguments for insert / update * * @var string */ private $set_string; - + /** * Order by clause * * @var string */ private $order_string; - + /** * Group by clause * * @var string */ private $group_string; - + // -------------------------------------------------------------------------- // ! SQL Clause Arrays // -------------------------------------------------------------------------- @@ -91,21 +91,21 @@ class Query_Builder { * @var array */ private $set_array_keys; - - /** + + /** * Key/val pairs for order by clause * * @var array */ private $order_array; - + /** * Key/val pairs for group by clause * * @var array */ private $group_array; - + // -------------------------------------------------------------------------- // ! Other Class vars // -------------------------------------------------------------------------- @@ -123,7 +123,7 @@ class Query_Builder { * @var int */ private $limit; - + /** * Value for offset in limit string * @@ -138,7 +138,7 @@ class Query_Builder { */ public $sql; - /** + /** * Query component order mapping * for complex select queries * @@ -153,7 +153,7 @@ class Query_Builder { * @var array */ private $query_map; - + /** * Map for having clause * @@ -161,6 +161,13 @@ class Query_Builder { */ private $having_map; + /** + * Query parser to safely escape conditions + * + * @var object + */ + private $parser; + /** * Convenience property for connection management * @@ -179,33 +186,33 @@ class Query_Builder { if (is_array($params)) { $p = new stdClass(); - + foreach($params as $k => $v) { $p->$k = $v; } - + $params = $p; } - - // Let the connection work with 'conn_db' or 'database' + + // Let the connection work with 'conn_db' or 'database' if (isset($params->database)) { $params->conn_db = $params->database; } - - + + $params->type = strtolower($params->type); $dbtype = ($params->type !== 'postgresql') ? $params->type : 'pgsql'; - + $dsn = ''; - + // Add the driver type to the dsn if ($dbtype !== 'firebird' && $dbtype !== 'sqlite') { $dsn = strtolower($dbtype).':'.$dsn; } - + // Make sure the class exists if ( ! class_exists($dbtype)) { @@ -227,7 +234,7 @@ class Query_Builder { { $dsn .= ";port={$params->port}"; } - + break; case "sqlite": @@ -238,9 +245,9 @@ class Query_Builder { $dsn = "{$params->host}:{$params->file}"; break; } - - - try + + + try { // Create the database connection if ( ! empty($params->user)) @@ -263,6 +270,8 @@ class Query_Builder { $this->conn_name = $params->name; } + // Instantiate the Query Parser + $this->parser = new Query_Parser(); // Make things just slightly shorter $this->sql =& $this->db->sql; @@ -314,9 +323,9 @@ class Query_Builder { return $this; } - + // -------------------------------------------------------------------------- - + /** * Method to simplify select_ methods * @@ -328,16 +337,16 @@ class Query_Builder { { // Escape the identifiers $field = $this->quote_ident($field); - - $as = ($as !== FALSE) + + $as = ($as !== FALSE) ? $this->quote_ident($as) : $field; - + return "({$field}) AS {$as} "; } - + // -------------------------------------------------------------------------- - + /** * Selects the maximum value of a field from a query * @@ -351,9 +360,9 @@ class Query_Builder { $this->select_string .= $this->sql->max().$this->_select($field, $as); return $this; } - + // -------------------------------------------------------------------------- - + /** * Selects the minimum value of a field from a query * @@ -362,14 +371,14 @@ class Query_Builder { * @return $this */ public function select_min($field, $as=FALSE) - { + { // Create the select string $this->select_string .= $this->sql->min().$this->_select($field, $as); return $this; } - + // -------------------------------------------------------------------------- - + /** * Selects the average value of a field from a query * @@ -383,9 +392,9 @@ class Query_Builder { $this->select_string .= $this->sql->avg().$this->_select($field, $as); return $this; } - + // -------------------------------------------------------------------------- - + /** * Selects the sum of a field from a query * @@ -401,7 +410,7 @@ class Query_Builder { } // -------------------------------------------------------------------------- - + /** * Adds the 'distinct' keyword to a query * @@ -413,7 +422,7 @@ class Query_Builder { $this->select_string = $this->sql->distinct() . $this->select_string; return $this; } - + // -------------------------------------------------------------------------- /** @@ -440,7 +449,7 @@ class Query_Builder { // -------------------------------------------------------------------------- // ! 'Like' methods // -------------------------------------------------------------------------- - + /** * Simplify 'like' methods * @@ -470,19 +479,19 @@ class Query_Builder { { $val = "%{$val}%"; } - + $this->query_map[] = array( 'type' => 'like', 'conjunction' => (empty($this->query_map)) ? ' WHERE ' : " {$conj} ", 'string' => $l ); - + // Add to the values array $this->values[] = $val; - + return $this; - } - + } + // -------------------------------------------------------------------------- /** @@ -542,11 +551,11 @@ class Query_Builder { { return $this->_like($field, $val, $pos, 'NOT LIKE', 'OR'); } - + // -------------------------------------------------------------------------- // ! Having methods // -------------------------------------------------------------------------- - + /** * Simplify building having clauses * @@ -558,7 +567,7 @@ class Query_Builder { private function _having($key, $val=array(), $conj='AND') { $where = $this->_where($key, $val); - + // Create key/value placeholders foreach($where as $f => &$val) { @@ -577,12 +586,12 @@ class Query_Builder { 'string' => $item ); } - + return $this; } - + // -------------------------------------------------------------------------- - + /** * Generates a 'Having' clause * @@ -594,9 +603,9 @@ class Query_Builder { { return $this->_having($key, $val, 'AND'); } - + // -------------------------------------------------------------------------- - + /** * Generates a 'Having' clause prefixed with 'OR' * @@ -642,9 +651,9 @@ class Query_Builder { return $where; } - + // -------------------------------------------------------------------------- - + /** * Simplify generating where string * @@ -679,9 +688,9 @@ class Query_Builder { return $this; } - + // -------------------------------------------------------------------------- - + /** * Simplify where_in methods * @@ -708,7 +717,7 @@ class Query_Builder { 'conjunction' => ( ! empty($this->query_map)) ? " {$conj} " : ' WHERE ', 'string' => $string ); - + return $this; } @@ -801,7 +810,7 @@ class Query_Builder { // -------------------------------------------------------------------------- // ! Other Query Modifier methods // -------------------------------------------------------------------------- - + /** * Sets values for inserts / updates / deletes * @@ -837,7 +846,7 @@ class Query_Builder { return $this; } - + // -------------------------------------------------------------------------- /** @@ -850,15 +859,26 @@ class Query_Builder { */ public function join($table, $condition, $type='') { - // TODO make able to handle operators without spaces - $table = implode(" ", array_map(array($this->db, 'quote_ident'), explode(' ', trim($table)))); - //$condition = preg_replace('`(\W)`', " $1 ", $condition); - $cond_array = explode(' ', trim($condition)); - $cond_array = array_map('trim', $cond_array); - $condition = $table . ' ON ' . $this->quote_ident($cond_array[0]) . $cond_array[1] . - ' ' . $this->quote_ident($cond_array[2]); + $parser = new query_parser(); + + // Parse out the join condition + $parts = $parser->parse_join($condition); + $count = count($parts['identifiers']); + + // Go through and quote the identifiers + for($i=0; $i <= $count; $i++) + { + if (in_array($parts['combined'][$i], $parts['identifiers']) && ! is_numeric($parts['combined'][$i])) + { + $parts['combined'][$i] = $this->quote_ident($parts['combined'][$i]); + } + } + + $parsed_condition = implode(' ', $parts['combined']); + + $condition = $table . ' ON ' . $parsed_condition; $this->query_map[] = array( 'type' => 'join', @@ -1022,7 +1042,7 @@ class Query_Builder { return $this; } - + // -------------------------------------------------------------------------- // ! Query execution methods // -------------------------------------------------------------------------- @@ -1070,7 +1090,7 @@ class Query_Builder { } // -------------------------------------------------------------------------- - + /** * Convience method for get() with a where clause * @@ -1084,13 +1104,13 @@ class Query_Builder { { // Create the where clause $this->where($where); - + // Return the result return $this->get($table, $limit, $offset); } - + // -------------------------------------------------------------------------- - + /** * Retreive the number of rows in the selected table * @@ -1103,9 +1123,9 @@ class Query_Builder { $res = $this->query($sql); return (int) count($res->fetchAll()); } - + // -------------------------------------------------------------------------- - + /** * Retrieve the number of results for the generated query - used * in place of the get() method @@ -1120,7 +1140,7 @@ class Query_Builder { { $this->from($table); } - + $sql = $this->_compile(); // Do prepared statements for anything involving a "where" clause @@ -1136,12 +1156,12 @@ class Query_Builder { // Reset for next query $this->_reset(); - + $rows = $result->fetchAll(); - + return (int) count($rows); } - + // -------------------------------------------------------------------------- /** @@ -1276,7 +1296,7 @@ class Query_Builder { // Set empty arrays $this->values = array(); - + // Set select string as an empty string, for proper handling // of the 'distinct' keyword $this->select_string = ''; @@ -1295,7 +1315,7 @@ class Query_Builder { private function _compile($type='', $table='') { $sql = ''; - + $table = $this->quote_ident($table); switch($type) @@ -1324,7 +1344,7 @@ class Query_Builder { { $sql .= $this->group_string; } - + // Set the having string if ( ! empty($this->having_map)) { @@ -1350,7 +1370,7 @@ class Query_Builder { case "insert": $param_count = count($this->set_array_keys); $params = array_fill(0, $param_count, '?'); - $sql = "INSERT INTO {$table} (" + $sql = "INSERT INTO {$table} (" . implode(', ', $this->set_array_keys) . ') VALUES ('.implode(', ', $params).')'; break; @@ -1382,7 +1402,7 @@ class Query_Builder { break; } - + //echo $sql . '
'; return $sql; diff --git a/classes/query_parser.php b/classes/query_parser.php index 5bc9e4b..55b69ba 100644 --- a/classes/query_parser.php +++ b/classes/query_parser.php @@ -27,9 +27,9 @@ class Query_Parser { * @var array */ private $match_patterns = array( - 'function' => '`([a-zA-Z0-9_]+\((.*?)\))`', - 'identifier' => '`([a-zA-Z0-9"_-]+\.?)+`', - 'operator' => '`=|AND|&&?|~|\|\|?|\^|/|>=?|<=?|-|%|OR|\+|NOT|\!=?|<>|XOR`' + 'function' => '([a-zA-Z0-9_]+\((.*?)\))', + 'identifier' => '([a-zA-Z0-9_-]+\.?)+', + 'operator' => '=|AND|&&?|~|\|\|?|\^|/|>=?|<=?|-|%|OR|\+|NOT|\!=?|<>|XOR' ); /** @@ -41,6 +41,7 @@ class Query_Parser { 'functions' => array(), 'identifiers' => array(), 'operators' => array(), + 'combined' => array(), ); /** @@ -50,9 +51,57 @@ class Query_Parser { */ public function __construct($sql = '') { - preg_match_all($this->match_patterns['function'], $sql, $this->matches['functions'], PREG_SET_ORDER); - preg_match_all($this->match_patterns['identifier'], $sql, $this->matches['identifiers'], PREG_SET_ORDER); - preg_match_all($this->match_patterns['operator'], $sql, $this->matches['operators'], PREG_SET_ORDER); + // Get sql clause components + preg_match_all('`'.$this->match_patterns['function'].'`', $sql, $this->matches['functions'], PREG_SET_ORDER); + preg_match_all('`'.$this->match_patterns['identifier'].'`', $sql, $this->matches['identifiers'], PREG_SET_ORDER); + preg_match_all('`'.$this->match_patterns['operator'].'`', $sql, $this->matches['operators'], PREG_SET_ORDER); + + // Get everything at once for ordering + $full_pattern = '`'.$this->match_patterns['function'].'+|'.$this->match_patterns['identifier'].'|('.$this->match_patterns['operator'].')+`i'; + preg_match_all($full_pattern, $sql, $this->matches['combined'], PREG_SET_ORDER); + + // Go through the matches, and get the most relevant matches + $this->matches = array_map(array($this, 'filter_array'), $this->matches); + } + + // -------------------------------------------------------------------------- + + /** + * Public parser method for seting the parse string + * + * @param string + */ + public function parse_join($sql) + { + $this->__construct($sql); + return $this->matches; + } + + // -------------------------------------------------------------------------- + + /** + * Returns a more useful match array + * + * @param array + * @return array + */ + private function filter_array($array) + { + $new_array = array(); + + foreach($array as $row) + { + if (is_array($row)) + { + $new_array[] = $row[0]; + } + else + { + $new_array[] = $row; + } + } + + return $new_array; } } diff --git a/tests/core/db_qp_test.php b/tests/core/db_qp_test.php index f19578a..442e3d2 100644 --- a/tests/core/db_qp_test.php +++ b/tests/core/db_qp_test.php @@ -13,6 +13,9 @@ // -------------------------------------------------------------------------- +/** + * Tests for the Query Parser + */ class QPTest extends UnitTestCase { public function __construct() @@ -22,16 +25,33 @@ class QPTest extends UnitTestCase { public function TestGeneric() { - $this->parser->__construct('table1.field1=table2.field2'); + $matches = $this->parser->parse_join('table1.field1=table2.field2'); + $this->assertIdentical($matches['combined'], array( + 'table1.field1', '=', 'table2.field2' + )); + } - //echo '
'.print_r($this->parser->matches, TRUE).'
'; + public function TestGeneric2() + { + $matches = $this->parser->parse_join('db1.table1.field1!=db2.table2.field2'); + $this->assertIdentical($matches['combined'], array( + 'db1.table1.field1','!=','db2.table2.field2' + )); + } + + public function TestWUnderscore() + { + $matches = $this->parser->parse_join('table_1.field1 = tab_le2.field_2'); + $this->assertIdentical($matches['combined'], array( + 'table_1.field1', '=', 'tab_le2.field_2' + )); } public function TestFunction() { - $this->parser->__construct('table1.field1 > SUM(3+5)'); - - //echo '
'.print_r($this->parser->matches, TRUE).'
'; + $matches = $this->parser->parse_join('table1.field1 > SUM(3+5)'); + $this->assertIdentical($matches['combined'], array( + 'table1.field1', '>', 'SUM(3+5)' + )); } - } \ No newline at end of file diff --git a/tests/db_files/FB_TEST_DB.FDB b/tests/db_files/FB_TEST_DB.FDB index f99f20a14e430639f85997b7926eae9628275a64..0ca8fa2ba98f6e3289083da94e23c7ce00953d2a 100644 GIT binary patch delta 2087 zcmb7FOK1~O6umP|+9a<@9{o`TF?O0-L@~83tx~iU5iKPYi6L5{6)IIJjWi3F4K8Nk zN^mitOTo3M)Qu>Lf-805s<>#1_@QXKbYZ~r-n`qXt=0O%pOx>&^mEuJ7&M8$0|Bl5Bh?G_depa31cNc#ymvF18A6PwS7(Eaf~;gj=s z-fE=xB25FyHd;+|Qfnej?eP>%BvUjQk5g;Xp&TN`+vr2RLyzh^=_cZ(`ZNtD96GN4 z!+I&=^iyCjyGQsskvkoo7Yx^r2efKA~+CxA~aK+IdH8wJ86+(;bmDrsuWiGJvY2nEft>Ra6Cf(Tdn>4;jVe7br3Vj|s+{0b{8ZU(>(_#Q~#;Si1y8 zL@1`bD@$UZ0>Ptt)W|;j3R$M%R$|O)FxF{c&h&4P?KiTBS{6~u?)(T@0A20nkEr&D zku5GmmT6d)G5gDyWeXc*3q}@E%Oc)hgujTN7STX4-9V{xxQv%>;o2Bxb=RR6{#Xoo zSC>Wm5R-GwjM;j|I$3zY6P#~$&Lo3~WDYj}K3XwP4Ait9sH~{%L2q+SzYXLh?=p3Gq9W2Zl!K1T2lt?%`48_djC7S|#2@=+ Tz-O43-&m}^)p)*V=*m($Qp55iG;-1-zK ztDJMneoAg0D04T)VSiPdXGd7WRa>}yXx_(o$6wQC7cGk1R;tKTn6HW zINwK{cSKeulG-XsdsQxIbMB82C4vye3sHQ*_H}lT|ED|$>1)doNiW7kB()>ubM8t; z`Y{=)DePmpDp919;w205$uz&fb{}=yQCYc8Qd=i!ug;xhnr8Qah!VkejuI0u8Hl$T zG(6bsBW^k(D-%iWEJ=H_+_JO0W?pn~s45YJC|)`%UWm;;V$%^>nMi7DB<ZfEI; z5