Merge pull request #4246 from michalsn/fix/sqlsrv_schema_read

Fix select queries in SQLSRV when using different schema

system/Database/BaseBuilder.php

@@ -2380,7 +2380,7 @@ protected function _replace(string $table, array $keys, array $values): string
 	 * Groups tables in FROM clauses if needed, so there is no confusion
 	 * about operator precedence.
 	 *
-	 * Note: This is only used (and overridden) by MySQL and CUBRID.
+	 * Note: This is only used (and overridden) by MySQL and SQLSRV.
 	 *
 	 * @return string
 	 */

system/Database/SQLSRV/Builder.php

@@ -62,6 +62,25 @@ class Builder extends BaseBuilder
 
 	//--------------------------------------------------------------------
 
+	/**
+	 * FROM tables
+	 *
+	 * Groups tables in FROM clauses if needed, so there is no confusion
+	 * about operator precedence.
+	 *
+	 * @return string
+	 */
+	protected function _fromTables(): string
+	{
+		$from = [];
+		foreach ($this->QBFrom as $value)
+		{
+			$from[] = $this->getFullName($value);
+		}
+
+		return implode(', ', $from);
+	}
+
 	/**
 	 * Truncate statement
 	 *
@@ -79,6 +98,94 @@ protected function _truncate(string $table): string
 		return 'TRUNCATE TABLE ' . $this->getFullName($table);
 	}
 
+	/**
+	 * JOIN
+	 *
+	 * Generates the JOIN portion of the query
+	 *
+	 * @param string  $table
+	 * @param string  $cond   The join condition
+	 * @param string  $type   The type of join
+	 * @param boolean $escape Whether not to try to escape identifiers
+	 *
+	 * @return $this
+	 */
+	public function join(string $table, string $cond, string $type = '', bool $escape = null)
+	{
+		if ($type !== '')
+		{
+			$type = strtoupper(trim($type));
+
+			if (! in_array($type, $this->joinTypes, true))
+			{
+				$type = '';
+			}
+			else
+			{
+				$type .= ' ';
+			}
+		}
+
+		// Extract any aliases that might exist. We use this information
+		// in the protectIdentifiers to know whether to add a table prefix
+		$this->trackAliases($table);
+
+		is_bool($escape) || $escape = $this->db->protectIdentifiers;
+
+		if (! $this->hasOperator($cond))
+		{
+			$cond = ' USING (' . ($escape ? $this->db->escapeIdentifiers($cond) : $cond) . ')';
+		}
+		elseif ($escape === false)
+		{
+			$cond = ' ON ' . $cond;
+		}
+		else
+		{
+			// Split multiple conditions
+			if (preg_match_all('/\sAND\s|\sOR\s/i', $cond, $joints, PREG_OFFSET_CAPTURE))
+			{
+				$conditions = [];
+				$joints     = $joints[0];
+				array_unshift($joints, ['', 0]);
+
+				for ($i = count($joints) - 1, $pos = strlen($cond); $i >= 0; $i --)
+				{
+					$joints[$i][1] += strlen($joints[$i][0]); // offset
+					$conditions[$i] = substr($cond, $joints[$i][1], $pos - $joints[$i][1]);
+					$pos            = $joints[$i][1] - strlen($joints[$i][0]);
+					$joints[$i]     = $joints[$i][0];
+				}
+				ksort($conditions);
+			}
+			else
+			{
+				$conditions = [$cond];
+				$joints     = [''];
+			}
+
+			$cond = ' ON ';
+			foreach ($conditions as $i => $condition)
+			{
+				$operator = $this->getOperator($condition);
+
+				$cond .= $joints[$i];
+				$cond .= preg_match("/(\(*)?([\[\]\w\.'-]+)" . preg_quote($operator) . '(.*)/i', $condition, $match) ? $match[1] . $this->db->protectIdentifiers($match[2]) . $operator . $this->db->protectIdentifiers($match[3]) : $condition;
+			}
+		}
+
+		// Do we want to escape the table name?
+		if ($escape === true)
+		{
+			$table = $this->db->protectIdentifiers($table, true, null, false);
+		}
+
+		// Assemble the JOIN statement
+		$this->QBJoin[] = $join = $type . 'JOIN ' . $this->getFullName($table) . $cond;
+
+		return $this;
+	}
+
 	/**
 	 * Insert statement
 	 *
@@ -189,11 +296,21 @@ public function decrement(string $column, int $value = 1)
 	 */
 	private function getFullName(string $table): string
 	{
+		$alias = '';
+
+		if (strpos($table, ' ') !== false)
+		{
+			$alias = explode(' ', $table);
+			$table = array_shift($alias);
+			$alias = ' ' . implode(' ', $alias);
+		}
+
 		if ($this->db->escapeChar === '"')
 		{
-			return '"' . $this->db->getDatabase() . '"."' . $this->db->schema . '"."' . str_replace('"', '', $table) . '"';
+			return '"' . $this->db->getDatabase() . '"."' . $this->db->schema . '"."' . str_replace('"', '', $table) . '"' . $alias;
 		}
-		return '[' . $this->db->getDatabase() . '].[' . $this->db->schema . '].[' . str_replace('"', '', $table) . ']';
+
+		return '[' . $this->db->getDatabase() . '].[' . $this->db->schema . '].[' . str_replace('"', '', $table) . ']' . str_replace('"', '', $alias);
 	}
 
 	/**

system/Test/CIDatabaseTestCase.php

@@ -12,6 +12,7 @@
 namespace CodeIgniter\Test;
 
 use CodeIgniter\Database\BaseConnection;
+use CodeIgniter\Database\BaseBuilder;
 use CodeIgniter\Database\Exceptions\DatabaseException;
 use CodeIgniter\Database\MigrationRunner;
 use CodeIgniter\Database\Seeder;
@@ -162,6 +163,22 @@ public function loadDependencies()
 
 	//--------------------------------------------------------------------
 
+	/**
+	 * Loads the Builder class appropriate for the current database.
+	 *
+	 * @param string $tableName
+	 *
+	 * @return BaseBuilder
+	 */
+	public function loadBuilder(string $tableName)
+	{
+		$builderClass = str_replace('Connection', 'Builder', get_class($this->db));
+
+		return new $builderClass($tableName, $this->db);
+	}
+
+	//--------------------------------------------------------------------
+
 	/**
 	 * Ensures that the database is cleaned up to a known state
 	 * before each test runs.

tests/system/Database/Builder/FromTest.php

@@ -1,6 +1,7 @@
 <?php namespace Builder;
 
 use CodeIgniter\Database\BaseBuilder;
+use CodeIgniter\Database\SQLSRV\Builder as SQLSRVBuilder;
 use CodeIgniter\Test\Mock\MockConnection;
 
 class FromTest extends \CodeIgniter\Test\CIUnitTestCase
@@ -98,4 +99,19 @@ public function testFromReset()
 	}
 
 	//--------------------------------------------------------------------
+
+	public function testFromWithMultipleTablesAsStringWithSQLSRV()
+	{
+		$this->db = new MockConnection(['DBDriver' => 'SQLSRV', 'database' => 'test', 'schema' => 'dbo']);
+
+		$builder = new SQLSRVBuilder('user', $this->db);
+
+		$builder->from(['jobs, roles']);
+
+		$expectedSQL = 'SELECT * FROM "test"."dbo"."user", "test"."dbo"."jobs", "test"."dbo"."roles"';
+
+		$this->assertEquals($expectedSQL, str_replace("\n", ' ', $builder->getCompiledSelect()));
+	}
+
+	//--------------------------------------------------------------------
 }

tests/system/Database/Builder/JoinTest.php

@@ -2,6 +2,7 @@
 
 use CodeIgniter\Database\BaseBuilder;
 use CodeIgniter\Database\Postgre\Builder as PostgreBuilder;
+use CodeIgniter\Database\SQLSRV\Builder as SQLSRVBuilder;
 use CodeIgniter\Test\Mock\MockConnection;
 
 class JoinTest extends \CodeIgniter\Test\CIUnitTestCase
@@ -84,4 +85,19 @@ public function testFullOuterJoin()
 
 	//--------------------------------------------------------------------
 
+	public function testJoinWithAlias()
+	{
+		$this->db = new MockConnection(['DBDriver' => 'SQLSRV', 'database' => 'test', 'schema' => 'dbo']);
+
+		$builder = new SQLSRVBuilder('jobs', $this->db);
+		$builder->testMode();
+		$builder->join('users u', 'u.id = jobs.id', 'LEFT');
+
+		$expectedSQL = 'SELECT * FROM "test"."dbo"."jobs" LEFT JOIN "test"."dbo"."users" "u" ON "u"."id" = "jobs"."id"';
+
+		$this->assertEquals($expectedSQL, str_replace("\n", ' ', $builder->getCompiledSelect()));
+	}
+
+	//--------------------------------------------------------------------
+
 }

tests/system/Database/Builder/SelectTest.php

@@ -2,6 +2,7 @@
 
 use CodeIgniter\Database\BaseBuilder;
 use CodeIgniter\Database\Exceptions\DataException;
+use CodeIgniter\Database\SQLSRV\Builder as SQLSRVBuilder;
 use CodeIgniter\Test\Mock\MockConnection;
 
 class SelectTest extends \CodeIgniter\Test\CIUnitTestCase
@@ -262,4 +263,17 @@ public function testSelectMinThrowsExceptionOnMultipleColumn()
 
 	//--------------------------------------------------------------------
 
+	public function testSimpleSelectWithSQLSRV()
+	{
+		$this->db = new MockConnection(['DBDriver' => 'SQLSRV', 'database' => 'test', 'schema' => 'dbo']);
+
+		$builder = new SQLSRVBuilder('users', $this->db);
+
+		$expected = 'SELECT * FROM "test"."dbo"."users"';
+
+		$this->assertEquals($expected, str_replace("\n", ' ', $builder->getCompiledSelect()));
+	}
+
+	//--------------------------------------------------------------------
+
 }

tests/system/Database/Live/OrderTest.php

@@ -68,18 +68,20 @@ public function testOrderRandom()
 			->orderBy('name', 'random')
 			->getCompiledSelect();
 
-		$key = 'RANDOM()';
+		$key   = 'RANDOM()';
+		$table = $this->db->protectIdentifiers('job', true);
 
 		if ($this->db->DBDriver === 'MySQLi')
 		{
 			$key = 'RAND()';
 		}
 		elseif ($this->db->DBDriver === 'SQLSRV')
 		{
-			$key = 'NEWID()';
+			$key   = 'NEWID()';
+			$table = '"' . $this->db->getDatabase() . '"."' . $this->db->schema . '".' . $table;
 		}
 
-		$expected = 'SELECT * FROM ' . $this->db->protectIdentifiers('job', true) . ' ORDER BY ' . $key;
+		$expected = 'SELECT * FROM ' . $table . ' ORDER BY ' . $key;
 
 		$this->assertEquals($expected, str_replace("\n", ' ', $sql));
 	}

tests/system/Models/CountAllModelTest.php

@@ -31,7 +31,7 @@ public function testcountAllResultsRecoverTempUseSoftDeletes(): void
 
 	public function testcountAllResultsFalseWithDeletedTrue(): void
 	{
-		$builder     = new BaseBuilder('user', $this->db);
+		$builder     = $this->loadBuilder('user');
 		$expectedSQL = $builder->testMode()->countAllResults();
 
 		$this->createModel(UserModel::class);
@@ -47,7 +47,7 @@ public function testcountAllResultsFalseWithDeletedTrue(): void
 
 	public function testcountAllResultsFalseWithDeletedFalse(): void
 	{
-		$builder     = new BaseBuilder('user', $this->db);
+		$builder     = $this->loadBuilder('user');
 		$expectedSQL = $builder->testMode()->where('user.deleted_at', null)->countAllResults();
 
 		$this->createModel(UserModel::class);
@@ -63,7 +63,7 @@ public function testcountAllResultsFalseWithDeletedFalse(): void
 
 	public function testcountAllResultsFalseWithDeletedTrueUseSoftDeletesFalse(): void
 	{
-		$builder     = new BaseBuilder('user', $this->db);
+		$builder     = $this->loadBuilder('user');
 		$expectedSQL = $builder->testMode()->countAllResults();
 
 		$this->createModel(UserModel::class);
@@ -80,7 +80,7 @@ public function testcountAllResultsFalseWithDeletedTrueUseSoftDeletesFalse(): vo
 
 	public function testcountAllResultsFalseWithDeletedFalseUseSoftDeletesFalse(): void
 	{
-		$builder     = new BaseBuilder('user', $this->db);
+		$builder     = $this->loadBuilder('user');
 		$expectedSQL = $builder->testMode()->where('user.deleted_at', null)->countAllResults();
 
 		$this->createModel(UserModel::class);