Protobuf-backed Mutations #8040
Conversation
Generated by 🚫 Danger |
|
Sorry I'm slow with the reviews -- I'm afraid I won't be able to get to these PRs until Wednesday. If this doesn't work for you, feel free to reassign. |
Firestore/core/src/model/mutation.h
Outdated
| #include "absl/types/optional.h" | ||
|
|
||
| namespace firebase { | ||
| namespace firestore { | ||
| namespace model { | ||
|
|
||
| class MaybeDocument; | ||
| typedef std::map<FieldPath, absl::optional<google_firestore_v1_Value>> |
There was a problem hiding this comment.
Question: does the map need to be sorted?
There was a problem hiding this comment.
It does, as it is used as an input to SetAll. SetAll is more efficient it the input map is sorted as it processes field maps one layer at a time.
There was a problem hiding this comment.
SetAll is more efficient it the input map is sorted as it processes field maps one layer at a time.
Can you add this part to the comment as well?
Firestore/core/src/model/mutation.h
Outdated
| #include "absl/types/optional.h" | ||
|
|
||
| namespace firebase { | ||
| namespace firestore { | ||
| namespace model { | ||
|
|
||
| class MaybeDocument; | ||
| typedef std::map<FieldPath, absl::optional<google_firestore_v1_Value>> |
There was a problem hiding this comment.
SetAll is more efficient it the input map is sorted as it processes field maps one layer at a time.
Can you add this part to the comment as well?
I recommend comparing to Android: https://github.com/firebase/firebase-android-sdk/tree/master/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation