Parsed join conditions fixes #2

classes/query_builder.php

@@ -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
 	 *
@@ -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;
@@ -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] .
+		$parser = new query_parser();
-				' ' . $this->quote_ident($cond_array[2]);
+
+		// 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',

classes/query_parser.php

@@ -27,9 +27,9 @@ class Query_Parser {
 	 * @var array
 	 */
 	private $match_patterns = array(
-		'function' => '`([a-zA-Z0-9_]+\((.*?)\))`',
+		'function' => '([a-zA-Z0-9_]+\((.*?)\))',
-		'identifier' => '`([a-zA-Z0-9"_-]+\.?)+`',
+		'identifier' => '([a-zA-Z0-9_-]+\.?)+',
-		'operator' => '`=|AND|&&?|~|\|\|?|\^|/|>=?|<=?|-|%|OR|\+|NOT|\!=?|<>|XOR`'
+		'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);
+		// Get sql clause components
-		preg_match_all($this->match_patterns['identifier'], $sql, $this->matches['identifiers'], PREG_SET_ORDER);
+		preg_match_all('`'.$this->match_patterns['function'].'`', $sql, $this->matches['functions'], PREG_SET_ORDER);
-		preg_match_all($this->match_patterns['operator'], $sql, $this->matches['operators'], 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;
 	}
 
 }

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 '<pre>'.print_r($this->parser->matches, TRUE).'</pre>';
+	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)');
+		$matches = $this->parser->parse_join('table1.field1 > SUM(3+5)');
-
+		$this->assertIdentical($matches['combined'], array(
-		//echo '<pre>'.print_r($this->parser->matches, TRUE).'</pre>';
+			'table1.field1', '>', 'SUM(3+5)'
+		));
 	}
-
 }