From 7d9569834fe11e27138562772ed43327d5a82acb Mon Sep 17 00:00:00 2001 From: Norbert Orzechowicz Date: Fri, 12 Jun 2026 19:40:26 +0200 Subject: [PATCH] fix(flow-php/postgresql): detect stale-cast drift in column/domain default comparison - model defaults as a `ColumnDefault` value object (literal, type, kind) - compare constant defaults type-aware so a wrong-typed stored cast is detected - add `ColumnType::isSameBaseType()` (base type, ignoring typmod) --- documentation/upgrading.md | 16 +- .../Tests/Unit/SchemaConverterTest.php | 2 +- .../Unit/MessengerCatalogProviderTest.php | 2 +- .../PgSql/PgCatalogProvider.php | 67 +------ .../QueryBuilder/Schema/ColumnType.php | 16 ++ .../src/Flow/PostgreSql/Schema/Column.php | 17 +- .../Flow/PostgreSql/Schema/ColumnDefault.php | 164 ++++++++++++++++++ .../Flow/PostgreSql/Schema/DefaultKind.php | 11 ++ .../PostgreSql/Schema/Diff/ColumnDiff.php | 26 ++- .../PostgreSql/Schema/Diff/DomainDiff.php | 9 +- .../Schema/Diff/SchemaComparator.php | 3 +- .../Flow/PostgreSql/Schema/Diff/TableDiff.php | 4 +- .../src/Flow/PostgreSql/Schema/Domain.php | 20 ++- .../src/Flow/PostgreSql/Schema/Table.php | 4 +- .../ColumnDefaultNoFalsePositiveDriftTest.php | 110 ++++++++++++ ...mnDefaultStaleCastDriftConvergenceTest.php | 123 +++++++++++++ ...ColumnTypeChangeDefaultConvergenceTest.php | 104 +++++++++++ ...gCatalogDomainDefaultNormalizationTest.php | 2 +- .../Tests/Integration/PostgreSqlContext.php | 31 ++++ .../Unit/DSL/SchemaColumnDefaultTest.php | 34 ++-- .../Unit/DSL/SchemaDomainDefaultTest.php | 6 +- .../QueryBuilder/Schema/ColumnTypeTest.php | 22 +++ .../Tests/Unit/Schema/CatalogTest.php | 42 +++-- .../Tests/Unit/Schema/ColumnDefaultTest.php | 114 ++++++++++++ .../Tests/Unit/Schema/ColumnTest.php | 2 +- .../Tests/Unit/Schema/Diff/ColumnDiffTest.php | 145 +++++++++++++++- .../Tests/Unit/Schema/DomainTest.php | 2 +- 27 files changed, 973 insertions(+), 125 deletions(-) create mode 100644 src/lib/postgresql/src/Flow/PostgreSql/Schema/ColumnDefault.php create mode 100644 src/lib/postgresql/src/Flow/PostgreSql/Schema/DefaultKind.php create mode 100644 src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/ColumnDefaultNoFalsePositiveDriftTest.php create mode 100644 src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/ColumnDefaultStaleCastDriftConvergenceTest.php create mode 100644 src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/ColumnTypeChangeDefaultConvergenceTest.php create mode 100644 src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/ColumnDefaultTest.php diff --git a/documentation/upgrading.md b/documentation/upgrading.md index 52abe38d84..5af6b5dde3 100644 --- a/documentation/upgrading.md +++ b/documentation/upgrading.md @@ -9,7 +9,20 @@ Please follow the instructions for your specific version to ensure a smooth upgr ## Upgrading from 0.39.x to 0.40.x -### 1) `flow-php/telemetry` - Log severity filtering moved to a pipeline middleware +### 1) `flow-php/postgresql` - column and domain defaults are modeled as `ColumnDefault` + +| Before | After | +|---------------------------------------------------------------|-----------------------------------------------------------------------------| +| `Column::$default` / `Domain::$default` type `?string` | `?Flow\PostgreSql\Schema\ColumnDefault` | +| `new Column('c', $type, true, "'0'")` | `new Column('c', $type, true, ColumnDefault::fromExpression("'0'", $type))` | +| `$column->default` (string) | `$column->default?->literal` / `$column->default?->applicableSql()` | +| `ColumnShape['default']` / `DomainShape['default']` `?string` | `?array{literal: string, type: ?ColumnTypeShape, kind: string}` | + +`Column::create()` / `Domain::create()` still accept `bool|float|int|string|Expression|null`. +Schema arrays serialized by `Column::normalize()` / `Domain::normalize()` before 0.40 must be +regenerated — `fromArray()` reads the nested `default` shape only. + +### 2) `flow-php/telemetry` - Log severity filtering moved to a pipeline middleware | Before | After | |-----------------------------------------------------------------|-----------------------------------------------------------------------------------------| @@ -122,6 +135,7 @@ to_pgsql_table($client, 'users')->withTypesMap(new EntryTypesMap( ``` ### 6) `flow-php/symfony-postgresql-messenger` - `messenger_messages` time columns use `timestamp` instead of + `timestamptz` | Column type for `created_at`, `available_at`, `delivered_at` | Before | After | diff --git a/src/adapter/etl-adapter-postgresql/tests/Flow/ETL/Adapter/PostgreSql/Tests/Unit/SchemaConverterTest.php b/src/adapter/etl-adapter-postgresql/tests/Flow/ETL/Adapter/PostgreSql/Tests/Unit/SchemaConverterTest.php index 65bfbbac58..7ec0ccc044 100644 --- a/src/adapter/etl-adapter-postgresql/tests/Flow/ETL/Adapter/PostgreSql/Tests/Unit/SchemaConverterTest.php +++ b/src/adapter/etl-adapter-postgresql/tests/Flow/ETL/Adapter/PostgreSql/Tests/Unit/SchemaConverterTest.php @@ -67,7 +67,7 @@ public function test_default_value(): void 'flags', ); - static::assertSame('true', $table->column('active')->default); + static::assertSame('true', $table->column('active')->default?->literal); } public function test_empty_schema_throws(): void diff --git a/src/bridge/symfony/postgresql-messenger/tests/Flow/Bridge/Symfony/PostgreSQLMessenger/Tests/Unit/MessengerCatalogProviderTest.php b/src/bridge/symfony/postgresql-messenger/tests/Flow/Bridge/Symfony/PostgreSQLMessenger/Tests/Unit/MessengerCatalogProviderTest.php index 6978c96f78..c9a8039f72 100644 --- a/src/bridge/symfony/postgresql-messenger/tests/Flow/Bridge/Symfony/PostgreSQLMessenger/Tests/Unit/MessengerCatalogProviderTest.php +++ b/src/bridge/symfony/postgresql-messenger/tests/Flow/Bridge/Symfony/PostgreSQLMessenger/Tests/Unit/MessengerCatalogProviderTest.php @@ -74,7 +74,7 @@ public function test_queue_name_has_default_value(): void $provider = new MessengerCatalogProvider(); $queueNameColumn = $provider->get()->get('public')->tables[0]->column('queue_name'); - static::assertSame("'default'", $queueNameColumn->default); + static::assertSame("'default'", $queueNameColumn->default?->literal); } public function test_table_has_custom_name_and_schema(): void diff --git a/src/lib/postgresql/src/Flow/PostgreSql/Client/Infrastructure/PgSql/PgCatalogProvider.php b/src/lib/postgresql/src/Flow/PostgreSql/Client/Infrastructure/PgSql/PgCatalogProvider.php index 1679c13db5..7d40b829c3 100644 --- a/src/lib/postgresql/src/Flow/PostgreSql/Client/Infrastructure/PgSql/PgCatalogProvider.php +++ b/src/lib/postgresql/src/Flow/PostgreSql/Client/Infrastructure/PgSql/PgCatalogProvider.php @@ -9,7 +9,6 @@ use Flow\PostgreSql\Parser\ColumnTypeParser; use Flow\PostgreSql\Parser\ExpressionParser; use Flow\PostgreSql\Parser\TriggerDefinitionParser; -use Flow\PostgreSql\Protobuf\AST\Integer; use Flow\PostgreSql\QueryBuilder\Condition\ComparisonOperator; use Flow\PostgreSql\QueryBuilder\Expression\Literal; use Flow\PostgreSql\QueryBuilder\Schema\ColumnType; @@ -17,6 +16,7 @@ use Flow\PostgreSql\Schema\Catalog; use Flow\PostgreSql\Schema\CatalogProvider; use Flow\PostgreSql\Schema\Column; +use Flow\PostgreSql\Schema\ColumnDefault; use Flow\PostgreSql\Schema\Constraint\CheckConstraint; use Flow\PostgreSql\Schema\Constraint\ExcludeConstraint; use Flow\PostgreSql\Schema\Constraint\ForeignKey; @@ -74,7 +74,6 @@ use function Flow\PostgreSql\DSL\when; use function Flow\Types\DSL\type_array; use function Flow\Types\DSL\type_boolean; -use function Flow\Types\DSL\type_instance_of; use function Flow\Types\DSL\type_integer; use function Flow\Types\DSL\type_null; use function Flow\Types\DSL\type_string; @@ -82,7 +81,6 @@ use function Flow\Types\DSL\type_union; use function preg_match; use function sprintf; -use function str_replace; use function strtolower; use function trim; @@ -131,57 +129,6 @@ private function mapReferentialAction(string $code): ReferentialAction return ReferentialAction::tryFrom($code) ?? ReferentialAction::NO_ACTION; } - /** - * Strip implicit type casts from default values that PostgreSQL adds for storage. - * - * PostgreSQL stores `'pending'` as `'pending'::character varying` — the cast is implicit - * and redundant since the column type is already known. This method parses the expression - * and removes the outer TypeCast when it wraps a simple constant. - */ - private function normalizeDefault(?string $default): ?string - { - if ($default === null) { - return null; - } - - $node = $this->expressionParser->parse($default); - $typeCast = $node->getTypeCast(); - - if ($typeCast === null) { - return $default; - } - - $inner = $typeCast->getArg(); - - if ($inner === null) { - return $default; - } - - $aConst = $inner->getAConst(); - - if ($aConst === null) { - return $default; - } - - $sval = $aConst->getSval(); - - if ($sval !== null) { - return "'" . str_replace("'", "''", $sval->getSval()) . "'"; - } - - if ($aConst->getIval() !== null) { - return (string) type_instance_of(Integer::class)->assert($aConst->getIval())->getIval(); - } - - $fval = $aConst->getFval(); - - if ($fval !== null) { - return $fval->getFval(); - } - - return $default; - } - /** * @return list */ @@ -318,12 +265,15 @@ private function readColumns(string $tableName, string $schemaName): array $isGenerated = $generated !== ''; $defaultValue = type_union(type_string(), type_null())->assert($row['default_value'] ?? null); + $columnType = $this->columnTypeParser->parse(type_string()->assert($row['type_name'])); $columns[] = new Column( type_string()->assert($row['name']), - $this->columnTypeParser->parse(type_string()->assert($row['type_name'])), + $columnType, type_boolean()->assert($row['nullable']), - $isGenerated || $isIdentity ? null : $this->normalizeDefault($defaultValue), + $isGenerated || $isIdentity || $defaultValue === null + ? null + : ColumnDefault::fromExpression($defaultValue, $columnType), $isIdentity, $isIdentity ? IdentityGeneration::from($identity) : null, $isGenerated, @@ -411,12 +361,13 @@ private function readDomains(string $schemaName): array $row = type_array()->assert($row); $name = type_string()->assert($row['name']); $defaultValue = type_union(type_string(), type_null())->assert($row['default_value'] ?? null); + $baseType = $this->columnTypeParser->parse(type_string()->assert($row['base_type'])); $domains[] = new Domain( $name, - $this->columnTypeParser->parse(type_string()->assert($row['base_type'])), + $baseType, type_boolean()->assert($row['nullable']), - $this->normalizeDefault($defaultValue), + $defaultValue === null ? null : ColumnDefault::fromExpression($defaultValue, $baseType), $this->readDomainCheckConstraints($name, $schemaName), ); } diff --git a/src/lib/postgresql/src/Flow/PostgreSql/QueryBuilder/Schema/ColumnType.php b/src/lib/postgresql/src/Flow/PostgreSql/QueryBuilder/Schema/ColumnType.php index 1050249e77..fb242b413f 100644 --- a/src/lib/postgresql/src/Flow/PostgreSql/QueryBuilder/Schema/ColumnType.php +++ b/src/lib/postgresql/src/Flow/PostgreSql/QueryBuilder/Schema/ColumnType.php @@ -265,6 +265,22 @@ public function isEqual(self $other): bool ); } + /** + * Same base type ignoring type modifiers (precision/scale). + * + * A column default's stored cast (e.g. '0'::numeric) carries the base type but no typmod, while + * the column itself is typed numeric(10,3). Comparing those for stale-cast drift must ignore the + * typmod and look only at the base type identity. + */ + public function isSameBaseType(self $other): bool + { + return ( + self::normalizedName($this->name) === self::normalizedName($other->name) + && $this->normalizedSchema() === $other->normalizedSchema() + && $this->isArray === $other->isArray + ); + } + /** * @return ColumnTypeShape */ diff --git a/src/lib/postgresql/src/Flow/PostgreSql/Schema/Column.php b/src/lib/postgresql/src/Flow/PostgreSql/Schema/Column.php index 61e7d93cd9..0ac6f6f4b8 100644 --- a/src/lib/postgresql/src/Flow/PostgreSql/Schema/Column.php +++ b/src/lib/postgresql/src/Flow/PostgreSql/Schema/Column.php @@ -10,8 +10,9 @@ /** * @phpstan-import-type ColumnTypeShape from ColumnType + * @phpstan-import-type ColumnDefaultShape from ColumnDefault * - * @phpstan-type ColumnShape = array{name: string, type: ColumnTypeShape, nullable: bool, default?: ?string, is_identity?: bool, identity_generation?: ?string, is_generated?: bool, generation_expression?: ?string, ordinal_position?: ?int} + * @phpstan-type ColumnShape = array{name: string, type: ColumnTypeShape, nullable: bool, default?: ?ColumnDefaultShape, is_identity?: bool, identity_generation?: ?string, is_generated?: bool, generation_expression?: ?string, ordinal_position?: ?int} */ final readonly class Column { @@ -21,7 +22,7 @@ public function __construct( public string $name, public ColumnType $type, public bool $nullable, - public ?string $default = null, + public ?ColumnDefault $default = null, public bool $isIdentity = false, public ?IdentityGeneration $identityGeneration = null, public bool $isGenerated = false, @@ -44,11 +45,13 @@ public static function create( ?string $generationExpression = null, ?int $ordinalPosition = null, ): self { + $formattedDefault = (new ColumnDefaultFormatter())->format($default); + return new self( $name, $type, $nullable, - (new ColumnDefaultFormatter())->format($default), + $formattedDefault === null ? null : ColumnDefault::fromExpression($formattedDefault, $type), $isIdentity, $identityGeneration, $isGenerated, @@ -66,7 +69,9 @@ public static function fromArray(array $data): self name: $data['name'], type: ColumnType::fromArray($data['type']), nullable: $data['nullable'], - default: $data['default'] ?? null, + default: array_key_exists('default', $data) && $data['default'] !== null + ? ColumnDefault::fromArray($data['default']) + : null, isIdentity: $data['is_identity'] ?? false, identityGeneration: array_key_exists('identity_generation', $data) && $data['identity_generation'] !== null ? IdentityGeneration::from($data['identity_generation']) @@ -87,7 +92,7 @@ public function isEqualStructure(self $other): bool return ( $this->type->isEqual($other->type) && $this->nullable === $other->nullable - && $this->default === $other->default + && ColumnDefault::nullableEquals($this->default, $other->default) && $this->isIdentity === $other->isIdentity && $this->identityGeneration === $other->identityGeneration && $this->isGenerated === $other->isGenerated @@ -104,7 +109,7 @@ public function normalize(): array 'name' => $this->name, 'type' => $this->type->normalize(), 'nullable' => $this->nullable, - 'default' => $this->default, + 'default' => $this->default?->normalize(), 'is_identity' => $this->isIdentity, 'identity_generation' => $this->identityGeneration?->value, 'is_generated' => $this->isGenerated, diff --git a/src/lib/postgresql/src/Flow/PostgreSql/Schema/ColumnDefault.php b/src/lib/postgresql/src/Flow/PostgreSql/Schema/ColumnDefault.php new file mode 100644 index 0000000000..baa98df611 --- /dev/null +++ b/src/lib/postgresql/src/Flow/PostgreSql/Schema/ColumnDefault.php @@ -0,0 +1,164 @@ +parse($expression); + + $typeCast = $node->getTypeCast(); + + if ($typeCast !== null) { + $aConst = $typeCast->getArg()?->getAConst(); + $literal = $aConst !== null ? self::renderConstant($aConst) : null; + + if ($literal !== null) { + $typeName = $typeCast->getTypeName(); + + return new self( + $literal, + $typeName !== null ? ColumnType::fromAst($typeName) : $columnType, + DefaultKind::CONSTANT, + ); + } + } else { + $aConst = $node->getAConst(); + $literal = $aConst !== null ? self::renderConstant($aConst) : null; + + if ($literal !== null) { + return new self($literal, $columnType, DefaultKind::CONSTANT); + } + } + + return new self((new ExpressionParser())->normalize($expression), null, DefaultKind::EXPRESSION); + } + + /** + * @param ColumnDefaultShape $data + */ + public static function fromArray(array $data): self + { + return new self( + $data['literal'], + array_key_exists('type', $data) && $data['type'] !== null ? ColumnType::fromArray($data['type']) : null, + DefaultKind::from($data['kind']), + ); + } + + public static function nullableEquals(?self $a, ?self $b): bool + { + if ($a === null || $b === null) { + return $a === null && $b === null; + } + + return $a->equals($b); + } + + /** + * The expression to feed into a `SET DEFAULT` / `DEFAULT` clause. + * + * For a constant this is the bare literal (no cast) so PostgreSQL re-types it against the column, + * which is exactly what converges a stale cast back to the column's type. + */ + public function applicableSql(): string + { + return $this->literal; + } + + public function equals(self $other): bool + { + if ($this->kind !== $other->kind) { + return false; + } + + if ($this->kind === DefaultKind::EXPRESSION) { + return $this->literal === $other->literal; + } + + return ( + $this->literal === $other->literal + && $this->effectiveType !== null + && $other->effectiveType !== null + && $this->effectiveType->isSameBaseType($other->effectiveType) + ); + } + + /** + * @return ColumnDefaultShape + */ + public function normalize(): array + { + return [ + 'literal' => $this->literal, + 'type' => $this->effectiveType?->normalize(), + 'kind' => $this->kind->value, + ]; + } + + private static function renderConstant(A_Const $aConst): ?string + { + $sval = $aConst->getSval(); + + if ($sval !== null) { + return "'" . str_replace("'", "''", $sval->getSval()) . "'"; + } + + if ($aConst->getIval() !== null) { + return (string) type_instance_of(Integer::class)->assert($aConst->getIval())->getIval(); + } + + $fval = $aConst->getFval(); + + if ($fval !== null) { + return $fval->getFval(); + } + + if ($aConst->getBoolval() !== null) { + return type_instance_of(Boolean::class)->assert($aConst->getBoolval())->getBoolval() ? 'true' : 'false'; + } + + return null; + } +} diff --git a/src/lib/postgresql/src/Flow/PostgreSql/Schema/DefaultKind.php b/src/lib/postgresql/src/Flow/PostgreSql/Schema/DefaultKind.php new file mode 100644 index 0000000000..3bfde1f002 --- /dev/null +++ b/src/lib/postgresql/src/Flow/PostgreSql/Schema/DefaultKind.php @@ -0,0 +1,11 @@ +target->default !== null) { - $colDef = $colDef->defaultRaw(ExpressionFactory::fromAst((new ExpressionParser())->parse($this->target->default))); + $colDef = $colDef->defaultRaw(ExpressionFactory::fromAst( + (new ExpressionParser())->parse($this->target->default->applicableSql()), + )); } if ($this->target->isGenerated && $this->target->generationExpression !== null) { @@ -66,7 +69,9 @@ public function generate(): array return $sqls; } - if (!$this->target->type->isEqual($this->source->type)) { + $typeChanged = !$this->target->type->isEqual($this->source->type); + + if ($typeChanged) { $sqls[] = alter()->table($this->qualifiedTableName)->alterColumnType($columnName, $this->target->type); } @@ -76,15 +81,26 @@ public function generate(): array : alter()->table($this->qualifiedTableName)->alterColumnSetNotNull($columnName); } - if ($this->target->default !== $this->source->default) { + if (!ColumnDefault::nullableEquals($this->target->default, $this->source->default)) { $sqls[] = $this->target->default === null ? alter()->table($this->qualifiedTableName)->alterColumnDropDefault($columnName) : alter() ->table($this->qualifiedTableName) ->alterColumnSetDefault( $columnName, - ExpressionFactory::fromAst((new ExpressionParser())->parse($this->target->default)), + ExpressionFactory::fromAst( + (new ExpressionParser())->parse($this->target->default->applicableSql()), + ), ); + } elseif ($typeChanged && $this->target->default !== null) { + $sqls[] = alter() + ->table($this->qualifiedTableName) + ->alterColumnSetDefault( + $columnName, + ExpressionFactory::fromAst( + (new ExpressionParser())->parse($this->target->default->applicableSql()), + ), + ); } return $sqls; @@ -92,7 +108,7 @@ public function generate(): array public function hasDefaultChanged(): bool { - return $this->source->default !== $this->target->default; + return !ColumnDefault::nullableEquals($this->source->default, $this->target->default); } public function hasGenerationChanged(): bool diff --git a/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/DomainDiff.php b/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/DomainDiff.php index fc4bb0dcb1..173ed60f92 100644 --- a/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/DomainDiff.php +++ b/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/DomainDiff.php @@ -8,6 +8,7 @@ use Flow\PostgreSql\QueryBuilder\Condition\ConditionFactory; use Flow\PostgreSql\QueryBuilder\Expression\ExpressionFactory; use Flow\PostgreSql\QueryBuilder\Sql; +use Flow\PostgreSql\Schema\ColumnDefault; use Flow\PostgreSql\Schema\Constraint\CheckConstraint; use Flow\PostgreSql\Schema\Domain; use RuntimeException; @@ -46,12 +47,14 @@ public function generate(): array : alter()->domain($this->target->name)->setNotNull(); } - if ($this->target->default !== $this->source->default) { + if (!ColumnDefault::nullableEquals($this->target->default, $this->source->default)) { $sqls[] = $this->target->default === null ? alter()->domain($this->target->name)->dropDefault() : alter() ->domain($this->target->name) - ->setDefault(ExpressionFactory::fromAst((new ExpressionParser())->parse($this->target->default))); + ->setDefault(ExpressionFactory::fromAst( + (new ExpressionParser())->parse($this->target->default->applicableSql()), + )); } foreach ($this->removedCheckConstraints as $cc) { @@ -88,7 +91,7 @@ public function hasBaseTypeChanged(): bool public function hasDefaultChanged(): bool { - return $this->source->default !== $this->target->default; + return !ColumnDefault::nullableEquals($this->source->default, $this->target->default); } public function hasNullableChanged(): bool diff --git a/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/SchemaComparator.php b/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/SchemaComparator.php index 7593b71116..9ad8664dd9 100644 --- a/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/SchemaComparator.php +++ b/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/SchemaComparator.php @@ -5,6 +5,7 @@ namespace Flow\PostgreSql\Schema\Diff; use Flow\PostgreSql\Parser; +use Flow\PostgreSql\Schema\ColumnDefault; use Flow\PostgreSql\Schema\Domain; use Flow\PostgreSql\Schema\ExecutionOrderStrategy; use Flow\PostgreSql\Schema\Extension; @@ -169,7 +170,7 @@ function (Domain $a, Domain $b): ?DomainDiff { $hasPropertyChange = !$a->baseType->isEqual($b->baseType) || $a->nullable !== $b->nullable - || $a->default !== $b->default; + || !ColumnDefault::nullableEquals($a->default, $b->default); if (!$hasPropertyChange && $checkDiff->added === [] && $checkDiff->removed === []) { return null; diff --git a/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/TableDiff.php b/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/TableDiff.php index 1c3e4120f1..563f118687 100644 --- a/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/TableDiff.php +++ b/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/TableDiff.php @@ -183,7 +183,9 @@ public function generate(): array } if ($col->default !== null) { - $colDef = $colDef->defaultRaw(ExpressionFactory::fromAst((new ExpressionParser())->parse($col->default))); + $colDef = $colDef->defaultRaw(ExpressionFactory::fromAst( + (new ExpressionParser())->parse($col->default->applicableSql()), + )); } if ($col->isIdentity) { diff --git a/src/lib/postgresql/src/Flow/PostgreSql/Schema/Domain.php b/src/lib/postgresql/src/Flow/PostgreSql/Schema/Domain.php index a723ac1502..11ccaae36e 100644 --- a/src/lib/postgresql/src/Flow/PostgreSql/Schema/Domain.php +++ b/src/lib/postgresql/src/Flow/PostgreSql/Schema/Domain.php @@ -12,14 +12,16 @@ use Flow\PostgreSql\QueryBuilder\Sql; use Flow\PostgreSql\Schema\Constraint\CheckConstraint; +use function array_key_exists; use function array_map; use function Flow\PostgreSql\DSL\create; /** * @phpstan-import-type ColumnTypeShape from ColumnType + * @phpstan-import-type ColumnDefaultShape from ColumnDefault * @phpstan-import-type CheckConstraintShape from CheckConstraint * - * @phpstan-type DomainShape = array{name: string, base_type: ColumnTypeShape, nullable: bool, default: ?string, check_constraints: list} + * @phpstan-type DomainShape = array{name: string, base_type: ColumnTypeShape, nullable: bool, default: ?ColumnDefaultShape, check_constraints: list} */ final readonly class Domain { @@ -30,7 +32,7 @@ public function __construct( public string $name, public ColumnType $baseType, public bool $nullable = true, - public ?string $default = null, + public ?ColumnDefault $default = null, public array $checkConstraints = [], ) {} @@ -44,11 +46,13 @@ public static function create( bool|float|int|string|Expression|null $default = null, array $checkConstraints = [], ): self { + $formattedDefault = (new ColumnDefaultFormatter())->format($default); + return new self( $name, $baseType, $nullable, - (new ColumnDefaultFormatter())->format($default), + $formattedDefault === null ? null : ColumnDefault::fromExpression($formattedDefault, $baseType), $checkConstraints, ); } @@ -62,7 +66,9 @@ public static function fromArray(array $data): self name: $data['name'], baseType: ColumnType::fromArray($data['base_type']), nullable: $data['nullable'], - default: $data['default'] ?? null, + default: array_key_exists('default', $data) && $data['default'] !== null + ? ColumnDefault::fromArray($data['default']) + : null, checkConstraints: array_map(static fn(array $cc): CheckConstraint => CheckConstraint::fromArray( $cc, ), $data['check_constraints']), @@ -78,7 +84,7 @@ public function normalize(): array 'name' => $this->name, 'base_type' => $this->baseType->normalize(), 'nullable' => $this->nullable, - 'default' => $this->default, + 'default' => $this->default?->normalize(), 'check_constraints' => array_map( static fn(CheckConstraint $cc): array => $cc->normalize(), $this->checkConstraints, @@ -95,7 +101,9 @@ public function toSql(): Sql } if ($this->default !== null) { - $builder = $builder->default(ExpressionFactory::fromAst((new ExpressionParser())->parse($this->default))); + $builder = $builder->default(ExpressionFactory::fromAst( + (new ExpressionParser())->parse($this->default->applicableSql()), + )); } foreach ($this->checkConstraints as $cc) { diff --git a/src/lib/postgresql/src/Flow/PostgreSql/Schema/Table.php b/src/lib/postgresql/src/Flow/PostgreSql/Schema/Table.php index f4b568fe14..9a67051c94 100644 --- a/src/lib/postgresql/src/Flow/PostgreSql/Schema/Table.php +++ b/src/lib/postgresql/src/Flow/PostgreSql/Schema/Table.php @@ -379,7 +379,9 @@ public function toSql(): array } if ($col->default !== null) { - $colDef = $colDef->defaultRaw(ExpressionFactory::fromAst((new ExpressionParser())->parse($col->default))); + $colDef = $colDef->defaultRaw(ExpressionFactory::fromAst( + (new ExpressionParser())->parse($col->default->applicableSql()), + )); } if ($col->isIdentity) { diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/ColumnDefaultNoFalsePositiveDriftTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/ColumnDefaultNoFalsePositiveDriftTest.php new file mode 100644 index 0000000000..35a364f68b --- /dev/null +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/ColumnDefaultNoFalsePositiveDriftTest.php @@ -0,0 +1,110 @@ +pgsqlContext()->dropSchemaIfExists(self::SCHEMA); + $this->pgsqlContext()->client()->execute(create()->schema(self::SCHEMA)->toSql()); + $this->pgsqlContext()->client()->execute(create()->sequence('seq', self::SCHEMA)->toSql()); + + $this + ->pgsqlContext() + ->client() + ->execute( + create() + ->table(self::TABLE, self::SCHEMA) + ->column(column('n', column_type_integer())->default(5)) + ->column(column('s', column_type_varchar(255))->default('pending')) + ->column(column('t', column_type_text())->default('plain')) + ->column(column('ts', column_type_timestamptz())->default(func('now'))) + ->column(column('seq_col', column_type_integer())->default(func('nextval', [literal(self::SCHEMA + . '.seq')]))) + ->toSql(), + ); + } + + protected function tearDown(): void + { + $this->pgsqlContext()->dropSchemaIfExists(self::SCHEMA); + + parent::tearDown(); + } + + public function test_declared_defaults_do_not_drift_against_introspected_implicit_casts(): void + { + $table = client_catalog_provider($this->pgsqlContext()->client(), [self::SCHEMA]) + ->get() + ->get(self::SCHEMA) + ->table(self::TABLE); + + $declared = [ + 'n' => Column::create('n', column_type_integer(), true, 5), + 's' => Column::create('s', column_type_varchar(255), true, 'pending'), + 't' => Column::create('t', column_type_text(), true, 'plain'), + 'ts' => Column::create('ts', column_type_timestamptz(), true, func('now')), + ]; + + foreach ($declared as $name => $declaredColumn) { + static::assertSame( + [], + (new ColumnDiff(self::SCHEMA . '.' . self::TABLE, $table->column($name), $declaredColumn))->generate(), + sprintf('Column "%s" reported spurious drift', $name), + ); + } + } + + public function test_all_defaults_including_sequence_round_trip_without_drift(): void + { + $first = client_catalog_provider($this->pgsqlContext()->client(), [self::SCHEMA]) + ->get() + ->get(self::SCHEMA) + ->table(self::TABLE); + $second = client_catalog_provider($this->pgsqlContext()->client(), [self::SCHEMA]) + ->get() + ->get(self::SCHEMA) + ->table(self::TABLE); + + foreach (['n', 's', 't', 'ts', 'seq_col'] as $name) { + static::assertSame( + [], + (new ColumnDiff( + self::SCHEMA . '.' . self::TABLE, + $first->column($name), + $second->column($name), + ))->generate(), + sprintf('Column "%s" reported spurious drift on round-trip', $name), + ); + } + } +} diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/ColumnDefaultStaleCastDriftConvergenceTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/ColumnDefaultStaleCastDriftConvergenceTest.php new file mode 100644 index 0000000000..c2d8a61810 --- /dev/null +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/ColumnDefaultStaleCastDriftConvergenceTest.php @@ -0,0 +1,123 @@ +pgsqlContext()->dropSchemaIfExists(self::SCHEMA); + $this->pgsqlContext()->client()->execute(create()->schema(self::SCHEMA)->toSql()); + + // Stale state: create the column as double precision with a default, then change only its type + // to numeric(10,3) WITHOUT re-issuing SET DEFAULT. pg_attrdef keeps '0'::double precision. + $this + ->pgsqlContext() + ->client() + ->execute( + create() + ->table(self::SOURCE_TABLE, self::SCHEMA) + ->column(column('amount', column_type_double_precision())->default('0')) + ->toSql(), + ); + $this + ->pgsqlContext() + ->client() + ->execute(sprintf( + 'ALTER TABLE %s.%s ALTER COLUMN amount TYPE numeric(10, 3)', + self::SCHEMA, + self::SOURCE_TABLE, + )); + + $this + ->pgsqlContext() + ->client() + ->execute( + create() + ->table(self::TARGET_TABLE, self::SCHEMA) + ->column(column('amount', column_type_numeric(10, 3))->default('0')) + ->toSql(), + ); + } + + protected function tearDown(): void + { + $this->pgsqlContext()->dropSchemaIfExists(self::SCHEMA); + + parent::tearDown(); + } + + public function test_stale_cast_drift_is_detected_and_converges_without_type_change(): void + { + static::assertSame("'0'::double precision", $this->pgsqlContext()->columnDefaultExpression( + self::SCHEMA, + self::SOURCE_TABLE, + 'amount', + )); + + $source = client_catalog_provider($this->pgsqlContext()->client(), [self::SCHEMA]) + ->get() + ->get(self::SCHEMA) + ->table(self::SOURCE_TABLE) + ->column('amount'); + $target = client_catalog_provider($this->pgsqlContext()->client(), [self::SCHEMA]) + ->get() + ->get(self::SCHEMA) + ->table(self::TARGET_TABLE) + ->column('amount'); + + $sqls = (new ColumnDiff(self::SCHEMA . '.' . self::SOURCE_TABLE, $source, $target))->generate(); + + static::assertCount(1, $sqls); + static::assertSame( + sprintf('ALTER TABLE %s.%s ALTER COLUMN amount SET DEFAULT \'0\'', self::SCHEMA, self::SOURCE_TABLE), + $sqls[0]->toSql(), + ); + + foreach ($sqls as $sql) { + $this->pgsqlContext()->client()->execute($sql->toSql()); + } + + static::assertSame("'0'::numeric", $this->pgsqlContext()->columnDefaultExpression( + self::SCHEMA, + self::SOURCE_TABLE, + 'amount', + )); + + $sourceAfter = client_catalog_provider($this->pgsqlContext()->client(), [self::SCHEMA]) + ->get() + ->get(self::SCHEMA) + ->table(self::SOURCE_TABLE) + ->column('amount'); + + static::assertSame( + [], + (new ColumnDiff(self::SCHEMA . '.' . self::SOURCE_TABLE, $sourceAfter, $target))->generate(), + ); + } +} diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/ColumnTypeChangeDefaultConvergenceTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/ColumnTypeChangeDefaultConvergenceTest.php new file mode 100644 index 0000000000..cfb7406445 --- /dev/null +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/ColumnTypeChangeDefaultConvergenceTest.php @@ -0,0 +1,104 @@ +pgsqlContext()->dropSchemaIfExists(self::SCHEMA); + $this->pgsqlContext()->client()->execute(create()->schema(self::SCHEMA)->toSql()); + + $this + ->pgsqlContext() + ->client() + ->execute( + create() + ->table(self::SOURCE_TABLE, self::SCHEMA) + ->column(column('amount', column_type_double_precision())->default('0')) + ->toSql(), + ); + + $this + ->pgsqlContext() + ->client() + ->execute( + create() + ->table(self::TARGET_TABLE, self::SCHEMA) + ->column(column('amount', column_type_numeric(10, 3))->default('0')) + ->toSql(), + ); + } + + protected function tearDown(): void + { + $this->pgsqlContext()->dropSchemaIfExists(self::SCHEMA); + + parent::tearDown(); + } + + public function test_type_change_reapplies_default_so_pg_attrdef_is_retyped(): void + { + static::assertSame("'0'::double precision", $this->pgsqlContext()->columnDefaultExpression( + self::SCHEMA, + self::SOURCE_TABLE, + 'amount', + )); + + $source = client_catalog_provider($this->pgsqlContext()->client(), [self::SCHEMA]) + ->get() + ->get(self::SCHEMA) + ->table(self::SOURCE_TABLE) + ->column('amount'); + $target = client_catalog_provider($this->pgsqlContext()->client(), [self::SCHEMA]) + ->get() + ->get(self::SCHEMA) + ->table(self::TARGET_TABLE) + ->column('amount'); + + foreach ((new ColumnDiff(self::SCHEMA . '.' . self::SOURCE_TABLE, $source, $target))->generate() as $sql) { + $this->pgsqlContext()->client()->execute($sql->toSql()); + } + + static::assertSame("'0'::numeric", $this->pgsqlContext()->columnDefaultExpression( + self::SCHEMA, + self::SOURCE_TABLE, + 'amount', + )); + + $sourceAfter = client_catalog_provider($this->pgsqlContext()->client(), [self::SCHEMA]) + ->get() + ->get(self::SCHEMA) + ->table(self::SOURCE_TABLE) + ->column('amount'); + + static::assertSame( + [], + (new ColumnDiff(self::SCHEMA . '.' . self::SOURCE_TABLE, $sourceAfter, $target))->generate(), + ); + } +} diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/PgCatalogDomainDefaultNormalizationTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/PgCatalogDomainDefaultNormalizationTest.php index 1469ac8744..287de20e67 100644 --- a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/PgCatalogDomainDefaultNormalizationTest.php +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/Client/PgCatalogDomainDefaultNormalizationTest.php @@ -52,6 +52,6 @@ public function test_domain_default_round_trip_with_implicit_cast_on_varchar_lit static::assertCount(1, $schema->domains); static::assertSame('status_type', $schema->domains[0]->name); - static::assertSame("'pending'", $schema->domains[0]->default); + static::assertSame("'pending'", $schema->domains[0]->default?->literal); } } diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/PostgreSqlContext.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/PostgreSqlContext.php index 3e71d72135..779c5a8a83 100644 --- a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/PostgreSqlContext.php +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/PostgreSqlContext.php @@ -9,9 +9,16 @@ use function fclose; use function file_get_contents; +use function Flow\PostgreSql\DSL\and_; +use function Flow\PostgreSql\DSL\col; use function Flow\PostgreSql\DSL\drop; +use function Flow\PostgreSql\DSL\eq; +use function Flow\PostgreSql\DSL\func; +use function Flow\PostgreSql\DSL\literal; use function Flow\PostgreSql\DSL\pgsql_client; use function Flow\PostgreSql\DSL\pgsql_connection_dsn; +use function Flow\PostgreSql\DSL\select; +use function Flow\PostgreSql\DSL\table; use function getcwd; use function getenv; use function is_file; @@ -94,6 +101,30 @@ public function close(): void $this->client->close(); } + /** + * Reads the raw default expression stored in pg_attrdef for a column, + * including the implicit type cast PostgreSQL keeps (e.g. '0'::numeric). + * The column must have a stored default. + */ + public function columnDefaultExpression(string $schema, string $table, string $column): string + { + return $this->client->fetchScalarString( + select(func('pg_catalog.pg_get_expr', [col('adbin', 'd'), col('adrelid', 'd')])) + ->from(table('pg_attrdef', 'pg_catalog')->as('d')) + ->join( + table('pg_attribute', 'pg_catalog')->as('a'), + and_(eq(col('attrelid', 'a'), col('adrelid', 'd')), eq(col('attnum', 'a'), col('adnum', 'd'))), + ) + ->join(table('pg_class', 'pg_catalog')->as('c'), eq(col('oid', 'c'), col('adrelid', 'd'))) + ->join(table('pg_namespace', 'pg_catalog')->as('n'), eq(col('oid', 'n'), col('relnamespace', 'c'))) + ->where(and_( + eq(col('relname', 'c'), literal($table)), + eq(col('nspname', 'n'), literal($schema)), + eq(col('attname', 'a'), literal($column)), + )), + ); + } + public function dropDomainIfExists(string $domain): void { $this->client->execute(drop()->domain($domain)->ifExists()->cascade()->toSql()); diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/DSL/SchemaColumnDefaultTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/DSL/SchemaColumnDefaultTest.php index 1ecd5e6b98..65250a1549 100644 --- a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/DSL/SchemaColumnDefaultTest.php +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/DSL/SchemaColumnDefaultTest.php @@ -27,52 +27,52 @@ public function test_bool_false_default_is_stored_as_literal(): void { $column = schema_column_boolean('active', default: false); - static::assertSame('false', $column->default); + static::assertSame('false', $column->default?->literal); } public function test_bool_true_default_is_stored_as_literal(): void { $column = schema_column_boolean('active', default: true); - static::assertSame('true', $column->default); + static::assertSame('true', $column->default?->literal); } public function test_float_default_is_stored_as_literal(): void { $column = schema_column_real('price', default: 3.14); - static::assertSame('3.14', $column->default); + static::assertSame('3.14', $column->default?->literal); } public function test_func_call_expression_is_stored_as_deparsed_sql(): void { $column = schema_column_timestamp_tz('created_at', default: func('now')); - static::assertSame('now()', $column->default); + static::assertSame('now()', $column->default?->literal); } public function test_func_call_uuid_default(): void { $column = schema_column_uuid('external_id', default: func('gen_random_uuid')); - static::assertSame('gen_random_uuid()', $column->default); + static::assertSame('gen_random_uuid()', $column->default?->literal); } public function test_int_default_is_stored_as_literal(): void { - static::assertSame('0', schema_column_integer('age', default: 0)->default); - static::assertSame('-42', schema_column_integer('delta', default: -42)->default); + static::assertSame('0', schema_column_integer('age', default: 0)->default?->literal); + static::assertSame('-42', schema_column_integer('delta', default: -42)->default?->literal); } public function test_literal_expression_matches_plain_scalar_equivalent(): void { static::assertSame( - schema_column('status', column_type_varchar(50), default: 'active')->default, - schema_column('status', column_type_varchar(50), default: literal('active'))->default, + schema_column('status', column_type_varchar(50), default: 'active')->default?->literal, + schema_column('status', column_type_varchar(50), default: literal('active'))->default?->literal, ); static::assertSame( - schema_column_integer('age', default: 0)->default, - schema_column_integer('age', default: literal(0))->default, + schema_column_integer('age', default: 0)->default?->literal, + schema_column_integer('age', default: literal(0))->default?->literal, ); } @@ -80,7 +80,7 @@ public function test_negative_float_default_is_stored_as_literal(): void { $column = schema_column_real('delta', default: -2.5); - static::assertSame('-2.5', $column->default); + static::assertSame('-2.5', $column->default?->literal); } public function test_null_default_is_stored_as_null(): void @@ -93,7 +93,7 @@ public function test_plain_string_default_is_stored_as_pg_quoted_literal(): void { $column = schema_column('status', column_type_varchar(50), default: 'active'); - static::assertSame("'active'", $column->default); + static::assertSame("'active'", $column->default?->literal); } public function test_round_trip_through_to_sql(): void @@ -116,27 +116,27 @@ public function test_schema_column_text_accepts_typed_string_default(): void { $column = schema_column_text('label', default: 'unknown'); - static::assertSame("'unknown'", $column->default); + static::assertSame("'unknown'", $column->default?->literal); } public function test_schema_column_varchar_accepts_typed_default(): void { $column = schema_column_varchar('status', 50, default: 'pending'); - static::assertSame("'pending'", $column->default); + static::assertSame("'pending'", $column->default?->literal); } public function test_string_with_backslash_is_stored_verbatim_inside_literal(): void { $column = schema_column('path', column_type_varchar(50), default: 'a\\b'); - static::assertSame("'a\\b'", $column->default); + static::assertSame("'a\\b'", $column->default?->literal); } public function test_string_with_single_quote_is_escaped(): void { $column = schema_column('note', column_type_varchar(50), default: "it's"); - static::assertSame("'it''s'", $column->default); + static::assertSame("'it''s'", $column->default?->literal); } } diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/DSL/SchemaDomainDefaultTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/DSL/SchemaDomainDefaultTest.php index c6557f1ac1..2b932bf801 100644 --- a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/DSL/SchemaDomainDefaultTest.php +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/DSL/SchemaDomainDefaultTest.php @@ -18,14 +18,14 @@ public function test_func_call_expression_is_stored_as_deparsed_sql(): void { $domain = schema_domain('ts', column_type_timestamptz(), default: func('now')); - static::assertSame('now()', $domain->default); + static::assertSame('now()', $domain->default?->literal); } public function test_int_default_is_stored_as_literal(): void { $domain = schema_domain('positive_int', column_type_integer(), default: 0); - static::assertSame('0', $domain->default); + static::assertSame('0', $domain->default?->literal); } public function test_null_default_is_stored_as_null(): void @@ -37,7 +37,7 @@ public function test_plain_string_default_is_stored_as_pg_quoted_literal(): void { $domain = schema_domain('email', column_type_text(), default: 'unknown'); - static::assertSame("'unknown'", $domain->default); + static::assertSame("'unknown'", $domain->default?->literal); } public function test_round_trip_through_to_sql(): void diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/QueryBuilder/Schema/ColumnTypeTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/QueryBuilder/Schema/ColumnTypeTest.php index 691ba0dc0d..a7ed8631df 100644 --- a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/QueryBuilder/Schema/ColumnTypeTest.php +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/QueryBuilder/Schema/ColumnTypeTest.php @@ -271,6 +271,28 @@ public function test_is_equal_with_same_type_and_typmods(): void static::assertTrue(column_type_varchar(255)->isEqual(column_type_varchar(255))); } + public function test_is_same_base_type_array_vs_scalar(): void + { + static::assertFalse(ColumnType::array(column_type_text())->isSameBaseType(column_type_text())); + } + + public function test_is_same_base_type_different_base_types(): void + { + static::assertFalse(ColumnType::numeric(10, 3)->isSameBaseType(ColumnType::doublePrecision())); + static::assertFalse(column_type_integer()->isSameBaseType(column_type_bigint())); + } + + public function test_is_same_base_type_ignores_typmods(): void + { + static::assertTrue(ColumnType::numeric(10, 3)->isSameBaseType(ColumnType::numeric())); + static::assertTrue(column_type_varchar(255)->isSameBaseType(column_type_varchar(100))); + } + + public function test_is_same_base_type_serial_normalizes_to_integer(): void + { + static::assertTrue(ColumnType::serial()->isSameBaseType(column_type_integer())); + } + public function test_json(): void { $type = ColumnType::json(); diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/CatalogTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/CatalogTest.php index 14279eaba6..525c472096 100644 --- a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/CatalogTest.php +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/CatalogTest.php @@ -8,6 +8,7 @@ use Flow\PostgreSql\QueryBuilder\Schema\ReferentialAction; use Flow\PostgreSql\Schema\Catalog; use Flow\PostgreSql\Schema\Column; +use Flow\PostgreSql\Schema\ColumnDefault; use Flow\PostgreSql\Schema\Constraint\CheckConstraint; use Flow\PostgreSql\Schema\Constraint\ExcludeConstraint; use Flow\PostgreSql\Schema\Constraint\ForeignKey; @@ -608,7 +609,12 @@ public function test_normalize_and_from_array_with_table_all_constraints(): void new Column('id', ColumnType::integer(), false), new Column('user_id', ColumnType::integer(), false), new Column('amount', ColumnType::numeric(10, 2), false), - new Column('status', ColumnType::text(), false, default: "'pending'"), + new Column( + 'status', + ColumnType::text(), + false, + default: ColumnDefault::fromExpression("'pending'", ColumnType::text()), + ), ], primaryKey: new PrimaryKey(['id'], 'orders_pkey'), indexes: [ @@ -735,8 +741,18 @@ public function test_normalize_produces_expected_structure(): void public function test_round_trip_preserves_column_default_value(): void { $table = new Table('public', 'defaults', [ - new Column('status', ColumnType::text(), false, default: "'active'"), - new Column('count', ColumnType::integer(), false, default: '0'), + new Column( + 'status', + ColumnType::text(), + false, + default: ColumnDefault::fromExpression("'active'", ColumnType::text()), + ), + new Column( + 'count', + ColumnType::integer(), + false, + default: ColumnDefault::fromExpression('0', ColumnType::integer()), + ), new Column('no_default', ColumnType::text(), true), ]); @@ -744,17 +760,23 @@ public function test_round_trip_preserves_column_default_value(): void $restored = Catalog::fromArray($catalog->normalize()); $restoredTable = $restored->get('public')->table('defaults'); - static::assertSame("'active'", $restoredTable->column('status')->default); - static::assertSame('0', $restoredTable->column('count')->default); + static::assertSame("'active'", $restoredTable->column('status')->default?->literal); + static::assertSame('0', $restoredTable->column('count')->default?->literal); static::assertNull($restoredTable->column('no_default')->default); } public function test_round_trip_preserves_domain_with_check_constraints(): void { - $domain = new Domain('positive_int', ColumnType::integer(), nullable: false, default: '0', checkConstraints: [ - new CheckConstraint('VALUE > 0', 'chk_positive'), - new CheckConstraint('VALUE < 1000000', 'chk_max'), - ]); + $domain = new Domain( + 'positive_int', + ColumnType::integer(), + nullable: false, + default: ColumnDefault::fromExpression('0', ColumnType::integer()), + checkConstraints: [ + new CheckConstraint('VALUE > 0', 'chk_positive'), + new CheckConstraint('VALUE < 1000000', 'chk_max'), + ], + ); $catalog = new Catalog([new Schema('public', domains: [$domain])]); $restored = Catalog::fromArray($catalog->normalize()); @@ -762,7 +784,7 @@ public function test_round_trip_preserves_domain_with_check_constraints(): void $restoredDomain = $restored->get('public')->domains[0]; static::assertSame('positive_int', $restoredDomain->name); static::assertFalse($restoredDomain->nullable); - static::assertSame('0', $restoredDomain->default); + static::assertSame('0', $restoredDomain->default?->literal); static::assertCount(2, $restoredDomain->checkConstraints); static::assertSame('value > 0', $restoredDomain->checkConstraints[0]->expression); static::assertSame('chk_positive', $restoredDomain->checkConstraints[0]->name); diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/ColumnDefaultTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/ColumnDefaultTest.php new file mode 100644 index 0000000000..c7f95eeb3b --- /dev/null +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/ColumnDefaultTest.php @@ -0,0 +1,114 @@ +kind); + static::assertSame('5', $default->literal); + static::assertNotNull($default->effectiveType); + static::assertTrue($default->effectiveType->isSameBaseType(ColumnType::integer())); + } + + public function test_bool_constants_render_as_keywords(): void + { + static::assertSame('true', ColumnDefault::fromExpression('true', ColumnType::boolean())->literal); + static::assertSame('false', ColumnDefault::fromExpression('false', ColumnType::boolean())->literal); + } + + public function test_constant_and_expression_are_never_equal(): void + { + $constant = ColumnDefault::fromExpression("'now()'", ColumnType::text()); + $expression = ColumnDefault::fromExpression('now()', ColumnType::timestamptz()); + + static::assertSame(DefaultKind::CONSTANT, $constant->kind); + static::assertSame(DefaultKind::EXPRESSION, $expression->kind); + static::assertFalse($constant->equals($expression)); + } + + public function test_equal_expression_defaults_compare_equal(): void + { + static::assertTrue(ColumnDefault::fromExpression( + 'now()', + ColumnType::timestamptz(), + )->equals(ColumnDefault::fromExpression('now()', ColumnType::timestamptz()))); + } + + public function test_expression_default_strips_implicit_casts_like_generation_expressions(): void + { + $default = ColumnDefault::fromExpression('upper(name::text)', ColumnType::text()); + + static::assertSame(DefaultKind::EXPRESSION, $default->kind); + static::assertSame('upper(name)', $default->literal); + static::assertTrue($default->equals(ColumnDefault::fromExpression('upper(name)', ColumnType::text()))); + } + + public function test_float_constant_is_rendered_verbatim(): void + { + static::assertSame('3.14', ColumnDefault::fromExpression('3.14', ColumnType::doublePrecision())->literal); + } + + public function test_implicit_varchar_cast_equals_bare_string(): void + { + static::assertTrue(ColumnDefault::fromExpression( + "'pending'::character varying", + ColumnType::varchar(255), + )->equals(ColumnDefault::fromExpression("'pending'", ColumnType::varchar(255)))); + } + + public function test_nullable_equals(): void + { + $default = ColumnDefault::fromExpression('0', ColumnType::integer()); + + static::assertTrue(ColumnDefault::nullableEquals(null, null)); + static::assertFalse(ColumnDefault::nullableEquals($default, null)); + static::assertFalse(ColumnDefault::nullableEquals(null, $default)); + static::assertTrue(ColumnDefault::nullableEquals($default, $default)); + } + + public function test_round_trips_through_normalize_and_from_array(): void + { + foreach ([ + ColumnDefault::fromExpression("'pending'", ColumnType::varchar(255)), + ColumnDefault::fromExpression("'0'::double precision", ColumnType::numeric(10, 3)), + ColumnDefault::fromExpression('now()', ColumnType::timestamptz()), + ColumnDefault::fromExpression('5', ColumnType::integer()), + ] as $default) { + static::assertTrue($default->equals(ColumnDefault::fromArray($default->normalize()))); + } + } + + public function test_same_base_type_cast_equals_bare_constant(): void + { + static::assertTrue(ColumnDefault::fromExpression("'0'::numeric", ColumnType::numeric( + 10, + 3, + ))->equals(ColumnDefault::fromExpression("'0'", ColumnType::numeric(10, 3)))); + } + + public function test_stale_cast_is_not_equal_to_clean_default(): void + { + $stale = ColumnDefault::fromExpression("'0'::double precision", ColumnType::numeric(10, 3)); + $clean = ColumnDefault::fromExpression("'0'", ColumnType::numeric(10, 3)); + + static::assertSame("'0'", $stale->literal); + static::assertSame("'0'", $clean->literal); + static::assertFalse($stale->equals($clean)); + } + + public function test_string_constant_is_quoted_and_escaped(): void + { + static::assertSame("'it''s'", ColumnDefault::fromExpression("'it''s'", ColumnType::text())->literal); + } +} diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/ColumnTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/ColumnTest.php index d11748e023..8a53967e47 100644 --- a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/ColumnTest.php +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/ColumnTest.php @@ -54,7 +54,7 @@ public function test_column_with_default(): void { $column = schema_column('name', column_type_varchar(255), default: 'unknown'); - static::assertSame("'unknown'", $column->default); + static::assertSame("'unknown'", $column->default?->literal); } public function test_is_equal_for_identical_columns(): void diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/Diff/ColumnDiffTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/Diff/ColumnDiffTest.php index 3a4043ba27..bf22db1fc6 100644 --- a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/Diff/ColumnDiffTest.php +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/Diff/ColumnDiffTest.php @@ -6,6 +6,7 @@ use Flow\PostgreSql\QueryBuilder\Schema\ColumnType; use Flow\PostgreSql\Schema\Column; +use Flow\PostgreSql\Schema\ColumnDefault; use Flow\PostgreSql\Schema\Diff\ColumnDiff; use Flow\PostgreSql\Schema\IdentityGeneration; use PHPUnit\Framework\TestCase; @@ -138,8 +139,18 @@ public function test_detects_default_change(): void { $diff = new ColumnDiff( 'public.test_table', - new Column('col', ColumnType::text(), true, 'old_default'), - new Column('col', ColumnType::text(), true, 'new_default'), + new Column( + 'col', + ColumnType::text(), + true, + ColumnDefault::fromExpression("'old_default'", ColumnType::text()), + ), + new Column( + 'col', + ColumnType::text(), + true, + ColumnDefault::fromExpression("'new_default'", ColumnType::text()), + ), ); static::assertTrue($diff->hasDefaultChanged()); @@ -218,7 +229,7 @@ public function test_drops_default(): void { $diff = new ColumnDiff( 'public.users', - new Column('col', ColumnType::text(), true, '42'), + new Column('col', ColumnType::text(), true, ColumnDefault::fromExpression("'42'", ColumnType::text())), new Column('col', ColumnType::text(), true, null), ); @@ -260,12 +271,120 @@ public function test_generated_column_with_not_null(): void ); } + public function test_type_change_reapplies_default_with_same_literal(): void + { + $diff = new ColumnDiff( + 'public.users', + new Column( + 'amount', + ColumnType::doublePrecision(), + true, + ColumnDefault::fromExpression("'0'", ColumnType::doublePrecision()), + ), + new Column( + 'amount', + ColumnType::numeric(10, 3), + true, + ColumnDefault::fromExpression("'0'", ColumnType::numeric(10, 3)), + ), + ); + + $sqls = $diff->generate(); + + static::assertCount(2, $sqls); + static::assertSame('ALTER TABLE public.users ALTER COLUMN amount TYPE numeric(10, 3)', $sqls[0]->toSql()); + static::assertSame("ALTER TABLE public.users ALTER COLUMN amount SET DEFAULT '0'", $sqls[1]->toSql()); + } + + public function test_type_change_with_changed_default_emits_single_set_default(): void + { + $diff = new ColumnDiff( + 'public.users', + new Column( + 'amount', + ColumnType::doublePrecision(), + true, + ColumnDefault::fromExpression("'0'", ColumnType::doublePrecision()), + ), + new Column( + 'amount', + ColumnType::numeric(10, 3), + true, + ColumnDefault::fromExpression("'1'", ColumnType::numeric(10, 3)), + ), + ); + + $sqls = $diff->generate(); + + static::assertCount(2, $sqls); + static::assertSame('ALTER TABLE public.users ALTER COLUMN amount TYPE numeric(10, 3)', $sqls[0]->toSql()); + static::assertSame("ALTER TABLE public.users ALTER COLUMN amount SET DEFAULT '1'", $sqls[1]->toSql()); + } + + public function test_type_change_with_no_default_emits_no_set_default(): void + { + $diff = new ColumnDiff( + 'public.users', + new Column('amount', ColumnType::doublePrecision(), true, null), + new Column('amount', ColumnType::numeric(10, 3), true, null), + ); + + $sqls = $diff->generate(); + + static::assertCount(1, $sqls); + static::assertSame('ALTER TABLE public.users ALTER COLUMN amount TYPE numeric(10, 3)', $sqls[0]->toSql()); + } + + public function test_type_change_to_null_default_drops_default(): void + { + $diff = new ColumnDiff( + 'public.users', + new Column( + 'amount', + ColumnType::doublePrecision(), + true, + ColumnDefault::fromExpression("'0'", ColumnType::doublePrecision()), + ), + new Column('amount', ColumnType::numeric(10, 3), true, null), + ); + + $sqls = $diff->generate(); + + static::assertCount(2, $sqls); + static::assertSame('ALTER TABLE public.users ALTER COLUMN amount TYPE numeric(10, 3)', $sqls[0]->toSql()); + static::assertSame('ALTER TABLE public.users ALTER COLUMN amount DROP DEFAULT', $sqls[1]->toSql()); + } + + public function test_type_change_with_generation_transition_keeps_drop_and_readd(): void + { + $diff = new ColumnDiff( + 'public.users', + new Column('total', ColumnType::doublePrecision(), true), + new Column( + 'total', + ColumnType::numeric(10, 3), + true, + isGenerated: true, + generationExpression: 'price * qty', + ), + ); + + $sqls = $diff->generate(); + + static::assertCount(2, $sqls); + static::assertSame('ALTER TABLE public.users DROP total', $sqls[0]->toSql()); + static::assertSame( + 'ALTER TABLE public.users ADD COLUMN total numeric(10, 3) GENERATED ALWAYS AS (price * qty) STORED', + $sqls[1]->toSql(), + ); + } + public function test_handles_multiple_changes_type_nullable_default(): void { $diff = new ColumnDiff( 'public.users', new Column('col', ColumnType::text(), true, null), - new Column('col', ColumnType::integer(), false, '0'), + new Column('col', ColumnType::integer(), false, ColumnDefault::fromExpression('0', ColumnType::integer())), ); $sqls = $diff->generate(); @@ -319,7 +438,7 @@ public function test_identity_with_default_and_not_null(): void 'id', ColumnType::integer(), false, - default: '0', + default: ColumnDefault::fromExpression('0', ColumnType::integer()), isIdentity: true, identityGeneration: IdentityGeneration::BY_DEFAULT, ), @@ -339,8 +458,18 @@ public function test_no_changes_when_identical(): void { $diff = new ColumnDiff( 'public.test_table', - new Column('col', ColumnType::text(), true, 'default_val'), - new Column('col', ColumnType::text(), true, 'default_val'), + new Column( + 'col', + ColumnType::text(), + true, + ColumnDefault::fromExpression("'default_val'", ColumnType::text()), + ), + new Column( + 'col', + ColumnType::text(), + true, + ColumnDefault::fromExpression("'default_val'", ColumnType::text()), + ), ); static::assertFalse($diff->hasNameChanged()); @@ -473,7 +602,7 @@ public function test_sets_default(): void $diff = new ColumnDiff( 'public.users', new Column('col', ColumnType::text(), true, null), - new Column('col', ColumnType::text(), true, '42'), + new Column('col', ColumnType::text(), true, ColumnDefault::fromExpression('42', ColumnType::text())), ); $sqls = $diff->generate(); diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/DomainTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/DomainTest.php index c237e5e898..f80e673669 100644 --- a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/DomainTest.php +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/DomainTest.php @@ -34,7 +34,7 @@ public function test_domain_with_constraints(): void ); static::assertFalse($domain->nullable); - static::assertSame('0', $domain->default); + static::assertSame('0', $domain->default?->literal); static::assertCount(1, $domain->checkConstraints); static::assertSame('value > 0', $domain->checkConstraints[0]->expression); }