Skip to content

refactor(dynamodb): replace badger with sqlite; drop the dependency - #92

Merged
skyoo2003 merged 5 commits into
mainfrom
refactor/dynamodb-sqlite
Jul 17, 2026
Merged

refactor(dynamodb): replace badger with sqlite; drop the dependency#92
skyoo2003 merged 5 commits into
mainfrom
refactor/dynamodb-sqlite

Conversation

@skyoo2003

Copy link
Copy Markdown
Owner

Summary

Rewrite the DynamoDB store on the shared SQLite backend used by every other service, removing the sole dependency on dgraph-io/badger — and with it an entire transitive tree: ristretto, flatbuffers, klauspost/compress, go-humanize, opentelemetry (×4), go-logr, xxhash, protobuf, golang.org/x/sys.

go.mod direct deps go 5 → 4; indirect 14 → 5.

Changes

  • items / tables / ttl / tags now live in SQLite tables; table metadata is still cached in memory and loaded on open (same model as before).
  • GSI/LSI queries are projected on the fly from the base table — no denormalized index rows to keep in sync (the badger version leaked stale index entries on update; this avoids that class of bug entirely).
  • Dropped the dead badger-transaction surfaceExecTransaction, PutItemTxn, GetItemTxn, DeleteItemTxn, PutItemWithGSI all had 0 callers.
  • Public store API and all types (TableInfo, AttributeValue, Item, sentinel errors, NewDynamoStore(dir)) are unchanged, so the provider and tests are untouched.

Verification

  • go build ./...
  • go test ./...108 packages ok, 0 fail (incl. dynamodb store_test + provider_test driving CreateTable/PutItem/GetItem/DeleteItem/ListTables/Scan through the real HandleRequest) ✅
  • Live server round-trip: CreateTable → PutItem → GetItem returns the item; after a full restart on the same data dir, ListTables and GetItem still return the persisted data. A real dynamodb/dynamodb.db (SQLite + WAL) is created. ✅

Context

