-
Notifications
You must be signed in to change notification settings - Fork 11
[PoC] Add TypedDocument for supporting flat collection writes #261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #261 +/- ##
============================================
- Coverage 80.41% 80.15% -0.27%
Complexity 1337 1337
============================================
Files 231 232 +1
Lines 6060 6197 +137
Branches 545 568 +23
============================================
+ Hits 4873 4967 +94
- Misses 813 849 +36
- Partials 374 381 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| return fields; | ||
| } | ||
|
|
||
| /** Returns a specific field by name, or null if not present. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of returning null, it'd be safer to return Optional.
| public enum FieldKind { | ||
| SCALAR, | ||
| ARRAY, | ||
| JSONB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JSONB is specific to Postgres. Can we call it NESTED or something like that?
| * @param value the value (typically a Map or object that can be serialized to JSON) | ||
| * @return this builder | ||
| */ | ||
| public Builder jsonbField(String name, Object value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same naming nit.
| } | ||
|
|
||
| @Override | ||
| public String toJson() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how does this behave for existing Document implementations?
| } | ||
|
|
||
| /** Returns the DataType for scalar fields, or the element type for array fields. */ | ||
| public DataType getDataType() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how would this behave for jsonb?
Description
Currently, write methods accept a
Documentwhich is just a JSON string with no type info. This works for Mongo because of its schemaless nature, but for Postgres, we need the type info for these fields. This PR introduces aTypedDocumentwhich contains this info.