From a4f730b0815bbbba6d97fdb20acbe29208ba8252 Mon Sep 17 00:00:00 2001 From: Timothy Warren Date: Thu, 23 Aug 2012 01:17:27 +0000 Subject: [PATCH] Fixed issue with multiple fields in order_by, removed extranious spaces in SQL generation --- autoload.php | 14 +++ classes/db_pdo.php | 18 +++- classes/query_builder.php | 23 ++--- tests/core/db_qb_test.php | 161 ++++++++++++++++++---------------- tests/db_files/FB_TEST_DB.FDB | Bin 802816 -> 802816 bytes 5 files changed, 124 insertions(+), 92 deletions(-) diff --git a/autoload.php b/autoload.php index 1846e2c..4bbe26b 100644 --- a/autoload.php +++ b/autoload.php @@ -42,6 +42,20 @@ if ( ! function_exists('do_include')) } } +if ( ! function_exists('mb_trim')) +{ + /** + * Multibyte-safe trim function + * + * @param string + * @return string + */ + function mb_trim($string) + { + return preg_replace("/(^\s+)|(\s+$)/us", "", $string); + } +} + /** * Load a Query class * diff --git a/classes/db_pdo.php b/classes/db_pdo.php index eca348a..7a07f81 100644 --- a/classes/db_pdo.php +++ b/classes/db_pdo.php @@ -211,14 +211,19 @@ 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) + // Handle comma-separated identifiers + if (strpos($ident, ',') !== FALSE) { - return $ident; + $parts = explode(',', $ident); + $parts = array_map('mb_trim', $parts); + $parts = array_map(array($this, 'quote_ident'), $parts); + $ident = implode(',', $parts); } + // Split each identifier by the period $hiers = explode('.', $ident); + $hiers = array_map('mb_trim', $hiers); // Return the re-compiled string return implode('.', array_map(array($this, '_quote'), $hiers)); @@ -234,11 +239,18 @@ abstract class DB_PDO extends PDO { */ protected function _quote($str) { + // Don't quote numbers if ( ! is_string($str) && is_numeric($str)) { return $str; } + // Don't add additional quotes + if (strpos($str, $this->escape_char) === 0 || strrpos($str, $this->escape_char) === 0) + { + return $str; + } + return "{$this->escape_char}{$str}{$this->escape_char}"; } diff --git a/classes/query_builder.php b/classes/query_builder.php index 1e8a910..a9059cd 100644 --- a/classes/query_builder.php +++ b/classes/query_builder.php @@ -161,13 +161,6 @@ class Query_Builder { */ private $having_map; - /** - * Query parser to safely escape conditions - * - * @var object - */ - private $parser; - /** * Convenience property for connection management * @@ -317,7 +310,7 @@ class Query_Builder { } } - $this->select_string .= implode(', ', $safe_array); + $this->select_string .= implode(',', $safe_array); unset($safe_array); @@ -676,7 +669,7 @@ class Query_Builder { $item = $this->quote_ident($f_array[0]); // Simple key value, or an operator - $item .= (count($f_array) === 1) ? '= ?' : " {$f_array[1]} ?"; + $item .= (count($f_array) === 1) ? '=?' : " {$f_array[1]} ?"; // Put in the query map for select statements $this->query_map[] = array( @@ -841,7 +834,7 @@ class Query_Builder { $this->set_array_keys = array_map(array($this->db, 'quote_ident'), $this->set_array_keys); // Generate the "set" string - $this->set_string = implode('=?, ', $this->set_array_keys); + $this->set_string = implode('=?,', $this->set_array_keys); $this->set_string .= '=?'; return $this; @@ -859,7 +852,7 @@ class Query_Builder { */ public function join($table, $condition, $type='') { - $table = implode(" ", array_map(array($this->db, 'quote_ident'), explode(' ', trim($table)))); + $table = implode(' ', array_map(array($this->db, 'quote_ident'), explode(' ', trim($table)))); $parser = new query_parser(); @@ -876,7 +869,7 @@ class Query_Builder { } } - $parsed_condition = implode(' ', $parts['combined']); + $parsed_condition = implode('', $parts['combined']); $condition = $table . ' ON ' . $parsed_condition; @@ -908,7 +901,7 @@ class Query_Builder { $this->group_array[] = $this->quote_ident($field); } - $this->group_string = ' GROUP BY ' . implode(', ', $this->group_array); + $this->group_string = ' GROUP BY ' . implode(',', $this->group_array); return $this; } @@ -1371,8 +1364,8 @@ class Query_Builder { $param_count = count($this->set_array_keys); $params = array_fill(0, $param_count, '?'); $sql = "INSERT INTO {$table} (" - . implode(', ', $this->set_array_keys) . - ') VALUES ('.implode(', ', $params).')'; + . implode(',', $this->set_array_keys) . + ') VALUES ('.implode(',', $params).')'; break; case "update": diff --git a/tests/core/db_qb_test.php b/tests/core/db_qb_test.php index fe3a3c3..19ff326 100644 --- a/tests/core/db_qb_test.php +++ b/tests/core/db_qb_test.php @@ -21,7 +21,7 @@ abstract class QBTest extends UnitTestCase { // -------------------------------------------------------------------------- // ! Get Tests // -------------------------------------------------------------------------- - + public function TestGet() { if (empty($this->db)) return; @@ -30,7 +30,7 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- public function TestGetLimit() @@ -41,7 +41,7 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- public function TestGetLimitSkip() @@ -52,59 +52,59 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- - + public function TestGetWhere() { if (empty($this->db)) return; - + $query = $this->db->get_where('create_test', array('id !=' => 1), 2, 1); - + $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- - + public function TestHaving() { if (empty($this->db)) return; - + $query = $this->db->select('id') ->from('create_test') ->group_by('id') ->having(array('id >' => 1)) ->having('id !=', 3) ->get(); - + $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- - + public function TestOrHaving() { if (empty($this->db)) return; - + $query = $this->db->select('id') ->from('create_test') ->group_by('id') ->having(array('id >' => 1)) ->or_having('id !=', 3) ->get(); - + $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- - + public function TestGetViews() { if (empty($this->db)) return; - + $this->assertTrue(is_array($this->db->get_views())); } - + // -------------------------------------------------------------------------- // ! Select Tests // -------------------------------------------------------------------------- @@ -120,7 +120,7 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- public function TestSelectWhereGet2() @@ -133,68 +133,81 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- - + public function TestSelectMax() { if (empty($this->db)) return; - + $query = $this->db->select_max('id', 'di') ->get('create_test'); - + $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- - + public function TestSelectMin() { if (empty($this->db)) return; - + $query = $this->db->select_min('id', 'di') ->get('create_test'); - + $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- - + + public function TestMultiOrderBy() + { + if (empty($this->db)) return; + + $query = $this->db->from('create_test') + ->order_by('id, key') + ->get(); + + $this->assertIsA($query, 'PDOStatement'); + } + + // -------------------------------------------------------------------------- + public function TestSelectAvg() { if (empty($this->db)) return; - + $query = $this->db->select_avg('id', 'di') ->get('create_test'); - + $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- - + public function TestSelectSum() { if (empty($this->db)) return; - + $query = $this->db->select_sum('id', 'di') ->get('create_test'); - + $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- - + public function TestSelectDistinct() { if (empty($this->db)) return; - + $query = $this->db->select_sum('id', 'di') ->distinct() ->get('create_test'); - + $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- public function TestSelectGet() @@ -206,7 +219,7 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- public function TestSelectFromGet() @@ -220,7 +233,7 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- public function TestSelectFromLimitGet() @@ -235,7 +248,7 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- // ! Query modifier tests // -------------------------------------------------------------------------- @@ -255,7 +268,7 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- public function TestOrderByRandom() @@ -272,7 +285,7 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- public function TestGroupBy() @@ -293,7 +306,7 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- public function TestOrWhere() @@ -309,7 +322,7 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- public function TestLike() @@ -322,7 +335,7 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- public function TestJoin() @@ -335,7 +348,7 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- // ! DB update tests // -------------------------------------------------------------------------- @@ -351,7 +364,7 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- public function TestUpdate() @@ -366,7 +379,7 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- public function TestDelete() @@ -377,58 +390,58 @@ abstract class QBTest extends UnitTestCase { $this->assertIsA($query, 'PDOStatement'); } - + // -------------------------------------------------------------------------- // ! Non-data read queries // -------------------------------------------------------------------------- - + public function TestCountAll() { if (empty($this->db)) return; $query = $this->db->count_all('create_test'); - + $this->assertTrue(is_numeric($query)); } - + // -------------------------------------------------------------------------- - + public function TestCountAllResults() { if (empty($this->db)) return; $query = $this->db->count_all_results('create_test'); - + $this->assertTrue(is_numeric($query)); } - + // -------------------------------------------------------------------------- - + public function TestCountAllResults2() { if (empty($this->db)) return; - + $query = $this->db->select('id, key as k, val') ->from('create_test') ->where(' id ', 1) ->or_where('key >', 0) ->limit(2, 1) ->count_all_results(); - + $this->assertTrue(is_numeric($query)); } - + // -------------------------------------------------------------------------- - + public function TestNumRows() { $query = $this->db->get('create_test'); - + $this->assertTrue(is_numeric($this->db->num_rows())); } - + // -------------------------------------------------------------------------- // ! Error Tests // -------------------------------------------------------------------------- - + /** * Handles invalid drivers */ @@ -442,14 +455,14 @@ abstract class QBTest extends UnitTestCase { 'pass' => NULL, 'type' => 'QGYFHGEG' ); - + $this->expectException('BadDBDriverException'); - + $this->db = new Query_Builder($params); } - + // -------------------------------------------------------------------------- - + public function TestBadConnection() { $params = array( @@ -460,11 +473,11 @@ abstract class QBTest extends UnitTestCase { 'pass' => NULL, 'type' => 'pgsql' ); - + $this->expectException('BadConnectionException'); - + $this->db = new Query_Builder($params); - + } } diff --git a/tests/db_files/FB_TEST_DB.FDB b/tests/db_files/FB_TEST_DB.FDB index 0ca8fa2ba98f6e3289083da94e23c7ce00953d2a..6939e6f7712222a0665908ae52c4a6220637576b 100644 GIT binary patch delta 1838 zcmb`H&ubG=5Xa|j^P|l(*;iXiMOvCRXzAqM-0;8sgrse5SvK;j8tu%{aUmruV-JUQkp5MbbMN(+Y9(!k5;)C+S_*n-)@ zutke4e75l0B4CT4En02Swjx44!5**r`$FZ`hZDkFtqz3(L3=E;GRA$+@6K|^H(jr= z4#2wW>_4Uew z!YIn*CXWV=Vxakq+$r^4=!zNeF!Z8!JHDK~!A4%VX_-=43_L zzQxb62QpPgI->^RrgW4o=BnvPYg(iuZECvN^@4}4(1fnw(5Y?q(D@J2;o@B^BxZdG zU_tJEL%<~k7`@url#a4B>U5+vEz*%zRcGnh^}L5J--IrIXzj2W_ghr8!Z_gKztk@8 z)b75#ROL}QUFQ8M%0_n@b|7ipX)dklF#DV*`<%{RmdkvU<#eWU>oXrzHf}SE$lDb@ zj0&43CWos$!ECu#;gczHq?-T(k)}jQJ1zejEKgv|71(kGMjLlPj%;Hn%i&$ry~+%^ zw#{QHa-{2nfk{&$q@8YLu;2;2;0nCp1U^|oVziN1ZhYY<(AGV4NB58%{lraqqs%)} zx!LqL8IM(&N|SyRFNlB_yISgi6q|e;E8wuf)F4G-i&b*-^}i#_rrcL-0bGgVdUh09?ZfV6f*i0O^kDrpE{E009R61~p)K%tl^^244jp2~#jr z7^WyQh07FfQb{zi!U8elNc0K+~X)CD4t#ua>F_*xZ#H&xB>1lQYDjnL(fK##hH z<0JRMZ+?Vm<-h_@vW$$+@p^f9jaR4P#vPS{fzw{&ZD~yV9by-rVIw!k2$w00=L!29 zoO0q71Iy=IQ}XQ`4?PT67`r(JyW5Wy-2@S@6THsdlZ6Cm$ z!BC$C!$mr#xG+X>6=%CuBAE9<ZbI`Kit8dw5dh za6G9!P+`Gdw(yd0xjxGe8E#8Xf9H*?UtU_}vE9mrQbm4k4Pj-BvWBoO55&EWHQFk) zrq{M6_+Qp25*jW}TDc3p^x;7I`??;}}V?go%rAK)nbol`bb7EI)b{93(w)=+2$M2lp_M*CVw+OJR!YcPWMc`X6qPN@yb1s7 zZFovs$XxRMGLI(dcB&x4Bu`k&WAEtPCf8}pb=q>BIx#u3!jEwkX%9oa!z4Q?*%~HD z45UbytRzf|*=>qhTQO@ZX63WlqLo?sZI(w3uTw2ob@Rnk@2+C1L`10(CT9uj2g6jU z_tHB|+rnuj91CvkiP$*XU1^>DalZwdsXMI@vD%()oVZ$U{1W7Ir*n2k47HMywg4;D y@oj4x4N#Qx{C2NM+r^Vn-o}Evd|^_$0$kXIbQ^AJAR79lkQlY&$&Ga`Qhxv;s7#gs