From b740ce501c5cb387db6aacbdae9e4e364e29a277 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Mon, 9 Nov 2026 13:03:57 +0200 Subject: [PATCH 1/3] perf: cheaper import preflight and positional SQLite case binds The shared SQL preflight ran up to eight namespaced is_*/is_finite calls per cell. Branch on the value type first, so a numeric cell costs two type tests and one finiteness test. Rules, order and messages are unchanged for list rows; a new data-provider test pins each rejection and its order and passes against both the old and the new loop. The SQLite importer bound every cell through a named placeholder. Use positional placeholders with the same values and encoding. 50k x 201 SAV, SQLite: preflight 2.5s -> 0.9s, import 19-20s -> 12-13s. --- src/Sql/AbstractPdoSqlProfile.php | 57 +++++++++++++++++------------ src/Sql/SqliteWideTableImporter.php | 8 ++-- tests/Sql/PdoSqlProfileTest.php | 32 ++++++++++++++++ 3 files changed, 69 insertions(+), 28 deletions(-) diff --git a/src/Sql/AbstractPdoSqlProfile.php b/src/Sql/AbstractPdoSqlProfile.php index dd6ea9e..43d2d63 100644 --- a/src/Sql/AbstractPdoSqlProfile.php +++ b/src/Sql/AbstractPdoSqlProfile.php @@ -62,7 +62,7 @@ public function assertCanRepresent(int $sourceVariableCount): void /** * @param list> $variables - * @param list> $rows + * @param list> $rows Untrusted cells; this is the type gate. */ public function assertDataset(array $variables, array $rows, ?PDO $pdo = null): void { @@ -93,37 +93,38 @@ public function assertDataset(array $variables, array $rows, ?PDO $pdo = null): throw new UnsupportedOperation(DiagnosticCode::InvalidSourceDataset, 'Every source case must contain exactly one value per source variable.'); } $encodedRowBytes = 0; + // Branch on the value type first: a numeric cell costs two type + // tests and one finiteness test. Rules, order and messages match + // the previous per-cell checks for list rows. foreach ($row as $index => $value) { $kind = $storageKinds[$index]; - if ($kind === 'string' && !is_string($value)) { - throw new UnsupportedOperation( - DiagnosticCode::InvalidSourceDataset, - 'SPSS string values must be non-null strings.', - ); + if (is_string($value)) { + if ($kind === 'numeric') { + $this->invalidNumericValue(); + } + if (strlen($value) > $maximumValueBytes) { + $this->capabilityExceeded('encoded string value', strlen($value), $maximumValueBytes); + } + $encodedRowBytes += strlen($value); + continue; } - if ($kind === 'numeric' - && $value !== null - && !is_int($value) - && !is_float($value) - ) { + if ($kind === 'string') { throw new UnsupportedOperation( DiagnosticCode::InvalidSourceDataset, - 'SPSS numeric values must be binary64 numbers or system-missing NULL.', - ); - } - if ($kind === 'numeric' - && (is_int($value) || is_float($value)) - && !is_finite((float) $value) - ) { - throw new UnsupportedOperation( - DiagnosticCode::TargetCapabilityExceeded, - $this->driverName() . ' rejects non-finite SPSS numeric values before mutation.', + 'SPSS string values must be non-null strings.', ); } - if (is_string($value) && strlen($value) > $maximumValueBytes) { - $this->capabilityExceeded('encoded string value', strlen($value), $maximumValueBytes); + if (is_float($value)) { + if (!is_finite($value)) { + throw new UnsupportedOperation( + DiagnosticCode::TargetCapabilityExceeded, + $this->driverName() . ' rejects non-finite SPSS numeric values before mutation.', + ); + } + } elseif ($value !== null && !is_int($value)) { + $this->invalidNumericValue(); } - $encodedRowBytes += is_string($value) ? strlen($value) : 8; + $encodedRowBytes += 8; } if ($encodedRowBytes > $maximumEncodedCaseBytes) { $this->capabilityExceeded('encoded case payload', $encodedRowBytes, $maximumEncodedCaseBytes); @@ -167,6 +168,14 @@ protected function rowStorageBytes(string $kind, int $declaredWidth): int return $kind === 'string' ? $declaredWidth : 8; } + private function invalidNumericValue(): never + { + throw new UnsupportedOperation( + DiagnosticCode::InvalidSourceDataset, + 'SPSS numeric values must be binary64 numbers or system-missing NULL.', + ); + } + private function capabilityExceeded(string $limit, int $actual, int $maximum): never { throw new UnsupportedOperation( diff --git a/src/Sql/SqliteWideTableImporter.php b/src/Sql/SqliteWideTableImporter.php index 9dac1e8..32928b1 100644 --- a/src/Sql/SqliteWideTableImporter.php +++ b/src/Sql/SqliteWideTableImporter.php @@ -302,16 +302,16 @@ private function insertCases(string $tableName, array $variables, array $rows): { $columns = array_merge(['__case_ordinal'], array_column($variables, 'column')); $quoted = array_map(fn(string $column): string => $this->quote($column), $columns); - $params = array_map(static fn(string $column): string => ':' . $column, $columns); - $statement = $this->pdo->prepare('INSERT INTO ' . $this->quote($tableName) . ' (' . implode(', ', $quoted) . ') VALUES (' . implode(', ', $params) . ')'); + // Positional placeholders: named ones cost a name lookup per bound cell. + $statement = $this->pdo->prepare('INSERT INTO ' . $this->quote($tableName) . ' (' . implode(', ', $quoted) . ') VALUES (' . implode(', ', array_fill(0, count($columns), '?')) . ')'); foreach ($rows as $caseOrdinal => $row) { - $values = ['__case_ordinal' => $caseOrdinal + 1]; + $values = [$caseOrdinal + 1]; foreach ($variables as $position => $variable) { $value = is_array($row) ? ($row[$variable['source']] ?? $row[$position] ?? null) : null; if ($value === null && $variable['kind'] === 'string') { throw new UnsupportedOperation(DiagnosticCode::InvalidSourceDataset, 'SPSS string values must not be represented as NULL.'); } - $values[$variable['column']] = is_float($value) ? Binary64::encode($value) : $value; + $values[] = is_float($value) ? Binary64::encode($value) : $value; } $statement->execute($values); } diff --git a/tests/Sql/PdoSqlProfileTest.php b/tests/Sql/PdoSqlProfileTest.php index 02cdebb..1c42126 100644 --- a/tests/Sql/PdoSqlProfileTest.php +++ b/tests/Sql/PdoSqlProfileTest.php @@ -14,6 +14,7 @@ use OpenStatSpec\Sql\OperationJournal; use PDO; use PDOStatement; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; final class PdoSqlProfileTest extends TestCase @@ -122,6 +123,37 @@ public function testMySqlPreflightRejectsCombinedCasePayloadBeyondActivePacket() ); } + /** @return iterable>, DiagnosticCode, string}> */ + public static function invalidCases(): iterable + { + yield 'bool in numeric column' => [[[true, 'a']], DiagnosticCode::InvalidSourceDataset, 'binary64 numbers or system-missing NULL']; + yield 'string in numeric column' => [[['1', 'a']], DiagnosticCode::InvalidSourceDataset, 'binary64 numbers or system-missing NULL']; + yield 'infinity' => [[[INF, 'a']], DiagnosticCode::TargetCapabilityExceeded, 'rejects non-finite']; + yield 'nan' => [[[NAN, 'a']], DiagnosticCode::TargetCapabilityExceeded, 'rejects non-finite']; + yield 'null string' => [[[1.0, null]], DiagnosticCode::InvalidSourceDataset, 'non-null strings']; + yield 'int in string column' => [[[1.0, 5]], DiagnosticCode::InvalidSourceDataset, 'non-null strings']; + yield 'case payload' => [[[1.0, str_repeat('a', 65_528)]], DiagnosticCode::TargetCapabilityExceeded, 'encoded case payload is 65536 bytes']; + yield 'first cell wins within a case' => [[[true, null]], DiagnosticCode::InvalidSourceDataset, 'binary64 numbers']; + yield 'first case wins' => [[[1.0, null], [INF, 'a']], DiagnosticCode::InvalidSourceDataset, 'non-null strings']; + yield 'cell rule before payload' => [[[NAN, str_repeat('a', 65_528)]], DiagnosticCode::TargetCapabilityExceeded, 'rejects non-finite']; + } + + /** @param list> $rows */ + #[DataProvider('invalidCases')] + public function testPreflightRejectsInvalidCellsInSourceOrder(array $rows, DiagnosticCode $code, string $message): void + { + $variables = [['name' => 'n', 'type' => 'numeric'], ['name' => 's', 'type' => 'string', 'width' => 8]]; + (new MySqlProfile())->assertDataset($variables, [[null, ''], [1, 'a'], [-1.5, str_repeat('b', 65_527)]]); + + try { + (new MySqlProfile())->assertDataset($variables, $rows); + self::fail('Preflight accepted an invalid case.'); + } catch (UnsupportedOperation $exception) { + self::assertSame($code, $exception->diagnosticCode); + self::assertStringContainsString($message, $exception->getMessage()); + } + } + public function testPostgreSqlMapsLongCollidingSourceNamesWithinIdentifierLimit(): void { $profile = new PostgreSqlProfile(); From aa2a31df5f12d3cee44ff53650562b2fa4fb8b04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Mon, 9 Nov 2026 13:04:08 +0200 Subject: [PATCH 2/3] docs: note faster import preflight in changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26fbdcc..c8cc3f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## [Unreleased] +### Changed + +- Make the shared import preflight cheaper per cell (same rules, order and + messages) and bind SQLite case rows with positional placeholders. A 50,000 + case by 201 variable SAV imports into SQLite in 12-13 s instead of 19-20 s. + ## [0.8.0] - 2026-09-10 This release consolidates the never-tagged, never-published 0.7.2 preparation From a0f4633e8288fa9c62328d345ce962c6f58cd5ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Mon, 9 Nov 2026 13:11:05 +0200 Subject: [PATCH 3/3] Prepare PHP 0.8.1 release metadata --- CHANGELOG.md | 5 ++++- README.md | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8cc3f2..a765ea3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## [Unreleased] +## [0.8.1] - 2026-11-09 + ### Changed - Make the shared import preflight cheaper per cell (same rules, order and @@ -185,7 +187,8 @@ ownership checks are retained; this release requires no new migration. - Pinned active conformance fixtures and capabilities to released OpenStatSpec specification v0.1.0 at commit `d287c2cde9ade71f04e27dd012caec876901aed5`. -[Unreleased]: https://github.com/OpenStatSpec/php/compare/v0.8.0...HEAD +[Unreleased]: https://github.com/OpenStatSpec/php/compare/v0.8.1...HEAD +[0.8.1]: https://github.com/OpenStatSpec/php/compare/v0.8.0...v0.8.1 [0.8.0]: https://github.com/OpenStatSpec/php/compare/v0.7.1...v0.8.0 [0.7.1]: https://github.com/OpenStatSpec/php/compare/v0.7.0...v0.7.1 [0.7.0]: https://github.com/OpenStatSpec/php/compare/v0.6.0...v0.7.0 diff --git a/README.md b/README.md index c21ca68..9c01f42 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ It imports an unencrypted SPSS `.sav` or `.zsav` dataset into a relational datab ## Status -PHP v0.8.0 targets released OpenStatSpec specification `v0.5.0` at immutable +PHP v0.8.1 targets released OpenStatSpec specification `v0.5.0` at immutable commit `864e84479f554b8ee250ffed44c4dfb963750d4a` (`specification_status: released`). Composer derives the package version from its Git tag; it is independent of the specification version.