Skip to content
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

Experimental/json serializable columns #695

Merged
merged 8 commits into from
Jul 18, 2021

Conversation

jwoertink
Copy link
Member

@jwoertink jwoertink commented Jul 5, 2021

Fixes #691

This is an experimental draft to serializing json columns in to objects. In order to do this, it requires a patch to how the PG shard decodes JSON columns. @matthewmcgarvey submitted will/crystal-pg#232 as a means to fix that hack.

To use them, your migrations would implement the JSON::Any as they do now, so nothing changes there. This means existing columns don't need any sort of migration. In your model, you can continue to use JSON::Any as is without changing anything. If you want to use the serialized column, you'll change the type from JSON::Any to your serializable object type, and then add the column option serialize: true.

struct UserPreferences
  include JSON::Serializable
  property theme : String = "dark"
  property email_notify : Bool = false
end

class User < BaseModel
  table do
    column preferences : UserPreferences, serialize: true
    column media : JSON::Any
  end
end

user = UserQuery.find(4)
user.preferences.theme == "dark"
user.media["image"]?.try(&.as_s)

There's still a few questions left to answer:

  • How does the operation handle a serialized column coming in from params?
  • What if your column is nullable, but the serialized object has no nilable properties?
  • Can we support more complex structures?
  • Should there be some sort of runtime escape hatch to get the raw value from the DB of a serialized object?
  • Can we provide type-safe compile-time error handling when things aren't setup correctly for this?

Comment on lines +5 to +7
require "db"
require "pg"
require "uuid"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These had to be moved up so our patch to "pg" comes after the definition

…ted to this PR. There's already enough noise.
src/avram/model.cr Outdated Show resolved Hide resolved
@jwoertink jwoertink marked this pull request as ready for review July 7, 2021 01:52
@jwoertink jwoertink requested a review from paulcsmith July 7, 2021 14:56
@jkthorne
Copy link
Contributor

I am excited for this feature. I have been using json more and more these days.

@jkthorne
Copy link
Contributor

I think there might also be some query features we can add.

@jwoertink
Copy link
Member Author

@wontruefree Have any ideas for an API for query stuff using this? I know one that I'd like to do is some sort of "has_key?" type method. The json query stuff in postgres is pretty gnarly. Also related: #197 We could add this in and do a separate PR unless it's something minimal...

Copy link
Member

@paulcsmith paulcsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm! This is a solid setup to play around with and see how it works

@jwoertink jwoertink merged commit 401b1ae into master Jul 18, 2021
@jwoertink jwoertink deleted the experimental/json_serializable_columns branch July 18, 2021 19:01
akadusei added a commit to GrottoPress/shield that referenced this pull request Aug 16, 2021
This is a `JSON::Serializable` column, as an alternative to
the `UserOptions` model.

Made possible by luckyframework/avram#695.
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.

New serializer columns
4 participants