Add StateStore query API#1153
Conversation
dmitsh
changed the title
dmitsh
changed the title
| token string | ||
| } | ||
|
|
||
| func (q *Query) VisitEQ(f *query.EQ) (string, error) { |
There was a problem hiding this comment.
In addition to the query text, can this also contain the map of parameters as a response? I think we should support prepared statement natively, otherwise, user code would still be vulnerable to SQL injection if the value is hand crafted for that.
There was a problem hiding this comment.
The function argument query.EQ already contains the exact key/value. What is the purpose and content of the suggested map of parameters?
| return str, nil | ||
| } | ||
|
|
||
| func (q *Query) visitFilters(op string, filters []query.Filter) (string, error) { |
There was a problem hiding this comment.
Also, instead of returning string, it could return interface{}, so for databases like MongoDb, it would already be the struct that the filter expects.
There was a problem hiding this comment.
This function returns string because it generates a textual query
| opts = append(opts, documentdb.Continuation(q.token)) | ||
| } | ||
| items := []CosmosItem{} | ||
| resp, err := client.QueryDocuments(collection.Self, &q.query, &items, opts...) |
There was a problem hiding this comment.
|
|
||
| func (q *Query) VisitEQ(f *query.EQ) (string, error) { | ||
| // { <key>: <val> } | ||
| return fmt.Sprintf("{ %q: %q }", f.Key, f.Val), nil |
There was a problem hiding this comment.
key or value could have a content that could turn this string into an invalid JSON. Instead, build the query object directly in the visitor methods.
Add StateStore query API implementation for MongoDB, CosmosDB
Codecov Report
@@ Coverage Diff @@
## master #1153 +/- ##
==========================================
+ Coverage 34.34% 35.00% +0.65%
==========================================
Files 140 145 +5
Lines 11993 12479 +486
==========================================
+ Hits 4119 4368 +249
- Misses 7450 7651 +201
- Partials 424 460 +36
Continue to review full report at Codecov.
|
artursouza
left a comment
artursouza
left a comment
There was a problem hiding this comment.
I've found the fix for the base64 encoded result. We basically need to create a separate response object and use jasonite.RawMessage:
For gRPC API it seems to simply be byte[] in data field.
| Key string `json:"key"` | ||
| Data []byte `json:"data"` | ||
| ETag *string `json:"etag,omitempty"` | ||
| Error string `json:"error,omitempty"` |
There was a problem hiding this comment.
Is there a reason why there can be error per key? For bulk get it makes sense but why for query?
There was a problem hiding this comment.
Is there a reason why there can be error per key? For bulk get it makes sense but why for query?
If the data is encoded, we decode each entry separately (similarly to bulk-get), hence the error could happen for individual keys.
Thanks, I tried this fix and it worked!!!, but your change belongs to the corresponding PR in dapr/dapr. |
Add StateStore query API implementation for MongoDB, CosmosDB
3 participants
Description
Added StateStore query API and its implementation for MongoDB
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: dapr/dapr#3662
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: