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

[NFR] Added json dataType support for MySQL dialect. #13225

Closed
wants to merge 2 commits into
base: 3.3.x
from

Conversation

Projects
6 participants
@zGaron
Copy link

zGaron commented Dec 18, 2017

Hello!

instead of PR #13216

  • I have read and understood the Contributing Guidelines
  • I have checked that another pull request for this purpose does not exist.
  • I wrote some tests for this PR.

Small description of change:

Thanks

@zGaron zGaron referenced this pull request Dec 18, 2017

Closed

[NFR] add dataType JSON support for MySQL dialect. #13216

3 of 3 tasks complete
@zGaron

This comment has been minimized.

Copy link
Author

zGaron commented Dec 18, 2017

MySQL version >= 5.7. = =!

@zGaron zGaron changed the title Added json dataType support for MySQL dialect. [NFR] Added json dataType support for MySQL dialect. Dec 18, 2017

@sergeyklay sergeyklay added this to the unplanned milestone Dec 18, 2017

@jellisii

This comment has been minimized.

Copy link

jellisii commented Apr 3, 2018

Is this expected for 3.3.2, or will it be in 4.x?

@sergeyklay

This comment has been minimized.

Copy link
Member

sergeyklay commented Apr 3, 2018

I apologize @zGaron for completely overlooking this. Thank you for the PR. Could you please send this PR to the 4.0.x branch? And again I am sorry about the delay.

@sergeyklay sergeyklay modified the milestones: unplanned, 4.0.0 Apr 3, 2018

@virgofx

This comment has been minimized.

Copy link
Member

virgofx commented Apr 4, 2018

To clarify --- does this mean one can bind native PHP arrays that automatically get transformed into or from JSON at the DB layer? e.g. does the following work? This is what I would expect: ( I didn't see any code changes or tests that would test this?)

/**
 * @var array
 * @Column(type='json')
 */
protected $json;
$model->setJson([ 'arrayOf' => 'data' ]);
$model->save();

DB

+----+--------------------+
| id | json               |
+----+--------------------+
| 1  | {"arrayOf":"data"} |
+----+--------------------+
@sergeyklay

This comment has been minimized.

Copy link
Member

sergeyklay commented Apr 4, 2018

@virgofx I think so

@virgofx

This comment has been minimized.

Copy link
Member

virgofx commented Apr 4, 2018

I believe Since PDO doesn't bind native JSON ... I would think somewhere in the Phalcon PHQL/database layer I was expecting to see json_encode()/json_decode() just prior to save --- I didn't see that ... At least a test to outline above showing that it works would also be good.

@benfavre

This comment has been minimized.

Copy link

benfavre commented May 3, 2018

This should be on the top of all lists.

@niden

This comment has been minimized.

Copy link
Member

niden commented Oct 10, 2018

@virgofx I don't think that there is an automatic conversion towards a JSON column from PHP.

JSON columns in MySQL/MariaDB are pretty much storing text or a string. They do however have an internal mechanism that checks the syntax of the JSON string and throws an exception when you try to update it.

@zGaron would you be so kind as to rebase this and send it to the 4.0.x branch?

Thank you

@niden niden referenced this pull request Oct 31, 2018

Merged

[#13543] Add more pdo types #13562

3 of 3 tasks complete
@niden

This comment has been minimized.

Copy link
Member

niden commented Oct 31, 2018

Addressed in #13562

@niden niden closed this Oct 31, 2018

@niden niden referenced this pull request Oct 31, 2018

Open

Update 4.x Documents #1935

0 of 10 tasks complete

@niden niden added this to Done in 4.0 Release Dec 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment