From 3f4c003f8c1ffabff70e9b82467eb3a180c5a388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Mon, 3 Aug 2026 16:33:57 +0300 Subject: [PATCH 01/16] docs: record SPSS codec 3.0.3 adoption --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd2f28e..a1b84c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## [Unreleased] +### Changed + +- Updated the installed and exact CI-tested `openstatspec/spss-sav` codec from + 3.0.2 to 3.0.3, and aligned the reported engine identity and codec + documentation with the separately versioned OpenStatSpec package. + ## [0.4.0] - 2026-07-31 ### Added From 85636428714c4f97700c3e372dd000369a760d06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 23:57:49 +0300 Subject: [PATCH 02/16] Add PHP variable transformations --- .../Spss/Ast/DeleteVariablesStatement.php | 19 +++ src/Frontend/Spss/Ast/StringStatement.php | 20 +++ src/Frontend/Spss/Binder.php | 16 ++ .../Spss/Binding/BoundCreateVariable.php | 14 ++ .../Spss/Binding/BoundDeleteVariable.php | 10 ++ src/Frontend/Spss/Compiler.php | 16 ++ src/Frontend/Spss/Parser.php | 36 +++++ .../InPlaceTransformationExecutor.php | 146 +++++++++++++++++- .../Model/CreateVariableOperation.php | 42 +++++ .../Model/DeleteVariableOperation.php | 22 +++ .../Validation/PlanValidator.php | 12 +- .../InPlaceTransformationExecutorTest.php | 26 ++++ 12 files changed, 374 insertions(+), 5 deletions(-) create mode 100644 src/Frontend/Spss/Ast/DeleteVariablesStatement.php create mode 100644 src/Frontend/Spss/Ast/StringStatement.php create mode 100644 src/Frontend/Spss/Binding/BoundCreateVariable.php create mode 100644 src/Frontend/Spss/Binding/BoundDeleteVariable.php create mode 100644 src/Transformation/Model/CreateVariableOperation.php create mode 100644 src/Transformation/Model/DeleteVariableOperation.php diff --git a/src/Frontend/Spss/Ast/DeleteVariablesStatement.php b/src/Frontend/Spss/Ast/DeleteVariablesStatement.php new file mode 100644 index 0000000..cb36b74 --- /dev/null +++ b/src/Frontend/Spss/Ast/DeleteVariablesStatement.php @@ -0,0 +1,19 @@ + $variables */ + public function __construct( + public int $lineNumber, + public array $variables, + ) {} + + public function line(): int + { + return $this->lineNumber; + } +} diff --git a/src/Frontend/Spss/Ast/StringStatement.php b/src/Frontend/Spss/Ast/StringStatement.php new file mode 100644 index 0000000..5e50af2 --- /dev/null +++ b/src/Frontend/Spss/Ast/StringStatement.php @@ -0,0 +1,20 @@ + $variables */ + public function __construct( + public int $lineNumber, + public array $variables, + public int $width, + ) {} + + public function line(): int + { + return $this->lineNumber; + } +} diff --git a/src/Frontend/Spss/Binder.php b/src/Frontend/Spss/Binder.php index f3c901b..ac27682 100644 --- a/src/Frontend/Spss/Binder.php +++ b/src/Frontend/Spss/Binder.php @@ -5,8 +5,10 @@ namespace OpenStatSpec\Frontend\Spss; use OpenStatSpec\Frontend\Spss\Ast\ElseInput; +use OpenStatSpec\Frontend\Spss\Ast\DeleteVariablesStatement; use OpenStatSpec\Frontend\Spss\Ast\ExecuteStatement; use OpenStatSpec\Frontend\Spss\Ast\MissingInput; +use OpenStatSpec\Frontend\Spss\Ast\StringStatement; use OpenStatSpec\Frontend\Spss\Ast\Program; use OpenStatSpec\Frontend\Spss\Ast\RangeInput; use OpenStatSpec\Frontend\Spss\Ast\RecodeStatement; @@ -15,6 +17,8 @@ use OpenStatSpec\Frontend\Spss\Binding\BoundProgram; use OpenStatSpec\Frontend\Spss\Binding\BoundRecode; use OpenStatSpec\Frontend\Spss\Binding\BoundValueLabels; +use OpenStatSpec\Frontend\Spss\Binding\BoundCreateVariable; +use OpenStatSpec\Frontend\Spss\Binding\BoundDeleteVariable; use OpenStatSpec\Frontend\Spss\Binding\BoundVariableLabel; final class Binder @@ -30,6 +34,18 @@ public function bind(string $datasetId, Program $program): BoundProgram if ($statement instanceof ExecuteStatement) { continue; } + if ($statement instanceof StringStatement) { + foreach ($statement->variables as $variable) { + $bound[] = new BoundCreateVariable($variable, 'string', $statement->width); + } + continue; + } + if ($statement instanceof DeleteVariablesStatement) { + foreach ($statement->variables as $variable) { + $bound[] = new BoundDeleteVariable($variable); + } + continue; + } if ($statement instanceof RecodeStatement) { $targets = $statement->targets === [] ? $statement->sources : $statement->targets; if (count($statement->sources) !== count($targets)) { diff --git a/src/Frontend/Spss/Binding/BoundCreateVariable.php b/src/Frontend/Spss/Binding/BoundCreateVariable.php new file mode 100644 index 0000000..c1f37e1 --- /dev/null +++ b/src/Frontend/Spss/Binding/BoundCreateVariable.php @@ -0,0 +1,14 @@ +statements as $statement) { + if ($statement instanceof BoundCreateVariable) { + $operations[] = new CreateVariableOperation( + $statement->variable, + $statement->storageKind, + $statement->storageKind === 'string' ? $statement->declaredStringWidth : null, + ); + continue; + } + if ($statement instanceof BoundDeleteVariable) { + $operations[] = new DeleteVariableOperation($statement->variable); + continue; + } if ($statement instanceof BoundRecode) { $rules = []; $hasElse = false; diff --git a/src/Frontend/Spss/Parser.php b/src/Frontend/Spss/Parser.php index 63a0875..c0f30ef 100644 --- a/src/Frontend/Spss/Parser.php +++ b/src/Frontend/Spss/Parser.php @@ -4,6 +4,7 @@ namespace OpenStatSpec\Frontend\Spss; +use OpenStatSpec\Frontend\Spss\Ast\DeleteVariablesStatement; use OpenStatSpec\Frontend\Spss\Ast\ElseInput; use OpenStatSpec\Frontend\Spss\Ast\ExecuteStatement; use OpenStatSpec\Frontend\Spss\Ast\MissingInput; @@ -16,6 +17,7 @@ use OpenStatSpec\Frontend\Spss\Ast\RecodeStatement; use OpenStatSpec\Frontend\Spss\Ast\ScalarValue; use OpenStatSpec\Frontend\Spss\Ast\SystemMissingInput; +use OpenStatSpec\Frontend\Spss\Ast\StringStatement; use OpenStatSpec\Frontend\Spss\Ast\ValueInput; use OpenStatSpec\Frontend\Spss\Ast\ValueLabel; use OpenStatSpec\Frontend\Spss\Ast\ValueLabelGroup; @@ -60,6 +62,11 @@ public function parseTokens(array $tokens): Program $statements[] = $this->valueLabels($command); } elseif ($this->matchKeyword('EXECUTE')) { $statements[] = new ExecuteStatement($command->line); + } elseif ($this->matchKeyword('STRING')) { + $statements[] = $this->string($command); + } elseif ($this->matchKeyword('DELETE')) { + $this->consumeKeyword('VARIABLES', 'Expected VARIABLES after DELETE.'); + $statements[] = $this->deleteVariables($command); } else { $this->fail($command, sprintf('Unsupported SPSS command %s.', $command->lexeme === '' ? '' : $command->lexeme)); } @@ -180,6 +187,35 @@ private function valueLabels(Token $command): ValueLabelsStatement return new ValueLabelsStatement($command->line, $groups); } + private function string(Token $command): StringStatement + { + $variables = []; + do { + $variables[] = $this->consumeIdentifier('Expected a variable name in STRING.')->lexeme; + } while ($this->check(TokenType::Identifier)); + $this->consume(TokenType::LeftParenthesis, 'Expected a width declaration in STRING.'); + $width = $this->consume(TokenType::Identifier, 'Expected a string width such as A20.')->lexeme; + if (preg_match('/\AA([1-9][0-9]*)\z/i', $width, $matches) !== 1) { + $this->fail($this->previous(), 'STRING width must use the SPSS A form.'); + } + $widthValue = (int) $matches[1]; + if ($widthValue > 32767) { + $this->fail($this->previous(), 'STRING width must be at most 32767.'); + } + $this->consume(TokenType::RightParenthesis, 'Expected ) after STRING width.'); + + return new StringStatement($command->line, $variables, $widthValue); + } + + private function deleteVariables(Token $command): DeleteVariablesStatement + { + $variables = []; + do { + $variables[] = $this->consumeIdentifier('Expected a variable name in DELETE VARIABLES.')->lexeme; + } while ($this->check(TokenType::Identifier)); + + return new DeleteVariablesStatement($command->line, $variables); + } private function scalar(string $message): ScalarValue { if ($this->match(TokenType::String)) { diff --git a/src/Transformation/Execution/InPlaceTransformationExecutor.php b/src/Transformation/Execution/InPlaceTransformationExecutor.php index ed1998a..d613bb1 100644 --- a/src/Transformation/Execution/InPlaceTransformationExecutor.php +++ b/src/Transformation/Execution/InPlaceTransformationExecutor.php @@ -11,6 +11,8 @@ use OpenStatSpec\Sql\Connection; use OpenStatSpec\Sql\NormativeCatalog; use OpenStatSpec\Sql\OperationJournal; +use OpenStatSpec\Transformation\Model\CreateVariableOperation; +use OpenStatSpec\Transformation\Model\DeleteVariableOperation; use OpenStatSpec\Transformation\Model\Action\AssignValueAction; use OpenStatSpec\Transformation\Model\Action\CopySourceAction; use OpenStatSpec\Transformation\Model\Action\SetMissingAction; @@ -210,10 +212,49 @@ private function preflight(TransformationPlan $plan, array $variables): array $nextOrdinal = max($nextOrdinal, $variable->sourceOrdinal + 1); } + $active = array_fill_keys(array_keys($variables), true); foreach ($plan->operations() as $operation) { + if ($operation instanceof CreateVariableOperation) { + if (isset($variables[$operation->targetVariable()])) { + throw $this->invalidCatalog(sprintf( + 'Create variable "%s" collides with an existing catalog variable.', + $operation->targetVariable(), + )); + } + $this->assertCanAlterSchema(count($active), true); + $physical = $this->connection->profile->physicalIdentifier($operation->targetVariable(), $used); + $variables[$operation->targetVariable()] = new VariableBinding( + NormativeCatalog::uuid(), + $operation->targetVariable(), + $physical, + $operation->storageKind(), + $nextOrdinal++, + $operation->declaredStringWidth(), + false, + ); + $used[$physical] = true; + $active[$operation->targetVariable()] = true; + continue; + } + if ($operation instanceof DeleteVariableOperation) { + $variableName = $operation->targetVariable(); + if (!isset($active[$variableName])) { + throw $this->invalidCatalog(sprintf( + 'Delete variable "%s" is not registered for dataset_id %s.', + $variableName, + $plan->datasetId(), + )); + } + if (count($active) === 1) { + throw $this->invalidCatalog('A transformation cannot delete the final dataset variable.'); + } + $this->assertCanAlterSchema(count($active), false); + unset($active[$variableName]); + continue; + } if ($operation instanceof RecodeOperation) { $source = $variables[$operation->sourceVariable()] ?? null; - if ($source === null) { + if ($source === null || !isset($active[$operation->sourceVariable()])) { throw $this->invalidCatalog(sprintf( 'Recode source variable "%s" is not registered for dataset_id %s.', $operation->sourceVariable(), @@ -221,6 +262,12 @@ private function preflight(TransformationPlan $plan, array $variables): array )); } $target = $variables[$operation->targetVariable()] ?? null; + if ($target !== null && !isset($active[$operation->targetVariable()])) { + throw $this->invalidCatalog(sprintf( + 'Recode target variable "%s" is no longer active.', + $operation->targetVariable(), + )); + } if ($target === null) { $this->assertCanCreateTarget($source, count($variables)); $physical = $this->connection->profile->physicalIdentifier($operation->targetVariable(), $used); @@ -241,7 +288,7 @@ private function preflight(TransformationPlan $plan, array $variables): array } $target = $variables[$operation->targetVariable()] ?? null; - if ($target === null) { + if ($target === null || !isset($active[$operation->targetVariable()])) { throw $this->invalidCatalog(sprintf( 'Metadata target variable "%s" is not registered for dataset_id %s.', $operation->targetVariable(), @@ -258,6 +305,35 @@ private function preflight(TransformationPlan $plan, array $variables): array return $variables; } + private function assertCanAlterSchema(int $registeredVariableCount, bool $creation): void + { + if (!in_array($this->connection->profileName, ['sqlite', 'postgresql'], true) + || !$this->connection->profile->ddlAtomic() + ) { + throw new UnsupportedOperation( + DiagnosticCode::TargetCapabilityExceeded, + sprintf( + '%s cannot atomically alter an existing wide table for an in-place variable transformation.', + $this->connection->profileName, + ), + ); + } + if (!$creation) { + return; + } + + $maximum = $this->connection->profile->effectiveMaximumSourceVariables($this->connection->pdo); + if ($registeredVariableCount >= $maximum) { + throw new UnsupportedOperation( + DiagnosticCode::TargetCapabilityExceeded, + sprintf( + '%s supports at most %d source variables in one OpenStatSpec wide table.', + $this->connection->profileName, + $maximum, + ), + ); + } + } private function assertCanCreateTarget(VariableBinding $source, int $registeredVariableCount): void { if ($source->storageKind === 'string') { @@ -374,6 +450,16 @@ private function applyOperation( array $variables, array &$created, ): void { + if ($operation instanceof CreateVariableOperation) { + $target = $variables[$operation->targetVariable()]; + $this->ensureTargetExists($dataset, $target, $created); + return; + } + if ($operation instanceof DeleteVariableOperation) { + $target = $variables[$operation->targetVariable()]; + $this->deleteVariable($dataset, $target); + return; + } $target = $variables[$operation->targetVariable()]; if ($operation instanceof RecodeOperation) { $this->ensureTargetExists($dataset, $target, $created); @@ -402,10 +488,10 @@ private function ensureTargetExists(DatasetBinding $dataset, VariableBinding $ta )); $this->statement( 'INSERT INTO variable ' - . '(variable_id, dataset_id, source_ordinal, source_name, physical_name, storage_kind, ' + . '(variable_id, dataset_id, source_ordinal, source_name, physical_name, storage_kind, declared_string_width, ' . 'print_format_family, print_format_width, print_format_decimals, ' . 'write_format_family, write_format_width, write_format_decimals) ' - . 'VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)', + . 'VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)', )->execute([ $target->variableId, $dataset->datasetId, @@ -413,6 +499,7 @@ private function ensureTargetExists(DatasetBinding $dataset, VariableBinding $ta $target->sourceName, $target->physicalName, $target->storageKind, + $target->declaredStringWidth, 5, 8, 0, @@ -423,6 +510,57 @@ private function ensureTargetExists(DatasetBinding $dataset, VariableBinding $ta $created[$target->sourceName] = true; } + private function deleteVariable(DatasetBinding $dataset, VariableBinding $target): void + { + $this->connection->pdo->exec(sprintf( + 'ALTER TABLE %s DROP COLUMN %s', + $this->qualifiedTable($dataset), + $this->quote($target->physicalName), + )); + $association = $this->statement( + 'SELECT vvls.value_label_set_id, vls.dataset_id FROM variable_value_label_set vvls ' + . 'JOIN value_label_set vls ON vls.value_label_set_id = vvls.value_label_set_id ' + . 'WHERE vvls.variable_id = ?', + ); + $association->execute([$target->variableId]); + $rows = $association->fetchAll(PDO::FETCH_ASSOC); + if (count($rows) > 1 || (isset($rows[0]['dataset_id']) && $rows[0]['dataset_id'] !== $dataset->datasetId)) { + throw $this->invalidCatalog('The variable value-label association is malformed or crosses dataset identity.'); + } + $setId = isset($rows[0]['value_label_set_id']) && is_string($rows[0]['value_label_set_id']) + ? $rows[0]['value_label_set_id'] + : null; + + foreach ([ + 'DELETE FROM dataset_weight_variable WHERE variable_id = ?', + 'DELETE FROM variable_set_member WHERE variable_id = ?', + 'DELETE FROM multiple_response_member WHERE variable_id = ?', + 'DELETE FROM variable_attribute WHERE variable_id = ?', + 'DELETE FROM missing_rule WHERE variable_id = ?', + 'DELETE FROM variable_value_label_set WHERE variable_id = ?', + ] as $sql) { + $this->statement($sql)->execute([$target->variableId]); + } + $this->statement('DELETE FROM variable WHERE variable_id = ? AND dataset_id = ?')->execute([ + $target->variableId, + $dataset->datasetId, + ]); + + if ($setId === null) { + return; + } + $references = $this->statement( + 'SELECT COUNT(*) FROM variable_value_label_set WHERE value_label_set_id = ?', + ); + $references->execute([$setId]); + if ((int) $references->fetchColumn() === 0) { + $this->statement('DELETE FROM value_label WHERE value_label_set_id = ?')->execute([$setId]); + $this->statement('DELETE FROM value_label_set WHERE value_label_set_id = ? AND dataset_id = ?')->execute([ + $setId, + $dataset->datasetId, + ]); + } + } private function applyRecode( RecodeOperation $operation, DatasetBinding $dataset, diff --git a/src/Transformation/Model/CreateVariableOperation.php b/src/Transformation/Model/CreateVariableOperation.php new file mode 100644 index 0000000..9219365 --- /dev/null +++ b/src/Transformation/Model/CreateVariableOperation.php @@ -0,0 +1,42 @@ +variable; } + public function targetVariable(): string { return $this->variable; } + public function storageKind(): string { return $this->storageKind; } + public function declaredStringWidth(): ?int { return $this->declaredStringWidth; } + + public function canonicalArray(): array + { + return array_filter([ + 'type' => $this->type(), + 'source_variable' => $this->variable, + 'target_variable' => $this->variable, + 'storage_kind' => $this->storageKind, + 'declared_string_width' => $this->declaredStringWidth, + ], static fn(mixed $value): bool => $value !== null); + } +} diff --git a/src/Transformation/Model/DeleteVariableOperation.php b/src/Transformation/Model/DeleteVariableOperation.php new file mode 100644 index 0000000..7b85e48 --- /dev/null +++ b/src/Transformation/Model/DeleteVariableOperation.php @@ -0,0 +1,22 @@ +variable; } + public function targetVariable(): string { return $this->variable; } + public function canonicalArray(): array + { + return [ + 'type' => $this->type(), + 'source_variable' => $this->variable, + 'target_variable' => $this->variable, + ]; + } +} diff --git a/src/Transformation/Validation/PlanValidator.php b/src/Transformation/Validation/PlanValidator.php index 6ff6544..53bced6 100644 --- a/src/Transformation/Validation/PlanValidator.php +++ b/src/Transformation/Validation/PlanValidator.php @@ -7,6 +7,8 @@ use OpenStatSpec\Transformation\Model\Action\AssignValueAction; use OpenStatSpec\Transformation\Model\Action\CopySourceAction; use OpenStatSpec\Transformation\Model\Action\SetMissingAction; +use OpenStatSpec\Transformation\Model\CreateVariableOperation; +use OpenStatSpec\Transformation\Model\DeleteVariableOperation; use OpenStatSpec\Transformation\Model\RecodeAction; use OpenStatSpec\Transformation\Model\RecodeOperation; use OpenStatSpec\Transformation\Model\RecodeSelector; @@ -57,6 +59,8 @@ public function assertValid(TransformationPlan $plan): void private function validateOperation(TransformationOperation $operation, string $path): void { if (!in_array($operation::class, [ + CreateVariableOperation::class, + DeleteVariableOperation::class, RecodeOperation::class, SetVariableLabelOperation::class, SetValueLabelsOperation::class, @@ -73,7 +77,13 @@ private function validateOperation(TransformationOperation $operation, string $p $this->validateVariableName($operation->sourceVariable(), $path . '.source_variable'); $this->validateVariableName($operation->targetVariable(), $path . '.target_variable'); - if ($operation instanceof RecodeOperation) { + if ($operation instanceof CreateVariableOperation) { + if ($operation->storageKind() === 'string' && ($operation->declaredStringWidth() === null || $operation->declaredStringWidth() < 1)) { + $this->violation('create_variable.string_width_required', $path . '.declared_string_width', 'String variables require a positive declared string width.'); + } + } elseif ($operation instanceof DeleteVariableOperation) { + return; + } elseif ($operation instanceof RecodeOperation) { $this->validateRecode($operation, $path); } elseif ($operation instanceof SetVariableLabelOperation) { $this->validateText($operation->label(), $path . '.label'); diff --git a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php index dbcbdc1..5dd5021 100644 --- a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php +++ b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php @@ -13,6 +13,8 @@ use OpenStatSpec\Transformation\Model\Action\AssignValueAction; use OpenStatSpec\Transformation\Model\Action\CopySourceAction; use OpenStatSpec\Transformation\Model\Action\SetMissingAction; +use OpenStatSpec\Transformation\Model\CreateVariableOperation; +use OpenStatSpec\Transformation\Model\DeleteVariableOperation; use OpenStatSpec\Transformation\Model\RecodeOperation; use OpenStatSpec\Transformation\Model\RecodeRule; use OpenStatSpec\Transformation\Model\ScalarValue; @@ -153,6 +155,30 @@ public function testSqliteAddsANewTargetInsideTheSameWideTableAndTransaction(): )->fetchAll(PDO::FETCH_COLUMN)); } + public function testExplicitStringCreateAndDeleteRemovesDataAndMetadata(): void + { + $plan = new TransformationPlan(self::DATASET_ID, [ + new CreateVariableOperation('TempText', 'string', 20), + new SetVariableLabelOperation('TempText', 'Temporary text'), + new SetValueLabelsOperation('TempText', [ + new ValueLabel(ScalarValue::string('yes'), 'Yes'), + ]), + new DeleteVariableOperation('TempText'), + ]); + + (new InPlaceTransformationExecutor(new Connection($this->pdo)))->execute($plan); + + self::assertSame(2, (int) $this->query('SELECT COUNT(*) FROM variable')->fetchColumn()); + self::assertFalse(in_array('temptext', $this->tableColumns(), true)); + self::assertSame(0, (int) $this->query( + 'SELECT COUNT(*) FROM variable_value_label_set', + )->fetchColumn()); + self::assertSame(0, (int) $this->query( + 'SELECT COUNT(*) FROM value_label', + )->fetchColumn()); + } + + public function testNewTargetIsRejectedBeforeAlterAtTheEffectiveColumnLimit(): void { $connection = new Connection($this->pdo); From 86c64f368d87427834f3709b7a4594bb4164d281 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 08:48:49 +0300 Subject: [PATCH 03/16] Fix PHP transformation formatting and add pre-commit gate --- .githooks/pre-commit | 12 +++++++++ README.md | 6 +++++ .../Model/CreateVariableOperation.php | 25 +++++++++++++++---- .../Model/DeleteVariableOperation.php | 15 ++++++++--- tools/install-git-hooks.sh | 7 ++++++ 5 files changed, 57 insertions(+), 8 deletions(-) create mode 100755 .githooks/pre-commit create mode 100755 tools/install-git-hooks.sh diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 0000000..4dcf992 --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,12 @@ +#!/usr/bin/env bash +set -euo pipefail + +repo_root="$(git rev-parse --show-toplevel)" +cd "$repo_root" + +if [[ ! -x vendor/bin/php-cs-fixer || ! -x vendor/bin/phpunit || ! -x vendor/bin/phpstan ]]; then + echo "Pre-commit checks require Composer dependencies. Run: composer install" >&2 + exit 1 +fi + +composer check diff --git a/README.md b/README.md index a0d013f..676b1f3 100644 --- a/README.md +++ b/README.md @@ -189,6 +189,12 @@ composer install composer check ``` +Install the tracked pre-commit hook once per clone: + +```bash +bash tools/install-git-hooks.sh +``` + `composer check` validates Composer configuration, lints PHP, checks style, runs PHPStan and runs PHPUnit. Use `composer fix` for safe style fixes, then rerun `composer check`. GitHub Actions runs the regular suite on PHP 8.4 and 8.5. It also runs real diff --git a/src/Transformation/Model/CreateVariableOperation.php b/src/Transformation/Model/CreateVariableOperation.php index 9219365..82c48fc 100644 --- a/src/Transformation/Model/CreateVariableOperation.php +++ b/src/Transformation/Model/CreateVariableOperation.php @@ -23,11 +23,26 @@ public function __construct( } } - public function type(): string { return 'create_variable'; } - public function sourceVariable(): string { return $this->variable; } - public function targetVariable(): string { return $this->variable; } - public function storageKind(): string { return $this->storageKind; } - public function declaredStringWidth(): ?int { return $this->declaredStringWidth; } + public function type(): string + { + return 'create_variable'; + } + public function sourceVariable(): string + { + return $this->variable; + } + public function targetVariable(): string + { + return $this->variable; + } + public function storageKind(): string + { + return $this->storageKind; + } + public function declaredStringWidth(): ?int + { + return $this->declaredStringWidth; + } public function canonicalArray(): array { diff --git a/src/Transformation/Model/DeleteVariableOperation.php b/src/Transformation/Model/DeleteVariableOperation.php index 7b85e48..27a7110 100644 --- a/src/Transformation/Model/DeleteVariableOperation.php +++ b/src/Transformation/Model/DeleteVariableOperation.php @@ -8,9 +8,18 @@ final readonly class DeleteVariableOperation implements TransformationOperation { public function __construct(private string $variable) {} - public function type(): string { return 'delete_variable'; } - public function sourceVariable(): string { return $this->variable; } - public function targetVariable(): string { return $this->variable; } + public function type(): string + { + return 'delete_variable'; + } + public function sourceVariable(): string + { + return $this->variable; + } + public function targetVariable(): string + { + return $this->variable; + } public function canonicalArray(): array { return [ diff --git a/tools/install-git-hooks.sh b/tools/install-git-hooks.sh new file mode 100755 index 0000000..bfcdeb9 --- /dev/null +++ b/tools/install-git-hooks.sh @@ -0,0 +1,7 @@ +#!/usr/bin/env bash +set -euo pipefail + +repo_root="$(git rev-parse --show-toplevel)" +cd "$repo_root" +git config core.hooksPath .githooks +echo "Git hooks installed: .githooks" From 0f93c983f3f21d80103e949761f154454d96c0d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 09:29:25 +0300 Subject: [PATCH 04/16] Address Codex review findings for variable transformations --- .githooks/pre-commit | 15 +++++++ .../InPlaceTransformationExecutor.php | 28 ++++++++++--- .../InPlaceTransformationExecutorTest.php | 40 +++++++++++++++++++ 3 files changed, 77 insertions(+), 6 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 4dcf992..b281a1c 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -9,4 +9,19 @@ if [[ ! -x vendor/bin/php-cs-fixer || ! -x vendor/bin/phpunit || ! -x vendor/bin exit 1 fi +if [[ -z "${OPENSTATSPEC_SPECIFICATION_DIR:-}" && -d "$repo_root/../specification" ]]; then + export OPENSTATSPEC_SPECIFICATION_DIR="$repo_root/../specification" +fi + +check_dir="$(mktemp -d "${TMPDIR:-/tmp}/openstatspec-php-check.XXXXXX")" +cleanup() { + rm -rf "$check_dir" +} +trap cleanup EXIT + +staged_tree="$(git write-tree)" +git -c core.autocrlf=false archive "$staged_tree" | tar -x -C "$check_dir" +ln -s "$repo_root/vendor" "$check_dir/vendor" + +cd "$check_dir" composer check diff --git a/src/Transformation/Execution/InPlaceTransformationExecutor.php b/src/Transformation/Execution/InPlaceTransformationExecutor.php index d613bb1..e05c6a0 100644 --- a/src/Transformation/Execution/InPlaceTransformationExecutor.php +++ b/src/Transformation/Execution/InPlaceTransformationExecutor.php @@ -31,6 +31,7 @@ use PDO; use PDOException; use PDOStatement; +use SPSS\Sav\Variable; use Throwable; /** Executes a canonical plan against its registered wide table, in place. */ @@ -282,6 +283,7 @@ private function preflight(TransformationPlan $plan, array $variables): array ); $variables[$operation->targetVariable()] = $target; $used[$physical] = true; + $active[$operation->targetVariable()] = true; } $this->assertRecodeKinds($operation, $source, $target); continue; @@ -319,6 +321,15 @@ private function assertCanAlterSchema(int $registeredVariableCount, bool $creati ); } if (!$creation) { + if ($this->connection->profileName === 'sqlite' + && version_compare($this->connection->serverVersion, '3.35.0', '<') + ) { + throw new UnsupportedOperation( + DiagnosticCode::TargetCapabilityExceeded, + 'SQLite versions below 3.35.0 do not support DROP COLUMN for DELETE VARIABLES.', + ); + } + return; } @@ -480,12 +491,17 @@ private function ensureTargetExists(DatasetBinding $dataset, VariableBinding $ta $type = $target->storageKind === 'numeric' ? $this->connection->profile->numericType() : $this->connection->profile->textType(); + $columnDefinition = $target->storageKind === 'string' + ? $type . " NOT NULL DEFAULT ''" + : $type . ' NULL'; $this->connection->pdo->exec(sprintf( - 'ALTER TABLE %s ADD COLUMN %s %s NULL', + 'ALTER TABLE %s ADD COLUMN %s %s', $this->qualifiedTable($dataset), $this->quote($target->physicalName), - $type, + $columnDefinition, )); + $formatFamily = $target->storageKind === 'string' ? Variable::FORMAT_TYPE_A : 5; + $formatWidth = $target->storageKind === 'string' ? $this->stringWidth($target) : 8; $this->statement( 'INSERT INTO variable ' . '(variable_id, dataset_id, source_ordinal, source_name, physical_name, storage_kind, declared_string_width, ' @@ -500,11 +516,11 @@ private function ensureTargetExists(DatasetBinding $dataset, VariableBinding $ta $target->physicalName, $target->storageKind, $target->declaredStringWidth, - 5, - 8, + $formatFamily, + $formatWidth, 0, - 5, - 8, + $formatFamily, + $formatWidth, 0, ]); $created[$target->sourceName] = true; diff --git a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php index 5dd5021..87cf2f3 100644 --- a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php +++ b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php @@ -155,6 +155,22 @@ public function testSqliteAddsANewTargetInsideTheSameWideTableAndTransaction(): )->fetchAll(PDO::FETCH_COLUMN)); } + public function testImplicitRecodeTargetRemainsActiveForLaterMetadataOperation(): void + { + $plan = new TransformationPlan(self::DATASET_ID, [ + new RecodeOperation('SourceValue', 'CreatedTarget', [ + new RecodeRule(new ElseSelector(), new CopySourceAction()), + ]), + new SetVariableLabelOperation('CreatedTarget', 'Created target'), + ]); + + (new InPlaceTransformationExecutor(new Connection($this->pdo)))->execute($plan); + + self::assertSame('Created target', $this->query( + "SELECT variable_label FROM variable WHERE source_name = 'CreatedTarget'", + )->fetchColumn()); + } + public function testExplicitStringCreateAndDeleteRemovesDataAndMetadata(): void { $plan = new TransformationPlan(self::DATASET_ID, [ @@ -179,6 +195,30 @@ public function testExplicitStringCreateAndDeleteRemovesDataAndMetadata(): void } + public function testExplicitStringCreateInitializesBlankValuesAndAFormats(): void + { + $plan = new TransformationPlan(self::DATASET_ID, [ + new CreateVariableOperation('TempText', 'string', 20), + ]); + + (new InPlaceTransformationExecutor(new Connection($this->pdo)))->execute($plan); + + self::assertSame(['', '', '', '', ''], $this->query( + 'SELECT temptext FROM respondents ORDER BY __case_ordinal', + )->fetchAll(PDO::FETCH_COLUMN)); + self::assertSame(0, (int) $this->query( + 'SELECT COUNT(*) FROM respondents WHERE temptext IS NULL', + )->fetchColumn()); + self::assertSame( + ['1', 20, 0, '1', 20, 0], + $this->query( + "SELECT print_format_family, print_format_width, print_format_decimals, " + . "write_format_family, write_format_width, write_format_decimals " + . "FROM variable WHERE source_name = 'TempText'", + )->fetch(PDO::FETCH_NUM), + ); + } + public function testNewTargetIsRejectedBeforeAlterAtTheEffectiveColumnLimit(): void { $connection = new Connection($this->pdo); From 85c29a7bea834d2b44e4e3119c8d088271b29777 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 09:52:43 +0300 Subject: [PATCH 05/16] Allow recode targets to reuse deleted variable slots --- .../InPlaceTransformationExecutor.php | 2 +- .../InPlaceTransformationExecutorTest.php | 46 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/Transformation/Execution/InPlaceTransformationExecutor.php b/src/Transformation/Execution/InPlaceTransformationExecutor.php index e05c6a0..29fa48e 100644 --- a/src/Transformation/Execution/InPlaceTransformationExecutor.php +++ b/src/Transformation/Execution/InPlaceTransformationExecutor.php @@ -270,7 +270,7 @@ private function preflight(TransformationPlan $plan, array $variables): array )); } if ($target === null) { - $this->assertCanCreateTarget($source, count($variables)); + $this->assertCanCreateTarget($source, count($active)); $physical = $this->connection->profile->physicalIdentifier($operation->targetVariable(), $used); $target = new VariableBinding( NormativeCatalog::uuid(), diff --git a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php index 87cf2f3..81217a6 100644 --- a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php +++ b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php @@ -219,6 +219,52 @@ public function testExplicitStringCreateInitializesBlankValuesAndAFormats(): voi ); } + public function testRecodeTargetCanReuseSlotFreedByEarlierDelete(): void + { + $connection = new Connection($this->pdo); + $maximum = $connection->profile->effectiveMaximumSourceVariables($this->pdo); + $columns = ['__case_ordinal INTEGER NOT NULL PRIMARY KEY']; + for ($ordinal = 1; $ordinal <= $maximum; ++$ordinal) { + $columns[] = 'v' . $ordinal . ' REAL NULL'; + } + + $this->pdo->exec('DROP TABLE respondents'); + $this->pdo->exec('CREATE TABLE respondents (' . implode(', ', $columns) . ')'); + $this->pdo->exec('DELETE FROM variable'); + $insert = $this->pdo->prepare( + 'INSERT INTO variable ' + . '(variable_id, dataset_id, source_ordinal, source_name, physical_name, storage_kind) ' + . 'VALUES (?, ?, ?, ?, ?, ?)', + ); + $this->pdo->beginTransaction(); + for ($ordinal = 1; $ordinal <= $maximum; ++$ordinal) { + $name = 'V' . $ordinal; + $insert->execute([ + sprintf('00000000-0000-4000-8000-%012d', $ordinal), + self::DATASET_ID, + $ordinal, + $name, + strtolower($name), + 'numeric', + ]); + } + $this->pdo->commit(); + + $plan = new TransformationPlan(self::DATASET_ID, [ + new DeleteVariableOperation('V' . $maximum), + new RecodeOperation('V1', 'Replacement', [ + new RecodeRule(new ElseSelector(), new CopySourceAction()), + ]), + ]); + + (new InPlaceTransformationExecutor($connection))->execute($plan); + + self::assertSame($maximum, (int) $this->query('SELECT COUNT(*) FROM variable')->fetchColumn()); + self::assertSame($maximum, count($this->query('PRAGMA table_info(respondents)')->fetchAll()) - 1); + self::assertFalse(in_array('v' . $maximum, $this->tableColumns(), true)); + self::assertTrue(in_array('replacement', $this->tableColumns(), true)); + } + public function testNewTargetIsRejectedBeforeAlterAtTheEffectiveColumnLimit(): void { $connection = new Connection($this->pdo); From db75ac34a92e72777182cd43f9271944d0cf189e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 10:30:14 +0300 Subject: [PATCH 06/16] Bound canonical string transformation widths --- .../InPlaceTransformationExecutor.php | 4 +++- .../Model/CreateVariableOperation.php | 6 ++++++ .../Validation/PlanValidator.php | 9 +++++++-- .../Canonical/TransformationPlanTest.php | 18 ++++++++++++++++++ .../InPlaceTransformationExecutorTest.php | 18 ++++++++++++++++++ 5 files changed, 52 insertions(+), 3 deletions(-) diff --git a/src/Transformation/Execution/InPlaceTransformationExecutor.php b/src/Transformation/Execution/InPlaceTransformationExecutor.php index 29fa48e..b4ea237 100644 --- a/src/Transformation/Execution/InPlaceTransformationExecutor.php +++ b/src/Transformation/Execution/InPlaceTransformationExecutor.php @@ -501,7 +501,9 @@ private function ensureTargetExists(DatasetBinding $dataset, VariableBinding $ta $columnDefinition, )); $formatFamily = $target->storageKind === 'string' ? Variable::FORMAT_TYPE_A : 5; - $formatWidth = $target->storageKind === 'string' ? $this->stringWidth($target) : 8; + $formatWidth = $target->storageKind === 'string' + ? min(255, $this->stringWidth($target)) + : 8; $this->statement( 'INSERT INTO variable ' . '(variable_id, dataset_id, source_ordinal, source_name, physical_name, storage_kind, declared_string_width, ' diff --git a/src/Transformation/Model/CreateVariableOperation.php b/src/Transformation/Model/CreateVariableOperation.php index 82c48fc..038e298 100644 --- a/src/Transformation/Model/CreateVariableOperation.php +++ b/src/Transformation/Model/CreateVariableOperation.php @@ -7,6 +7,7 @@ /** Declares and adds one variable to the existing dataset. */ final readonly class CreateVariableOperation implements TransformationOperation { + public const MAX_STRING_WIDTH = 32767; public function __construct( private string $variable, private string $storageKind, @@ -18,6 +19,11 @@ public function __construct( if ($storageKind === 'string' && ($declaredStringWidth === null || $declaredStringWidth < 1)) { throw new \InvalidArgumentException('String variables require a positive declared string width.'); } + if ($storageKind === 'string' && $declaredStringWidth > self::MAX_STRING_WIDTH) { + throw new \InvalidArgumentException( + 'String variables support at most ' . self::MAX_STRING_WIDTH . ' bytes.', + ); + } if ($storageKind === 'numeric' && $declaredStringWidth !== null) { throw new \InvalidArgumentException('Numeric variables cannot declare a string width.'); } diff --git a/src/Transformation/Validation/PlanValidator.php b/src/Transformation/Validation/PlanValidator.php index 53bced6..2fd866c 100644 --- a/src/Transformation/Validation/PlanValidator.php +++ b/src/Transformation/Validation/PlanValidator.php @@ -78,8 +78,13 @@ private function validateOperation(TransformationOperation $operation, string $p $this->validateVariableName($operation->targetVariable(), $path . '.target_variable'); if ($operation instanceof CreateVariableOperation) { - if ($operation->storageKind() === 'string' && ($operation->declaredStringWidth() === null || $operation->declaredStringWidth() < 1)) { - $this->violation('create_variable.string_width_required', $path . '.declared_string_width', 'String variables require a positive declared string width.'); + if ($operation->storageKind() === 'string') { + $width = $operation->declaredStringWidth(); + if ($width === null || $width < 1) { + $this->violation('create_variable.string_width_required', $path . '.declared_string_width', 'String variables require a positive declared string width.'); + } elseif ($width > CreateVariableOperation::MAX_STRING_WIDTH) { + $this->violation('create_variable.string_width_too_large', $path . '.declared_string_width', 'String variables support at most ' . CreateVariableOperation::MAX_STRING_WIDTH . ' bytes.'); + } } } elseif ($operation instanceof DeleteVariableOperation) { return; diff --git a/tests/Transformation/Canonical/TransformationPlanTest.php b/tests/Transformation/Canonical/TransformationPlanTest.php index bf1724e..e58e531 100644 --- a/tests/Transformation/Canonical/TransformationPlanTest.php +++ b/tests/Transformation/Canonical/TransformationPlanTest.php @@ -7,6 +7,7 @@ use OpenStatSpec\Transformation\Model\Action\AssignValueAction; use OpenStatSpec\Transformation\Model\Action\CopySourceAction; use OpenStatSpec\Transformation\Model\Action\SetMissingAction; +use OpenStatSpec\Transformation\Model\CreateVariableOperation; use OpenStatSpec\Transformation\Model\RecodeOperation; use OpenStatSpec\Transformation\Model\RecodeRule; use OpenStatSpec\Transformation\Model\ScalarValue; @@ -95,6 +96,23 @@ public function testValidatorCollectsIdentityNameAndLabelViolations(): void $result->throwIfInvalid(); } + public function testCreateVariableRejectsStringWidthAboveSpssMaximum(): void + { + $this->expectException('InvalidArgumentException'); + $this->expectExceptionMessage('at most 32767'); + + new CreateVariableOperation('TooWide', 'string', 32768); + } + + public function testValidatorAllowsMaximumCanonicalStringWidth(): void + { + $plan = new TransformationPlan(self::DATASET_ID, [ + new CreateVariableOperation('MaxText', 'string', 32767), + ]); + + self::assertTrue((new PlanValidator())->validate($plan)->isValid()); + } + public function testValidatorAllowsSequentialOperationsOnTheSameTarget(): void { $plan = new TransformationPlan(self::DATASET_ID, [ diff --git a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php index 81217a6..4a79a2a 100644 --- a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php +++ b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php @@ -265,6 +265,24 @@ public function testRecodeTargetCanReuseSlotFreedByEarlierDelete(): void self::assertTrue(in_array('replacement', $this->tableColumns(), true)); } + public function testExplicitLongStringCreateCapsAFormatWidthsButKeepsStorageWidth(): void + { + $plan = new TransformationPlan(self::DATASET_ID, [ + new CreateVariableOperation('LongText', 'string', 400), + ]); + + (new InPlaceTransformationExecutor(new Connection($this->pdo)))->execute($plan); + + self::assertSame( + [400, '1', 255, '1', 255], + $this->query( + "SELECT declared_string_width, print_format_family, print_format_width, " + . "write_format_family, write_format_width " + . "FROM variable WHERE source_name = 'LongText'", + )->fetch(PDO::FETCH_NUM), + ); + } + public function testNewTargetIsRejectedBeforeAlterAtTheEffectiveColumnLimit(): void { $connection = new Connection($this->pdo); From 49aa494abd925537950a7d8978bfc91fb0bf95db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 10:46:16 +0300 Subject: [PATCH 07/16] Reject unsupported SPSS STRING ranges --- src/Frontend/Spss/Parser.php | 6 +++++- tests/Frontend/Spss/ParserTest.php | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/Frontend/Spss/Parser.php b/src/Frontend/Spss/Parser.php index c0f30ef..3eab73f 100644 --- a/src/Frontend/Spss/Parser.php +++ b/src/Frontend/Spss/Parser.php @@ -191,7 +191,11 @@ private function string(Token $command): StringStatement { $variables = []; do { - $variables[] = $this->consumeIdentifier('Expected a variable name in STRING.')->lexeme; + $variable = $this->consumeIdentifier('Expected a variable name in STRING.')->lexeme; + if (strcasecmp($variable, 'TO') === 0) { + $this->fail($this->previous(), 'STRING variable ranges using TO are not supported.'); + } + $variables[] = $variable; } while ($this->check(TokenType::Identifier)); $this->consume(TokenType::LeftParenthesis, 'Expected a width declaration in STRING.'); $width = $this->consume(TokenType::Identifier, 'Expected a string width such as A20.')->lexeme; diff --git a/tests/Frontend/Spss/ParserTest.php b/tests/Frontend/Spss/ParserTest.php index 6bdef05..cae2011 100644 --- a/tests/Frontend/Spss/ParserTest.php +++ b/tests/Frontend/Spss/ParserTest.php @@ -68,6 +68,20 @@ public function testParsesVariableAndValueLabelGroups(): void self::assertSame(['band'], $program->statements[1]->groups[1]->variables); } + public function testRejectsStringVariableRangesBeforeCreation(): void + { + try { + (new Parser())->parse('STRING c1 TO c4 (A7).'); + self::fail('STRING variable ranges unexpectedly parsed.'); + } catch (SpssSyntaxException $exception) { + self::assertCount(1, $exception->diagnostics); + self::assertSame( + 'STRING variable ranges using TO are not supported.', + $exception->diagnostics[0]->message, + ); + } + } + public function testFailsClosedForUnknownOrUnterminatedCommands(): void { foreach (['COMPUTE score=1.', 'RECODE score (1=2)'] as $syntax) { From dcfb1e351ecae7fc323ca8583487b02c72493a3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 11:12:15 +0300 Subject: [PATCH 08/16] Harden SPSS variable range handling --- src/Frontend/Spss/Parser.php | 6 ++++- .../InPlaceTransformationExecutor.php | 26 +++++++++++++------ tests/Frontend/Spss/ParserTest.php | 14 ++++++++++ .../InPlaceTransformationExecutorTest.php | 20 ++++++++++++++ 4 files changed, 57 insertions(+), 9 deletions(-) diff --git a/src/Frontend/Spss/Parser.php b/src/Frontend/Spss/Parser.php index 3eab73f..5d994fd 100644 --- a/src/Frontend/Spss/Parser.php +++ b/src/Frontend/Spss/Parser.php @@ -215,7 +215,11 @@ private function deleteVariables(Token $command): DeleteVariablesStatement { $variables = []; do { - $variables[] = $this->consumeIdentifier('Expected a variable name in DELETE VARIABLES.')->lexeme; + $variable = $this->consumeIdentifier('Expected a variable name in DELETE VARIABLES.')->lexeme; + if (strcasecmp($variable, 'TO') === 0) { + $this->fail($this->previous(), 'DELETE VARIABLES ranges using TO are not supported.'); + } + $variables[] = $variable; } while ($this->check(TokenType::Identifier)); return new DeleteVariablesStatement($command->line, $variables); diff --git a/src/Transformation/Execution/InPlaceTransformationExecutor.php b/src/Transformation/Execution/InPlaceTransformationExecutor.php index b4ea237..5eaec20 100644 --- a/src/Transformation/Execution/InPlaceTransformationExecutor.php +++ b/src/Transformation/Execution/InPlaceTransformationExecutor.php @@ -39,6 +39,9 @@ final class InPlaceTransformationExecutor { private readonly PlanValidator $validator; + /** @var array> */ + private array $operationBindings = []; + public function __construct( private readonly Connection $connection, ?PlanValidator $validator = null, @@ -80,8 +83,8 @@ public function execute(TransformationPlan $plan): ExecutionResult throw new PDOException('The SQL driver did not start the transformation transaction.'); } $created = []; - foreach ($plan->operations() as $operation) { - $this->applyOperation($operation, $dataset, $plannedVariables, $created); + foreach ($plan->operations() as $operationIndex => $operation) { + $this->applyOperation($operation, $dataset, $plannedVariables, $created, $operationIndex); } $doltAfter = $doltBefore === null ? null @@ -214,9 +217,10 @@ private function preflight(TransformationPlan $plan, array $variables): array } $active = array_fill_keys(array_keys($variables), true); - foreach ($plan->operations() as $operation) { + $this->operationBindings = []; + foreach ($plan->operations() as $operationIndex => $operation) { if ($operation instanceof CreateVariableOperation) { - if (isset($variables[$operation->targetVariable()])) { + if (isset($variables[$operation->targetVariable()]) && isset($active[$operation->targetVariable()])) { throw $this->invalidCatalog(sprintf( 'Create variable "%s" collides with an existing catalog variable.', $operation->targetVariable(), @@ -235,6 +239,7 @@ private function preflight(TransformationPlan $plan, array $variables): array ); $used[$physical] = true; $active[$operation->targetVariable()] = true; + $this->operationBindings[$operationIndex] = ['target' => $variables[$operation->targetVariable()]]; continue; } if ($operation instanceof DeleteVariableOperation) { @@ -250,6 +255,7 @@ private function preflight(TransformationPlan $plan, array $variables): array throw $this->invalidCatalog('A transformation cannot delete the final dataset variable.'); } $this->assertCanAlterSchema(count($active), false); + $this->operationBindings[$operationIndex] = ['target' => $variables[$variableName]]; unset($active[$variableName]); continue; } @@ -286,6 +292,7 @@ private function preflight(TransformationPlan $plan, array $variables): array $active[$operation->targetVariable()] = true; } $this->assertRecodeKinds($operation, $source, $target); + $this->operationBindings[$operationIndex] = ['source' => $source, 'target' => $target]; continue; } @@ -297,6 +304,7 @@ private function preflight(TransformationPlan $plan, array $variables): array $plan->datasetId(), )); } + $this->operationBindings[$operationIndex] = ['target' => $target]; if ($operation instanceof SetValueLabelsOperation) { foreach ($operation->labels() as $label) { $this->assertScalarKind($label->value(), $target, 'value label'); @@ -460,21 +468,23 @@ private function applyOperation( DatasetBinding $dataset, array $variables, array &$created, + int $operationIndex, ): void { + $bindings = $this->operationBindings[$operationIndex] ?? []; if ($operation instanceof CreateVariableOperation) { - $target = $variables[$operation->targetVariable()]; + $target = $bindings['target'] ?? $variables[$operation->targetVariable()]; $this->ensureTargetExists($dataset, $target, $created); return; } if ($operation instanceof DeleteVariableOperation) { - $target = $variables[$operation->targetVariable()]; + $target = $bindings['target'] ?? $variables[$operation->targetVariable()]; $this->deleteVariable($dataset, $target); return; } - $target = $variables[$operation->targetVariable()]; + $target = $bindings['target'] ?? $variables[$operation->targetVariable()]; if ($operation instanceof RecodeOperation) { $this->ensureTargetExists($dataset, $target, $created); - $this->applyRecode($operation, $dataset, $variables[$operation->sourceVariable()], $target); + $this->applyRecode($operation, $dataset, $bindings['source'] ?? $variables[$operation->sourceVariable()], $target); } elseif ($operation instanceof SetVariableLabelOperation) { $this->applyVariableLabel($operation, $dataset, $target); } elseif ($operation instanceof SetValueLabelsOperation) { diff --git a/tests/Frontend/Spss/ParserTest.php b/tests/Frontend/Spss/ParserTest.php index cae2011..ed46790 100644 --- a/tests/Frontend/Spss/ParserTest.php +++ b/tests/Frontend/Spss/ParserTest.php @@ -82,6 +82,20 @@ public function testRejectsStringVariableRangesBeforeCreation(): void } } + public function testRejectsDeleteVariableRangesBeforeExecution(): void + { + try { + (new Parser())->parse('DELETE VARIABLES c1 TO c4.'); + self::fail('DELETE VARIABLES ranges unexpectedly parsed.'); + } catch (SpssSyntaxException $exception) { + self::assertCount(1, $exception->diagnostics); + self::assertSame( + 'DELETE VARIABLES ranges using TO are not supported.', + $exception->diagnostics[0]->message, + ); + } + } + public function testFailsClosedForUnknownOrUnterminatedCommands(): void { foreach (['COMPUTE score=1.', 'RECODE score (1=2)'] as $syntax) { diff --git a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php index 4a79a2a..68dd833 100644 --- a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php +++ b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php @@ -195,6 +195,26 @@ public function testExplicitStringCreateAndDeleteRemovesDataAndMetadata(): void } + public function testDeleteThenCreateCanReuseVariableName(): void + { + $plan = new TransformationPlan(self::DATASET_ID, [ + new DeleteVariableOperation('SourceValue'), + new CreateVariableOperation('SourceValue', 'string', 20), + ]); + + (new InPlaceTransformationExecutor(new Connection($this->pdo)))->execute($plan); + + $physicalName = (string) $this->query( + "SELECT physical_name FROM variable WHERE source_name = 'SourceValue'", + )->fetchColumn(); + self::assertNotSame('source_value', $physicalName); + self::assertSame('string', $this->query( + "SELECT storage_kind FROM variable WHERE source_name = 'SourceValue'", + )->fetchColumn()); + self::assertTrue(in_array($physicalName, $this->tableColumns(), true)); + self::assertFalse(in_array('source_value', $this->tableColumns(), true)); + } + public function testExplicitStringCreateInitializesBlankValuesAndAFormats(): void { $plan = new TransformationPlan(self::DATASET_ID, [ From bc31e7372e4e4b7eb50a95040d96ab6d1d53ab54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 11:17:15 +0300 Subject: [PATCH 09/16] Remove orphaned multiple-response sets --- .../Execution/InPlaceTransformationExecutor.php | 12 ++++++++++++ .../InPlaceTransformationExecutorTest.php | 15 +++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/Transformation/Execution/InPlaceTransformationExecutor.php b/src/Transformation/Execution/InPlaceTransformationExecutor.php index 5eaec20..17ae66b 100644 --- a/src/Transformation/Execution/InPlaceTransformationExecutor.php +++ b/src/Transformation/Execution/InPlaceTransformationExecutor.php @@ -574,6 +574,18 @@ private function deleteVariable(DatasetBinding $dataset, VariableBinding $target $dataset->datasetId, ]); + $orphanedSets = $this->statement( + 'SELECT multiple_response_set_id FROM multiple_response_set WHERE dataset_id = ? ' + . 'AND NOT EXISTS (SELECT 1 FROM multiple_response_member WHERE multiple_response_set_id = multiple_response_set.multiple_response_set_id)', + ); + $orphanedSets->execute([$dataset->datasetId]); + foreach ($orphanedSets->fetchAll(PDO::FETCH_COLUMN) as $orphanedSetId) { + if (!is_string($orphanedSetId)) { + continue; + } + $this->statement('DELETE FROM multiple_response_set WHERE multiple_response_set_id = ? AND dataset_id = ?')->execute([$orphanedSetId, $dataset->datasetId]); + } + if ($setId === null) { return; } diff --git a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php index 68dd833..4cf930c 100644 --- a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php +++ b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php @@ -215,6 +215,21 @@ public function testDeleteThenCreateCanReuseVariableName(): void self::assertFalse(in_array('source_value', $this->tableColumns(), true)); } + public function testDeletingLastMultipleResponseMemberRemovesSet(): void + { + $this->pdo->prepare('INSERT INTO multiple_response_set (multiple_response_set_id, dataset_id, source_ordinal, set_name, set_kind) VALUES (?, ?, ?, ?, ?)')->execute(['mr-source', self::DATASET_ID, 1, '$MR', 'MC']); + $this->pdo->prepare('INSERT INTO multiple_response_member (multiple_response_set_id, variable_id, source_ordinal) VALUES (?, ?, ?)')->execute(['mr-source', '018f47f2-8b6a-7c3d-9e1f-123456789abd', 1]); + + $plan = new TransformationPlan(self::DATASET_ID, [ + new DeleteVariableOperation('SourceValue'), + ]); + + (new InPlaceTransformationExecutor(new Connection($this->pdo)))->execute($plan); + + self::assertSame(0, (int) $this->query('SELECT COUNT(*) FROM multiple_response_set')->fetchColumn()); + self::assertSame(0, (int) $this->query('SELECT COUNT(*) FROM multiple_response_member')->fetchColumn()); + } + public function testExplicitStringCreateInitializesBlankValuesAndAFormats(): void { $plan = new TransformationPlan(self::DATASET_ID, [ From 24708344a1d603f780212b8c9d931116b7789a98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 11:25:00 +0300 Subject: [PATCH 10/16] Recreate variables after same-plan deletion --- .../Execution/InPlaceTransformationExecutor.php | 1 + .../InPlaceTransformationExecutorTest.php | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/src/Transformation/Execution/InPlaceTransformationExecutor.php b/src/Transformation/Execution/InPlaceTransformationExecutor.php index 17ae66b..2bca8ba 100644 --- a/src/Transformation/Execution/InPlaceTransformationExecutor.php +++ b/src/Transformation/Execution/InPlaceTransformationExecutor.php @@ -479,6 +479,7 @@ private function applyOperation( if ($operation instanceof DeleteVariableOperation) { $target = $bindings['target'] ?? $variables[$operation->targetVariable()]; $this->deleteVariable($dataset, $target); + unset($created[$target->sourceName]); return; } $target = $bindings['target'] ?? $variables[$operation->targetVariable()]; diff --git a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php index 4cf930c..37bc1da 100644 --- a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php +++ b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php @@ -230,6 +230,22 @@ public function testDeletingLastMultipleResponseMemberRemovesSet(): void self::assertSame(0, (int) $this->query('SELECT COUNT(*) FROM multiple_response_member')->fetchColumn()); } + public function testCreateDeleteCreateRecreatesVariable(): void + { + $plan = new TransformationPlan(self::DATASET_ID, [ + new CreateVariableOperation('TempText', 'string', 20), + new DeleteVariableOperation('TempText'), + new CreateVariableOperation('TempText', 'string', 30), + ]); + + (new InPlaceTransformationExecutor(new Connection($this->pdo)))->execute($plan); + + self::assertTrue(in_array('temptext_2', $this->tableColumns(), true)); + self::assertSame(30, (int) $this->query( + "SELECT declared_string_width FROM variable WHERE source_name = 'TempText'", + )->fetchColumn()); + } + public function testExplicitStringCreateInitializesBlankValuesAndAFormats(): void { $plan = new TransformationPlan(self::DATASET_ID, [ From 7ff6dd2087a4b6b15ebfa4e33369fc0436eae44e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 11:34:58 +0300 Subject: [PATCH 11/16] Fix ordered recode recreation and staged hook checks --- .githooks/pre-commit | 3 ++- .../InPlaceTransformationExecutor.php | 5 +---- .../InPlaceTransformationExecutorTest.php | 21 +++++++++++++++++++ 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index b281a1c..711fbd5 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -21,7 +21,8 @@ trap cleanup EXIT staged_tree="$(git write-tree)" git -c core.autocrlf=false archive "$staged_tree" | tar -x -C "$check_dir" -ln -s "$repo_root/vendor" "$check_dir/vendor" +mkdir "$check_dir/vendor" +cp -a "$repo_root/vendor/." "$check_dir/vendor/" cd "$check_dir" composer check diff --git a/src/Transformation/Execution/InPlaceTransformationExecutor.php b/src/Transformation/Execution/InPlaceTransformationExecutor.php index 2bca8ba..e4e746c 100644 --- a/src/Transformation/Execution/InPlaceTransformationExecutor.php +++ b/src/Transformation/Execution/InPlaceTransformationExecutor.php @@ -270,10 +270,7 @@ private function preflight(TransformationPlan $plan, array $variables): array } $target = $variables[$operation->targetVariable()] ?? null; if ($target !== null && !isset($active[$operation->targetVariable()])) { - throw $this->invalidCatalog(sprintf( - 'Recode target variable "%s" is no longer active.', - $operation->targetVariable(), - )); + $target = null; } if ($target === null) { $this->assertCanCreateTarget($source, count($active)); diff --git a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php index 37bc1da..1ad73a9 100644 --- a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php +++ b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php @@ -171,6 +171,27 @@ public function testImplicitRecodeTargetRemainsActiveForLaterMetadataOperation() )->fetchColumn()); } + public function testDeleteThenRecodeCanReuseVariableName(): void + { + $plan = new TransformationPlan(self::DATASET_ID, [ + new DeleteVariableOperation('Destination'), + new RecodeOperation('SourceValue', 'Destination', [ + new RecodeRule(new ElseSelector(), new AssignValueAction(ScalarValue::number(7))), + ]), + ]); + + (new InPlaceTransformationExecutor(new Connection($this->pdo)))->execute($plan); + + $physicalName = (string) $this->query( + "SELECT physical_name FROM variable WHERE source_name = 'Destination'", + )->fetchColumn(); + self::assertNotSame('destination', $physicalName); + self::assertSame( + [7.0, 7.0, 7.0, 7.0, 7.0], + $this->query('SELECT ' . $physicalName . ' FROM respondents ORDER BY __case_ordinal')->fetchAll(PDO::FETCH_COLUMN), + ); + } + public function testExplicitStringCreateAndDeleteRemovesDataAndMetadata(): void { $plan = new TransformationPlan(self::DATASET_ID, [ From 3d6fdc31b446e4a9a23c4ee23df8dc15dbc388b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 12:33:54 +0300 Subject: [PATCH 12/16] Allow final-variable replacement within a plan --- .../Execution/InPlaceTransformationExecutor.php | 12 +++++++++++- .../InPlaceTransformationExecutorTest.php | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/Transformation/Execution/InPlaceTransformationExecutor.php b/src/Transformation/Execution/InPlaceTransformationExecutor.php index e4e746c..2194738 100644 --- a/src/Transformation/Execution/InPlaceTransformationExecutor.php +++ b/src/Transformation/Execution/InPlaceTransformationExecutor.php @@ -251,7 +251,7 @@ private function preflight(TransformationPlan $plan, array $variables): array $plan->datasetId(), )); } - if (count($active) === 1) { + if (count($active) === 1 && !$this->hasFutureCreateOperation($plan->operations(), $operationIndex)) { throw $this->invalidCatalog('A transformation cannot delete the final dataset variable.'); } $this->assertCanAlterSchema(count($active), false); @@ -312,6 +312,16 @@ private function preflight(TransformationPlan $plan, array $variables): array return $variables; } + /** @param list $operations */ + private function hasFutureCreateOperation(array $operations, int $operationIndex): bool + { + foreach ($operations as $futureIndex => $operation) { + if ($futureIndex > $operationIndex && $operation instanceof CreateVariableOperation) { + return true; + } + } + return false; + } private function assertCanAlterSchema(int $registeredVariableCount, bool $creation): void { if (!in_array($this->connection->profileName, ['sqlite', 'postgresql'], true) diff --git a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php index 1ad73a9..1faf7cb 100644 --- a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php +++ b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php @@ -216,6 +216,21 @@ public function testExplicitStringCreateAndDeleteRemovesDataAndMetadata(): void } + public function testDeletingOnlyVariableThenRecreatingWithinPlan(): void + { + $plan = new TransformationPlan(self::DATASET_ID, [ + new DeleteVariableOperation('Destination'), + new DeleteVariableOperation('SourceValue'), + new CreateVariableOperation('SourceValue', 'string', 20), + ]); + (new InPlaceTransformationExecutor(new Connection($this->pdo)))->execute($plan); + self::assertSame(1, (int) $this->query('SELECT COUNT(*) FROM variable')->fetchColumn()); + $physicalName = (string) $this->query("SELECT physical_name FROM variable WHERE source_name = 'SourceValue'")->fetchColumn(); + self::assertNotSame('source_value', $physicalName); + self::assertSame('string', $this->query("SELECT storage_kind FROM variable WHERE source_name = 'SourceValue'")->fetchColumn()); + self::assertTrue(in_array($physicalName, $this->tableColumns(), true)); + self::assertFalse(in_array('source_value', $this->tableColumns(), true)); + } public function testDeleteThenCreateCanReuseVariableName(): void { $plan = new TransformationPlan(self::DATASET_ID, [ From 2f92e44ca7eb8baf68fce9b87dd324cc5ddeb72a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 12:45:21 +0300 Subject: [PATCH 13/16] Reserve technical case ordinal column --- .../Execution/InPlaceTransformationExecutor.php | 2 +- .../Execution/InPlaceTransformationExecutorTest.php | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Transformation/Execution/InPlaceTransformationExecutor.php b/src/Transformation/Execution/InPlaceTransformationExecutor.php index 2194738..dd5e6b5 100644 --- a/src/Transformation/Execution/InPlaceTransformationExecutor.php +++ b/src/Transformation/Execution/InPlaceTransformationExecutor.php @@ -209,7 +209,7 @@ private function resolveVariables(DatasetBinding $dataset): array */ private function preflight(TransformationPlan $plan, array $variables): array { - $used = []; + $used = ['__case_ordinal' => true]; $nextOrdinal = 1; foreach ($variables as $variable) { $used[$variable->physicalName] = true; diff --git a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php index 1faf7cb..e7aaca3 100644 --- a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php +++ b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php @@ -231,6 +231,16 @@ public function testDeletingOnlyVariableThenRecreatingWithinPlan(): void self::assertTrue(in_array($physicalName, $this->tableColumns(), true)); self::assertFalse(in_array('source_value', $this->tableColumns(), true)); } + public function testCreatedVariableCannotUseCaseOrdinalColumn(): void + { + $plan = new TransformationPlan(self::DATASET_ID, [ + new CreateVariableOperation('__case_ordinal', 'string', 1), + ]); + (new InPlaceTransformationExecutor(new Connection($this->pdo)))->execute($plan); + $physicalName = (string) $this->query("SELECT physical_name FROM variable WHERE source_name = '__case_ordinal'")->fetchColumn(); + self::assertNotSame('__case_ordinal', $physicalName); + self::assertTrue(in_array($physicalName, $this->tableColumns(), true)); + } public function testDeleteThenCreateCanReuseVariableName(): void { $plan = new TransformationPlan(self::DATASET_ID, [ From a8aafecfe45d40bdb09833e066c85016351e0527 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 13:45:31 +0300 Subject: [PATCH 14/16] Remove orphaned variable sets --- .../Execution/InPlaceTransformationExecutor.php | 12 ++++++++++++ .../InPlaceTransformationExecutorTest.php | 14 ++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/Transformation/Execution/InPlaceTransformationExecutor.php b/src/Transformation/Execution/InPlaceTransformationExecutor.php index dd5e6b5..d7ffa7a 100644 --- a/src/Transformation/Execution/InPlaceTransformationExecutor.php +++ b/src/Transformation/Execution/InPlaceTransformationExecutor.php @@ -594,6 +594,18 @@ private function deleteVariable(DatasetBinding $dataset, VariableBinding $target $this->statement('DELETE FROM multiple_response_set WHERE multiple_response_set_id = ? AND dataset_id = ?')->execute([$orphanedSetId, $dataset->datasetId]); } + $orphanedVariableSets = $this->statement( + 'SELECT variable_set_id FROM variable_set WHERE dataset_id = ? ' + . 'AND NOT EXISTS (SELECT 1 FROM variable_set_member WHERE variable_set_id = variable_set.variable_set_id)', + ); + $orphanedVariableSets->execute([$dataset->datasetId]); + foreach ($orphanedVariableSets->fetchAll(PDO::FETCH_COLUMN) as $orphanedVariableSetId) { + if (!is_string($orphanedVariableSetId)) { + continue; + } + $this->statement('DELETE FROM variable_set WHERE variable_set_id = ? AND dataset_id = ?')->execute([$orphanedVariableSetId, $dataset->datasetId]); + } + if ($setId === null) { return; } diff --git a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php index e7aaca3..4b7a5ab 100644 --- a/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php +++ b/tests/Transformation/Execution/InPlaceTransformationExecutorTest.php @@ -276,6 +276,20 @@ public function testDeletingLastMultipleResponseMemberRemovesSet(): void self::assertSame(0, (int) $this->query('SELECT COUNT(*) FROM multiple_response_member')->fetchColumn()); } + public function testDeletingLastVariableSetMemberRemovesSet(): void + { + $this->pdo->prepare('INSERT INTO variable_set (variable_set_id, dataset_id, source_ordinal, set_name) VALUES (?, ?, ?, ?)')->execute(['vs-source', self::DATASET_ID, 1, 'Core']); + $this->pdo->prepare('INSERT INTO variable_set_member (variable_set_id, variable_id, source_ordinal) VALUES (?, ?, ?)')->execute(['vs-source', '018f47f2-8b6a-7c3d-9e1f-123456789abd', 1]); + $plan = new TransformationPlan(self::DATASET_ID, [ + new DeleteVariableOperation('SourceValue'), + ]); + + (new InPlaceTransformationExecutor(new Connection($this->pdo)))->execute($plan); + + self::assertSame(0, (int) $this->query('SELECT COUNT(*) FROM variable_set')->fetchColumn()); + self::assertSame(0, (int) $this->query('SELECT COUNT(*) FROM variable_set_member')->fetchColumn()); + } + public function testCreateDeleteCreateRecreatesVariable(): void { $plan = new TransformationPlan(self::DATASET_ID, [ From faa1fdf50e182db61e5e03b6a5e643646ca4cdbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 14:01:20 +0300 Subject: [PATCH 15/16] Preflight PostgreSQL column slots --- .../InPlaceTransformationExecutor.php | 56 +++++++++++----- .../PostgreSqlSpssRoundTripTest.php | 64 +++++++++++++++++++ 2 files changed, 104 insertions(+), 16 deletions(-) diff --git a/src/Transformation/Execution/InPlaceTransformationExecutor.php b/src/Transformation/Execution/InPlaceTransformationExecutor.php index d7ffa7a..6db9237 100644 --- a/src/Transformation/Execution/InPlaceTransformationExecutor.php +++ b/src/Transformation/Execution/InPlaceTransformationExecutor.php @@ -64,7 +64,7 @@ public function execute(TransformationPlan $plan): ExecutionResult $dataset = $this->resolveDataset($plan->datasetId()); $variables = $this->resolveVariables($dataset); - $plannedVariables = $this->preflight($plan, $variables); + $plannedVariables = $this->preflight($plan, $dataset, $variables); $guard = $this->doltGuard(); $doltBefore = $guard?->beforeExecution(); @@ -207,8 +207,11 @@ private function resolveVariables(DatasetBinding $dataset): array * @param array $variables * @return array */ - private function preflight(TransformationPlan $plan, array $variables): array + private function preflight(TransformationPlan $plan, DatasetBinding $dataset, array $variables): array { + $physicalColumnSlots = $this->connection->profileName === 'postgresql' + ? $this->postgresqlPhysicalColumnSlots($dataset) + : null; $used = ['__case_ordinal' => true]; $nextOrdinal = 1; foreach ($variables as $variable) { @@ -226,7 +229,10 @@ private function preflight(TransformationPlan $plan, array $variables): array $operation->targetVariable(), )); } - $this->assertCanAlterSchema(count($active), true); + $this->assertCanAlterSchema(count($active), true, $physicalColumnSlots); + if ($physicalColumnSlots !== null) { + ++$physicalColumnSlots; + } $physical = $this->connection->profile->physicalIdentifier($operation->targetVariable(), $used); $variables[$operation->targetVariable()] = new VariableBinding( NormativeCatalog::uuid(), @@ -273,7 +279,10 @@ private function preflight(TransformationPlan $plan, array $variables): array $target = null; } if ($target === null) { - $this->assertCanCreateTarget($source, count($active)); + $this->assertCanCreateTarget($source, count($active), $physicalColumnSlots); + if ($physicalColumnSlots !== null) { + ++$physicalColumnSlots; + } $physical = $this->connection->profile->physicalIdentifier($operation->targetVariable(), $used); $target = new VariableBinding( NormativeCatalog::uuid(), @@ -322,7 +331,7 @@ private function hasFutureCreateOperation(array $operations, int $operationIndex } return false; } - private function assertCanAlterSchema(int $registeredVariableCount, bool $creation): void + private function assertCanAlterSchema(int $registeredVariableCount, bool $creation, ?int $physicalColumnSlots = null): void { if (!in_array($this->connection->profileName, ['sqlite', 'postgresql'], true) || !$this->connection->profile->ddlAtomic() @@ -349,6 +358,19 @@ private function assertCanAlterSchema(int $registeredVariableCount, bool $creati } $maximum = $this->connection->profile->effectiveMaximumSourceVariables($this->connection->pdo); + if ($this->connection->profileName === 'postgresql' + && $physicalColumnSlots !== null + && $physicalColumnSlots >= $maximum + 1 + ) { + throw new UnsupportedOperation( + DiagnosticCode::TargetCapabilityExceeded, + sprintf( + 'PostgreSQL cannot add a source variable because the wide table already uses %d physical PostgreSQL column slots, including dropped columns; the table limit is %d.', + $physicalColumnSlots, + $maximum + 1, + ), + ); + } if ($registeredVariableCount >= $maximum) { throw new UnsupportedOperation( DiagnosticCode::TargetCapabilityExceeded, @@ -360,7 +382,7 @@ private function assertCanAlterSchema(int $registeredVariableCount, bool $creati ); } } - private function assertCanCreateTarget(VariableBinding $source, int $registeredVariableCount): void + private function assertCanCreateTarget(VariableBinding $source, int $registeredVariableCount, ?int $physicalColumnSlots = null): void { if ($source->storageKind === 'string') { throw new UnsupportedOperation( @@ -380,17 +402,19 @@ private function assertCanCreateTarget(VariableBinding $source, int $registeredV ); } - $maximum = $this->connection->profile->effectiveMaximumSourceVariables($this->connection->pdo); - if ($registeredVariableCount >= $maximum) { - throw new UnsupportedOperation( - DiagnosticCode::TargetCapabilityExceeded, - sprintf( - '%s supports at most %d source variables in one OpenStatSpec wide table.', - $this->connection->profileName, - $maximum, - ), - ); + $this->assertCanAlterSchema($registeredVariableCount, true, $physicalColumnSlots); + } + + private function postgresqlPhysicalColumnSlots(DatasetBinding $dataset): int + { + $statement = $this->statement('SELECT COUNT(*) FROM pg_attribute WHERE attrelid = to_regclass(?) AND attnum > 0'); + $statement->execute([$this->qualifiedTable($dataset)]); + $slots = filter_var($statement->fetchColumn(), FILTER_VALIDATE_INT); + if (!is_int($slots) || $slots < 1) { + throw $this->invalidCatalog('The PostgreSQL wide table has no physical column slots.'); } + + return $slots; } private function assertRecodeKinds( diff --git a/tests/Integration/PostgreSqlSpssRoundTripTest.php b/tests/Integration/PostgreSqlSpssRoundTripTest.php index 3a2ecfa..15c6e93 100644 --- a/tests/Integration/PostgreSqlSpssRoundTripTest.php +++ b/tests/Integration/PostgreSqlSpssRoundTripTest.php @@ -4,7 +4,16 @@ namespace OpenStatSpec\Tests\Integration; +use OpenStatSpec\Core\DiagnosticCode; +use OpenStatSpec\Core\UnsupportedOperation; +use OpenStatSpec\Sql\CatalogOwnership; +use OpenStatSpec\Sql\Connection; +use OpenStatSpec\Sql\NormativeCatalog; use OpenStatSpec\Spss\PhpSpssEngine; +use OpenStatSpec\Transformation\Execution\InPlaceTransformationExecutor; +use OpenStatSpec\Transformation\Model\CreateVariableOperation; +use OpenStatSpec\Transformation\Model\DeleteVariableOperation; +use OpenStatSpec\Transformation\Model\TransformationPlan; use OpenStatSpec\Spss\SpssAdapter; use PDO; use PDOException; @@ -117,6 +126,61 @@ public function testRealEngineRoundTripsSavAndZsavThroughPostgreSql(): void } } + public function testDeleteThenCreateAtThePostgreSqlPhysicalColumnLimitFailsBeforeMutation(): void + { + $pdo = $this->postgres(); + $datasetId = NormativeCatalog::uuid(); + $tableName = 'transform_slots_' . bin2hex(random_bytes(6)); + + try { + (new NormativeCatalog($pdo))->createTables(); + CatalogOwnership::markCurrentVersion($pdo); + $columns = ['__case_ordinal BIGINT NOT NULL PRIMARY KEY']; + for ($ordinal = 1; $ordinal <= 1599; ++$ordinal) { + $columns[] = 'v' . $ordinal . ' DOUBLE PRECISION NULL'; + } + $pdo->exec('CREATE TABLE ' . $this->quote($tableName) . ' (' . implode(', ', $columns) . ')'); + $pdo->prepare( + 'INSERT INTO dataset ' + . '(dataset_id, spec_version, source_format, physical_table_schema, physical_table_name, dataset_name, source_case_count, imported_at) ' + . 'VALUES (?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP)', + )->execute([$datasetId, '1.0', 'fixture', null, $tableName, 'PostgreSQL slot fixture', 0]); + $insert = $pdo->prepare( + 'INSERT INTO variable ' + . '(variable_id, dataset_id, source_ordinal, source_name, physical_name, storage_kind) ' + . 'VALUES (?, ?, ?, ?, ?, ?)', + ); + $pdo->beginTransaction(); + for ($ordinal = 1; $ordinal <= 1599; ++$ordinal) { + $insert->execute([NormativeCatalog::uuid(), $datasetId, $ordinal, 'V' . $ordinal, 'v' . $ordinal, 'numeric']); + } + $pdo->commit(); + + $plan = new TransformationPlan($datasetId, [ + new DeleteVariableOperation('V1599'), + new CreateVariableOperation('Replacement', 'string', 1), + ]); + + try { + (new InPlaceTransformationExecutor(new Connection($pdo)))->execute($plan); + self::fail('PostgreSQL accepted a replacement after the physical column limit was reached.'); + } catch (UnsupportedOperation $exception) { + self::assertSame(DiagnosticCode::TargetCapabilityExceeded, $exception->diagnosticCode); + self::assertStringContainsString('physical PostgreSQL column slots', $exception->getMessage()); + } + + self::assertSame(1599, (int) $this->scalar($pdo, 'SELECT COUNT(*) FROM variable WHERE dataset_id = ?', [$datasetId])); + self::assertSame( + 1600, + (int) $this->scalar($pdo, 'SELECT COUNT(*) FROM pg_attribute WHERE attrelid = to_regclass(?) AND attnum > 0', ['"' . $tableName . '"']), + ); + } finally { + $pdo->prepare('DELETE FROM variable WHERE dataset_id = ?')->execute([$datasetId]); + $pdo->prepare('DELETE FROM dataset WHERE dataset_id = ?')->execute([$datasetId]); + $pdo->exec('DROP TABLE IF EXISTS ' . $this->quote($tableName)); + } + } + private function postgres(): PDO { $dsn = getenv('OPENSTATSPEC_PG_DSN'); From 8594ffdbceb1bb29cb8633e897e7463450f713dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 14:05:55 +0300 Subject: [PATCH 16/16] Use valid UUID in PostgreSQL slot test --- tests/Integration/PostgreSqlSpssRoundTripTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/PostgreSqlSpssRoundTripTest.php b/tests/Integration/PostgreSqlSpssRoundTripTest.php index 15c6e93..b6beec5 100644 --- a/tests/Integration/PostgreSqlSpssRoundTripTest.php +++ b/tests/Integration/PostgreSqlSpssRoundTripTest.php @@ -129,7 +129,7 @@ public function testRealEngineRoundTripsSavAndZsavThroughPostgreSql(): void public function testDeleteThenCreateAtThePostgreSqlPhysicalColumnLimitFailsBeforeMutation(): void { $pdo = $this->postgres(); - $datasetId = NormativeCatalog::uuid(); + $datasetId = '018f47f2-8b6a-7c3d-9e1f-123456789abc'; $tableName = 'transform_slots_' . bin2hex(random_bytes(6)); try {