diff --git a/CHANGELOG.md b/CHANGELOG.md index f39da61..f9aa501 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,11 +15,19 @@ - `mariadb` test suite, run with `vendor/bin/talon run mariadb`. Backed by `resources/phpunit.mariadb.xml` and `resources/schema/mariadb.sql`. [#26](https://github.com/phalcon/talon/issues/26) - `mariadb:11.4` service in `docker-compose.yml`, and the matching service in both CI jobs. [#26](https://github.com/phalcon/talon/issues/26) - `phpunit/phpcov` (^9) as a dev dependency, used to merge the per-suite coverage into `tests/_output/coverage.xml`. [#26](https://github.com/phalcon/talon/issues/26) +- `Phalcon\Talon\Database\Dialect`, an enum resolving a PDO connection to `Mysql`, `Pgsql`, or `Sqlite`, with per-dialect identifier quoting. [#27](https://github.com/phalcon/talon/issues/27) +- `DatabaseTrait::getDialect()`, returning the connection's `Dialect`. Use it to choose SQL syntax; use `getDriver()` to tell MariaDB from MySQL. [#27](https://github.com/phalcon/talon/issues/27) ### Fixed +- `Connection::select()` now matches a NULL criterion with `IS NULL` instead of `col = :col`, which matched nothing. `assertNotInDatabase('t', ['x' => null])` previously passed regardless of the data. [#27](https://github.com/phalcon/talon/issues/27) +- `Connection::select()` now quotes table and column identifiers per dialect, so reserved words such as `order` and `key` work. [#27](https://github.com/phalcon/talon/issues/27) +- `DATA_POSTGRES_SCHEMA` is now applied to the connection as `SET search_path`. It was previously read into the options and ignored. [#27](https://github.com/phalcon/talon/issues/27) + ### Removed +- `resources/schema/mariadb.sql`, a byte-identical copy of `mysql.sql`. `DatabaseIntegrationTest` now loads the schema named by each suite's `dump_file` rather than deriving the path from the driver name. [#27](https://github.com/phalcon/talon/issues/27) + ## [0.8.0](https://github.com/phalcon/talon/releases/tag/v0.8.0) (2026-07-15) ### Changed diff --git a/resources/phpunit.mariadb.xml b/resources/phpunit.mariadb.xml index 3d116ce..f6686a4 100644 --- a/resources/phpunit.mariadb.xml +++ b/resources/phpunit.mariadb.xml @@ -16,7 +16,7 @@ - + diff --git a/resources/schema/mariadb.sql b/resources/schema/mariadb.sql deleted file mode 100644 index 317c75f..0000000 --- a/resources/schema/mariadb.sql +++ /dev/null @@ -1,2 +0,0 @@ -DROP TABLE IF EXISTS users; -CREATE TABLE users (id INT PRIMARY KEY, email VARCHAR(255)); diff --git a/src/Database/Connection.php b/src/Database/Connection.php index 436ece6..2e1afae 100644 --- a/src/Database/Connection.php +++ b/src/Database/Connection.php @@ -57,6 +57,16 @@ public function getPdo(): PDO $this->pdo->exec('PRAGMA journal_mode = WAL'); } + // Before initial_queries, so those can rely on the search path. + if ($this->driver === 'pgsql') { + $schema = $options['schema'] ?? ''; + if (is_string($schema) && $schema !== '') { + $this->pdo->exec( + 'SET search_path TO ' . Dialect::Pgsql->quoteIdentifier($schema) + ); + } + } + $initialQueries = $this->settings->get('initial_queries', ''); if (is_string($initialQueries) && $initialQueries !== '') { $this->pdo->exec($initialQueries); @@ -86,15 +96,27 @@ public function loadSchema(string $dumpFile): void */ public function select(string $table, array $criteria = []): array { + $dialect = Dialect::fromPdo($this->getPdo()); + $where = []; $params = []; + $index = 0; foreach ($criteria as $key => $value) { - $where[] = $key . ' = :' . $key; - $params[$key] = $value; + $column = $dialect->quoteIdentifier((string) $key); + + // `col = :p` never matches NULL in any dialect; bind no parameter. + if ($value === null) { + $where[] = $column . ' IS NULL'; + continue; + } + + $placeholder = 'p' . $index++; + $where[] = $column . ' = :' . $placeholder; + $params[$placeholder] = $value; } - $sql = 'SELECT * FROM ' . $table; + $sql = 'SELECT * FROM ' . $dialect->quoteIdentifier($table); if ($where !== []) { $sql .= ' WHERE ' . implode(' AND ', $where); } diff --git a/src/Database/Dialect.php b/src/Database/Dialect.php new file mode 100644 index 0000000..51270b7 --- /dev/null +++ b/src/Database/Dialect.php @@ -0,0 +1,59 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Phalcon\Talon\Database; + +use PDO; +use Phalcon\Talon\Exceptions\UnknownDriver; + +use function explode; +use function implode; +use function is_string; +use function str_replace; + +enum Dialect: string +{ + case Mysql = 'mysql'; + case Pgsql = 'pgsql'; + case Sqlite = 'sqlite'; + + /** + * MariaDB connects through pdo_mysql, so PDO reports 'mysql' for it. That + * is correct here - the SQL dialect is the same. Use the configured driver + * name when a test needs to tell the two servers apart. + */ + public static function fromPdo(PDO $pdo): self + { + $name = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); + $name = is_string($name) ? $name : ''; + + return self::tryFrom($name) ?? throw new UnknownDriver($name); + } + + public function quoteIdentifier(string $name): string + { + $delimiter = match ($this) { + self::Mysql => '`', + self::Pgsql, self::Sqlite => '"', + }; + + $quoted = []; + foreach (explode('.', $name) as $segment) { + $quoted[] = $delimiter + . str_replace($delimiter, $delimiter . $delimiter, $segment) + . $delimiter; + } + + return implode('.', $quoted); + } +} diff --git a/src/Traits/DatabaseTrait.php b/src/Traits/DatabaseTrait.php index db5c6f9..5abad41 100644 --- a/src/Traits/DatabaseTrait.php +++ b/src/Traits/DatabaseTrait.php @@ -16,6 +16,7 @@ use Phalcon\Talon\Contracts\Connection as ConnectionContract; use Phalcon\Talon\Contracts\Settings; use Phalcon\Talon\Database\Connection; +use Phalcon\Talon\Database\Dialect; use Phalcon\Talon\Talon; use function getenv; @@ -74,6 +75,11 @@ public function getConnection(): ConnectionContract return self::$connections[$driver]; } + public function getDialect(): Dialect + { + return Dialect::fromPdo($this->getConnection()->getPdo()); + } + public function getDriver(): string { return $this->databaseDriver(); diff --git a/tests/Database/ConnectionDriverTest.php b/tests/Database/ConnectionDriverTest.php new file mode 100644 index 0000000..c0608e3 --- /dev/null +++ b/tests/Database/ConnectionDriverTest.php @@ -0,0 +1,70 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Phalcon\Talon\Tests\Database; + +use PDO; +use Phalcon\Talon\Database\Dialect; +use Phalcon\Talon\PHPUnit\AbstractDatabaseTestCase; +use Phalcon\Talon\Settings; +use Phalcon\Talon\Talon; + +final class ConnectionDriverTest extends AbstractDatabaseTestCase +{ + protected function setUp(): void + { + parent::setUp(); + + Talon::useSettings(Settings::fromEnv()); + self::resetConnections(); + } + + protected function tearDown(): void + { + Talon::reset(); + + parent::tearDown(); + } + + public function testDialectMatchesTheConfiguredDriver(): void + { + $expected = match ($this->getDriver()) { + 'mariadb', 'mysql' => Dialect::Mysql, + 'pgsql' => Dialect::Pgsql, + default => Dialect::Sqlite, + }; + + $this->assertSame($expected, $this->getDialect()); + } + + public function testMariadbReportsTheMysqlDialect(): void + { + if ($this->getDriver() !== 'mariadb') { + $this->markTestSkipped('MariaDB only'); + } + + $version = $this->getConnection()->getPdo()->getAttribute(PDO::ATTR_SERVER_VERSION); + + $this->assertIsString($version); + $this->assertStringContainsString('MariaDB', $version); + $this->assertSame(Dialect::Mysql, $this->getDialect()); + } + + public function testTheDialectValueIsThePdoDriverName(): void + { + $this->assertSame( + $this->getDialect()->value, + $this->getConnection()->getPdo()->getAttribute(PDO::ATTR_DRIVER_NAME) + ); + } +} diff --git a/tests/Database/DatabaseIntegrationTest.php b/tests/Database/DatabaseIntegrationTest.php index c7a2a97..10dd64a 100644 --- a/tests/Database/DatabaseIntegrationTest.php +++ b/tests/Database/DatabaseIntegrationTest.php @@ -17,11 +17,12 @@ use Phalcon\Talon\Settings; use Phalcon\Talon\Talon; -use function getenv; +use function is_string; /** - * Driver-agnostic: runs against sqlite, mysql, or pgsql depending on the `driver` - * env set by the chosen phpunit config. + * Driver-agnostic: runs against sqlite, mysql, mariadb, or pgsql depending on the + * `driver` env set by the chosen phpunit config. The schema comes from that + * config's `dump_file`, so a suite can point at any schema it likes. */ final class DatabaseIntegrationTest extends AbstractDatabaseTestCase { @@ -32,8 +33,12 @@ protected function setUp(): void Talon::useSettings(Settings::fromEnv()); self::resetConnections(); - $driver = getenv('driver') ?: 'sqlite'; - $this->getConnection()->loadSchema($this->getSettings()->rootPath('resources/schema/' . $driver . '.sql')); + $dumpFile = $this->getSettings()->get('dump_file'); + $this->getConnection()->loadSchema( + $this->getSettings()->rootPath( + is_string($dumpFile) && $dumpFile !== '' ? $dumpFile : 'resources/schema/sqlite.sql' + ) + ); $this->getConnection()->execute("INSERT INTO users (id, email) VALUES (1, 'john.connor@skynet.dev')"); } diff --git a/tests/Database/Fixtures/AbstractDriverSchema.php b/tests/Database/Fixtures/AbstractDriverSchema.php new file mode 100644 index 0000000..11313e4 --- /dev/null +++ b/tests/Database/Fixtures/AbstractDriverSchema.php @@ -0,0 +1,51 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Phalcon\Talon\Tests\Database\Fixtures; + +use Phalcon\Talon\Database\Dialect; + +/** + * Per-dialect schema statements. The methods are abstract on purpose: a new + * dialect cannot be silently forgotten by a fixture that only implements some + * of them. + */ +abstract class AbstractDriverSchema +{ + /** + * @return list + */ + public function sqlFor(Dialect $dialect): array + { + return match ($dialect) { + Dialect::Mysql => $this->sqlMysql(), + Dialect::Pgsql => $this->sqlPgsql(), + Dialect::Sqlite => $this->sqlSqlite(), + }; + } + + /** + * @return list + */ + abstract protected function sqlMysql(): array; + + /** + * @return list + */ + abstract protected function sqlPgsql(): array; + + /** + * @return list + */ + abstract protected function sqlSqlite(): array; +} diff --git a/tests/Database/PostgresSchemaTest.php b/tests/Database/PostgresSchemaTest.php new file mode 100644 index 0000000..4a7472c --- /dev/null +++ b/tests/Database/PostgresSchemaTest.php @@ -0,0 +1,61 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Phalcon\Talon\Tests\Database; + +use Phalcon\Talon\Database\Connection; +use Phalcon\Talon\PHPUnit\AbstractDatabaseTestCase; +use Phalcon\Talon\Settings; +use Phalcon\Talon\Talon; + +final class PostgresSchemaTest extends AbstractDatabaseTestCase +{ + protected function setUp(): void + { + parent::setUp(); + + if ($this->getDriver() !== 'pgsql') { + $this->markTestSkipped('search_path applies to PostgreSQL only'); + } + + Talon::useSettings(Settings::fromEnv()); + self::resetConnections(); + + $this->getConnection()->execute('CREATE SCHEMA IF NOT EXISTS talon_alt'); + $this->getConnection()->execute('DROP TABLE IF EXISTS talon_alt.widgets'); + $this->getConnection()->execute('CREATE TABLE talon_alt.widgets (id INTEGER)'); + $this->getConnection()->execute('INSERT INTO talon_alt.widgets VALUES (7)'); + } + + protected function tearDown(): void + { + if ($this->getDriver() === 'pgsql') { + $this->getConnection()->execute('DROP SCHEMA IF EXISTS talon_alt CASCADE'); + } + + Talon::reset(); + + parent::tearDown(); + } + + public function testSearchPathResolvesUnqualifiedNames(): void + { + $settings = Settings::fromEnv(['DATA_POSTGRES_SCHEMA' => 'talon_alt']); + $scoped = new Connection($settings, 'pgsql'); + + $rows = $scoped->select('widgets'); + + $this->assertCount(1, $rows); + $this->assertEquals(7, $rows[0]['id']); + } +} diff --git a/tests/Database/SchemaLoadingTest.php b/tests/Database/SchemaLoadingTest.php new file mode 100644 index 0000000..3073502 --- /dev/null +++ b/tests/Database/SchemaLoadingTest.php @@ -0,0 +1,94 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Phalcon\Talon\Tests\Database; + +use Phalcon\Talon\PHPUnit\AbstractDatabaseTestCase; +use Phalcon\Talon\Settings; +use Phalcon\Talon\Talon; +use Phalcon\Talon\Tests\Database\Fixtures\AbstractDriverSchema; + +use function file_put_contents; +use function implode; +use function unlink; + +final class SchemaLoadingTest extends AbstractDatabaseTestCase +{ + private string $dumpFile = ''; + + protected function setUp(): void + { + parent::setUp(); + + Talon::useSettings(Settings::fromEnv()); + self::resetConnections(); + + $this->dumpFile = $this->getSettings()->outputPath('schema-loading.sql'); + } + + protected function tearDown(): void + { + if ($this->dumpFile !== '') { + @unlink($this->dumpFile); + } + + $this->getConnection()->execute('DROP TABLE IF EXISTS widgets'); + + Talon::reset(); + + parent::tearDown(); + } + + public function testLoadSchemaExecutesEveryStatement(): void + { + $fixture = new class extends AbstractDriverSchema { + protected function sqlMysql(): array + { + return [ + 'DROP TABLE IF EXISTS widgets;', + 'CREATE TABLE widgets (id INT PRIMARY KEY, label VARCHAR(64)) ' + . 'DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin;', + "INSERT INTO widgets VALUES (1, 'first');", + "INSERT INTO widgets VALUES (2, 'second');", + ]; + } + + protected function sqlPgsql(): array + { + return [ + 'DROP TABLE IF EXISTS widgets;', + 'CREATE TABLE widgets (id INTEGER PRIMARY KEY, label VARCHAR(64));', + "INSERT INTO widgets VALUES (1, 'first');", + "INSERT INTO widgets VALUES (2, 'second');", + ]; + } + + protected function sqlSqlite(): array + { + return [ + 'DROP TABLE IF EXISTS widgets;', + 'CREATE TABLE widgets (id INTEGER PRIMARY KEY, label TEXT);', + "INSERT INTO widgets VALUES (1, 'first');", + "INSERT INTO widgets VALUES (2, 'second');", + ]; + } + }; + + file_put_contents($this->dumpFile, implode("\n", $fixture->sqlFor($this->getDialect()))); + + $this->getConnection()->loadSchema($this->dumpFile); + + $this->assertCount(2, $this->getFromDatabase('widgets')); + $this->assertInDatabase('widgets', ['label' => 'second']); + } +} diff --git a/tests/Database/SelectSemanticsTest.php b/tests/Database/SelectSemanticsTest.php new file mode 100644 index 0000000..d869b59 --- /dev/null +++ b/tests/Database/SelectSemanticsTest.php @@ -0,0 +1,86 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Phalcon\Talon\Tests\Database; + +use Phalcon\Talon\PHPUnit\AbstractDatabaseTestCase; +use Phalcon\Talon\Settings; +use Phalcon\Talon\Talon; +use PHPUnit\Framework\AssertionFailedError; + +final class SelectSemanticsTest extends AbstractDatabaseTestCase +{ + protected function setUp(): void + { + parent::setUp(); + + Talon::useSettings(Settings::fromEnv()); + self::resetConnections(); + + $dialect = $this->getDialect(); + $table = $dialect->quoteIdentifier('order'); + $key = $dialect->quoteIdentifier('key'); + $note = $dialect->quoteIdentifier('note'); + + $this->getConnection()->execute('DROP TABLE IF EXISTS ' . $table); + $this->getConnection()->execute( + 'CREATE TABLE ' . $table . ' (' . $key . ' INTEGER, ' . $note . ' VARCHAR(64))' + ); + $this->getConnection()->execute('INSERT INTO ' . $table . ' VALUES (1, NULL)'); + $this->getConnection()->execute('INSERT INTO ' . $table . " VALUES (2, 'present')"); + } + + protected function tearDown(): void + { + $this->getConnection()->execute( + 'DROP TABLE IF EXISTS ' . $this->getDialect()->quoteIdentifier('order') + ); + + Talon::reset(); + + parent::tearDown(); + } + + public function testEmptyCriteriaReturnsEveryRow(): void + { + $this->assertCount(2, $this->getFromDatabase('order')); + } + + public function testMultipleCriteriaAreCombined(): void + { + $this->assertCount(1, $this->getFromDatabase('order', ['key' => 2, 'note' => 'present'])); + $this->assertCount(0, $this->getFromDatabase('order', ['key' => 1, 'note' => 'present'])); + } + + public function testNotInDatabaseWithNullIsNotVacuous(): void + { + $this->expectException(AssertionFailedError::class); + + $this->assertNotInDatabase('order', ['note' => null]); + } + + public function testNullCriterionMatchesOnlyTheNullRow(): void + { + $rows = $this->getFromDatabase('order', ['note' => null]); + + $this->assertCount(1, $rows); + // Loose comparison on purpose: pgsql and mysql return the integer as a + // string, sqlite as an int, and Talon does not normalize result types. + $this->assertEquals(1, $rows[0]['key']); + } + + public function testReservedWordIdentifiersAreQuoted(): void + { + $this->assertInDatabase('order', ['key' => 2]); + } +} diff --git a/tests/Unit/Database/DialectTest.php b/tests/Unit/Database/DialectTest.php new file mode 100644 index 0000000..6346be7 --- /dev/null +++ b/tests/Unit/Database/DialectTest.php @@ -0,0 +1,67 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Phalcon\Talon\Tests\Unit\Database; + +use PDO; +use Phalcon\Talon\Database\Dialect; +use Phalcon\Talon\Exceptions\UnknownDriver; +use PHPUnit\Framework\TestCase; + +final class DialectTest extends TestCase +{ + public function testFromPdoMapsDriverNames(): void + { + $this->assertSame(Dialect::Mysql, Dialect::fromPdo($this->pdoReporting('mysql'))); + $this->assertSame(Dialect::Pgsql, Dialect::fromPdo($this->pdoReporting('pgsql'))); + $this->assertSame(Dialect::Sqlite, Dialect::fromPdo($this->pdoReporting('sqlite'))); + } + + public function testFromPdoThrowsForUnknownDriver(): void + { + $this->expectException(UnknownDriver::class); + $this->expectExceptionMessage("Unknown database driver 'oci'"); + + Dialect::fromPdo($this->pdoReporting('oci')); + } + + public function testQuoteIdentifierEscapesTheDelimiter(): void + { + $this->assertSame('`we``ird`', Dialect::Mysql->quoteIdentifier('we`ird')); + $this->assertSame('"we""ird"', Dialect::Pgsql->quoteIdentifier('we"ird')); + $this->assertSame('"we""ird"', Dialect::Sqlite->quoteIdentifier('we"ird')); + } + + public function testQuoteIdentifierQuotesEachSegmentOfAQualifiedName(): void + { + $this->assertSame('`private`.`users`', Dialect::Mysql->quoteIdentifier('private.users')); + $this->assertSame('"private"."users"', Dialect::Pgsql->quoteIdentifier('private.users')); + } + + public function testQuoteIdentifierWrapsInTheDialectDelimiter(): void + { + $this->assertSame('`order`', Dialect::Mysql->quoteIdentifier('order')); + $this->assertSame('"order"', Dialect::Pgsql->quoteIdentifier('order')); + $this->assertSame('"order"', Dialect::Sqlite->quoteIdentifier('order')); + } + + private function pdoReporting(string $driverName): PDO + { + $pdo = $this->createMock(PDO::class); + $pdo->method('getAttribute') + ->with(PDO::ATTR_DRIVER_NAME) + ->willReturn($driverName); + + return $pdo; + } +}