Versioning store changes#213
Conversation
There was a problem hiding this comment.
Pull request overview
Implements SQLite-backed object versioning primitives in support of Issue #201 by extending the schema and updating core object/multipart/lifecycle operations to read/write versioned rows (including delete markers) and expose version-aware listing and retrieval.
Changes:
- Add versioning state machine in the SQLite store (per-bucket versioning status, per-object versions with
version_id,seq,is_latest, and delete markers). - Update object and multipart code paths to be version-aware (Put/Get/Delete/Copy, object parts keyed by version, and version listing).
- Add/extend schema initialization + migration to support versioning and add targeted tests for versioning and precondition behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sia/persist/sqlite/versioning.go | Adds versioning helpers and write/delete state machine (new versions, delete markers, null version handling). |
| sia/persist/sqlite/store.go | Adjusts store wiring/comments around interface assertion while splitting versioning work. |
| sia/persist/sqlite/precondition_test.go | Adds regression test ensuring suspended-bucket delete checks preconditions against the current version. |
| sia/persist/sqlite/objects.go | Introduces version-aware object APIs (Put/Get/Delete/Copy), parts keyed by version, and adds ListObjectVersions implementation. |
| sia/persist/sqlite/objects_test.go | Updates existing tests and adds new test coverage for versioning lifecycle + version listing semantics. |
| sia/persist/sqlite/multipart.go | Makes CompleteMultipartUpload version-aware and writes parts under the created version. |
| sia/persist/sqlite/migrations.go | Adds migration to rebuild objects/object_parts for versioning columns and adds bucket versioning status. |
| sia/persist/sqlite/lifecycle.go | Updates object expiration to delete current versions using the versioning-aware delete logic. |
| sia/persist/sqlite/init.sql | Updates initial schema for versioned objects/object_parts and bucket versioning status. |
| sia/persist/sqlite/buckets.go | Adds Get/Put bucket versioning status and internal lookup helper. |
| sia/objects/objects.go | Extends stored object model with VersionID, Versioned flag, delete-marker flag, and ObjectForUpload.VersionID. |
| sia/objects.go | Adjusts bulk delete result construction in preparation for VersionID changing to *string. |
| s3/versioning.go | Adds shared versioning types (VersionRequest, ListObjectVersions pagination/result types, version formatting). |
| s3/objects.go | Updates multi-delete parsing to treat VersionId as optional and map wire "null" to internal empty string. |
| s3/messages.go | Changes ObjectID.VersionID in delete XML messages from string to *string (nil = unspecified). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
peterjan
left a comment
There was a problem hiding this comment.
Tough PR to go through but I think it's fine. I would repeat the benchmark results you did in the previous PR here in the PR description for documenting purposes. Especially since you were able to make it so there's minimal regression.
50f57bc to
46ca062
Compare
|
hard to approve considering the build is failing but the PR looks good to me 👍 |
|
Thank you |
ChrisSchinnerl
left a comment
There was a problem hiding this comment.
Haven't been able to go through all of it in detail but it looks promising so far.
Re: #201
Due to some interdependent things, it's difficult to get everything to build. But if you run the sqlite tests specifically:
They all pass.
Before:
After: