endtoend: run the corpus through the analysis core with an opt-in context - #4548
Merged
Merged
Conversation
TestReplay's contexts could only mutate the config, so the coreanalyzer experiment could reach a case only through its own exec.json. Contexts now also name the experiments every case in them runs with, and a new "core" context runs the corpus through the analysis core. The two paths still disagree, so the context is opt-in: it runs only when SQLC_TEST_CORE is set. That gate is an environment variable rather than a test flag because the documented workflow runs the whole module, and a flag defined in one test binary fails every package that does not define it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0161e7oMkzNW9DZMUPtyQibH
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0161e7oMkzNW9DZMUPtyQibH
A set-returning function in FROM takes its arguments from the items before
it, so bindRangeFunction types the call while the scope is still being
assembled. The scope under construction was never installed on the analyzer,
so a.scope was nil and resolving one of those arguments dereferenced it:
FROM transactions,
jsonb_each(jsonb_extract_path(transactions.data, '...')) AS x
buildScope and relationScope now install the scope they are filling for as
long as they fill it, and put back the one they replaced on the way out.
Resolving against a nil scope also reports the column as unresolved rather
than crashing, so no other half-built statement can panic on the way to the
error it was going to report anyway.
Both queries now analyze to an ordinary error instead of taking down the
process, which is what the rest of the corpus needed: TestReplay's core
context runs to completion in one process rather than aborting partway.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0161e7oMkzNW9DZMUPtyQibH
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TestReplayruns the whole testdata corpus once per context:baseascommitted,
managed-dbagainst a live database. This adds a third,core,which generates every case through the analysis core, so the two paths can be
compared case by case rather than one case at a time.
The context
Contexts could only mutate
config.Config, so thecoreanalyzerexperimentcould reach a case only through its own
exec.json.textContextnow alsonames the experiments every case in it runs with:
A case's own
SQLCEXPERIMENTis appended to the context's, so a case can stillturn one back off with the
noprefix.The two paths still disagree, so the context is opt-in, and needs no database:
The gate is an environment variable rather than a test flag because the
documented workflow runs the whole module, and a flag defined in one test
binary fails every package that does not define it:
It also matches how the rest of this suite is configured —
POSTGRESQL_SERVER_URI,MYSQL_SERVER_URI,SQLC_DUMMY_VALUE.The panic it found
Two cases took the test binary down, which ended the run partway through and
took the rest of the corpus with it. A set-returning function in
FROMtakesits arguments from the items before it, so
bindRangeFunctiontypes the callwhile the scope is still being assembled — but the scope under construction was
never installed on the analyzer, so
a.scopewas nil and resolving one of thosearguments dereferenced it:
buildScopeandrelationScopenow install the scope they are filling for aslong as they fill it, and put back the one they replaced on the way out.
Resolving against a nil scope also reports the column as unresolved rather than
crashing, so no other half-built statement can panic on the way to the error it
was going to report anyway.
Both queries now analyze to an ordinary error, and the core context runs to
completion in a single process.
Worth a look on review: installing the partial scope also means a
FROMitemcan now resolve columns from the items before it, which is what
bindRangeFunction's comment always claimed it did. That is a smallpermissiveness increase for subqueries in
FROM— a non-LATERAL subquery cannow see an earlier
FROMitem. Nothing in the suite depends on the stricterbehavior, but it is a semantic change rather than a purely defensive one.
Where the paths differ today
838 cases run under
core, 311 pass, 527 fail. Ranked:*not expanded in the emitted SQLsqlc generateexits non-zerosql.Null*/ pgtype wrappers)interface{}models.gouintvsint)Star expansion is one behavior and 43% of the failures. Of the hard errors, 47
are
illegal character U+003F '?'— parameters get dropped, so the emitted Godoes not compile — then catalog
sql: no rows in result seton CTEs andschema-qualified relations, unsupported AST nodes (
*ast.NamedArgExpr,*ast.MultiAssignRef), andunknown columnonunnest.No goldens were regenerated and no divergence was papered over. The failures are
the report.
Testing
go test --tags=examples -timeout 20m ./...passes with PostgreSQL and MySQLlive — 31 packages, no failures. Covers
base,managed-db, and thelive-database example tests.
SQLC_TEST_CORE=1 go test ./internal/endtoend -run 'TestReplay/core'completesin one process with zero panics.
SQLC_TEST_CORE.🤖 Generated with Claude Code
https://claude.ai/code/session_0161e7oMkzNW9DZMUPtyQibH
Generated by Claude Code