refactor(cache): slim FieldProjection to (cacheKey, fieldName) (PR-009g-ii) - #1049
Open
AnthonyMDev wants to merge 1 commit into
Open
Conversation
Removes columnShape and cardinality from FieldProjection, along with the ColumnShape/Cardinality enums and the OutputType classification machinery. Per the amended ADR 0007 decision: - Column projection is dropped, not deferred. SQLite is row-oriented: rows live whole in B-tree leaf pages (and the records table is WITHOUT ROWID, clustered by primary key), so narrowing the SELECT column list saves no IO — a NULL column costs ~1 header byte. The savings would be minor per-row CPU, while per-field column sets fragment the SQL statement shapes and complicate statement caching. - Position predicates are dropped. On well-formed data they read exactly the rows the row filter already reads (a field has one shape at a time). Their only effect was converting a declared-vs- stored shape mismatch — a non-backwards-compatible schema change shipped without clearing the cache — from a caller-visible JSONDecodingError.wrongType into a silent refetch. The error is the intended behavior (matching 2.x): the developer decides how to respond to a schema change that invalidated their cache. With no consumer for shape metadata, the slim (cacheKey, fieldName) pair is the entire 3.0 projection contract for custom NormalizedCache implementors, and Set<FieldProjection> now dedupes on exactly what backends read — removing the conflicting-duplicate-projections precondition hazard. If the profile-gated __typename SQL filter lands later, it needs type-condition metadata that arrives as an additive property, not a breaking change. Also removes the now-redundant ProjectionKey dedupe struct in ApolloSQLiteDatabase and rewrites stale doc comments that promised SQL-level projection from PR-009g. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 0 new, 4 changed, 0 removedBuild ID: f8aa7d26535ead4195702961 URL: https://www.apollographql.com/docs/deploy-preview/f8aa7d26535ead4195702961 ✅ AI Style Review — No Changes DetectedNo MDX files were changed in this pull request. Review Log: View detailed log
|
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
1 participant
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.
Goal
Remove the unconsumed
columnShape/cardinalitymetadata fromFieldProjection, making the slim(cacheKey, fieldName)pair the entire 3.0 projection contract.Design doc reference
__typenameSQL filter is deferred behind perf measurement.Position in execution plan
PR-009g-ii of the cache rewrite Phase 1 stack. See cache-rewrite-phase1-execution.md §8.
Stacks on
cache-rewrite/phase-1a-sqlite-projection(PR-009g, feat(sqlite): field-aware selectFields with column projection (PR-009g) #1023)Followups in this stack
selectFieldsscenarios (docs PR againstcache-rewrite/phase-1-plan)SQLiteNormalizedCacheswitches to field-aware path + drop-and-rebuild migrationFiles changed
apollo-ios/Sources/Apollo/Caching/FieldProjection.swift— slimmed to(cacheKey, fieldName);ColumnShape,Cardinality, and all OutputType classification machinery removedNormalizedCache.swift— conflicting-duplicate-projections precondition removed (no longer expressible); stale 1A.5 migration narration rewrittenCacheDependentKey.swift,ApolloStore.swift— stale shape/PR-009g references rewrittenapollo-ios/Sources/Apollo/Execution/FieldProjectionCollector.swift— constructs slim projections; over-fetch comment now cites the accepted-cost decisionapollo-ios/Sources/ApolloSQLite/ApolloSQLiteDatabase.swift— redundantProjectionKeydedupe struct removed;Set<FieldProjection>used directlySQLiteDatabase.swift—selectFieldsdoc updated (stored shape inferred frompositionvalues)Tests/ApolloTests/—FieldProjectionTestsrewritten for the slim surface; shape arguments removed fromLoadFieldsTests,SQLiteSelectFieldsTests,ProjectionLoaderTests,FieldProjectionCollectorTestsTests added
FieldProjectionTestsretains construction, equality, and hashing coverage for the slim type; all existing projection/loader/read-path behavior tests pass unchanged, demonstrating the shape metadata had no behavioral consumers.Acceptance criteria
columnShape/ColumnShape/cardinality/Cardinalityremain inapollo-ios/SourcesorTestsSet<FieldProjection>dedupes on(cacheKey, fieldName)— the conflicting-duplicates hazard is structurally goneJSONDecodingError.wrongType, matching 2.xVerification
tuist generatesucceeds: yesXcodeListNavigatorIssues severity:"error"returns zero: yesNotes for reviewer
Rationale for dropping (not deferring) column projection: SQLite is row-oriented and the records table is
WITHOUT ROWID(clustered whole-row storage), so column narrowing saves no IO — NULL columns cost ~1 header byte each. Position predicates read identical rows on well-formed data; their only effect was converting a declared-vs-stored shape mismatch into a silent refetch, where the intended semantics (per discussion) is the 2.x-matching caller-visible error. If the profile-gated__typenameSQL filter is later justified, it requires type-condition metadata that can be added toFieldProjectionas an additive, non-breaking property.🤖 Generated with Claude Code