Skip to content

docstore/awsdynamodb: don't panic on in/not-in filter over a key field - #3760

Merged
vangent merged 1 commit into
google:masterfrom
herdiyana256:fix-awsdynamodb-key-in-op-panic
Jul 28, 2026
Merged

docstore/awsdynamodb: don't panic on in/not-in filter over a key field#3760
vangent merged 1 commit into
google:masterfrom
herdiyana256:fix-awsdynamodb-key-in-op-panic

Conversation

@herdiyana256

Copy link
Copy Markdown
Contributor

toKeyCondition panics in its default case when a filter on the partition or sort key uses an operator it cannot express as a DynamoDB KeyCondition. docstore's own Where() accepts in/not-in on any field, and bestQueryable selects a Query plan using an op-agnostic hasFilter check on the sort key, so a valid user query such as Where("pk", "=", x).Where("sk", "in", vals) reaches that path. Nothing in the package recovers the panic, so it crashes the calling process during pure-Go query planning, before any network I/O.

This returns false from that case instead, routing the filter to the FilterExpression path rather than panicking. Same class as the empty-in-slice fix in #3754, different code path (toKeyCondition vs toInCondition). Added a regression test covering in and not-in on a key field.

toKeyCondition panicked in its default case when a filter on the partition
or sort key used an op it cannot express as a KeyCondition. docstore's Where()
accepts in/not-in on any field, and bestQueryable picks a Query plan using an
op-agnostic hasFilter check on the sort key, so a valid user query such as
Where("pk","=",x).Where("sk","in",vals) reached that panic and crashed
the calling process. Return false instead so the filter is routed to the
FilterExpression path.
@herdiyana256
herdiyana256 force-pushed the fix-awsdynamodb-key-in-op-panic branch from 268debf to 17b65a4 Compare July 28, 2026 15:00
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.40%. Comparing base (45d204f) to head (17b65a4).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3760      +/-   ##
==========================================
+ Coverage   75.37%   75.40%   +0.02%     
==========================================
  Files         104      104              
  Lines       14241    14241              
==========================================
+ Hits        10734    10738       +4     
+ Misses       2770     2766       -4     
  Partials      737      737              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vangent
vangent merged commit 65c466d into google:master Jul 28, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants