-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
Support json type #263
Support json type #263
Conversation
Tigrov
commented
Aug 1, 2023
•
edited
Loading
edited
Q | A |
---|---|
Is bugfix? | ❌ |
New feature? | ✔️ |
Breaks BC? | ✔️ |
Fixed issues | - |
PR Summary
|
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #263 +/- ##
============================================
+ Coverage 98.02% 98.47% +0.44%
- Complexity 328 343 +15
============================================
Files 17 19 +2
Lines 1013 1046 +33
============================================
+ Hits 993 1030 +37
+ Misses 20 16 -4
☔ View full report in Codecov by Sentry. |
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.
Overall looks good 👍 Please, add line to changelog
} | ||
|
||
if ($this->getType() === SchemaInterface::TYPE_JSON) { | ||
return json_decode((string) $value, true, 512, JSON_THROW_ON_ERROR); |
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.
return json_decode((string) $value, true, 512, JSON_THROW_ON_ERROR); | |
return Json::decode((string) $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.
Or you can replace Json::encode()
in other place to json_encode
and drop dependency yiisoft/json
. It's even better this way.
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.
Json::encode()
is used according to MySQL and PostgreSQL
https://github.com/yiisoft/db-mysql/blob/master/src/Builder/JsonExpressionBuilder.php#L53
https://github.com/yiisoft/db-pgsql/blob/master/src/Builder/JsonExpressionBuilder.php#L56
Json::encode()
Pre-processes the data before sending it to json_encode()
:
https://github.com/yiisoft/json/blob/master/src/Json.php#L100
Json::decode()
is the same as json_decode()
but with default arguments:
https://github.com/yiisoft/json/blob/master/src/Json.php#L81
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.
Still good idea to use the helper's function
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.
@xepozz Do you mean use Json::decode()
?
MySQL and PostgreSQL use json_decode()
directly
https://github.com/yiisoft/db-mysql/blob/master/src/ColumnSchema.php#L59
https://github.com/yiisoft/db-pgsql/blob/master/src/ColumnSchema.php#L145
Co-authored-by: Sergei Predvoditelev <[email protected]>
# Conflicts: # tests/ColumnSchemaTest.php # tests/Provider/SchemaProvider.php # tests/Support/Fixture/sqlite.sql
👍 |