[TonisOrmisson]

Follow-up to the Python finding that SAV import spent most of its time in per-cell validation. This checks whether the PHP adapter has the same problem and fixes the parts that measured as real.

What profiling showed (50,000 cases x 200 numeric + 1 string var, value labels, 10 MB SAV)

Phase (PHP, Xdebug off)	SQLite	Dolt 2.2.3
php-spss read + normalize	5-7 s	5-6 s
AbstractPdoSqlProfile::assertDataset preflight	2.5-3 s	2.7 s
case encoding (caseRows/caseValue)	n/a	3.7 s
PreparedCaseBatch size bookkeeping	n/a	0.6 s
inserts	~10 s, row by row with named placeholders	~42 s waiting on the Dolt server

The PHP adapter has the same pattern but a much smaller share than Python: the preflight was 11-13% of a SQLite import and about 4% of a Dolt import. MySQL and PostgreSQL already batch rows. SQLite bound each cell through a named placeholder, and that was the biggest PHP-side cost.

What changed

1. Preflight (AbstractPdoSqlProfile::assertDataset): the old loop ran up to eight namespaced is_*/is_finite calls per cell. These calls are resolved at runtime, so the compiler cannot turn them into type-check opcodes. The new loop branches on the value type first, so a numeric cell costs two type tests and one finiteness test. A microbenchmark on the same 10M cells: 2.46 s -> 0.93 s. The @param for rows now says list<array<mixed>>, because this method is the type gate for untrusted cells.
2. SQLite insertCases: switched from named to positional placeholders. The values, Binary64 encoding and the null-string guard are unchanged.

What was kept, and why

• One per-cell type and finiteness pass stays. In PHP it is a real trust boundary. The importers are public and take arrays. SpssEngine is pluggable. php-spss Dataset documents that "string nullability is validated by the SAV adapter". php-spss can also return non-finite doubles.
• All error codes and messages, and their order (row-major, left to right, cell rules before the case-payload limit), are unchanged for list rows. The new testPreflightRejectsInvalidCellsInSourceOrder data provider passes against both the old and the new loop. One difference: name-keyed rows (outside the documented list<list<...>> contract, and already raising "Undefined array key" warnings in preflight) now get numeric type and finiteness checks in preflight instead of later. They can only be rejected earlier, never accepted where they used to be rejected.
• The per-column limit rewrite was not done. The length and payload checks only touch string cells, and the type pass visits every cell anyway, so a per-column max would add a second pass instead of removing one.
• The caseValue re-checks in MySQL/PostgreSQL were kept. Removing them saves about 0.2 s. They are the only check for name-keyed rows.
• The PreparedCaseBatch row sizing was kept. It measures encoded wire bytes for grouping, which is not the preflight quantity. Computing it with builtins saved only 0.12 s.

Timings (same file, same machine, SpssAdapter::import)

Target	Before	After
SQLite, quiet machine	19.0 s, 20.4 s	11.6 s, 13.0 s
SQLite, under heavy shared load (alternating runs)	90.7 / 98.8 / 49.8 s	58.5 / 43.5 / 20.0 s
Dolt 2.2.3 (wall)	62.0 s	61.6 s (server-bound)
Dolt 2.2.3 (PHP CPU)	21.6 s	19.5 s
Preflight alone	2.5-3.1 s	0.9-1.2 s

Checks

• composer check on the staged tree (validate, lint, php-cs-fixer, phpstan level 8, phpunit): 769 tests OK, 112 skipped (server DSNs not set), with the specification pinned at 864e844.
• Dolt CI filter against dolthub/dolt-sql-server:2.2.3: 99 tests OK, 48 skipped.
• MySQL, MariaDB and PostgreSQL integration suites were not run locally (left to CI).

No version bump, tag or release.

Summary by CodeRabbit

• Performance
  • Large SAV imports can complete faster, with a reported 50,000-case import improving from about 19–20 seconds to 12–13 seconds.
• Changed
  • Import validation rules, check order, and error messages remain unchanged.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The change updates PDO dataset preflight cell validation, adds tests for invalid-cell diagnostics and ordering, and changes SQLite case inserts to positional placeholders. The changelog reports the associated SAV import timing comparison.

Changes: Import validation and insertion

Layer / File(s)	Summary
PDO dataset validation and diagnostics src/Sql/AbstractPdoSqlProfile.php, tests/Sql/PdoSqlProfileTest.php	Preflight validation checks cell runtime types before storage kind and uses the existing numeric-value diagnostic. Tests cover invalid cells, diagnostic precedence, and validation before payload-size checks.
SQLite case insertion and changelog src/Sql/SqliteWideTableImporter.php, CHANGELOG.md	SQLite case insertion uses positional placeholders and an ordered value list. The changelog reports the import-preflight and placeholder updates and a SAV import timing comparison.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Merge Risk: Minimal · up to aa2a3

No actionable issue is identified in the import-preflight or SQLite insertion changes; merge after normal checks.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title names the two main changes: faster import preflight and faster SQLite inserts. The changeset matches this. The preflight change branches on cell type in assertDataset, and the SQLite change …

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 3 files. (1 skipped: 1 unsupported.)

Full details: Title check

Explanation

The title names the two main changes: faster import preflight and faster SQLite inserts. The changeset matches this. The preflight change branches on cell type in assertDataset, and the SQLite change switches case inserts to positional placeholders. The version tag '(0.8.1)' is not a change in the changeset, since the objectives state no version bump is included. That detail is noise but does not make the title misleading.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 68511a0c-9f30-4faf-aabf-850bd3df7c9e

Commits

Reviewing files that changed from the base of the PR and between 6b8edb8 and aa2a31d.

Files selected for processing: 4

• CHANGELOG.md
• src/Sql/AbstractPdoSqlProfile.php
• src/Sql/SqliteWideTableImporter.php
• tests/Sql/PdoSqlProfileTest.php

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.