Fix & reimplement getLastInsertedId for various sequence names

src/Drivers/PdoPgsql/PdoPgsqlDriver.php

@@ -76,18 +76,19 @@ public function createPlatform(IConnection $connection): IPlatform
 	public function getLastInsertedId(string|Fqn|null $sequenceName = null): mixed
 	{
 		if ($sequenceName === null) {
-			throw new InvalidArgumentException('PgsqlDriver requires to pass sequence name for getLastInsertedId() method.');
+			throw new InvalidArgumentException('PgsqlDriver requires passing a sequence name for getLastInsertedId() method.');
 		}
 
 		$this->checkConnection();
 		assert($this->connection !== null);
 
-		$sequenceName = match (true) {
-			$sequenceName instanceof Fqn => $this->convertIdentifierToSql($sequenceName->schema) . '.' .
-				$this->convertIdentifierToSql($sequenceName->name),
-			default => $this->convertIdentifierToSql($sequenceName),
-		};
-		$sql = 'SELECT CURRVAL(\'' . $sequenceName . '\')';
+		if ($sequenceName instanceof Fqn) {
+			$sequenceName = $this->convertIdentifierToSql($sequenceName);
+			$sql = 'SELECT CURRVAL(\'' . $sequenceName . '\')';
+		} else {
+			$sequenceName = $this->convertStringToSql($sequenceName);
+			$sql = "SELECT CURRVAL($sequenceName)";
+		}
 		return $this->loggedQuery($sql)->fetchField();
 	}
 
@@ -117,12 +118,11 @@ protected function createResultAdapter(PDOStatement $statement): IResultAdapter
 
 	protected function convertIdentifierToSql(string|Fqn $identifier): string
 	{
-		$escaped = match (true) {
-			$identifier instanceof Fqn => str_replace('"', '""', $identifier->schema) . '.'
-				. str_replace('"', '""', $identifier->name),
-			default => str_replace('"', '""', $identifier),
+		return match (true) {
+			$identifier instanceof Fqn => '"' . str_replace('"', '""', $identifier->schema) . '"."'
+				. str_replace('"', '""', $identifier->name) . '"',
+			default => '"' . str_replace('"', '""', $identifier) . '"',
 		};
-		return '"' . $escaped . '"';
 	}
 
 

src/Drivers/Pgsql/PgsqlDriver.php

@@ -182,13 +182,18 @@ public function query(string $query): Result
 	public function getLastInsertedId(string|Fqn|null $sequenceName = null): mixed
 	{
 		if ($sequenceName === null) {
-			throw new InvalidArgumentException('PgsqlDriver requires to pass sequence name for getLastInsertedId() method.');
+			throw new InvalidArgumentException('PgsqlDriver requires passing a sequence name for getLastInsertedId() method.');
 		}
 		$this->checkConnection();
 		assert($this->connection !== null);
 
-		$sequenceName = $this->convertIdentifierToSql($sequenceName);
-		$sql = 'SELECT CURRVAL(\'' . $sequenceName . '\')';
+		if ($sequenceName instanceof Fqn) {
+			$sequenceName = $this->convertIdentifierToSql($sequenceName);
+			$sql = 'SELECT CURRVAL(\'' . $sequenceName . '\')';
+		} else {
+			$sequenceName = $this->convertStringToSql($sequenceName);
+			$sql = "SELECT CURRVAL($sequenceName)";
+		}
 		return $this->loggedQuery($sql)->fetchField();
 	}
 

src/IConnection.php

@@ -7,6 +7,9 @@
 use Nextras\Dbal\Drivers\Exception\DriverException;
 use Nextras\Dbal\Drivers\Exception\QueryException;
 use Nextras\Dbal\Drivers\IDriver;
+use Nextras\Dbal\Drivers\PdoPgsql\PdoPgsqlDriver;
+use Nextras\Dbal\Drivers\Pgsql\PgsqlDriver;
+use Nextras\Dbal\Platforms\Data\Column;
 use Nextras\Dbal\Platforms\Data\Fqn;
 use Nextras\Dbal\Platforms\IPlatform;
 use Nextras\Dbal\QueryBuilder\QueryBuilder;
@@ -86,6 +89,18 @@ public function queryByQueryBuilder(QueryBuilder $queryBuilder): Result;
 
 	/**
 	 * Returns last inserted ID.
+	 *
+	 * The sequence name's implementation depends on a particular database platform and driver.
+	 *
+	 * This method accepts the very same value obtained through platform reflection, e.g., through
+	 * {@see IPlatform::getLastInsertedId} or alternatively through {@see IPlatform::getColumns()} and
+	 * its {@see Column::$meta} property: `$column->meta['sequence']`.
+	 *
+	 * In case of {@see PgsqlDriver} or {@see PdoPgsqlDriver} the name is a string that may
+	 * container double-quotes for handling cases sensitive names or names with special characters.
+	 * I.e. `public."MySchemaName"` is a valid sequence name argument. Alternatively, you may pass a Fqn instance
+	 * that will properly double-quote the schema and name.
+	 *
 	 * @return int|string|null
 	 */
 	public function getLastInsertedId(string|Fqn|null $sequenceName = null);

tests/cases/integration/connection.postgres.phpt

@@ -33,11 +33,13 @@ class ConnectionPostgresTest extends IntegrationTestCase
 
 		$this->connection->query('INSERT INTO publishers %values', ['name' => 'FOO']);
 		Assert::same(2, $this->connection->getLastInsertedId('publishers_id_seq'));
+		Assert::same(2, $this->connection->getLastInsertedId('public.publishers_id_seq'));
+		Assert::same(2, $this->connection->getLastInsertedId('"public"."publishers_id_seq"'));
 		Assert::same(2, $this->connection->getLastInsertedId(new Fqn(schema: 'public', name: 'publishers_id_seq')));
 
 		Assert::exception(function() {
 			$this->connection->getLastInsertedId();
-		}, InvalidArgumentException::class, 'PgsqlDriver requires to pass sequence name for getLastInsertedId() method.');
+		}, InvalidArgumentException::class, 'PgsqlDriver requires passing a sequence name for getLastInsertedId() method.');
 	}
 
 
@@ -47,7 +49,7 @@ class ConnectionPostgresTest extends IntegrationTestCase
 		$this->connection->query('DROP SEQUENCE IF EXISTS %column', "MySequence");
 		$this->connection->query('CREATE SEQUENCE %column INCREMENT 5 START 10;', "MySequence");
 		$this->connection->query('SELECT NEXTVAL(\'%column\')', "MySequence");
-		Assert::same(10, $this->connection->getLastInsertedId("MySequence"));
+		Assert::same(10, $this->connection->getLastInsertedId('"MySequence"'));
 	}
 }
 

tests/cases/unit/CachedPlatformTest.phpt

@@ -6,11 +6,16 @@
 
 namespace NextrasTests\Dbal;
 
+
 use Mockery;
 use Mockery\MockInterface;
 use Nette\Caching\Cache;
-use Nette\Caching\IStorage;
+use Nette\Caching\Storages\FileStorage;
 use Nextras\Dbal\Bridges\NetteCaching\CachedPlatform;
+use Nextras\Dbal\Platforms\Data\Column;
+use Nextras\Dbal\Platforms\Data\ForeignKey;
+use Nextras\Dbal\Platforms\Data\Fqn;
+use Nextras\Dbal\Platforms\Data\Table;
 use Nextras\Dbal\Platforms\IPlatform;
 use Tester\Assert;
 
@@ -20,103 +25,84 @@ require_once __DIR__ . '/../../bootstrap.php';
 
 class CachedPlatformTest extends TestCase
 {
-	/** @var CachedPlatform */
-	private $platform;
-
-	/** @var IStorage|MockInterface */
-	private $storageMock;
+	private CachedPlatform $cache;
 
-	/** @var IPlatform|MockInterface */
+	/** @var MockInterface */
 	private $platformMock;
 
 
-	public function setUp()
+	public function setUp(): void
 	{
 		parent::setUp();
-		$this->storageMock = Mockery::mock(IStorage::class);
 		$this->platformMock = Mockery::mock(IPlatform::class);
-		$this->platform = new CachedPlatform($this->platformMock, new Cache($this->storageMock));
+		$this->cache = new CachedPlatform($this->platformMock, new Cache(new FileStorage(TEMP_DIR)));
 	}
 
 
-	public function testCachedColumn()
+	public function testColumns(): void
 	{
-		$expectedCols = ['one', 'two'];
-		$this->storageMock->shouldReceive('read')->with(Mockery::type('string'))->once()->andReturn($expectedCols);
-
-		$cols = $this->platform->getColumns('foo');
-		Assert::same($expectedCols, $cols);
-
-		$this->storageMock->shouldReceive('clean');
-		$this->platform->clearCache();
+		$column = new Column(
+			name: 'bar',
+			type: 'type',
+			size: 128,
+			default: null,
+			isPrimary: true,
+			isAutoincrement: false,
+			isUnsigned: false,
+			isNullable: true,
+			meta: ['sequence' => 'a.b'],
+		);
+
+		$this->platformMock->shouldReceive('getColumns')->with('foo', null)->once()->andReturn([clone $column]);
+
+		Assert::equal([clone $column], $this->cache->getColumns('foo'));
+		Assert::equal([clone $column], $this->cache->getColumns('foo'));
 	}
 
 
-	public function testQueryColumn()
+	public function testTables(): void
 	{
-		$expectedCols = ['one', 'two'];
-		$this->storageMock->shouldReceive('read')->with(Mockery::type('string'))->once()->andReturnNull();
-		$this->storageMock->shouldReceive('lock')->with(Mockery::type('string'))->once();
-		$this->storageMock->shouldReceive('write')->with(Mockery::type('string'), $expectedCols, [])->once();
-		$this->platformMock->shouldReceive('getColumns')->with('foo', null)->once()->andReturn($expectedCols);
-
-		$cols = $this->platform->getColumns('foo');
-		Assert::same($expectedCols, $cols);
+		$table = new Table(
+			fqnName: new Fqn('one', 'two'),
+			isView: false,
+		);
+		$this->platformMock->shouldReceive('getTables')->once()->andReturn([clone $table]);
+
+		Assert::equal([clone $table], $this->cache->getTables());
+		Assert::equal([clone $table], $this->cache->getTables());
 	}
 
 
-	public function testQueryTables()
+	public function testForeignKeys(): void
 	{
-		$expectedTables = ['one', 'two'];
-		$this->storageMock->shouldReceive('read')->with(Mockery::type('string'))->once()->andReturnNull();
-		$this->storageMock->shouldReceive('lock')->with(Mockery::type('string'))->once();
-		$this->storageMock->shouldReceive('write')->with(Mockery::type('string'), $expectedTables, [])->once();
-		$this->platformMock->shouldReceive('getTables')->once()->andReturn($expectedTables);
-
-		$cols = $this->platform->getTables();
-		Assert::same($expectedTables, $cols);
+		$fk = new ForeignKey(
+			fqnName: new Fqn('one', 'two'),
+			column: 'col',
+			refTable: new Fqn('three', 'four'),
+			refColumn: 'refCol',
+		);
+		$this->platformMock->shouldReceive('getForeignKeys')->with('foo', null)->once()->andReturn([clone $fk]);
+
+		Assert::equal([clone $fk], $this->cache->getForeignKeys('foo'));
+		Assert::equal([clone $fk], $this->cache->getForeignKeys('foo'));
 	}
 
 
-	public function testQueryFk()
+	public function testQueryPrimarySequence(): void
 	{
-		$expectedFk = ['one', 'two'];
-		$this->storageMock->shouldReceive('read')->with(Mockery::type('string'))->once()->andReturnNull();
-		$this->storageMock->shouldReceive('lock')->with(Mockery::type('string'))->once();
-		$this->storageMock->shouldReceive('write')->with(Mockery::type('string'), $expectedFk, [])->once();
-		$this->platformMock->shouldReceive('getForeignKeys')->with('foo', null)->once()->andReturn($expectedFk);
-
-		$cols = $this->platform->getForeignKeys('foo');
-		Assert::same($expectedFk, $cols);
-	}
-
-
-	public function testQueryPS()
-	{
-		$expectedPs = 'ps_name';
-		$this->storageMock->shouldReceive('read')->with(Mockery::type('string'))->once()->andReturnNull();
-		$this->storageMock->shouldReceive('lock')->with(Mockery::type('string'))->once();
-		$this->storageMock->shouldReceive('write')->with(Mockery::type('string'), [$expectedPs], [])->once();
-		$this->platformMock->shouldReceive('getPrimarySequenceName')->with('foo', null)->once()->andReturn($expectedPs);
-
-		$cols = $this->platform->getPrimarySequenceName('foo');
-		Assert::same($expectedPs, $cols);
-
-
-		$this->storageMock->shouldReceive('read')->with(Mockery::type('string'))->once()->andReturnNull();
-		$this->storageMock->shouldReceive('lock')->with(Mockery::type('string'))->once();
-		$this->storageMock->shouldReceive('write')->with(Mockery::type('string'), [null], [])->once();
-		$this->platformMock->shouldReceive('getPrimarySequenceName')->with('foo', null)->once()->andReturn(null);
+		$this->platformMock->shouldReceive('getPrimarySequenceName')->with('foo', null)->once()
+			->andReturn('ps_name');
 
-		$cols = $this->platform->getPrimarySequenceName('foo');
-		Assert::same(null, $cols);
+		Assert::equal('ps_name', $this->cache->getPrimarySequenceName('foo'));
+		Assert::equal('ps_name', $this->cache->getPrimarySequenceName('foo'));
 	}
 
 
-	public function testName()
+	public function testName(): void
 	{
-		$this->platformMock->shouldReceive('getName')->once()->andReturn('foo');
-		Assert::same('foo', $this->platform->getName());
+		$this->platformMock->shouldReceive('getName')->twice()->andReturn('foo');
+		Assert::same('foo', $this->cache->getName());
+		Assert::same('foo', $this->cache->getName()); // no cache
 	}
 }