Final item of the over-engineering cleanup series (follows #91). This was the one genuine rewrite; #91 covered the pure deletions and de-duplication.

Rewrite the DynamoDB store on the shared SQLite backend used by every other
service, removing the sole dependency on dgraph-io/badger and its transitive
tree (ristretto, flatbuffers, klauspost/compress, go-humanize, otel, go-logr,
xxhash, protobuf).

- items/tables/ttl/tags now live in SQLite tables; table metadata is still
  cached in memory and loaded on open
- GSI/LSI queries are projected on the fly from the base table (no denormalized
  index rows to keep in sync)
- drop the dead badger-transaction surface (ExecTransaction, PutItemTxn,
  GetItemTxn, DeleteItemTxn, PutItemWithGSI — all had 0 callers)

Verified: go build ./..., go test ./... (108 ok, 0 fail), and a live server
round-trip (CreateTable/PutItem/GetItem) that survives a restart on the same
data dir.
@sourcery-ai

sourcery-ai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Refactors the DynamoDB store to use the shared SQLite backend instead of Badger, introduces concrete SQLite schemas and helper utilities, implements GSI/LSI projection maintenance and querying on top of SQL, and removes unused Badger-specific transaction APIs and the Badger dependency tree.

File-Level Changes

Change Details Files
Replace Badger-backed DynamoStore with a SQLite-backed implementation using the shared sqlite.Store, with explicit tables for items, table metadata, TTL, tags, and GSI projections.
  • Introduce migrations defining ddb_tables, ddb_items, ddb_ttl, ddb_tags, and ddb_gsi schemas, plus an index for GSI lookups.
  • Change DynamoStore to hold *sqlite.Store instead of *badger.DB and open dynamodb.db via sqlite.Open() in NewDynamoStore.
  • Reimplement loadTableMeta, CreateTable, DeleteTable, UpdateTable, and Close to operate via database/sql queries and transactions instead of Badger keys and iterators.
internal/services/dynamodb/store.go
Rework item CRUD and querying (Query, Scan, QueryGSI) to use SQL rows, add key derivation helpers, and maintain GSI projections transactionally to avoid stale index entries.
  • Add attributeStringValue and keyValues helpers to derive PK/SK strings from AttributeValue maps.
  • Implement PutItem and DeleteItem using SQL transactions that upsert/delete ddb_items rows and synchronize corresponding ddb_gsi projections via syncItemGSI.
  • Add syncItemGSI, indexHashKeyName, queryItems, and escapeLike to manage consistent secondary index rows and perform JSON-based SELECTs for Query, Scan, and QueryGSI.
internal/services/dynamodb/store.go
Convert TTL and tag storage from Badger key-value records to dedicated SQLite tables and centralize tag writes.
  • Implement PutTTLConfig and GetTTLConfig using ddb_ttl with INSERT OR REPLACE and SELECT, returning empty TTLConfig when unset.
  • Implement GetTags, PutTags, RemoveTags via ddb_tags SELECT/INSERT OR REPLACE, adding writeTags helper to marshal and persist tag maps.
  • Ensure all TTL and tag operations rely on JSON stored as TEXT columns, matching prior semantics.
internal/services/dynamodb/store.go
Remove unused Badger transaction-based APIs and the Badger dependency, updating module metadata accordingly.
  • Delete ExecTransaction, PutItemTxn, DeleteItemTxn, GetItemTxn, PutItemWithGSI, writeGSIEntry, and uniqueSuffix along with Badger-specific key prefix constants.
  • Drop github.com/dgraph-io/badger/v4 and its transitive dependencies from go.mod and go.sum, leaving only the shared SQLite stack.
  • Add minimal new indirect test dependencies (kr/pretty, rogpeppe/go-internal, gopkg.in/check.v1) in go.mod/go.sum for testify tooling.
internal/services/dynamodb/store.go
go.mod
go.sum
Add targeted tests for the new SQLite-backed GSI querying to validate projection sync on insert, update, delete, and unknown index behavior.
  • Introduce TestDynamoStore_QueryGSI to exercise CreateTable, PutItem, QueryGSI, and DeleteItem against a Users table with a ByEmail GSI.
  • Assert correct result counts for multiple items sharing an email, empty results for unknown index names, and index projection updates on key changes and deletions.
  • Use existing newTestStore helper so tests run against the real DynamoStore implementation and shared SQLite backend.
internal/services/dynamodb/store_test.go

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@github-actions github-actions Bot added dependencies Dependency updates services AWS service implementations labels Jul 17, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • The new QueryGSI implementation scans all items in the base table and filters in memory, which may be fine for small datasets but could become a bottleneck as data grows; consider either constraining its use or adding a more targeted SQL pattern for common index shapes.
  • When QueryGSI receives an unknown indexName it silently returns nil, nil; if this is unexpected for callers, consider returning a dedicated error or distinguishing between “no such index” and “no matching items” to avoid confusing behavior.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new `QueryGSI` implementation scans all items in the base table and filters in memory, which may be fine for small datasets but could become a bottleneck as data grows; consider either constraining its use or adding a more targeted SQL pattern for common index shapes.
- When `QueryGSI` receives an unknown `indexName` it silently returns `nil, nil`; if this is unexpected for callers, consider returning a dedicated error or distinguishing between “no such index” and “no matching items” to avoid confusing behavior.

## Individual Comments

### Comment 1
<location path="internal/services/dynamodb/store.go" line_range="248-251" />
<code_context>
-		return nil
-	}); err != nil {
-		return fmt.Errorf("scan items for deletion: %w", err)
+	if _, err := s.db.DB().Exec(`DELETE FROM ddb_items WHERE table_name = ?`, name); err != nil {
+		return fmt.Errorf("delete table items: %w", err)
 	}
-
-	// Delete metadata and all items in a single batch.
-	keysToDelete = append(keysToDelete, metaKey(name))
-
-	if err := s.db.Update(func(txn *badger.Txn) error {
-		for _, k := range keysToDelete {
-			if err := txn.Delete(k); err != nil {
-				return err
-			}
-		}
-		return nil
-	}); err != nil {
-		return fmt.Errorf("delete table records: %w", err)
+	if _, err := s.db.DB().Exec(`DELETE FROM ddb_tables WHERE name = ?`, name); err != nil {
+		return fmt.Errorf("delete table metadata: %w", err)
 	}
</code_context>
<issue_to_address>
**issue (bug_risk):** Table deletion is now split across two non-transactional statements, which can leave partial state if the second fails.

With Badger, metadata and items were deleted in one atomic batch; with SQLite, they’re now removed via two separate `Exec` calls on `ddb_items` then `ddb_tables`. If the second delete fails (or call order changes), you can end up with orphaned records. Please wrap both statements in a single transaction so the deletion is all-or-nothing.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread internal/services/dynamodb/store.go Outdated
@skyoo2003 skyoo2003 self-assigned this Jul 17, 2026
…offs

Address code review on the sqlite store:
- DeleteTable now removes items + metadata inside a single transaction, so a
  mid-way failure can't leave orphaned rows (was two separate Exec calls).
- QueryGSI: document the O(n) scan ceiling + upgrade path, and why an unknown
  index returns an empty result rather than an error (matches prior behavior;
  an error would surface as an opaque 500 — a ValidationException belongs in
  the provider).

Verified: go build ./..., go test ./... (108 ok, 0 fail).
Resolve the ponytail debt in QueryGSI: instead of scanning every item in the
table and filtering in memory, maintain a ddb_gsi projection (keyed by base-item
identity) that PutItem/DeleteItem keep in sync, and serve QueryGSI with an
indexed lookup on (table_name, index_name, gsi_pk).

- correct for all key types (stores the same attributeStringValue used on the
  query side), and re-synced per write so an update that changes/removes an
  index key can't leave stale rows (the bug the old badger projection had)
- GSIs are immutable after CreateTable in the provider, so no rebuild-on-update
  is needed
- add TestDynamoStore_QueryGSI (query, unknown-index, update re-sync, delete);
  QueryGSI previously had no store-level test

Verified: go vet, go test ./... (108 ok, 0 fail).
@github-actions github-actions Bot added the tests Test code and test infrastructure label Jul 17, 2026
@skyoo2003

Copy link
Copy Markdown
Owner Author

@sourcery-ai review

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 2 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="internal/services/dynamodb/store_test.go" line_range="177-186" />
<code_context>
+	got, err := store.QueryGSI("Users", "ByEmail", "a@x.com")
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for QueryGSI error paths and index variants (missing table, LSIs, sort-keyed GSIs)

The new store logic adds several QueryGSI edge cases that aren’t covered by tests:

- Querying a non-existent table should return ErrTableNotFound; add a test that calls QueryGSI with an unknown table and asserts that error.
- syncItemGSI now handles both GSIs and LSIs; add a test using a table with an LSI and assert QueryGSI returns the expected items.
- IndexDef.KeySchema can include a RANGE key; add a test for an index with HASH + RANGE keys where items vary only by RANGE, and verify the projected items are written and read correctly.

These tests will help ensure the new SQLite-backed index path behaves correctly after the Badger-specific code removal.

Suggested implementation:

```golang
		GlobalSecondaryIndexes: []IndexDef{{
			IndexName: "ByEmail",
			KeySchema: []KeyDef{{Name: "Email", Type: "S", KeyType: "HASH"}},
		}},
	}))

	// Simple HASH-only GSI returns all matching items.
	require.NoError(t, store.PutItem("Users", Item{"UserID": {S: strPtr("u1")}, "Email": {S: strPtr("a@x.com")}}))
	require.NoError(t, store.PutItem("Users", Item{"UserID": {S: strPtr("u2")}, "Email": {S: strPtr("a@x.com")}}))
	require.NoError(t, store.PutItem("Users", Item{"UserID": {S: strPtr("u3")}, "Email": {S: strPtr("b@x.com")}}))

	got, err := store.QueryGSI("Users", "ByEmail", "a@x.com")
	require.NoError(t, err)
	assert.Len(t, got, 2)

	// Unknown index → empty result, not an error.
	none, err := store.QueryGSI("Users", "Nope", "a@x.com")
	require.NoError(t, err)
	assert.Empty(t, none)

	// Updating an item's index key re-syncs its projection (no stale rows).
	require.NoError(t, store.PutItem("Users", Item{"UserID": {S: strPtr("u1")}, "Email": {S: strPtr("b@x.com")}}))

	// Querying a non-existent table should return ErrTableNotFound.
	t.Run("missing table returns ErrTableNotFound", func(t *testing.T) {
		_, err := store.QueryGSI("NoSuchTable", "ByEmail", "a@x.com")
		assert.ErrorIs(t, err, ErrTableNotFound)
	})

	// syncItemGSI supports LSIs as well as GSIs – define a table with an LSI and
	// ensure QueryGSI returns the expected items.
	t.Run("LSI is queryable via QueryGSI", func(t *testing.T) {
		require.NoError(t, store.CreateTable(TableDef{
			TableName: "Orders",
			KeySchema: []KeyDef{
				{Name: "OrderID", Type: "S", KeyType: "HASH"},
			},
			LocalSecondaryIndexes: []IndexDef{{
				IndexName: "ByStatus",
				KeySchema: []KeyDef{{Name: "Status", Type: "S", KeyType: "HASH"}},
			}},
		}))

		require.NoError(t, store.PutItem("Orders", Item{
			"OrderID": {S: strPtr("o1")},
			"Status":  {S: strPtr("pending")},
		}))
		require.NoError(t, store.PutItem("Orders", Item{
			"OrderID": {S: strPtr("o2")},
			"Status":  {S: strPtr("pending")},
		}))
		require.NoError(t, store.PutItem("Orders", Item{
			"OrderID": {S: strPtr("o3")},
			"Status":  {S: strPtr("shipped")},
		}))

		orders, err := store.QueryGSI("Orders", "ByStatus", "pending")
		require.NoError(t, err)
		assert.Len(t, orders, 2)
	})

	// IndexDef.KeySchema can include a RANGE key; verify a HASH+RANGE GSI indexes
	// and projects items correctly.
	t.Run("GSI with HASH and RANGE keys", func(t *testing.T) {
		require.NoError(t, store.CreateTable(TableDef{
			TableName: "Messages",
			KeySchema: []KeyDef{
				{Name: "MessageID", Type: "S", KeyType: "HASH"},
			},
			GlobalSecondaryIndexes: []IndexDef{{
				IndexName: "ByUserAndCreatedAt",
				KeySchema: []KeyDef{
					{Name: "UserID", Type: "S", KeyType: "HASH"},
					{Name: "CreatedAt", Type: "N", KeyType: "RANGE"},
				},
			}},
		}))

		require.NoError(t, store.PutItem("Messages", Item{
			"MessageID": {S: strPtr("m1")},
			"UserID":    {S: strPtr("u1")},
			"CreatedAt": {N: int64Ptr(1)},
			"Body":      {S: strPtr("first")},
		}))
		require.NoError(t, store.PutItem("Messages", Item{
			"MessageID": {S: strPtr("m2")},
			"UserID":    {S: strPtr("u1")},
			"CreatedAt": {N: int64Ptr(2)},
			"Body":      {S: strPtr("second")},
		}))
		require.NoError(t, store.PutItem("Messages", Item{
			"MessageID": {S: strPtr("m3")},
			"UserID":    {S: strPtr("u2")},
			"CreatedAt": {N: int64Ptr(3)},
			"Body":      {S: strPtr("third")},
		}))

		msgs, err := store.QueryGSI("Messages", "ByUserAndCreatedAt", "u1")
		require.NoError(t, err)
		assert.Len(t, msgs, 2)

		// Verify that the projected attributes from the GSI path include the
		// primary key and non-key attributes.
		var ids []string
		for _, it := range msgs {
			if v := it["MessageID"].S; v != nil {
				ids = append(ids, *v)
			}
			// CreatedAt and Body should also be present on the projected rows.
			assert.NotNil(t, it["CreatedAt"].N)
			assert.NotNil(t, it["Body"].S)
		}
		assert.ElementsMatch(t, []string{"m1", "m2"}, ids)
	})

```

- Ensure `ErrTableNotFound`, `TableDef`, `KeyDef`, `IndexDef`, `LocalSecondaryIndexes`, `GlobalSecondaryIndexes`, `Item`, `int64Ptr`, and `CreateTable` already exist and are imported/accessible in this test file. If the package name used in tests is different (e.g. `dynamodb_test`), you may need to prefix `ErrTableNotFound` with the package name.
- If your `QueryGSI` API expects both HASH and RANGE key values (e.g. via an options struct or additional parameters), adapt the LSI and HASH+RANGE tests to pass the appropriate arguments instead of a single `"pending"` / `"u1"` string.
- If your index modeling for LSIs requires the HASH key to match the table's primary HASH, adjust the `Orders` table definition so the LSI `KeySchema` matches your constraints while still covering the LSI path in `syncItemGSI`.
</issue_to_address>

### Comment 2
<location path="internal/services/dynamodb/store_test.go" line_range="164-173" />
<code_context>
+func TestDynamoStore_QueryGSI(t *testing.T) {
+	store := newTestStore(t)
+
+	require.NoError(t, store.CreateTable(TableInfo{
+		Name:         "Users",
+		PartitionKey: KeyDef{Name: "UserID", Type: "S"},
+		GlobalSecondaryIndexes: []IndexDef{{
+			IndexName: "ByEmail",
+			KeySchema: []KeyDef{{Name: "Email", Type: "S", KeyType: "HASH"}},
+		}},
+	}))
+
+	require.NoError(t, store.PutItem("Users", Item{"UserID": {S: strPtr("u1")}, "Email": {S: strPtr("a@x.com")}}))
+	require.NoError(t, store.PutItem("Users", Item{"UserID": {S: strPtr("u2")}, "Email": {S: strPtr("a@x.com")}}))
+	require.NoError(t, store.PutItem("Users", Item{"UserID": {S: strPtr("u3")}, "Email": {S: strPtr("b@x.com")}}))
+
+	got, err := store.QueryGSI("Users", "ByEmail", "a@x.com")
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for keyValues / AttributeValue edge cases used by QueryGSI and the new SQLite paths

With the refactor, keyValues and attributeStringValue now sit on the hot path for PutItem, GetItem, DeleteItem, Query, and QueryGSI. The new GSI test exercises only the normal string-key case; it would be good to add targeted coverage for:

- Missing keys: PutItem/GetItem/DeleteItem/QueryGSI with items that omit the partition or sort key, asserting the expected “missing partition/sort key attribute” errors.
- Non-string key types: primary and GSI hash keys using N and B AttributeValue, to ensure those cases remain supported.
- Query sortKeyPrefix with special characters (`%`, `_`, `\`): insert items with such sort keys and query with literal prefixes to validate escapeLike behavior.

A small table-driven test around these helpers (using the test store) would give stronger confidence in the new SQLite-backed implementation.

Suggested implementation:

```golang
	// Updating an item's index key re-syncs its projection (no stale rows).
	require.NoError(t, store.PutItem("Users", Item{"UserID": {S: strPtr("u1")}, "Email": {S: strPtr("b@x.com")}}))

	// Querying again reflects the updated index key.
	got, err = store.QueryGSI("Users", "ByEmail", "a@x.com")
	require.NoError(t, err)
	assert.Len(t, got, 1)
	assert.Equal(t, strPtr("u2"), got[0]["UserID"].S)

	got, err = store.QueryGSI("Users", "ByEmail", "b@x.com")
	require.NoError(t, err)
	assert.Len(t, got, 2)

	// Deleting an item removes it from the index projection.
	require.NoError(t, store.DeleteItem("Users", Item{
		"UserID": {S: strPtr("u2")},
	}))
	got, err = store.QueryGSI("Users", "ByEmail", "b@x.com")
	require.NoError(t, err)
	assert.Len(t, got, 1)
	assert.Equal(t, strPtr("u1"), got[0]["UserID"].S)
}

func TestDynamoStore_KeyValuesMissingKeys(t *testing.T) {
	store := newTestStore(t)

	require.NoError(t, store.CreateTable(TableInfo{
		Name:         "Orders",
		PartitionKey: KeyDef{Name: "OrderID", Type: "S"},
		SortKey:      &KeyDef{Name: "LineID", Type: "S"},
	}))

	t.Run("PutItem missing partition key", func(t *testing.T) {
		err := store.PutItem("Orders", Item{
			"LineID": {S: strPtr("L1")},
		})
		require.Error(t, err)
		assert.Contains(t, err.Error(), "missing partition key attribute")

		// Make sure nothing was written.
		_, getErr := store.GetItem("Orders", Item{
			"OrderID": {S: strPtr("O1")},
			"LineID":  {S: strPtr("L1")},
		})
		require.Error(t, getErr)
	})

	t.Run("PutItem missing sort key", func(t *testing.T) {
		err := store.PutItem("Orders", Item{
			"OrderID": {S: strPtr("O2")},
		})
		require.Error(t, err)
		assert.Contains(t, err.Error(), "missing sort key attribute")

		_, getErr := store.GetItem("Orders", Item{
			"OrderID": {S: strPtr("O2")},
			"LineID":  {S: strPtr("L2")},
		})
		require.Error(t, getErr)
	})

	t.Run("GetItem missing partition key", func(t *testing.T) {
		// Insert a valid item first.
		require.NoError(t, store.PutItem("Orders", Item{
			"OrderID": {S: strPtr("O3")},
			"LineID":  {S: strPtr("L3")},
		}))

		_, err := store.GetItem("Orders", Item{
			"LineID": {S: strPtr("L3")},
		})
		require.Error(t, err)
		assert.Contains(t, err.Error(), "missing partition key attribute")
	})

	t.Run("DeleteItem missing sort key", func(t *testing.T) {
		// Insert a valid item first.
		require.NoError(t, store.PutItem("Orders", Item{
			"OrderID": {S: strPtr("O4")},
			"LineID":  {S: strPtr("L4")},
		}))

		err := store.DeleteItem("Orders", Item{
			"OrderID": {S: strPtr("O4")},
		})
		require.Error(t, err)
		assert.Contains(t, err.Error(), "missing sort key attribute")
	})
}

func TestDynamoStore_NonStringKeyTypes(t *testing.T) {
	store := newTestStore(t)

	t.Run("numeric partition key", func(t *testing.T) {
		require.NoError(t, store.CreateTable(TableInfo{
			Name:         "Accounts",
			PartitionKey: KeyDef{Name: "AccountID", Type: "N"},
		}))

		require.NoError(t, store.PutItem("Accounts", Item{
			"AccountID": {N: strPtr("1001")},
			"Name":      {S: strPtr("Primary")},
		}))
		require.NoError(t, store.PutItem("Accounts", Item{
			"AccountID": {N: strPtr("1002")},
			"Name":      {S: strPtr("Secondary")},
		}))

		item, err := store.GetItem("Accounts", Item{
			"AccountID": {N: strPtr("1001")},
		})
		require.NoError(t, err)
		assert.Equal(t, strPtr("Primary"), item["Name"].S)

		items, err := store.Query("Accounts", Item{
			"AccountID": {N: strPtr("1002")},
		})
		require.NoError(t, err)
		require.Len(t, items, 1)
		assert.Equal(t, strPtr("Secondary"), items[0]["Name"].S)
	})

	t.Run("binary GSI hash key", func(t *testing.T) {
		require.NoError(t, store.CreateTable(TableInfo{
			Name:         "Blobs",
			PartitionKey: KeyDef{Name: "BlobID", Type: "S"},
			GlobalSecondaryIndexes: []IndexDef{{
				IndexName: "ByDigest",
				KeySchema: []KeyDef{{
					Name:    "Digest",
					Type:    "B",
					KeyType: "HASH",
				}},
			}},
		}))

		d1 := []byte{0x00, 0x01, 0x02}
		d2 := []byte{0x03, 0x04, 0x05}

		require.NoError(t, store.PutItem("Blobs", Item{
			"BlobID": {S: strPtr("b1")},
			"Digest": {B: d1},
		}))
		require.NoError(t, store.PutItem("Blobs", Item{
			"BlobID": {S: strPtr("b2")},
			"Digest": {B: d1},
		}))
		require.NoError(t, store.PutItem("Blobs", Item{
			"BlobID": {S: strPtr("b3")},
			"Digest": {B: d2},
		}))

		got, err := store.QueryGSI("Blobs", "ByDigest", d1)
		require.NoError(t, err)
		require.Len(t, got, 2)
		assert.Equal(t, strPtr("b1"), got[0]["BlobID"].S)
		assert.Equal(t, strPtr("b2"), got[1]["BlobID"].S)
	})
}

func TestDynamoStore_QuerySortKeyPrefixEscaping(t *testing.T) {
	store := newTestStore(t)

	require.NoError(t, store.CreateTable(TableInfo{
		Name:         "Files",
		PartitionKey: KeyDef{Name: "UserID", Type: "S"},
		SortKey:      &KeyDef{Name: "Path", Type: "S"},
	}))

	// Sort keys containing LIKE special characters.
	items := []Item{
		{
			"UserID": {S: strPtr("u1")},
			"Path":   {S: strPtr(`/%_literal`)},
		},
		{
			"UserID": {S: strPtr("u1")},
			"Path":   {S: strPtr(`/data%`),
		},
		{
			"UserID": {S: strPtr("u1")},
			"Path":   {S: strPtr(`/data_1`),
		},
		{
			"UserID": {S: strPtr("u1")},
			"Path":   {S: strPtr(`/data\\escaped`),
		},
	}

	for _, it := range items {
		require.NoError(t, store.PutItem("Files", it))
	}

	tests := []struct {
		name     string
		prefix   string
		expected int
	}{
		{
			name:     "literal percent",
			prefix:   "/%_",
			expected: 1,
		},
		{
			name:     "literal underscore",
			prefix:   "/data_",
			expected: 2, // /data_1 and /data\escaped do not match, only /data_1
		},
		{
			name:     "literal backslash",
			prefix:   `/data\`,
			expected: 1,
		},
	}

	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			got, err := store.QueryWithSortKeyPrefix("Files", Item{
				"UserID": {S: strPtr("u1")},
			}, tt.prefix)
			require.NoError(t, err)
			assert.Len(t, got, tt.expected)
		})
	}
}

```

The above tests assume and may need alignment with:

1. **Error strings**: The assertions on `err.Error()` for missing partition/sort keys should match the exact messages returned by your `keyValues` helper (e.g., `"missing partition key attribute"` / `"missing sort key attribute"`). Adjust the `Contains` substrings if your implementation uses different wording.
2. **Type signatures**:
   - `AttributeValue.B` is treated as a `[]byte`, and `QueryGSI` is called with a `[]byte` key for binary indexes. If your implementation uses a different representation (e.g., `string` or `*string`), update the test to pass and compare the correct type.
   - The tests use `store.Query(tableName, key Item)` and `store.QueryWithSortKeyPrefix(tableName, key Item, sortKeyPrefix string)` as the primary key query APIs. If your actual query helpers have different names or signatures (e.g., `Query(tableName, partitionKey Item, sortPrefix *string)`), adjust the calls accordingly and keep the intent: exercise sort key prefix matching with special characters and verify that `escapeLike` correctly treats `%`, `_`, and `\` as literals.
3. **Index projection semantics**: The extended `TestDynamoStore_QueryGSI` assertions assume that `QueryGSI` returns items with at least the primary key attributes. If your projection differs (e.g., returning only indexed attributes), adapt the expected fields while still verifying that updates and deletes do not leave stale rows in the SQLite-backed GSI tables.
4. Ensure that any new helper such as `QueryWithSortKeyPrefix` either already exists or is implemented to delegate to the internal `escapeLike` / prefix query logic; if your API already exposes sort-prefix querying via `Query`, you can inline those calls instead of introducing a new wrapper.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread internal/services/dynamodb/store_test.go
Comment thread internal/services/dynamodb/store_test.go
Address review: exercise the new SQLite-backed paths beyond the happy path.

- QueryGSI: unknown table (ErrTableNotFound), local secondary index, an index
  with HASH+RANGE keys where items share a gsi_pk but differ by base key, and a
  binary (B) hash key — confirming the projection is correct for all key types
- keyValues: missing partition/sort key errors on PutItem/GetItem/DeleteItem
- numeric (N) partition key round-trip
- escapeLike: sort-key prefix with %, _, \ treated as literals

go test ./... → 108 ok, 0 fail.
Address code-review findings on the badger->SQLite refactor:

- Query numeric ('N') sort keys numerically, not lexicographically
- Project composite-GSI items only when all index key attrs are present
- Order QueryGSI by the index sort key via a new gsi_sk column (migration v3)
- Propagate unsupported index-key type errors instead of silently skipping
- Hold the read lock across PutItem/DeleteItem writes so a racing DeleteTable
  can't orphan rows that resurrect on a same-name recreate
- Delete ddb_ttl/ddb_tags rows in DeleteTable so a recreated table starts clean
- UpdateTable persists before mutating the cache (no rollback-on-failure drift)
- begins_with prefix matches case-sensitively via a byte range, not LIKE
- Serialize the tag read-modify-write in PutTags/RemoveTags
- Build a fresh index slice instead of appending into the shared cached array
- Skip the ddb_gsi delete on writes to tables without secondary indexes
@skyoo2003
skyoo2003 merged commit 3d5b6bf into main Jul 17, 2026
10 checks passed
@skyoo2003
skyoo2003 deleted the refactor/dynamodb-sqlite branch July 17, 2026 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Dependency updates services AWS service implementations tests Test code and test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant