Skip to content

Commit ed9ef84

Browse files
TonisOrmissonTonisOrmisson
authored
Merge pull request #18 from OpenStatSpec/perf/column-preflight
Speed up import preflight and SQLite inserts (0.8.1)
2 parents 6b8edb8 + a0f4633 commit ed9ef84

5 files changed

Lines changed: 80 additions & 30 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,14 @@
22

33
## [Unreleased]
44

5+
## [0.8.1] - 2026-11-09
6+
7+
### Changed
8+
9+
- Make the shared import preflight cheaper per cell (same rules, order and
10+
messages) and bind SQLite case rows with positional placeholders. A 50,000
11+
case by 201 variable SAV imports into SQLite in 12-13 s instead of 19-20 s.
12+
513
## [0.8.0] - 2026-09-10
614

715
This release consolidates the never-tagged, never-published 0.7.2 preparation
@@ -179,7 +187,8 @@ ownership checks are retained; this release requires no new migration.
179187
- Pinned active conformance fixtures and capabilities to released OpenStatSpec
180188
specification v0.1.0 at commit `d287c2cde9ade71f04e27dd012caec876901aed5`.
181189

182-
[Unreleased]: https://github.com/OpenStatSpec/php/compare/v0.8.0...HEAD
190+
[Unreleased]: https://github.com/OpenStatSpec/php/compare/v0.8.1...HEAD
191+
[0.8.1]: https://github.com/OpenStatSpec/php/compare/v0.8.0...v0.8.1
183192
[0.8.0]: https://github.com/OpenStatSpec/php/compare/v0.7.1...v0.8.0
184193
[0.7.1]: https://github.com/OpenStatSpec/php/compare/v0.7.0...v0.7.1
185194
[0.7.0]: https://github.com/OpenStatSpec/php/compare/v0.6.0...v0.7.0

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ It imports an unencrypted SPSS `.sav` or `.zsav` dataset into a relational datab
66

77
## Status
88

9-
PHP v0.8.0 targets released OpenStatSpec specification `v0.5.0` at immutable
9+
PHP v0.8.1 targets released OpenStatSpec specification `v0.5.0` at immutable
1010
commit `864e84479f554b8ee250ffed44c4dfb963750d4a`
1111
(`specification_status: released`). Composer derives the package version from its
1212
Git tag; it is independent of the specification version.

‎src/Sql/AbstractPdoSqlProfile.php‎

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public function assertCanRepresent(int $sourceVariableCount): void
6262

6363
/**
6464
* @param list<array<string, mixed>> $variables
65-
* @param list<list<int|float|string|null>> $rows
65+
* @param list<array<mixed>> $rows Untrusted cells; this is the type gate.
6666
*/
6767
public function assertDataset(array $variables, array $rows, ?PDO $pdo = null): void
6868
{
@@ -93,37 +93,38 @@ public function assertDataset(array $variables, array $rows, ?PDO $pdo = null):
9393
throw new UnsupportedOperation(DiagnosticCode::InvalidSourceDataset, 'Every source case must contain exactly one value per source variable.');
9494
}
9595
$encodedRowBytes = 0;
96+
// Branch on the value type first: a numeric cell costs two type
97+
// tests and one finiteness test. Rules, order and messages match
98+
// the previous per-cell checks for list rows.
9699
foreach ($row as $index => $value) {
97100
$kind = $storageKinds[$index];
98-
if ($kind === 'string' && !is_string($value)) {
99-
throw new UnsupportedOperation(
100-
DiagnosticCode::InvalidSourceDataset,
101-
'SPSS string values must be non-null strings.',
102-
);
101+
if (is_string($value)) {
102+
if ($kind === 'numeric') {
103+
$this->invalidNumericValue();
104+
}
105+
if (strlen($value) > $maximumValueBytes) {
106+
$this->capabilityExceeded('encoded string value', strlen($value), $maximumValueBytes);
107+
}
108+
$encodedRowBytes += strlen($value);
109+
continue;
103110
}
104-
if ($kind === 'numeric'
105-
&& $value !== null
106-
&& !is_int($value)
107-
&& !is_float($value)
108-
) {
111+
if ($kind === 'string') {
109112
throw new UnsupportedOperation(
110113
DiagnosticCode::InvalidSourceDataset,
111-
'SPSS numeric values must be binary64 numbers or system-missing NULL.',
112-
);
113-
}
114-
if ($kind === 'numeric'
115-
&& (is_int($value) || is_float($value))
116-
&& !is_finite((float) $value)
117-
) {
118-
throw new UnsupportedOperation(
119-
DiagnosticCode::TargetCapabilityExceeded,
120-
$this->driverName() . ' rejects non-finite SPSS numeric values before mutation.',
114+
'SPSS string values must be non-null strings.',
121115
);
122116
}
123-
if (is_string($value) && strlen($value) > $maximumValueBytes) {
124-
$this->capabilityExceeded('encoded string value', strlen($value), $maximumValueBytes);
117+
if (is_float($value)) {
118+
if (!is_finite($value)) {
119+
throw new UnsupportedOperation(
120+
DiagnosticCode::TargetCapabilityExceeded,
121+
$this->driverName() . ' rejects non-finite SPSS numeric values before mutation.',
122+
);
123+
}
124+
} elseif ($value !== null && !is_int($value)) {
125+
$this->invalidNumericValue();
125126
}
126-
$encodedRowBytes += is_string($value) ? strlen($value) : 8;
127+
$encodedRowBytes += 8;
127128
}
128129
if ($encodedRowBytes > $maximumEncodedCaseBytes) {
129130
$this->capabilityExceeded('encoded case payload', $encodedRowBytes, $maximumEncodedCaseBytes);
@@ -167,6 +168,14 @@ protected function rowStorageBytes(string $kind, int $declaredWidth): int
167168
return $kind === 'string' ? $declaredWidth : 8;
168169
}
169170

171+
private function invalidNumericValue(): never
172+
{
173+
throw new UnsupportedOperation(
174+
DiagnosticCode::InvalidSourceDataset,
175+
'SPSS numeric values must be binary64 numbers or system-missing NULL.',
176+
);
177+
}
178+
170179
private function capabilityExceeded(string $limit, int $actual, int $maximum): never
171180
{
172181
throw new UnsupportedOperation(

‎src/Sql/SqliteWideTableImporter.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -302,16 +302,16 @@ private function insertCases(string $tableName, array $variables, array $rows):
302302
{
303303
$columns = array_merge(['__case_ordinal'], array_column($variables, 'column'));
304304
$quoted = array_map(fn(string $column): string => $this->quote($column), $columns);
305-
$params = array_map(static fn(string $column): string => ':' . $column, $columns);
306-
$statement = $this->pdo->prepare('INSERT INTO ' . $this->quote($tableName) . ' (' . implode(', ', $quoted) . ') VALUES (' . implode(', ', $params) . ')');
305+
// Positional placeholders: named ones cost a name lookup per bound cell.
306+
$statement = $this->pdo->prepare('INSERT INTO ' . $this->quote($tableName) . ' (' . implode(', ', $quoted) . ') VALUES (' . implode(', ', array_fill(0, count($columns), '?')) . ')');
307307
foreach ($rows as $caseOrdinal => $row) {
308-
$values = ['__case_ordinal' => $caseOrdinal + 1];
308+
$values = [$caseOrdinal + 1];
309309
foreach ($variables as $position => $variable) {
310310
$value = is_array($row) ? ($row[$variable['source']] ?? $row[$position] ?? null) : null;
311311
if ($value === null && $variable['kind'] === 'string') {
312312
throw new UnsupportedOperation(DiagnosticCode::InvalidSourceDataset, 'SPSS string values must not be represented as NULL.');
313313
}
314-
$values[$variable['column']] = is_float($value) ? Binary64::encode($value) : $value;
314+
$values[] = is_float($value) ? Binary64::encode($value) : $value;
315315
}
316316
$statement->execute($values);
317317
}

‎tests/Sql/PdoSqlProfileTest.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use OpenStatSpec\Sql\OperationJournal;
1515
use PDO;
1616
use PDOStatement;
17+
use PHPUnit\Framework\Attributes\DataProvider;
1718
use PHPUnit\Framework\TestCase;
1819

1920
final class PdoSqlProfileTest extends TestCase
@@ -122,6 +123,37 @@ public function testMySqlPreflightRejectsCombinedCasePayloadBeyondActivePacket()
122123
);
123124
}
124125

126+
/** @return iterable<string, array{list<list<mixed>>, DiagnosticCode, string}> */
127+
public static function invalidCases(): iterable
128+
{
129+
yield 'bool in numeric column' => [[[true, 'a']], DiagnosticCode::InvalidSourceDataset, 'binary64 numbers or system-missing NULL'];
130+
yield 'string in numeric column' => [[['1', 'a']], DiagnosticCode::InvalidSourceDataset, 'binary64 numbers or system-missing NULL'];
131+
yield 'infinity' => [[[INF, 'a']], DiagnosticCode::TargetCapabilityExceeded, 'rejects non-finite'];
132+
yield 'nan' => [[[NAN, 'a']], DiagnosticCode::TargetCapabilityExceeded, 'rejects non-finite'];
133+
yield 'null string' => [[[1.0, null]], DiagnosticCode::InvalidSourceDataset, 'non-null strings'];
134+
yield 'int in string column' => [[[1.0, 5]], DiagnosticCode::InvalidSourceDataset, 'non-null strings'];
135+
yield 'case payload' => [[[1.0, str_repeat('a', 65_528)]], DiagnosticCode::TargetCapabilityExceeded, 'encoded case payload is 65536 bytes'];
136+
yield 'first cell wins within a case' => [[[true, null]], DiagnosticCode::InvalidSourceDataset, 'binary64 numbers'];
137+
yield 'first case wins' => [[[1.0, null], [INF, 'a']], DiagnosticCode::InvalidSourceDataset, 'non-null strings'];
138+
yield 'cell rule before payload' => [[[NAN, str_repeat('a', 65_528)]], DiagnosticCode::TargetCapabilityExceeded, 'rejects non-finite'];
139+
}
140+
141+
/** @param list<list<mixed>> $rows */
142+
#[DataProvider('invalidCases')]
143+
public function testPreflightRejectsInvalidCellsInSourceOrder(array $rows, DiagnosticCode $code, string $message): void
144+
{
145+
$variables = [['name' => 'n', 'type' => 'numeric'], ['name' => 's', 'type' => 'string', 'width' => 8]];
146+
(new MySqlProfile())->assertDataset($variables, [[null, ''], [1, 'a'], [-1.5, str_repeat('b', 65_527)]]);
147+
148+
try {
149+
(new MySqlProfile())->assertDataset($variables, $rows);
150+
self::fail('Preflight accepted an invalid case.');
151+
} catch (UnsupportedOperation $exception) {
152+
self::assertSame($code, $exception->diagnosticCode);
153+
self::assertStringContainsString($message, $exception->getMessage());
154+
}
155+
}
156+
125157
public function testPostgreSqlMapsLongCollidingSourceNamesWithinIdentifierLimit(): void
126158
{
127159
$profile = new PostgreSqlProfile();

0 commit comments

Comments
 (0)