Skip to content

Commit 65c466d

Browse files
authored
docstore/awsdynamodb: don't panic on in/not-in filter over a key field (#3760)
1 parent 45d204f commit 65c466d

2 files changed

Lines changed: 46 additions & 1 deletion

File tree

docstore/awsdynamodb/v2/query.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,12 @@ func toKeyCondition(f driver.Filter, pkey, skey string) (expression.KeyCondition
419419
case ">":
420420
return expression.KeyGreaterThan(key, val), true
421421
default:
422-
panic(fmt.Sprint("invalid filter operation:", f.Op))
422+
// Ops such as "in"/"not-in" are valid docstore filters (Where()
423+
// accepts them on any field) but cannot form a DynamoDB
424+
// KeyConditionExpression. Report "not a key condition" so the
425+
// filter is routed to the FilterExpression path instead of
426+
// panicking and crashing the process.
427+
return expression.KeyConditionBuilder{}, false
423428
}
424429
}
425430
return expression.KeyConditionBuilder{}, false

docstore/awsdynamodb/v2/query_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -704,3 +704,43 @@ func Test_documentIterator_Next(t *testing.T) {
704704
})
705705
}
706706
}
707+
708+
// Regression test: docstore's Where() accepts "in"/"not-in" on any field,
709+
// including a table's key fields. bestQueryable can then pick a Query plan
710+
// while a key-field filter uses one of those ops, reaching toKeyCondition with
711+
// an op it cannot express as a KeyCondition. That path must not panic (an
712+
// unrecovered panic there crashes the calling process); the filter should be
713+
// handled as a regular FilterExpression instead. Covers the op on both the
714+
// partition key and the sort key.
715+
func TestKeyFilterWithInOpDoesNotPanic(t *testing.T) {
716+
c := &collection{
717+
table: "T",
718+
partitionKey: "pk",
719+
sortKey: "sk",
720+
description: &dyn2Types.TableDescription{},
721+
opts: &Options{AllowScans: true},
722+
}
723+
for _, op := range []string{"in", "not-in"} {
724+
for _, keyField := range []string{"pk", "sk"} {
725+
// An "=" on the partition key is always present so bestQueryable
726+
// selects a Query plan (reaching toKeyCondition); the in/not-in
727+
// filter targets a key field on top of that.
728+
q := &driver.Query{
729+
Filters: []driver.Filter{
730+
{FieldPath: []string{"pk"}, Op: driver.EqualOp, Value: "x"},
731+
{FieldPath: []string{keyField}, Op: op, Value: []string{"a", "b"}},
732+
},
733+
}
734+
func() {
735+
defer func() {
736+
if r := recover(); r != nil {
737+
t.Errorf("op %q on %q: planQuery panicked: %v", op, keyField, r)
738+
}
739+
}()
740+
// Only assert on the absence of a panic; whether the plan
741+
// returns nil or a graceful error is not the contract here.
742+
_, _ = c.planQuery(q)
743+
}()
744+
}
745+
}
746+
}

0 commit comments

Comments
 (0)