Skip to content

various fixes to postgres adapter#39

Merged
mde merged 3 commits intogeddy:masterfrom
der-On:master
Apr 10, 2013
Merged

various fixes to postgres adapter#39
mde merged 3 commits intogeddy:masterfrom
der-On:master

Conversation

@der-On
Copy link
Contributor

@der-On der-On commented Mar 21, 2013

No description provided.

der-On added 3 commits March 21, 2013 22:44
Objects that use the default toString() method will not create usefull serialized output so we make JSON out of them.
Also we now allow Arrays to be serialized to JSON this way too as the array datatype will not cover Arrays of objects with a default toString() method.
@mde
Copy link
Contributor

mde commented Mar 22, 2013

This looks great, but there are a couple of things I'd like to clarify --
there's a specific datatype for Array, so why do we need code for handling
Arrays in the Object serialization in
8ff337a?

Also, would it be possible to add some tests for these changes?

On Thu, Mar 21, 2013 at 2:44 PM, Ondrej notifications@github.com wrote:

Objects that use the default toString() method will not create usefull
serialized output so we make JSON out of them.
Also we now allow Arrays to be serialized to JSON this way too as the
array datatype will not cover Arrays of objects with a default toString()

method.

You can merge this Pull Request by running

git pull https://github.com/der-On/model master

Or view, comment on, or merge it at:

#39
Commit Summary

  • fixed wrong serialization of objects with default toString()

File Changes

Patch Links:

@der-On
Copy link
Contributor Author

der-On commented Mar 22, 2013

Yes there is an array datatype, but I tried to use it while having arrays containing objects. These get serialized to something like "[object Object],[object Object],...". I've looked into the pg parser code and it is not able to parse such arrays back and forth. It only supports arrays of strings/numbers. Also unserialization of object datatypes does not work. One has to do it manually in the model or somewhere else after a Model.load(). I know my code is not very standard compliant but see it as an idea to a possible solution.

mde added a commit that referenced this pull request Apr 10, 2013
various fixes to postgres adapter
@mde mde merged commit c4aa579 into geddy:master Apr 10, 2013
@mde
Copy link
Contributor

mde commented Apr 10, 2013

Merged, in master HEAD, thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants