Repository navigation
migrate-embedded-sqlite-to-pure-go-modernc - #786
Conversation
* docs(sqlfuncs): mirror CLAUDE.md as AGENTS.md for all agent sessions Co-authored-by: jeffreyaven <5146499+jeffreyaven@users.noreply.github.com> * deps: adopt any-sdk pure-Go sqlite backend, drop mattn/cgo fork replace Co-authored-by: jeffreyaven <5146499+jeffreyaven@users.noreply.github.com> * ci: retire cgo - CGO_ENABLED=0, drop C toolchains and sqlite_stackql tag, add per-artifact function smoke Co-authored-by: jeffreyaven <5146499+jeffreyaven@users.noreply.github.com> * docs+test: modernc backend invariants, release notes draft, robot ext-function smoke scenario Co-authored-by: jeffreyaven <5146499+jeffreyaven@users.noreply.github.com> * deps: bump any-sdk to alpha02 (aws_policy_equal tags parity, boolean decltype); pin sqlite_version fixture to 3.53.4 Co-authored-by: jeffreyaven <5146499+jeffreyaven@users.noreply.github.com> * burn: remove executed migration work order Co-authored-by: jeffreyaven <5146499+jeffreyaven@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jeffreyaven <5146499+jeffreyaven@users.noreply.github.com>
* retire remaining cgo configuration and stale migration artifacts Co-authored-by: jeffreyaven <5146499+jeffreyaven@users.noreply.github.com> * preserve migration smoke-test documentation Co-authored-by: jeffreyaven <5146499+jeffreyaven@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jeffreyaven <5146499+jeffreyaven@users.noreply.github.com>
Co-authored-by: jeffreyaven <5146499+jeffreyaven@users.noreply.github.com>
Co-authored-by: jeffreyaven <5146499+jeffreyaven@users.noreply.github.com>
There was a problem hiding this comment.
filename should not have spaces ideally please change to stackql_release_process.md
There was a problem hiding this comment.
Renamed to docs/stackql_release_process.md in 625b20a.
Also updated the three references to the old name so nothing is left dangling: the header comments in mcp-packaging.yml and mcp-registry-publish.yml, and packaging/mcpb/CLAUDE.md.
|
|
||
| build\stackql.exe --help | ||
|
|
||
| - name: Extension function smoke test |
There was a problem hiding this comment.
Move this to robot
There was a problem hiding this comment.
Done in 27f1c4d. The step is removed; the check is the robot scenario Sqlite Extension Functions Smoke Working in stackql_mocked_from_cmd_line.robot, which this job already runs via build.py --robot-test.
The scenario previously used a weaker aws_policy_equal comparison than this step did, so I moved the step's arguments (Version plus scalar-vs-array Resource) into the scenario so nothing is lost in the move. I ran the scenario's exact SQL against a CGO_ENABLED=0 build of this branch and the output matches the expected table.
| Write-Host $execOut | ||
| if ($execOut -notmatch 'StackQL') { throw "exec did not return 'StackQL'" } | ||
|
|
||
| # Extension function smoke: proves the pure Go sqlite function registration |
There was a problem hiding this comment.
move this to robot
There was a problem hiding this comment.
Removed in 27f1c4d, with one thing to flag.
This job has no robot harness: no checkout and no Python, it only downloads the amd64 exe from winbuild and runs it under x64 emulation. So after this change the extension functions are no longer exercised under emulation. The same binary is covered by the robot scenario in winbuild.
If you want emulation coverage kept, the options are wiring a minimal robot run into this job, or keeping a workflow-level check in this job only. Let me know which you prefer and I will add it.
I also dropped the stale "cgo" wording from the comment just above, since this PR makes it incorrect.
| run: | | ||
| python cicd/python/build.py --verbose --build-mcp-client | ||
|
|
||
| - name: Extension function smoke test |
There was a problem hiding this comment.
move this to robot
There was a problem hiding this comment.
Removed in 27f1c4d. linuxbuild does not run robot itself, but linuxtest needs it and runs the functional suite against this job's artifact, so the robot scenario Sqlite Extension Functions Smoke Working covers the linux amd64 binary there.
| run: | | ||
| python cicd/python/build.py --verbose --build-mcp-client | ||
|
|
||
| - name: Extension function smoke test |
There was a problem hiding this comment.
move this to robot
There was a problem hiding this comment.
Removed in 27f1c4d. This job runs the functional suite in-job, so the robot scenario Sqlite Extension Functions Smoke Working covers the native linux arm64 binary.
| python cicd/python/build.py --verbose --build | ||
| python cicd/python/build.py --verbose --build-mcp-client | ||
|
|
||
| - name: Extension function smoke test |
There was a problem hiding this comment.
move this to robot
There was a problem hiding this comment.
Removed in 27f1c4d. This job runs the functional suite in-job, so the robot scenario Sqlite Extension Functions Smoke Working covers the darwin intel binary.
| python cicd/python/build.py --verbose --build | ||
| python cicd/python/build.py --verbose --build-mcp-client | ||
|
|
||
| - name: Extension function smoke test |
There was a problem hiding this comment.
move this to robot
There was a problem hiding this comment.
Removed in 27f1c4d, with the same flag as on the winarmsmoke thread.
This job builds and uploads the darwin arm64 binary but does not run robot, so after this change the extension functions are not exercised on darwin arm64. macOS coverage comes from macosbuild (intel).
Adding a full robot run here would take this job from roughly 3 minutes to something like the 30 minutes of the intel job, so I have not done that unprompted. Let me know if you want it.
| db_backend: | ||
| - sqlite | ||
| - postgres_tcp | ||
| include: |
There was a problem hiding this comment.
This requires an assertion, at the conclusion of all shards, that every single test is run on each db and platform. Failure to verify both the total count and the presence of any expected line item must fail
There was a problem hiding this comment.
Added in c645a2b as a new dockertestverify job. It needs dockertest, so it runs once every shard has concluded. dockermerge now needs it as well, so an image cannot be pushed past a failed verification.
How it works:
- Every robot-running
dockertestleg uploads itsoutput.xml. The unsharded sqlite leg uploads as shard 1 of 1, so both db backends go through the same check. - The verifier lives in
robot_shard.pynext to the shard modifier. It parsestest/robot/functionalwith no modifier applied to get the expected tests, then compares that as a multiset against the union of the shard reports for each (platform, db) group. The total count and each line item are checked independently. - Failure to verify fails the job: an absent shard, an absent
output.xml, inconsistent shard counts, aNOT RUNtest, an expected group with no reports, or reports from a group that is not in the expected list. That last one means a new robot-running leg cannot be added without also being verified.
Testing: run locally against real output.xml files produced by dry-run shards (146 + 159 + 170 + 176 = 651, matching the unsharded 651). 13 mutated cases all exit 1. One of them renames a test so the count is still 651 but a line item is wrong, which shows the two checks are independent. actionlint 1.6.23 with shellcheck passes.
Scoping notes:
- The
linux/arm64docker leg is not an expected group because it has never run the robot suite. Thematrix.platform == 'linux/amd64'condition is on main and predates this PR. Native linux arm64 robot coverage is inlinuxarmbuild. - Only the docker postgres leg is sharded. The other platforms run the suite unsharded through
build.py --robot-testwith no modifier, so there is nothing to reassemble there. - The job skips verification under the same conditions where
dockertestskips robot (fork and Dependabot PRs,build-eliderefs, express tags). That condition is duplicated rather than shared. If the two drift, this job expects reports that do not exist and fails, which is the safe direction.
This has not run in CI yet. The artifact upload and download directory layout is the one part I could not exercise locally, so the first run on this push is the real test of it.
Rename `docs/stackql release process.md` to `docs/stackql_release_process.md` per review, and update the three references to it (two workflow header comments and the mcpb packaging notes) so nothing points at the old name. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Remove the six workflow-level extension function smoke steps from build.yml per review. The check lives in the robot scenario `Sqlite Extension Functions Smoke Working`, which now carries the stronger aws_policy_equal comparison the workflow steps used (Version plus scalar-vs-array Resource), so nothing is lost in the move. The scenario runs wherever the functional suite runs: winbuild, linuxtest, linuxarmbuild, macosbuild, wsltest and the docker sqlite leg. winarmsmoke and macosarmbuild have no robot harness, so the check is no longer exercised in those two jobs. Also drop the stale "cgo" wording from the winarmsmoke comment. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sharding splits one robot run across several dockertest legs, so no single leg can attest that the whole suite ran. Add a dockertestverify job that runs once all legs conclude and fails unless, for each (platform, db backend) group, the shards together account for every functional test exactly once. - Each robot-running dockertest leg uploads its output.xml; unsharded legs upload as shard 1 of 1. - robot_shard.py gains a verifier. The expected tests are parsed from test/robot/functional with no shard modifier applied, then compared as a multiset against the union of the shard reports, so the total count and each individual test are checked independently. - Anything that prevents verification fails it: absent shard, absent output.xml, inconsistent shard counts, a NOT RUN test, an expected group with no reports, or reports from a group that is not listed. - dockermerge now needs dockertestverify, so an image cannot be pushed past a failed verification. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Per review: now that GitHub hosts arm64 macOS and Windows runners, run the robot functional suite on both rather than leaving the extension function check unexercised there. - macosarmbuild: mirror the macosbuild robot harness (libpq, python deps, registry rewrite, certificates, robot run, report upload). The libpq path is resolved with `brew --prefix` because homebrew's prefix differs on Apple Silicon. - winarmsmoke: mirror the winbuild robot harness against the amd64 binary under x64 emulation. Python is x64 because psycopg2-binary, psycopg-binary and cryptography publish no win_arm64 wheels. The job now checks out the repo (before the artifact downloads, since checkout cleans the workspace) and moves from the short to the standard job timeout. - winbuild: upload the MCP client as an artifact so the MCP scenarios run in winarmsmoke rather than being skipped. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Description
Migrates the embedded SQLite backend from cgo to pure Go. The embedded
relational engine now uses
modernc.org/sqlitev1.59.0 (SQLite 3.53.4),consumed via any-sdk
v0.5.6-alpha01-pure-go-sqlite-migration->v0.5.6-alpha02-pure-go-sqlite-migration, which owns the driver seam(
public/sqlengine, driver namestackql-sqlite) and the six StackQLextension functions ported to pure Go (
public/sqlfuncs:split_part,regexp_like,regexp_substr,regexp_replace,json_equal,aws_policy_equal). All builds areCGO_ENABLED=0; thestackql-go-sqlite3cgo fork, itsgo.modreplace directive, thesqlite_stackqlbuild tag, and all C toolchain CI steps are removed.History. The original work order halted on a stop condition (the driver
seam lives in any-sdk, not this repo - see the first revision of this PR
description). The migration was executed upstream (any-sdk PR #144, defect
fixes in PR #145), then resumed here against the released alpha tags. The
robot suite surfaced three real behavior differences on first run; all
three were root-caused and resolved:
aws_policy_equalmissing two fork-only behaviors (Tags unordered; top-level array-vs-array unordered) - the cgo fork had drifted from sqlite-ext-functions, the port's sourcebooleancolumns rendered0/1(modernc lacks mattn's decltype bool conversion)sqlite_version()3.45.1 -> 3.53.4Result: robot suite 651/651 PASS on
v0.5.6-alpha02-pure-go-sqlite-migration, zero tests skipped, weakened, ordeleted.
User-visible behavior changes (documented in any-sdk
public/sqlfuncs/DIVERGENCES.md, to be reflected in release notes):regexp functions now use standard RE2 syntax (the retired tiny-regex-c
engine treated
(,),|as literals and silently failed on patternsover ~30 nodes - groups, alternation, and inverted classes now work);
JSON parsing is strict RFC 8259 (trailing content and lax number forms
rejected); external loadable SQLite extensions (
.load) are no longersupported; library embedders inherit the modernc dependency tree.
Follow-up before release: bump go.mod to any-sdk
v0.5.6-beta01onceany-sdk
feat/go-sqlite-migrationis promoted to main and tagged.Six commits in the three work-order groups:
8c36181,2f1cb75,e4d2d0a,9085b5b,f811bd5,6e87bbf(final commit deletes theexecuted work order per burn order).
Type of change
()|are now interpreted (previously literal under the retiredengine); strict JSON parsing rejects previously tolerated malformed
input;
.load-able extensions unsupported. All documented in any-sdkDIVERGENCES.md.
Issues referenced.
Resolves the unblocking request from this PR's original halt report via
any-sdk #144 and #145.
Evidence
module; the 5 previously-failing tests (findings A/B/C) all green.
startup pragma assertions, concurrency stress, and timestamp round-trip
all pass against alpha02 (upstream suites plus in-repo smoke).
go mod graph | grep "mattn/go-sqlite3"empty; repo-wide grep formattn/stackql-go-sqlite3/sqlite_stackql/SQLITE_ENABLE_STACKQLclean outside docs/changelog;internal/sqlfuncsremoved (superseded by any-sdk
public/sqlfuncs).CGO_ENABLED=0; C toolchain steps removed.v0.11.707-final-cgo(this repo, commit7013bd6)and any-sdk's final cgo release.
Performance - cgo baseline from the last 10 successful
build.ymlruns on
main(runs 673-707, 2026-09-09 -> 2026-09-20). Migrated columnspending maintainer approval of the gated workflow runs on this PR; red-flag
rule: >25% over median AND outside the baseline range on any platform.
Checklist:
failures as a result of these changes. (Local; cross-platform CI pending
run approval.)
testing.
gated CI runs on this PR.)
Variations
The only test fixture modified is the
sqlite_versionpin (finding C).The three declared-boolean fixtures are deliberately untouched: rendering
parity is preserved upstream (finding B).
Tech Debt
Retired in this change set: the stackql-go-sqlite3 fork dependency and its
replace directive, the sqlite-ext-functions C build coupling, the
sqlite_stackqlbuild tag, and the cgo cross-compile toolchain matrix.Remaining (tracked outside this PR): archive stackql-go-sqlite3 and
sqlite-ext-functions post-merge; bump to any-sdk
v0.5.6-beta01;omni-sdk adopts the same backend later.