chore(data): resolve five data-plane hygiene items (#178) - #208
xiaocui-big wants to merge 1 commit into
Conversation
1. embeddings_text: add UNIQUE (file_id, chunk_index) constraint with integration test proving duplicate rejection. 2. memories: add partial indexes on source_file_id and created_by_user_id (both ON DELETE SET NULL) to avoid sequential scans on file/user delete. 3. memory_relations: align FK referential actions with memories CASCADE (workspace_id, source_id, target_id all ON DELETE CASCADE). 4. indexgeneration List: replace N+1 per-build generation query with a single batch query using build_id = ANY($1). 5. ListRelations: add keyset cursor reusing memory/list.go's encodeListCursor/decodeListCursor helpers, with filter hash. All schema changes ship as migration 0024 with Down blocks.
Blocking: migration 0024 is already owned by another open PRThis PR is not ready to merge, for a reason that has nothing to do with the quality of the five items it addresses. The five items themselves look well-chosen; item 1 in particular ("the write path already deletes by The problem is the filename. The collision
But migration 0024 is already taken by #194: Both branches are based on The full current picture
Note the last two rows: What I need from you before this can mergeThe ordering constraint is not mine to waive —
Separately, on the bodyThe Test plan is still all unchecked: Every one of these is exactly the right thing to check, which is why I would rather see them checked than removed. In particular the first one — "applies cleanly on a populated database (not only empty schema)" — is the check that would have caught the collision above, since it is the populated path where a duplicate version number actually announces itself. Items 4 and 6 are behaviour-preservation claims about a return-type change ( This repo's convention is a provenance ledger pinned to a specific commit SHA. Please add one at the rebased head rather than referencing earlier runs. |
|
Closing in favor of #195. After comparing with
#195 additionally includes a defensive deduplication step before adding the uniqueness constraint, which is a nice safety improvement. Since #195 is part of the coordinated migration sequence (#194 → #197 → #195) and already owns the 0026 slot, this PR is redundant. Deferring to #195. |
2 participants
Summary
0024_data_plane_hygiene.sqlcovers items 1–3 withDownblocks.Changes
embeddings_textuniqueness — addedUNIQUE (file_id, chunk_index)constraint. The write path (indexer.go) deletes byfile_idthen batch-inserts; this gives the invariant database-level teeth. Integration test proves duplicate(file_id, chunk_index)is rejected.memoriesFK indexes — added partial indexes onsource_file_idandcreated_by_user_id(bothON DELETE SET NULL) to avoid sequential scans on every file/user delete.memory_relationscascade alignment — changed all three FKs (workspace_id,source_id,target_id) from defaultNO ACTIONtoON DELETE CASCADE, matching thememoriescascade so a workspace delete cannot fail on orphan edges.indexgeneration.List— replaced per-buildlistGenerationscall with a singlelistGenerationsForBuildsquery usingbuild_id = ANY($1).ListRelations— addedCursorfield toListRelationsQuery, return type changed to*ListRelationsResultwithNextCursor. Reusesmemory/list.go'sencodeListCursor/decodeListCursorhelpers with a relation-specific filter hash.Test plan
TestEmbeddingsTextChunkUniquenesspasses — duplicate(file_id, chunk_index)is rejected, differentchunk_indexfor same file succeeds.EXPLAINonDELETE FROM files WHERE id = $1shows index scan onidx_memories_source_file_idinstead of sequential scan.indexgeneration.Listreturns identical results to before, with one query instead of N+1.ListRelationswith a cursor returns the next page correctly; cursor with changed filters is rejected.ListRelationscallers (API handler, integration tests) pass with the new*ListRelationsResultreturn type.