fix: [SQLSRV] Query Builder always sets "<database>"."<schema>". to the table name.

[ping-yee]

Description
See #8697

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[kenjis]

@codeigniter4/database-team
This PR resolves the issue consistent with the current implementation.
However, such ad hoc code complicates the code and is not a fundamental solution.

Then I am not sure why only SQLSRV generates table names with database and schema names.
I am not sure if this PR should be merged.

[sclubricants]

Do we have this problem with postgre too? I know it uses schema sometimes as well..

[kenjis]

@sclubricants No. Only SQLSRV has the getFullName() method and use it.

CodeIgniter4/system/Database/SQLSRV/Builder.php

Line 282 in 971b0da

private function getFullName(string $table): string

This test passes.

--- a/tests/system/Database/Builder/FromTest.php
+++ b/tests/system/Database/Builder/FromTest.php
@@ -15,6 +15,7 @@ namespace CodeIgniter\Database\Builder;
 
 use CodeIgniter\Database\BaseBuilder;
 use CodeIgniter\Database\SQLSRV\Builder as SQLSRVBuilder;
+use CodeIgniter\Database\Postgre\Builder as PostgreBuilder;
 use CodeIgniter\Test\CIUnitTestCase;
 use CodeIgniter\Test\Mock\MockConnection;
 
@@ -153,4 +154,15 @@ final class FromTest extends CIUnitTestCase
 
         $this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->getCompiledSelect()));
     }
+
+    public function testConstructorWithMultipleSegmentTable(): void
+    {
+        $this->db = new MockConnection(['DBDriver' => 'Postgre', 'database' => 'test', 'schema' => 'dbo']);
+
+        $builder = new PostgreBuilder('database.dbo.table', $this->db);
+
+        $expectedSQL = 'SELECT * FROM "database"."dbo"."table"';
+
+        $this->assertSame($expectedSQL, str_replace("\n", ' ', $builder->getCompiledSelect()));
+    }
 }

[michalsn]

@kenjis

Then I am not sure why only SQLSRV generates table names with database and schema names.
I am not sure if this PR should be merged.

It was related to some reported issue, but I can't find it now.

I believe it was something related to using non-default schema. In that case, the generated queries weren't using the correct one...

AFAIR in SQLSRV you can't declare something like use mySchemaName. So if we're using the non-default schema, we have to declare it precisely every time in the query.

[kenjis]

@michalsn This? #4246
But there is no getFullName() (and Builder for sqlsrv) in CI3.
https://github.com/bcit-ci/CodeIgniter/tree/develop/system/database/drivers/sqlsrv

It seems SQLSRV was added in #3714, and it uses getFullName() from the beginning.
1e43715#diff-9824e937f41ead0d8d69015f5c47e4adce5518bf7aea4bf87fbed37eb783c67cR157

getFullName() in #3714:

        /**
	 * Get full name of the table
	 *
	 * @param string $table
	 *
	 * @return string
	 */
	private function getFullName(string $table): string
	{
		if ($this->db->escapeChar === '"')
		{
			return '"' . $this->db->getDatabase() . '"."' . $this->db->schema . '"."' . str_replace('"', '', $table) . '"';
		}
		return '[' . $this->db->getDatabase() . '].[' . $this->db->schema . '].[' . str_replace('"', '', $table) . ']';
	}

[kenjis]

AFAIR in SQLSRV you can't declare something like use mySchemaName. So if we're using the non-default schema, we have to declare it precisely every time in the query.

I googled, but yes, there seems no way to set schema in a connection.
https://stackoverflow.com/questions/4942023/set-default-schema-for-a-sql-query

[michalsn]

Yes, that was the issue/PR I had in mind.

The getFullName() method was probably created to simplify the code. I've never worked with non-default schema, so I can't determine the exact differences with CI4, but it seems to me that in CI3 we had to manually declare the schema if it wasn't the default one. In CI4 we have a config variable for that, so implementing the getFullName() was a convenient way to build the proper table name.

[kenjis]

Okay, in summary, it looks like this?

• in CI3, we cannot set schema in DB config, and CI3 does nothing to schema in gererated SQL statements.
• in CI4, we can set schema in DB config, and there is no way to set the schema in a DB connection. So CI4 generates full tablename (database.schema.table)

[michalsn]

Yes.

[kenjis]

@ping-yee Thank you!