From 245630f87696dec13b6d515003f73304d1408fa1 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 7 Sep 2026 19:55:03 +1000 Subject: [PATCH 1/2] Render ExpressionInterface check expressions and default the name Check documented its expression as string|ExpressionInterface but always wrapped it in a Literal, so an Expression was accepted at construction and threw a TypeError at render. An empty string rendered a bare CONSTRAINT header with no CHECK clause, and the name argument had no default. An ExpressionInterface expression now contributes its own specification and values to the check, so its identifiers and values are quoted by the platform. A plain string stays a literal SQL fragment by design. An empty string throws InvalidArgumentException at construction, matching Expression::setExpression(), and the name defaults to null like PrimaryKey and UniqueKey. Fixes #177 Signed-off-by: Simon Mundy --- docs/book/sql-ddl/constraints.md | 40 ++++++-- src/Sql/Ddl/Constraint/Check.php | 23 ++++- test/unit/Sql/Ddl/Constraint/CheckTest.php | 103 +++++++++++++++++++++ 3 files changed, 153 insertions(+), 13 deletions(-) diff --git a/docs/book/sql-ddl/constraints.md b/docs/book/sql-ddl/constraints.md index 887b1daa..e003eeba 100644 --- a/docs/book/sql-ddl/constraints.md +++ b/docs/book/sql-ddl/constraints.md @@ -217,26 +217,43 @@ $check = new Check( ### Using Expressions in Check Constraints -Check constraints can accept either string expressions or `Expression` objects. +Check constraints accept either a string or any `ExpressionInterface` implementation such as +`PhpDb\Sql\Expression`. The name is optional; without one the constraint renders as a bare +`CHECK (...)` clause. #### String Expressions (Simple) -For simple constraints, use strings: +A string is an expression slot by design: it is rendered verbatim as a literal SQL fragment, so +identifiers and values inside it are not quoted or escaped. Only pass trusted input. An empty +string throws `PhpDb\Sql\Exception\InvalidArgumentException` at construction. ```php use PhpDb\Sql\Ddl\Constraint\Check; -// Simple string expression +// Named $check = new Check('age >= 18', 'check_adult'); $check = new Check('price > 0', 'check_positive_price'); $check = new Check("status IN ('active', 'pending', 'completed')", 'check_valid_status'); + +// Unnamed +$check = new Check('price > 0'); +``` + +**Generated SQL:** + +```sql +CONSTRAINT "check_adult" CHECK (age >= 18) +CHECK (price > 0) ``` #### Expression Objects (Advanced) -For complex or parameterized constraints, use `Expression` objects: +An `Expression` is rendered through the platform, so its identifier and value arguments are +quoted. Values are always inlined into the DDL statement: DDL is never prepared, and MySQL rejects +parameter markers inside a `CHECK` clause. ```php +use PhpDb\Sql\Argument\Identifier; use PhpDb\Sql\Expression; use PhpDb\Sql\Ddl\Constraint\Check; @@ -247,12 +264,19 @@ $expr = new Expression( ); $check = new Check($expr, 'check_valid_age_range'); -// Complex expression +// Expression with quoted identifiers $expr = new Expression( - 'discount_percent BETWEEN ? AND ?', - [0, 100] + '? > ?', + [new Identifier('end_date'), new Identifier('start_date')] ); -$check = new Check($expr, 'check_discount_range'); +$check = new Check($expr, 'check_date_range'); +``` + +**Generated SQL:** + +```sql +CONSTRAINT "check_valid_age_range" CHECK (age >= '18' AND age <= '120') +CONSTRAINT "check_date_range" CHECK ("end_date" > "start_date") ``` ## Indexes diff --git a/src/Sql/Ddl/Constraint/Check.php b/src/Sql/Ddl/Constraint/Check.php index a6608f9b..1490d197 100644 --- a/src/Sql/Ddl/Constraint/Check.php +++ b/src/Sql/Ddl/Constraint/Check.php @@ -7,9 +7,11 @@ use Override; use PhpDb\Sql\Argument\Identifier; use PhpDb\Sql\Argument\Literal; +use PhpDb\Sql\Exception\InvalidArgumentException; use PhpDb\Sql\ExpressionInterface; use function implode; +use function sprintf; class Check extends AbstractConstraint { @@ -18,10 +20,17 @@ class Check extends AbstractConstraint protected string $specification = 'CHECK (%s)'; /** - * @param string|ExpressionInterface $expression + * A string expression is rendered verbatim as a literal SQL fragment. An ExpressionInterface + * expression is rendered through the platform, so its identifiers and values are quoted. + * + * @throws InvalidArgumentException When the expression is an empty string. */ - public function __construct($expression, ?string $name) + public function __construct(string|ExpressionInterface $expression, ?string $name = null) { + if ('' === $expression) { + throw new InvalidArgumentException('Check constraint expression must not be an empty string.'); + } + parent::__construct(null, $name); $this->expression = $expression; @@ -31,6 +40,10 @@ public function __construct($expression, ?string $name) #[Override] public function getExpressionData(): array { + $checkData = $this->expression instanceof ExpressionInterface + ? $this->expression->getExpressionData() + : ['spec' => '%s', 'values' => [new Literal($this->expression)]]; + $specParts = []; $values = []; @@ -39,9 +52,9 @@ public function getExpressionData(): array $values[] = new Identifier($this->name); } - if ('' !== $this->expression) { - $specParts[] = $this->specification; - $values[] = new Literal($this->expression); + $specParts[] = sprintf($this->specification, $checkData['spec']); + foreach ($checkData['values'] as $value) { + $values[] = $value; } return [ diff --git a/test/unit/Sql/Ddl/Constraint/CheckTest.php b/test/unit/Sql/Ddl/Constraint/CheckTest.php index 921b8da0..93fff318 100644 --- a/test/unit/Sql/Ddl/Constraint/CheckTest.php +++ b/test/unit/Sql/Ddl/Constraint/CheckTest.php @@ -5,14 +5,108 @@ namespace PhpDbTest\Sql\Ddl\Constraint; use PhpDb\Sql\Argument; +use PhpDb\Sql\Argument\Identifier; +use PhpDb\Sql\Ddl\Column\Integer; use PhpDb\Sql\Ddl\Constraint\Check; +use PhpDb\Sql\Ddl\Constraint\ConstraintInterface; +use PhpDb\Sql\Ddl\CreateTable; +use PhpDb\Sql\Exception\InvalidArgumentException; +use PhpDb\Sql\Expression; +use PhpDbTest\TestAsset\TrustingSql92Platform; use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\Attributes\Group; +use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; #[CoversMethod(Check::class, '__construct')] #[CoversMethod(Check::class, 'getExpressionData')] +#[Group('unit')] final class CheckTest extends TestCase { + /** + * Asserts what a table-level constraint renders to inside CREATE TABLE, with values quoted. + */ + private static function assertConstraintRenders(string $expected, ConstraintInterface $constraint): void + { + $createTable = new CreateTable('t'); + $createTable->addConstraint($constraint); + + static::assertSame( + "CREATE TABLE \"t\" ( \n {$expected} \n)", + $createTable->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + public function getExpressionDataMergesExpressionSpecAndValues(): void + { + $check = new Check(new Expression('a > ?', [1]), 'chk'); + + $expressionData = $check->getExpressionData(); + + static::assertSame('CONSTRAINT %s CHECK (a > %s)', $expressionData['spec']); + static::assertEquals( + [ + Argument::identifier('chk'), + Argument::value(1), + ], + $expressionData['values'], + ); + } + + #[Test] + public function rendersColumnLevelCheckInline(): void + { + $column = new Integer('age'); + $column->addConstraint(new Check('age >= 0', 'chk_age')); + + $createTable = new CreateTable('t'); + $createTable->addColumn($column); + + static::assertSame( + "CREATE TABLE \"t\" ( \n \"age\" INTEGER NOT NULL CONSTRAINT \"chk_age\" CHECK (age >= 0) \n)", + $createTable->getSqlString(), + ); + } + + #[Test] + public function rendersExpressionIdentifiersQuoted(): void + { + $check = new Check(new Expression('? > ?', [new Identifier('a'), new Identifier('b')]), 'chk'); + + static::assertConstraintRenders('CONSTRAINT "chk" CHECK ("a" > "b")', $check); + } + + #[Test] + public function rendersExpressionValuesQuoted(): void + { + $check = new Check(new Expression('a > ?', [1]), 'chk'); + + static::assertConstraintRenders('CONSTRAINT "chk" CHECK (a > \'1\')', $check); + } + + #[Test] + public function rendersPercentSignsInExpressionVerbatimWhenNamed(): void + { + $check = new Check(new Expression("email LIKE '%@%'"), 'chk'); + + static::assertConstraintRenders('CONSTRAINT "chk" CHECK (email LIKE \'%@%\')', $check); + } + + #[Test] + public function rendersPercentSignsInExpressionVerbatimWhenUnnamed(): void + { + $check = new Check(new Expression("email LIKE '%@%'")); + + static::assertConstraintRenders('CHECK (email LIKE \'%@%\')', $check); + } + + #[Test] + public function rendersWithoutConstraintClauseWhenUnnamed(): void + { + static::assertConstraintRenders('CHECK (id > 0)', new Check('id > 0')); + } + public function testGetExpressionData(): void { $check = new Check('id>0', 'foo'); @@ -28,4 +122,13 @@ public function testGetExpressionData(): void $expressionData['values'], ); } + + #[Test] + public function throwsWhenExpressionIsEmpty(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Check constraint expression must not be an empty string.'); + + new Check(''); + } } From 8745b2938373befde943f822c6bc1c7683d8c4dd Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 7 Sep 2026 21:14:53 +1000 Subject: [PATCH 2/2] Keep the QA toolchain green under Mago 1.47.6 CI installs the newest Mago, and 1.47.5 changed how the analyzer handles nested array shapes: prepareDataHierarchy() re-binds a reference into the MetadataData shape on every loop iteration, which no longer completes in useful time. Re-type the reference as a plain array so the shape is not re-derived per iteration, and adjust the expected findings accordingly. Regenerate the analyzer baseline for the reworded findings in Select, PredicateSet, Profiler and AbstractSql (the issues are unchanged, the messages are not), and reduce the continuation indent of two multi-line conditions in AbstractTableGateway to match the 1.47.4+ formatter. Signed-off-by: Simon Mundy --- analyzer-baseline.toml | 54 +++++------------------ src/Metadata/Source/AbstractSource.php | 10 +++-- src/TableGateway/AbstractTableGateway.php | 8 ++-- 3 files changed, 22 insertions(+), 50 deletions(-) diff --git a/analyzer-baseline.toml b/analyzer-baseline.toml index 26938f84..ae0b6959 100644 --- a/analyzer-baseline.toml +++ b/analyzer-baseline.toml @@ -1173,7 +1173,7 @@ count = 1 [[issues]] file = "src/Adapter/Profiler/Profiler.php" code = "invalid-property-assignment-value" -message = '''Invalid type for property `$profiles`: expected `array`, but got `array`.''' +message = '''Invalid type for property `$profiles`: expected `array`, but got `non-empty-array`.''' count = 1 [[issues]] @@ -1194,24 +1194,6 @@ code = "missing-property-type" message = "Property `$profiles` is missing a type hint." count = 1 -[[issues]] -file = "src/Adapter/Profiler/Profiler.php" -code = "possibly-null-operand" -message = "Right operand in arithmetic operation might be `null` (type `float|null`)." -count = 1 - -[[issues]] -file = "src/Adapter/Profiler/Profiler.php" -code = "possibly-undefined-int-array-index" -message = '''Possibly undefined array key `int` accessed on `array`.''' -count = 1 - -[[issues]] -file = "src/Adapter/Profiler/Profiler.php" -code = "possibly-undefined-string-array-index" -message = '''Possibly undefined array key accessed on `array{'elapse': float|null, 'end': float, 'parameters': PhpDb\Adapter\ParameterContainer|null, 'sql': string, 'start': float}|array{'end': float}`.''' -count = 1 - [[issues]] file = "src/Adapter/Profiler/Profiler.php" code = "unhandled-thrown-type" @@ -2289,7 +2271,7 @@ count = 1 [[issues]] file = "src/Sql/AbstractSql.php" code = "less-specific-nested-argument-type" -message = "Argument type mismatch for argument #2 of `vsprintf`: expected `array`, but provided type `array{}|non-empty-list` is less specific." +message = "Argument type mismatch for argument #2 of `vsprintf`: expected `array`, but provided type `list` is less specific." count = 1 [[issues]] @@ -3321,7 +3303,13 @@ count = 1 [[issues]] file = "src/Sql/Ddl/CreateTable.php" code = "possibly-undefined-string-array-index" -message = "Possibly undefined array key `string('combinedBy')` accessed on `array|string>`." +message = "Possibly undefined array key `string('combinedBy')` accessed on `array>`." +count = 1 + +[[issues]] +file = "src/Sql/Ddl/CreateTable.php" +code = "possibly-undefined-string-array-index" +message = "Possibly undefined array key `string('combinedBy')` accessed on `array`." count = 1 [[issues]] @@ -4353,7 +4341,7 @@ count = 1 [[issues]] file = "src/Sql/Predicate/PredicateSet.php" code = "less-specific-nested-argument-type" -message = "Argument type mismatch for argument #2 of `implode`: expected `array|null`, but provided type `array{}|non-empty-list` is less specific." +message = "Argument type mismatch for argument #2 of `implode`: expected `array|null`, but provided type `list` is less specific." count = 1 [[issues]] @@ -4365,7 +4353,7 @@ count = 1 [[issues]] file = "src/Sql/Predicate/PredicateSet.php" code = "less-specific-nested-return-statement" -message = '''Returned type `array{'spec': string, 'values': array{}|list}` is less specific than the declared return type `array{'spec': string, 'values': array}` for function `PhpDb\Sql\Predicate\PredicateSet::getExpressionData` due to nested 'mixed'.''' +message = '''Returned type `array{'spec': string, 'values': list}` is less specific than the declared return type `array{'spec': string, 'values': array}` for function `PhpDb\Sql\Predicate\PredicateSet::getExpressionData` due to nested 'mixed'.''' count = 1 [[issues]] @@ -4623,7 +4611,7 @@ count = 1 [[issues]] file = "src/Sql/Select.php" code = "invalid-property-assignment-value" -message = "Invalid type for property `$specifications`: expected `array>|array`, but got `array|string>`." +message = "Invalid type for property `$specifications`: expected `array>|array`, but got `non-empty-array|string>`." count = 1 [[issues]] @@ -5898,30 +5886,12 @@ code = "class-must-be-final" message = 'Class `PhpDb\TableGateway\Feature\GlobalAdapterFeature` should be declared `final`.' count = 1 -[[issues]] -file = "src/TableGateway/Feature/GlobalAdapterFeature.php" -code = "invalid-return-statement" -message = 'Invalid return type for function `PhpDb\TableGateway\Feature\GlobalAdapterFeature::getStaticAdapter`: expected `PhpDb\Adapter\AdapterInterface`, but found `PhpDb\Adapter\AdapterInterface|null`.' -count = 1 - [[issues]] file = "src/TableGateway/Feature/GlobalAdapterFeature.php" code = "missing-constructor" message = 'Class `PhpDb\TableGateway\Feature\GlobalAdapterFeature` has typed properties without default values but no constructor to initialize them.' count = 1 -[[issues]] -file = "src/TableGateway/Feature/GlobalAdapterFeature.php" -code = "nullable-return-statement" -message = 'Function `PhpDb\TableGateway\Feature\GlobalAdapterFeature::getStaticAdapter` is declared to return `PhpDb\Adapter\AdapterInterface` but possibly returns a nullable value (inferred as `PhpDb\Adapter\AdapterInterface|null`).' -count = 1 - -[[issues]] -file = "src/TableGateway/Feature/GlobalAdapterFeature.php" -code = "possibly-undefined-string-array-index" -message = '''Possibly undefined array key `class-string('PhpDb\TableGateway\Feature\GlobalAdapterFeature')` accessed on `array`.''' -count = 1 - [[issues]] file = "src/TableGateway/Feature/MasterSlaveFeature.php" code = "class-must-be-final" diff --git a/src/Metadata/Source/AbstractSource.php b/src/Metadata/Source/AbstractSource.php index 1c6c333e..7552e872 100644 --- a/src/Metadata/Source/AbstractSource.php +++ b/src/Metadata/Source/AbstractSource.php @@ -693,15 +693,17 @@ protected function loadTriggerData(string $schema): void * Prepare data hierarchy * * The by-reference walk builds arbitrary depths of the hierarchy, which - * cannot be expressed against the MetadataData shape. + * cannot be expressed against the MetadataData shape. The reference is + * re-typed as a plain array so the analyzer does not re-derive the shape + * on every iteration, which does not terminate in useful time on Mago + * 1.47.5 and later. * - * @mago-expect analysis:possibly-undefined-string-array-index - * @mago-expect analysis:possibly-undefined-int-array-index - * @mago-expect analysis:possibly-null-array-access + * @mago-expect analysis:mixed-assignment * @mago-expect lint:no-isset */ protected function prepareDataHierarchy(string $type, string ...$keys): void { + /** @var array $data */ $data = &$this->data; foreach ([$type, ...$keys] as $key) { if (! isset($data[$key])) { diff --git a/src/TableGateway/AbstractTableGateway.php b/src/TableGateway/AbstractTableGateway.php index 808a1a72..73c38bbf 100644 --- a/src/TableGateway/AbstractTableGateway.php +++ b/src/TableGateway/AbstractTableGateway.php @@ -347,8 +347,8 @@ protected function executeSelect(Select $select): ResultSetInterface if ( isset($selectState['columns']) - && [Select::SQL_STAR] === $selectState['columns'] - && [] !== $this->columns + && [Select::SQL_STAR] === $selectState['columns'] + && [] !== $this->columns ) { $select->columns($this->columns); } @@ -430,8 +430,8 @@ public function __clone(): void $this->table = clone $this->table; } elseif ( is_array($this->table) - && count($this->table) === 1 - && is_object(reset($this->table)) + && count($this->table) === 1 + && is_object(reset($this->table)) ) { foreach ($this->table as &$tableObject) { $tableObject = clone $tableObject;