fix storage issues

dynamodb/expression.odin

@@ -420,39 +420,34 @@ parse_expression_attribute_values :: proc(request_body: []byte) -> (map[string]A
 	return result, true
 }
 
-// NOTE: changed from Maybe(string) -> (string, bool) so callers can use or_return.
+// ============================================================================
+// FIX: Use JSON object lookup instead of fragile string scanning.
+// This handles whitespace, field ordering, and escape sequences correctly.
+// ============================================================================
 parse_key_condition_expression_string :: proc(request_body: []byte) -> (expr: string, ok: bool) {
-	body_str := string(request_body)
+	data, parse_err := json.parse(request_body, allocator = context.temp_allocator)
+	if parse_err != nil {
+		return
+	}
+	defer json.destroy_value(data)
 
-	marker :: "\"KeyConditionExpression\""
-	start_idx := strings.index(body_str, marker)
-	if start_idx < 0 {
+	root, root_ok := data.(json.Object)
+	if !root_ok {
 		return
 	}
 
-	after_marker := body_str[start_idx + len(marker):]
-	colon_idx := strings.index(after_marker, ":")
-	if colon_idx < 0 {
+	kce_val, found := root["KeyConditionExpression"]
+	if !found {
 		return
 	}
 
-	rest := after_marker[colon_idx + 1:]
-	quote_start := strings.index(rest, "\"")
-	if quote_start < 0 {
+	kce_str, str_ok := kce_val.(json.String)
+	if !str_ok {
 		return
 	}
 
-	value_start := quote_start + 1
-	pos := value_start
-	for pos < len(rest) {
-		if rest[pos] == '"' && (pos == 0 || rest[pos - 1] != '\\') {
-			expr = rest[value_start:pos]
-			ok = true
-			return
-		}
-		pos += 1
-	}
-
+	expr = string(kce_str)
+	ok = true
 	return
 }
 

dynamodb/json.odin

@@ -485,41 +485,131 @@ parse_limit :: proc(request_body: []byte) -> int {
 	return 0
 }
 
-// Parse ExclusiveStartKey from request body as binary key bytes
-// Returns nil if not present
-parse_exclusive_start_key :: proc(request_body: []byte) -> Maybe([]byte) {
+// ============================================================================
+// ExclusiveStartKey Parsing (Pagination Input)
+//
+// Parse ExclusiveStartKey from request body. Requires key_schema so we can
+// validate and extract the key, then convert it to a binary storage key.
+// Returns the binary key bytes that can be passed straight to scan/query.
+// Returns nil (not an error) when the field is absent.
+// ============================================================================
+
+parse_exclusive_start_key :: proc(
+	request_body: []byte,
+	table_name: string,
+	key_schema: []Key_Schema_Element,
+) -> (result: Maybe([]byte), ok: bool) {
 	data, parse_err := json.parse(request_body, allocator = context.temp_allocator)
 	if parse_err != nil {
-		return nil
+		return nil, true // no ESK is fine
 	}
 	defer json.destroy_value(data)
 
-	root, ok := data.(json.Object)
-	if !ok {
-		return nil
+	root, root_ok := data.(json.Object)
+	if !root_ok {
+		return nil, true
 	}
 
-	key_val, found := root["ExclusiveStartKey"]
+	esk_val, found := root["ExclusiveStartKey"]
 	if !found {
-		return nil
+		return nil, true // absent → no pagination, that's ok
 	}
 
-	// Parse as Item first
-	key_item, item_ok := parse_item_from_value(key_val)
+	// Parse ExclusiveStartKey as a DynamoDB Item
+	key_item, item_ok := parse_item_from_value(esk_val)
 	if !item_ok {
-		return nil
+		return nil, false // present but malformed → real error
 	}
 	defer item_destroy(&key_item)
 
-	// Convert to binary key bytes (this will be done by the storage layer)
-	// For now, just return nil - the storage layer will handle the conversion
-	return nil
+	// Validate and extract key struct using schema
+	key_struct, key_ok := key_from_item(key_item, key_schema)
+	if !key_ok {
+		return nil, false // missing required key attributes
+	}
+	defer key_destroy(&key_struct)
+
+	// Get raw byte values
+	key_values, kv_ok := key_get_values(&key_struct)
+	if !kv_ok {
+		return nil, false
+	}
+
+	// Build binary storage key
+	binary_key := build_data_key(table_name, key_values.pk, key_values.sk)
+	result = binary_key
+	ok = true
+	return
 }
 
-// Serialize a Key as ExclusiveStartKey for response
-serialize_last_evaluated_key :: proc(key: Key) -> string {
-	item := key_to_item(key, {}) // Empty key_schema since we don't need validation here
+// ============================================================================
+// LastEvaluatedKey Generation (Pagination Output)
+//
+// Decode a binary storage key back into a DynamoDB JSON fragment suitable
+// for the "LastEvaluatedKey" field in scan/query responses.
+//
+// Steps:
+//   1. Decode the binary key → table_name, pk_bytes, sk_bytes
+//   2. Look up attribute types from metadata (S/N/B)
+//   3. Build a Key struct with correctly-typed AttributeValues
+//   4. Convert Key → Item → DynamoDB JSON string
+// ============================================================================
+
+// Build a Key struct from a binary storage key using metadata for type info.
+// This mirrors the Zig buildKeyFromBinaryWithTypes helper.
+build_key_from_binary_with_types :: proc(
+	binary_key: []byte,
+	metadata: ^Table_Metadata,
+) -> (key: Key, ok: bool) {
+	decoder := Key_Decoder{data = binary_key, pos = 0}
+
+	// Skip entity type byte
+	_ = decoder_read_entity_type(&decoder) or_return
+
+	// Skip table name segment
+	_ = decoder_read_segment_borrowed(&decoder) or_return
+
+	// Read partition key bytes
+	pk_bytes := decoder_read_segment_borrowed(&decoder) or_return
+
+	// Read sort key bytes if present
+	sk_bytes: Maybe([]byte) = nil
+	if decoder_has_more(&decoder) {
+		sk := decoder_read_segment_borrowed(&decoder) or_return
+		sk_bytes = sk
+	}
+
+	// Get PK attribute type from metadata
+	pk_name := table_metadata_get_partition_key_name(metadata).? or_return
+	pk_type := table_metadata_get_attribute_type(metadata, pk_name).? or_return
+
+	pk_attr := build_attribute_value_with_type(pk_bytes, pk_type)
+
+	// Build SK attribute if present
+	sk_attr: Maybe(Attribute_Value) = nil
+	if sk, has_sk := sk_bytes.?; has_sk {
+		sk_name := table_metadata_get_sort_key_name(metadata).? or_return
+		sk_type := table_metadata_get_attribute_type(metadata, sk_name).? or_return
+		sk_attr = build_attribute_value_with_type(sk, sk_type)
+	}
+
+	return Key{pk = pk_attr, sk = sk_attr}, true
+}
+
+// Serialize a binary storage key as a LastEvaluatedKey JSON fragment.
+// Returns a string like: {"pk":{"S":"val"},"sk":{"N":"42"}}
+serialize_last_evaluated_key :: proc(
+	binary_key: []byte,
+	metadata: ^Table_Metadata,
+) -> (result: string, ok: bool) {
+	key, key_ok := build_key_from_binary_with_types(binary_key, metadata)
+	if !key_ok {
+		return "", false
+	}
+	defer key_destroy(&key)
+
+	item := key_to_item(key, metadata.key_schema)
 	defer item_destroy(&item)
 
-	return serialize_item(item)
+	return serialize_item(item), true
 }

dynamodb/storage.odin

@@ -527,7 +527,7 @@ create_table :: proc(
 	return desc, .None
 }
 
-// Delete table
+// Delete table — removes metadata AND all items with the table's data prefix
 delete_table :: proc(engine: ^Storage_Engine, table_name: string) -> Storage_Error {
 	table_lock := get_or_create_table_lock(engine, table_name)
 	sync.rw_mutex_lock(table_lock)
@@ -546,15 +546,48 @@ delete_table :: proc(engine: ^Storage_Engine, table_name: string) -> Storage_Err
 	}
 	delete(existing)
 
+	// Delete all data items using a prefix scan
+	table_prefix := build_table_prefix(table_name)
+	defer delete(table_prefix)
+
+	iter := rocksdb.rocksdb_create_iterator(engine.db.handle, engine.db.read_options)
+	if iter != nil {
+		defer rocksdb.rocksdb_iter_destroy(iter)
+
+		rocksdb.rocksdb_iter_seek(iter, raw_data(table_prefix), c.size_t(len(table_prefix)))
+
+		for rocksdb.rocksdb_iter_valid(iter) != 0 {
+			key_len: c.size_t
+			key_ptr := rocksdb.rocksdb_iter_key(iter, &key_len)
+			key_bytes := key_ptr[:key_len]
+
+			if !has_prefix(key_bytes, table_prefix) {
+				break
+			}
+
+			// Delete this item
+			err: cstring
+			rocksdb.rocksdb_delete(
+				engine.db.handle,
+				engine.db.write_options,
+				raw_data(key_bytes),
+				c.size_t(len(key_bytes)),
+				&err,
+			)
+			if err != nil {
+				rocksdb.rocksdb_free(rawptr(err))
+			}
+
+			rocksdb.rocksdb_iter_next(iter)
+		}
+	}
+
 	// Delete metadata
 	del_err := rocksdb.db_delete(&engine.db, meta_key)
 	if del_err != .None {
 		return .RocksDB_Error
 	}
 
-	// TODO: Delete all items in table using iterator
-	// For now, just delete metadata
-
 	remove_table_lock(engine, table_name)
 	return .None
 }
@@ -563,11 +596,11 @@ delete_table :: proc(engine: ^Storage_Engine, table_name: string) -> Storage_Err
 // Item Operations
 // ============================================================================
 
-// Put item
+// Put item — uses EXCLUSIVE lock (write operation)
 put_item :: proc(engine: ^Storage_Engine, table_name: string, item: Item) -> Storage_Error {
 	table_lock := get_or_create_table_lock(engine, table_name)
-	sync.rw_mutex_shared_lock(table_lock)
-	defer sync.rw_mutex_shared_unlock(table_lock)
+	sync.rw_mutex_lock(table_lock)
+	defer sync.rw_mutex_unlock(table_lock)
 
 	// Get table metadata
 	metadata, meta_err := get_table_metadata(engine, table_name)
@@ -576,6 +609,12 @@ put_item :: proc(engine: ^Storage_Engine, table_name: string, item: Item) -> Sto
 	}
 	defer table_metadata_destroy(&metadata, engine.allocator)
 
+	// Validate key attribute types match schema
+	validation_err := validate_item_key_types(item, metadata.key_schema, metadata.attribute_definitions)
+	if validation_err != .None {
+		return validation_err
+	}
+
 	// Extract key from item
 	key, key_ok := key_from_item(item, metadata.key_schema)
 	if !key_ok {
@@ -616,7 +655,7 @@ put_item :: proc(engine: ^Storage_Engine, table_name: string, item: Item) -> Sto
 	return .None
 }
 
-// Get item
+// Get item — uses SHARED lock (read operation)
 get_item :: proc(engine: ^Storage_Engine, table_name: string, key: Item) -> (Maybe(Item), Storage_Error) {
 	table_lock := get_or_create_table_lock(engine, table_name)
 	sync.rw_mutex_shared_lock(table_lock)
@@ -672,11 +711,11 @@ get_item :: proc(engine: ^Storage_Engine, table_name: string, key: Item) -> (May
 	return item, .None
 }
 
-// Delete item
+// Delete item — uses EXCLUSIVE lock (write operation)
 delete_item :: proc(engine: ^Storage_Engine, table_name: string, key: Item) -> Storage_Error {
 	table_lock := get_or_create_table_lock(engine, table_name)
-	sync.rw_mutex_shared_lock(table_lock)
-	defer sync.rw_mutex_shared_unlock(table_lock)
+	sync.rw_mutex_lock(table_lock)
+	defer sync.rw_mutex_unlock(table_lock)
 
 	// Get table metadata
 	metadata, meta_err := get_table_metadata(engine, table_name)
@@ -718,6 +757,14 @@ delete_item :: proc(engine: ^Storage_Engine, table_name: string, key: Item) -> S
 	return .None
 }
 
+// ============================================================================
+// Scan — with FIXED pagination
+//
+// FIX: LastEvaluatedKey must be the key of the LAST RETURNED item, not the
+//      next unread item. DynamoDB semantics: ExclusiveStartKey resumes
+//      *after* the given key, so we save the last key we actually returned.
+// ============================================================================
+
 scan :: proc(
 	engine: ^Storage_Engine,
 	table_name: string,
@@ -748,9 +795,8 @@ scan :: proc(
 
 	// Seek to start position
 	if start_key, has_start := exclusive_start_key.?; has_start {
-		// Resume from pagination token
+		// Resume from pagination token — seek to the key then skip it (exclusive)
 		rocksdb.rocksdb_iter_seek(iter, raw_data(start_key), c.size_t(len(start_key)))
-		// Skip the start key itself (it's exclusive)
 		if rocksdb.rocksdb_iter_valid(iter) != 0 {
 			rocksdb.rocksdb_iter_next(iter)
 		}
@@ -759,10 +805,13 @@ scan :: proc(
 		rocksdb.rocksdb_iter_seek(iter, raw_data(table_prefix), c.size_t(len(table_prefix)))
 	}
 
+	max_items := limit if limit > 0 else 1_000_000
+
 	// Collect items
 	items := make([dynamic]Item, context.temp_allocator)
 	count := 0
 	last_key: Maybe([]byte) = nil
+	has_more := false
 
 	for rocksdb.rocksdb_iter_valid(iter) != 0 {
 		// Get current key
@@ -775,10 +824,9 @@ scan :: proc(
 			break
 		}
 
-		// Check limit
-		if count >= limit {
-			// Save this key as pagination token
-			last_key = slice.clone(key_bytes, engine.allocator)
+		// Check limit — if we already have enough items, note there's more and stop
+		if count >= max_items {
+			has_more = true
 			break
 		}
 
@@ -798,12 +846,26 @@ scan :: proc(
 		append(&items, item)
 		count += 1
 
+		// Track the key of the last successfully returned item
+		if prev_key, had_prev := last_key.?; had_prev {
+			delete(prev_key)
+		}
+		last_key = slice.clone(key_bytes)
+
 		// Move to next
 		rocksdb.rocksdb_iter_next(iter)
 	}
 
-	// Convert to slice (owned by caller's allocator)
-	result_items := make([]Item, len(items), engine.allocator)
+	// Only emit LastEvaluatedKey if there are more items beyond what we returned
+	if !has_more {
+		if lk, had_lk := last_key.?; had_lk {
+			delete(lk)
+		}
+		last_key = nil
+	}
+
+	// Convert to slice
+	result_items := make([]Item, len(items))
 	copy(result_items, items[:])
 
 	return Scan_Result{
@@ -812,13 +874,17 @@ scan :: proc(
 	}, .None
 }
 
-// Query items by partition key with optional pagination
+// ============================================================================
+// Query — with sort key condition filtering and FIXED pagination
+// ============================================================================
+
 query :: proc(
 	engine: ^Storage_Engine,
 	table_name: string,
 	partition_key_value: []byte,
 	exclusive_start_key: Maybe([]byte),
 	limit: int,
+	sk_condition: Maybe(Sort_Key_Condition) = nil,
 ) -> (Query_Result, Storage_Error) {
 	table_lock := get_or_create_table_lock(engine, table_name)
 	sync.rw_mutex_shared_lock(table_lock)
@@ -860,6 +926,7 @@ query :: proc(
 	items := make([dynamic]Item)
 	count := 0
 	last_key: Maybe([]byte) = nil
+	has_more := false
 
 	for rocksdb.iter_valid(&iter) {
 		key := rocksdb.iter_key(&iter)
@@ -867,9 +934,9 @@ query :: proc(
 			break
 		}
 
-		// Hit limit — save this key as pagination token and stop
+		// Hit limit — note there's more and stop
 		if count >= max_items {
-			last_key = slice.clone(key)
+			has_more = true
 			break
 		}
 
@@ -885,11 +952,37 @@ query :: proc(
 			continue
 		}
 
+		// ---- Sort key condition filtering ----
+		if skc, has_skc := sk_condition.?; has_skc {
+			if !evaluate_sort_key_condition(item, &skc) {
+				// Item doesn't match SK condition — skip it
+				item_copy := item
+				item_destroy(&item_copy)
+				rocksdb.iter_next(&iter)
+				continue
+			}
+		}
+
 		append(&items, item)
 		count += 1
+
+		// Track key of last returned item
+		if prev_key, had_prev := last_key.?; had_prev {
+			delete(prev_key)
+		}
+		last_key = slice.clone(key)
+
 		rocksdb.iter_next(&iter)
 	}
 
+	// Only emit LastEvaluatedKey if there are more items
+	if !has_more {
+		if lk, had_lk := last_key.?; had_lk {
+			delete(lk)
+		}
+		last_key = nil
+	}
+
 	result_items := make([]Item, len(items))
 	copy(result_items, items[:])
 
@@ -899,6 +992,126 @@ query :: proc(
 	}, .None
 }
 
+// ============================================================================
+// Sort Key Condition Evaluation
+//
+// Extracts the sort key attribute from a decoded item and compares it against
+// the parsed Sort_Key_Condition using string comparison (matching DynamoDB's
+// byte-level comparison semantics for S/N/B types).
+// ============================================================================
+
+evaluate_sort_key_condition :: proc(item: Item, skc: ^Sort_Key_Condition) -> bool {
+	attr, found := item[skc.sk_name]
+	if !found {
+		return false
+	}
+
+	item_sk_str, ok1 := attr_value_to_string_for_compare(attr)
+	if !ok1 {
+		return false
+	}
+
+	cond_val_str, ok2 := attr_value_to_string_for_compare(skc.value)
+	if !ok2 {
+		return false
+	}
+
+	cmp := strings.compare(item_sk_str, cond_val_str)
+
+	switch skc.operator {
+	case .EQ:
+		return cmp == 0
+	case .LT:
+		return cmp < 0
+	case .LE:
+		return cmp <= 0
+	case .GT:
+		return cmp > 0
+	case .GE:
+		return cmp >= 0
+	case .BETWEEN:
+		if v2, has_v2 := skc.value2.?; has_v2 {
+			upper_str, ok3 := attr_value_to_string_for_compare(v2)
+			if !ok3 {
+				return false
+			}
+			cmp2 := strings.compare(item_sk_str, upper_str)
+			return cmp >= 0 && cmp2 <= 0
+		}
+		return false
+	case .BEGINS_WITH:
+		return strings.has_prefix(item_sk_str, cond_val_str)
+	}
+
+	return false
+}
+
+// Extract a comparable string from a scalar AttributeValue
+@(private = "file")
+attr_value_to_string_for_compare :: proc(attr: Attribute_Value) -> (string, bool) {
+	#partial switch v in attr {
+	case String:
+		return string(v), true
+	case Number:
+		return string(v), true
+	case Binary:
+		return string(v), true
+	}
+	return "", false
+}
+
+// ============================================================================
+// Item Key Validation
+//
+// Validates that an item's key attributes match the types declared in
+// AttributeDefinitions. E.g., if PK is declared as "S", the item must
+// have a String value for that attribute.
+// ============================================================================
+
+validate_item_key_types :: proc(
+	item: Item,
+	key_schema: []Key_Schema_Element,
+	attr_defs: []Attribute_Definition,
+) -> Storage_Error {
+	for ks in key_schema {
+		attr, found := item[ks.attribute_name]
+		if !found {
+			return .Missing_Key_Attribute
+		}
+
+		// Find the expected type from attribute definitions
+		expected_type: Maybe(Scalar_Attribute_Type) = nil
+		for ad in attr_defs {
+			if ad.attribute_name == ks.attribute_name {
+				expected_type = ad.attribute_type
+				break
+			}
+		}
+
+		et, has_et := expected_type.?
+		if !has_et {
+			continue // No definition found — skip validation (shouldn't happen)
+		}
+
+		// Check actual type matches expected
+		match := false
+		#partial switch _ in attr {
+		case String:
+			match = (et == .S)
+		case Number:
+			match = (et == .N)
+		case Binary:
+			match = (et == .B)
+		}
+
+		if !match {
+			return .Invalid_Key
+		}
+	}
+
+	return .None
+}
+
 // Helper to check if a byte slice has a prefix
 has_prefix :: proc(data: []byte, prefix: []byte) -> bool {
 	if len(data) < len(prefix) {
@@ -947,4 +1160,4 @@ list_tables :: proc(engine: ^Storage_Engine) -> ([]string, Storage_Error) {
 	}
 
 	return tables[:], .None
-}
+}