fix!: validate SQL extraction by content - #175
Merged
Merged
Conversation
eitsupi
marked this pull request as ready for review
August 29, 2026 10:13
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.
Summary
Correctness
The cache now misses for same-timestamp, same-size content rewrites and when unrelated projects share a custom cache directory with the same relative SQL path. Macro and vars changes still invalidate extraction conservatively. Corrupt, legacy, or version-mismatched cache files continue to fail open to cold computation.
The cache format intentionally has no independent schema version. A package version change remains the release-level compatibility boundary.
API scope
dlin_core::parser::cache::ExtractionCacheis now explicitly crate-private. This is an intentional pre-1.0 cleanup rather than a compatibility regression to preserve: the cache originated as a binary-crate implementation detail and was moved unchanged whendlin-corewas split out. It has no documented or known downstream use, and its lifecycle depends on internal macro-prefix, vars, extraction, persistence, and dirty-state invariants. Supported callers should use the graph builder cache options rather than operate the extraction cache backend directly.No compatibility wrapper is added because doing so would preserve an accidental low-level API and constrain later cache redesign.
Scope
This PR does not persist inferred columns or add model-to-macro dependency tracking. A temporary-file plus rename save path was tested but omitted after it materially increased persistent-cold time; the existing best-effort direct JSON write remains.
Verification
cargo test --workspace --all-targets --all-featurescargo clippy --workspace --all-targets --all-features -- -D warningsBenchmark
Deterministic medium workload, 512 models, release binary, 5 runs and 1 warmup:
SQL, manifest, and column semantic probes passed for no-cache/cold/warm. Single SQL file, macro, and vars invalidation probes all passed. The SQL cache was 98,006 bytes, down from the 100,931-byte baseline.
Timing is informational: other scenarios were also slower than the earlier baseline in this run, so absolute cross-run differences are not used as a gate.