-
Notifications
You must be signed in to change notification settings - Fork 590
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
feat(api): add JSON getitem support #4525
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4525 +/- ##
=========================================
Coverage ? 92.52%
=========================================
Files ? 184
Lines ? 19922
Branches ? 2935
=========================================
Hits ? 18432
Misses ? 1116
Partials ? 374
|
c1a439f
to
2dce0a1
Compare
| class JSONValue(StringValue): | ||
| pass # noqa: E701,E302 | ||
| class JSONValue(Value): | ||
| def __getitem__(self, key): |
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.
Could you please add a follow-up to provide .get(key, default) API similar to maps?
| @@ -27,6 +27,7 @@ class TestConf(UnorderedComparator, BackendTest, RoundHalfToEven): | |||
| supported_to_timestamp_units = {'s'} | |||
| supports_floating_modulus = False | |||
| bool_is_int = True | |||
| supports_json = False | |||
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.
Please add a follow-up issue to investigate it further.
2dce0a1
to
d3206f0
Compare
This PR adds the ability to extract object values or array elements of a JSON-typed column.
Notable things:
clickhouse_driver.JSONtype is return in Python. We handle this here by trying to parse the JSON and if that fails assuming the value is already deserialized.