From 6edbdc10bcb22a8d26623e766d72b2d76e2318d7 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 5 Oct 2026 11:03:29 +1100 Subject: [PATCH] Render aliased tables in data-changing statements as the bare table TableGateway keeps an aliased table array on Delete, Update and Insert, but its delete(), insert() and update() raised a TypeError: Sql passed the array to constructors that only accepted string|TableIdentifier, and rendering an array failed in resolveTable(). The constructors now accept the array the setters already took, and rendering drops the alias, since alias syntax in these statements is not portable. A TableGateway test drives each call through a real Sql and Adapter and checks the SQL that reaches the driver. Signed-off-by: Simon Mundy --- analyzer-baseline.toml | 62 ++++++++++---------- src/Sql/AbstractPreparableSql.php | 16 ++++++ src/Sql/Delete.php | 7 ++- src/Sql/Insert.php | 9 ++- src/Sql/Update.php | 7 ++- test/unit/Sql/DeleteTest.php | 24 ++++++++ test/unit/Sql/InsertTest.php | 27 +++++++++ test/unit/Sql/SqlTest.php | 23 ++++++++ test/unit/Sql/UpdateTest.php | 26 +++++++++ test/unit/TableGateway/TableGatewayTest.php | 64 +++++++++++++++++++++ 10 files changed, 227 insertions(+), 38 deletions(-) diff --git a/analyzer-baseline.toml b/analyzer-baseline.toml index 6a0f82f4..1b7b1e62 100644 --- a/analyzer-baseline.toml +++ b/analyzer-baseline.toml @@ -3030,6 +3030,12 @@ code = "invalid-return-statement" message = 'Invalid return type for function `PhpDb\Sql\Delete::processWhere`: expected `null|string`, but found `array|string`.' count = 1 +[[issues]] +file = "src/Sql/Delete.php" +code = "less-specific-argument" +message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\AbstractPreparableSql::unaliasTable`: expected `PhpDb\Sql\TableIdentifier|array|string`, but provided type `PhpDb\Sql\TableIdentifier|array|string` is less specific.' +count = 1 + [[issues]] file = "src/Sql/Delete.php" code = "less-specific-nested-argument-type" @@ -3054,12 +3060,6 @@ code = "missing-constant-type" message = "Class constant `SPECIFICATION_WHERE` is missing a type hint." count = 1 -[[issues]] -file = "src/Sql/Delete.php" -code = "possibly-invalid-argument" -message = 'Possible argument type mismatch for argument #1 of `PhpDb\Sql\AbstractSql::resolveTable`: expected `PhpDb\Sql\Select|PhpDb\Sql\TableIdentifier|null|string`, but possibly received `PhpDb\Sql\TableIdentifier|array|string`.' -count = 1 - [[issues]] file = "src/Sql/Delete.php" code = "possibly-null-argument" @@ -3264,6 +3264,12 @@ code = "less-specific-argument" message = 'Argument type mismatch for argument #1 of `PhpDb\Adapter\Platform\PlatformInterface::quoteIdentifier`: expected `string`, but provided type `array-key` is less specific.' count = 1 +[[issues]] +file = "src/Sql/Insert.php" +code = "less-specific-argument" +message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\AbstractPreparableSql::unaliasTable`: expected `PhpDb\Sql\TableIdentifier|array|string`, but provided type `PhpDb\Sql\TableIdentifier|array|string` is less specific.' +count = 2 + [[issues]] file = "src/Sql/Insert.php" code = "less-specific-nested-argument-type" @@ -3348,12 +3354,6 @@ code = "possibly-invalid-argument" message = 'Possible argument type mismatch for argument #1 of `PhpDb\Sql\AbstractSql::processSubSelect`: expected `PhpDb\Sql\Select`, but possibly received `PhpDb\Sql\Select|non-empty-array`.' count = 1 -[[issues]] -file = "src/Sql/Insert.php" -code = "possibly-invalid-argument" -message = 'Possible argument type mismatch for argument #1 of `PhpDb\Sql\AbstractSql::resolveTable`: expected `PhpDb\Sql\Select|PhpDb\Sql\TableIdentifier|null|string`, but possibly received `PhpDb\Sql\TableIdentifier|array|string`.' -count = 2 - [[issues]] file = "src/Sql/Insert.php" code = "possibly-invalid-argument" @@ -4562,39 +4562,39 @@ count = 1 [[issues]] file = "src/Sql/Sql.php" -code = "overly-wide-return-type" -message = 'Declared return type `PhpDb\Adapter\AdapterInterface|null` for `getadapter` has unused branches: `null`.' +code = "less-specific-argument" +message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Delete::__construct`: expected `PhpDb\Sql\TableIdentifier|array|null|string`, but provided type `PhpDb\Sql\TableIdentifier|array|null|string` is less specific.' count = 1 [[issues]] file = "src/Sql/Sql.php" -code = "overly-wide-return-type" -message = 'Declared return type `PhpDb\Sql\Platform\PlatformDecoratorInterface|null` for `getsqlplatform` has unused branches: `null`.' +code = "less-specific-argument" +message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Insert::__construct`: expected `PhpDb\Sql\TableIdentifier|array|null|string`, but provided type `PhpDb\Sql\TableIdentifier|array|null|string` is less specific.' count = 1 [[issues]] file = "src/Sql/Sql.php" -code = "possibly-invalid-argument" -message = 'Possible argument type mismatch for argument #1 of `PhpDb\Sql\Delete::__construct`: expected `PhpDb\Sql\TableIdentifier|null|string`, but possibly received `PhpDb\Sql\TableIdentifier|array|null|string`.' +code = "less-specific-argument" +message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Update::__construct`: expected `PhpDb\Sql\TableIdentifier|array|null|string`, but provided type `PhpDb\Sql\TableIdentifier|array|null|string` is less specific.' count = 1 [[issues]] file = "src/Sql/Sql.php" -code = "possibly-invalid-argument" -message = 'Possible argument type mismatch for argument #1 of `PhpDb\Sql\Exception\InvalidArgumentException::forForeignTable`: expected `string`, but possibly received `PhpDb\Sql\TableIdentifier|array|string`.' -count = 4 +code = "overly-wide-return-type" +message = 'Declared return type `PhpDb\Adapter\AdapterInterface|null` for `getadapter` has unused branches: `null`.' +count = 1 [[issues]] file = "src/Sql/Sql.php" -code = "possibly-invalid-argument" -message = 'Possible argument type mismatch for argument #1 of `PhpDb\Sql\Insert::__construct`: expected `PhpDb\Sql\TableIdentifier|null|string`, but possibly received `PhpDb\Sql\TableIdentifier|array|null|string`.' +code = "overly-wide-return-type" +message = 'Declared return type `PhpDb\Sql\Platform\PlatformDecoratorInterface|null` for `getsqlplatform` has unused branches: `null`.' count = 1 [[issues]] file = "src/Sql/Sql.php" code = "possibly-invalid-argument" -message = 'Possible argument type mismatch for argument #1 of `PhpDb\Sql\Update::__construct`: expected `PhpDb\Sql\TableIdentifier|null|string`, but possibly received `PhpDb\Sql\TableIdentifier|array|null|string`.' -count = 1 +message = 'Possible argument type mismatch for argument #1 of `PhpDb\Sql\Exception\InvalidArgumentException::forForeignTable`: expected `string`, but possibly received `PhpDb\Sql\TableIdentifier|array|string`.' +count = 4 [[issues]] file = "src/Sql/Sql.php" @@ -4710,6 +4710,12 @@ code = "invalid-return-statement" message = 'Invalid return type for function `PhpDb\Sql\Update::processWhere`: expected `null|string`, but found `array|string`.' count = 1 +[[issues]] +file = "src/Sql/Update.php" +code = "less-specific-argument" +message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\AbstractPreparableSql::unaliasTable`: expected `PhpDb\Sql\TableIdentifier|array|string`, but provided type `PhpDb\Sql\TableIdentifier|array|string` is less specific.' +count = 1 + [[issues]] file = "src/Sql/Update.php" code = "less-specific-nested-argument-type" @@ -4782,12 +4788,6 @@ code = "possible-method-access-on-null" message = "Attempting to call a method on `null`." count = 1 -[[issues]] -file = "src/Sql/Update.php" -code = "possibly-invalid-argument" -message = 'Possible argument type mismatch for argument #1 of `PhpDb\Sql\AbstractSql::resolveTable`: expected `PhpDb\Sql\Select|PhpDb\Sql\TableIdentifier|null|string`, but possibly received `PhpDb\Sql\TableIdentifier|array|string`.' -count = 1 - [[issues]] file = "src/Sql/Update.php" code = "possibly-invalid-argument" diff --git a/src/Sql/AbstractPreparableSql.php b/src/Sql/AbstractPreparableSql.php index 178623ce..83313f0a 100644 --- a/src/Sql/AbstractPreparableSql.php +++ b/src/Sql/AbstractPreparableSql.php @@ -9,6 +9,9 @@ use PhpDb\Adapter\ParameterContainer; use PhpDb\Adapter\StatementContainerInterface; +use function array_values; +use function is_array; + abstract class AbstractPreparableSql extends AbstractSql implements PreparableSqlInterface { #[Override] @@ -30,4 +33,17 @@ public function prepareStatement( return $statementContainer; } + + /** + * Returns the table to render, dropping the alias from an aliased table array + * + * TableGateway stores an aliased table on data-changing statements, and alias syntax in those + * statements is not portable, so only the table itself is rendered. + * + * @param array|string|TableIdentifier $table + */ + protected function unaliasTable(array|string|TableIdentifier $table): string|TableIdentifier + { + return is_array($table) ? array_values($table)[0] ?? '' : $table; + } } diff --git a/src/Sql/Delete.php b/src/Sql/Delete.php index 72006aaa..91c56592 100644 --- a/src/Sql/Delete.php +++ b/src/Sql/Delete.php @@ -44,8 +44,11 @@ class Delete extends AbstractPreparableSql /** * Constructor + * + * @param string|TableIdentifier|array|null $table An aliased table + * array is kept for TableGateway and rendered as the bare table */ - public function __construct(string|TableIdentifier|null $table = null) + public function __construct(string|TableIdentifier|array|null $table = null) { if ($table) { $this->from($table); @@ -96,7 +99,7 @@ protected function processDelete( ): string { return str_replace( '%1$s', - $this->resolveTable($this->table, $platform, $driver, $parameterContainer), + $this->resolveTable($this->unaliasTable($this->table), $platform, $driver, $parameterContainer), $this->specifications[static::SPECIFICATION_DELETE], ); } diff --git a/src/Sql/Insert.php b/src/Sql/Insert.php index 46f67ff4..f6fd57f9 100644 --- a/src/Sql/Insert.php +++ b/src/Sql/Insert.php @@ -50,8 +50,11 @@ class Insert extends AbstractPreparableSql /** * Constructor + * + * @param string|TableIdentifier|array|null $table An aliased table + * array is kept for TableGateway and rendered as the bare table */ - public function __construct(string|TableIdentifier|null $table = null) + public function __construct(string|TableIdentifier|array|null $table = null) { if ($table) { $this->into($table); @@ -173,7 +176,7 @@ protected function processInsert( return str_replace( ['%1$s', '%2$s', '%3$s'], [ - $this->resolveTable($this->table, $platform, $driver, $parameterContainer), + $this->resolveTable($this->unaliasTable($this->table), $platform, $driver, $parameterContainer), implode(', ', $columns), implode(', ', $values), ], @@ -198,7 +201,7 @@ protected function processSelect( return str_replace( ['%1$s', '%2$s', '%3$s'], [ - $this->resolveTable($this->table, $platform, $driver, $parameterContainer), + $this->resolveTable($this->unaliasTable($this->table), $platform, $driver, $parameterContainer), $columns ? "({$columns})" : '', $selectSql, ], diff --git a/src/Sql/Update.php b/src/Sql/Update.php index 13d4e849..4cf990ab 100644 --- a/src/Sql/Update.php +++ b/src/Sql/Update.php @@ -66,8 +66,11 @@ class Update extends AbstractPreparableSql /** * Constructor + * + * @param string|TableIdentifier|array|null $table An aliased table + * array is kept for TableGateway and rendered as the bare table */ - public function __construct(string|TableIdentifier|null $table = null) + public function __construct(string|TableIdentifier|array|null $table = null) { if ($table) { $this->table($table); @@ -216,7 +219,7 @@ protected function processUpdate( ): string { return str_replace( '%1$s', - $this->resolveTable($this->table, $platform, $driver, $parameterContainer), + $this->resolveTable($this->unaliasTable($this->table), $platform, $driver, $parameterContainer), $this->specifications[static::SPECIFICATION_UPDATE], ); } diff --git a/test/unit/Sql/DeleteTest.php b/test/unit/Sql/DeleteTest.php index 17734f81..1f6d37fb 100644 --- a/test/unit/Sql/DeleteTest.php +++ b/test/unit/Sql/DeleteTest.php @@ -7,6 +7,7 @@ use Override; use PhpDb\Adapter\Driver\DriverInterface; use PhpDb\Adapter\Driver\StatementInterface; +use PhpDb\Sql\AbstractPreparableSql; use PhpDb\Sql\Argument\Identifier; use PhpDb\Sql\Argument\Value; use PhpDb\Sql\Delete; @@ -25,6 +26,7 @@ use PhpDbTest\TestAsset\DeleteIgnore; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\CoversNothing; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\IgnoreDeprecations; use PHPUnit\Framework\Attributes\RequiresPhp; use PHPUnit\Framework\Attributes\Test; @@ -41,6 +43,7 @@ #[CoversMethod(Delete::class, 'processDelete')] #[CoversMethod(Delete::class, 'processWhere')] #[CoversMethod(Delete::class, '__get')] +#[CoversMethod(AbstractPreparableSql::class, 'unaliasTable')] final class DeleteTest extends TestCase { use AdapterTestTrait; @@ -48,6 +51,15 @@ final class DeleteTest extends TestCase protected Delete $delete; + /** @return array, string}> */ + public static function aliasedTableProvider(): array + { + return [ + 'table name' => [['f' => 'foo'], '"foo"'], + 'table identifier' => [['f' => new TableIdentifier('foo', 'sch')], '"sch"."foo"'], + ]; + } + #[Test] public function constructorWithTable(): void { @@ -118,6 +130,18 @@ public function getSqlString(): void static::assertSame('DELETE FROM "sch"."foo" WHERE x = y', $this->delete->getSqlString()); } + /** + * @param array $table + */ + #[Test] + #[DataProvider('aliasedTableProvider')] + public function getSqlStringRendersBareTableForAliasedTable(array $table, string $expected): void + { + $this->delete->from($table)->where('x = y'); + + static::assertSame("DELETE FROM {$expected} WHERE x = y", $this->delete->getSqlString()); + } + #[Test] public function getSqlStringWithEmptyWhere(): void { diff --git a/test/unit/Sql/InsertTest.php b/test/unit/Sql/InsertTest.php index 82cc0cbf..905afe63 100644 --- a/test/unit/Sql/InsertTest.php +++ b/test/unit/Sql/InsertTest.php @@ -10,6 +10,7 @@ use PhpDb\Adapter\Driver\StatementInterface; use PhpDb\Adapter\ParameterContainer; use PhpDb\Adapter\StatementContainer; +use PhpDb\Sql\AbstractPreparableSql; use PhpDb\Sql\Exception\InvalidArgumentException; use PhpDb\Sql\Expression; use PhpDb\Sql\Insert; @@ -21,6 +22,7 @@ use PhpDbTest\TestAsset\TrustingSql92Platform; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\CoversNothing; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\IgnoreDeprecations; use PHPUnit\Framework\Attributes\RequiresPhp; @@ -47,6 +49,7 @@ #[CoversMethod(Insert::class, '__unset')] #[CoversMethod(Insert::class, '__isset')] #[CoversMethod(Insert::class, '__get')] +#[CoversMethod(AbstractPreparableSql::class, 'unaliasTable')] final class InsertTest extends TestCase { use AdapterTestTrait; @@ -54,6 +57,15 @@ final class InsertTest extends TestCase protected Insert $insert; + /** @return array, string}> */ + public static function aliasedTableProvider(): array + { + return [ + 'table name' => [['f' => 'foo'], '"foo"'], + 'table identifier' => [['f' => new TableIdentifier('foo', 'sch')], '"sch"."foo"'], + ]; + } + // @codingStandardsIgnoreStart #[Test] public function _get(): void @@ -176,6 +188,21 @@ public function getSqlString(): void ); } + /** + * @param array $table + */ + #[Test] + #[DataProvider('aliasedTableProvider')] + public function getSqlStringRendersBareTableForAliasedTable(array $table, string $expected): void + { + $this->insert->into($table)->values(['bar' => 'baz']); + + static::assertSame( + "INSERT INTO {$expected} (\"bar\") VALUES ('baz')", + $this->insert->getSqlString(new TrustingSql92Platform()), + ); + } + #[Test] public function getSqlStringThrowsExceptionWhenNoValuesOrSelect(): void { diff --git a/test/unit/Sql/SqlTest.php b/test/unit/Sql/SqlTest.php index 42907641..c7d44ff9 100644 --- a/test/unit/Sql/SqlTest.php +++ b/test/unit/Sql/SqlTest.php @@ -22,6 +22,7 @@ use PhpDb\Sql\Update; use PhpDbTest\TestAsset; use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\Exception; use PHPUnit\Framework\MockObject\MockObject; @@ -49,6 +50,16 @@ final class SqlTest extends TestCase */ protected Sql $sql; + /** @return array */ + public static function dataChangingStatementProvider(): array + { + return [ + 'delete' => ['delete'], + 'insert' => ['insert'], + 'update' => ['update'], + ]; + } + // @codingStandardsIgnoreStart #[Test] public function _construct(): void @@ -96,6 +107,18 @@ public function buildSqlStringThrowsWhenPlatformNotSqlInterface(): void $sql->buildSqlString($this->sql->select()); } + /** + * @param 'delete'|'insert'|'update' $method + */ + #[Test] + #[DataProvider('dataChangingStatementProvider')] + public function dataChangingStatementKeepsAliasedTable(string $method): void + { + $sql = new Sql($this->mockAdapter, ['f' => 'foo']); + + static::assertSame(['f' => 'foo'], $sql->{$method}()->getRawState('table')); + } + #[Test] public function delete(): void { diff --git a/test/unit/Sql/UpdateTest.php b/test/unit/Sql/UpdateTest.php index 316c26d6..17182db4 100644 --- a/test/unit/Sql/UpdateTest.php +++ b/test/unit/Sql/UpdateTest.php @@ -31,6 +31,7 @@ use PhpDbTest\TestAsset\UpdateIgnore; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\CoversNothing; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\IgnoreDeprecations; use PHPUnit\Framework\Attributes\RequiresPhp; @@ -57,6 +58,7 @@ #[CoversMethod(Update::class, 'processSet')] #[CoversMethod(Update::class, 'processWhere')] #[CoversMethod(Update::class, 'processJoins')] +#[CoversMethod(AbstractPreparableSql::class, 'unaliasTable')] final class UpdateTest extends TestCase { use AdapterTestTrait; @@ -64,6 +66,15 @@ final class UpdateTest extends TestCase protected Update $update; + /** @return array, string}> */ + public static function aliasedTableProvider(): array + { + return [ + 'table name' => [['f' => 'foo'], '"foo"'], + 'table identifier' => [['f' => new TableIdentifier('foo', 'sch')], '"sch"."foo"'], + ]; + } + #[Test] public function cloneDeepCopiesSetWhereAndJoins(): void { @@ -212,6 +223,21 @@ public function getSqlStringForFalseUpdateValueParameter(): void ); } + /** + * @param array $table + */ + #[Test] + #[DataProvider('aliasedTableProvider')] + public function getSqlStringRendersBareTableForAliasedTable(array $table, string $expected): void + { + $this->update->table($table)->set(['bar' => 'baz']); + + static::assertSame( + "UPDATE {$expected} SET \"bar\" = 'baz'", + $this->update->getSqlString(new TrustingSql92Platform()), + ); + } + #[Test] public function getSqlStringWithEmptyWhere(): void { diff --git a/test/unit/TableGateway/TableGatewayTest.php b/test/unit/TableGateway/TableGatewayTest.php index 961ea07a..fc9187d2 100644 --- a/test/unit/TableGateway/TableGatewayTest.php +++ b/test/unit/TableGateway/TableGatewayTest.php @@ -10,6 +10,7 @@ use PhpDb\Adapter\Driver\DriverInterface; use PhpDb\Adapter\Driver\ResultInterface; use PhpDb\Adapter\Driver\StatementInterface; +use PhpDb\Adapter\ParameterContainer; use PhpDb\Adapter\Platform\PlatformInterface; use PhpDb\ResultSet\ResultSet; use PhpDb\Sql\Delete; @@ -21,6 +22,8 @@ use PhpDb\TableGateway\Feature; use PhpDb\TableGateway\Feature\FeatureSet; use PhpDb\TableGateway\TableGateway; +use PhpDbTest\TestAsset\TemporaryResultSet; +use PhpDbTest\TestAsset\TrustingSql92Platform; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; @@ -51,6 +54,16 @@ public static function aliasedTables(): array ]; } + /** @return array>, string}> */ + public static function dataChangingCallOnAliasedTable(): array + { + return [ + 'delete' => ['delete', [['id' => 5]], 'DELETE FROM "users" WHERE "id" = ?'], + 'insert' => ['insert', [['id' => 5]], 'INSERT INTO "users" ("id") VALUES (?)'], + 'update' => ['update', [['name' => 'x'], ['id' => 5]], 'UPDATE "users" SET "name" = ? WHERE "id" = ?'], + ]; + } + /** * Beside other tests checks for plain string table identifier */ @@ -160,6 +173,57 @@ public function constructorWithSingleFeature(): void static::assertSame($feature, $featureSet->getFeatureByClassName(Feature\SequenceFeature::class)); } + /** + * @param 'delete'|'insert'|'update' $method + * @param list> $arguments + */ + #[Test] + #[DataProvider('dataChangingCallOnAliasedTable')] + public function dataChangingCallOnAliasedTableRendersTheBareTable( + string $method, + array $arguments, + string $expectedSql, + ): void { + $executed = []; + $result = $this->createStub(ResultInterface::class); + $result->method('getAffectedRows')->willReturn(1); + + $driver = $this->createStub(DriverInterface::class); + $driver->method('formatParameterName')->willReturn('?'); + $driver->method('createStatement') + ->willReturnCallback( + function () use (&$executed, $result): StatementInterface { + $sql = ''; + $statement = $this->createStub(StatementInterface::class); + $container = new ParameterContainer(); + $statement->method('getParameterContainer')->willReturn($container); + $statement->method('setSql') + ->willReturnCallback( + static function (string $value) use (&$sql, $statement): StatementInterface { + $sql = $value; + return $statement; + }, + ); + $statement->method('execute') + ->willReturnCallback( + static function () use (&$executed, &$sql, $result): ResultInterface { + $executed[] = $sql; + return $result; + }, + ); + + return $statement; + }, + ); + + $adapter = new Adapter($driver, new TrustingSql92Platform(), new TemporaryResultSet()); + $table = new TableGateway(['u' => 'users'], $adapter); + + $table->{$method}(...$arguments); + + static::assertSame([$expectedSql], $executed); + } + /** * @param AliasedTable $tableValue */