rethinkdb-elixir icon indicating copy to clipboard operation
rethinkdb-elixir copied to clipboard

Should we support Structs?

Open hamiltop opened this issue 10 years ago • 24 comments
trafficstars

Right now, we support Maps, Lists and values in our data. What should happen if someone does:

table("people") |> insert(%Person{name: "Joe"}) |> run conn

Options are:

  1. Treat it as a map. Downside: when we pull it back out of the database we won't be able to parse it into a map.
  2. Mirror Poison's approach. Allow an as: Foo option to run that will parse the data into the correct struct.
  3. Don't allow structs at all. You must convert it to a map before inserting.
  4. Builder Pattern version of number 2 above. In the module where you define a struct, call use RethinkDB.StructSupport and it will define a Foo.parse that will set a flag in the %Query{} with the struct type that should be parsed.

I feel like 3 > 1 on grounds of explicit > implicit.

I feel like 4 requires 2 and is just a fancier syntax.

I think a combination of 2 and 4 should fit the pattern of everything else we're building. We provide the generic function and also provide a fancy macro solution to make things cleaner.

hamiltop avatar May 18 '15 05:05 hamiltop

I like the nr 2 approach

vorce avatar Aug 05 '15 14:08 vorce

Struct support would be very useful.

clessg avatar Sep 17 '15 02:09 clessg

@clessg Could you give me a few examples of how you might use them?

hamiltop avatar Sep 17 '15 04:09 hamiltop

Well, I'm new to Elixir, so I imagine having a User struct, for example.

But I suppose that structs are basically maps with a couple extra features, so there's no reason I can't just have a User module with functions that act on those maps. Would lose out on default attributes and pattern matching but it's probably not a big deal.

I'm happy with whatever is decided on!

clessg avatar Sep 17 '15 23:09 clessg

So it sound like you would like to be able to insert a struct. That's easy to do. You could either call Map.from_strict and then insert it or we can make this library do that automatically.

How about reading from the db? What would you expect to do there? While struct(User, data) works in theory, in practice keys in a rethinkDB response are strings, not atoms, and would be filtered out. So something to solve that would be handy.

On Thu, Sep 17, 2015, 4:54 PM Chris Gaudreau [email protected] wrote:

Well, I'm new to Elixir, so I imagine having a User struct, for example.

But I suppose that structs are basically maps with a couple extra features, so there's no reason I can't just have a User module with functions that act on those maps. Would lose out on default attributes and pattern matching but it's probably not a big deal.

I'm happy with whatever is decided on!

— Reply to this email directly or view it on GitHub https://github.com/hamiltop/rethinkdb-elixir/issues/45#issuecomment-141288138 .

hamiltop avatar Sep 18 '15 00:09 hamiltop

Oh, yeah, you're right about that apparently. In that case, I think a way to serialize to and from a struct would be a good idea. I'm still new to Elixir so I don't really know what would be the most idiomatic way to do so, though!

clessg avatar Sep 18 '15 02:09 clessg

How about a syntax like:

table("users") |> as_struct(User) |> run

What would it return?

Option 1:

%RethinkDB.Collection{data: [%User{id: ...}, %User{id: ...}])

Option 2:

[%User{id: ...}, %User{id: ...}]

Alternatively, the as_struct call could be moved to after run:

table("users") |> run |> as_struct(User)

That way, we don't need to change the semantics and return value of run.

Thoughts?

hamiltop avatar Sep 18 '15 05:09 hamiltop

My understanding is that the stuff before run is typically part of a query sent to the database (?), so my initial inclination was to have the as_struct call after run.

But I'm not sure what would happen in the case of an error in the query. Also, it might be easier to perform optimizations with as_struct before run.

clessg avatar Sep 18 '15 13:09 clessg

That's a good point. I could probably allow it anywhere in the query as well as after run.

As far as inserting structs, any reason not to automatically call Map.from_struct ?

On Fri, Sep 18, 2015, 6:31 AM Chris Gaudreau [email protected] wrote:

My understanding is that the stuff before run is typically part of a query sent to the database (?), so my initial inclination was to have the as_struct call after run.

But I'm not sure what would happen in the case of an error in the query. Also, it might be easier to perform optimizations with as_struct before run.

— Reply to this email directly or view it on GitHub https://github.com/hamiltop/rethinkdb-elixir/issues/45#issuecomment-141452279 .

hamiltop avatar Sep 18 '15 15:09 hamiltop

I see no reason not to have Map.from_struct called automatically, other than some potential initial confusion, but I doubt it's a big deal.

clessg avatar Sep 18 '15 15:09 clessg

Another question around as_struct will be what to do when an unknown key is encountered. Should it error? or ignore it?

hamiltop avatar Sep 19 '15 02:09 hamiltop

I think ignoring it makes the most sense. That's certainly what I'm used to in other languages, but Erlang/Elixir is definitely its own beast.

clessg avatar Sep 19 '15 12:09 clessg

I think I'm convinced that Ecto query building doesn't make sense for this driver. However, I think validations and other aspects of Ecto will be useful. I think this issue ties in with that.

Perhaps a simple use RethinkDB.Model might be useful, which defines CRUD for the struct and auto parses it.

hamiltop avatar Oct 16 '15 21:10 hamiltop

I think I'm going to add a protocol for RethinkDB.Encoder. It will include an encode and a decode function. It will also include a derive macro. This enables a few useful things:

  1. cleans up internal code around handling arrays, functions, etc.
  2. allows user to specify how to handle structs while providing sensible default via derive
  3. an implementation for Changeset can be included in a rethinkdb_ecto.

Encode will be called by default, but decode will still need some user input about what to decode to. I'm still not sure about how to do a List of Structs. Maybe two functions, result_as and each result_as, where the latter expects the result to be a List and will decode each. A corner case that could be supported in result_as might be [TypeA, TypeB, TypeC] which would strictly mean expect a 3 element list and decode each element into corresponding types. It's a weird case, so probably not worth building. Alternatively we can just do result_as and if the return type is a collection we would apply decode to each element.

result_as can come before or after run in a query, but will execute after the result is returned.

Going to put this up in a branch.

hamiltop avatar Nov 03 '15 17:11 hamiltop

As any though been given to create a project on top of this one to provide all the goodies so to speak that avoid some boilerplates ? In my opinion, the driver should try keep its focus rather narrow and its perimeter should be well defined.

Ideally, the project on top of the driver should provide:

  • Struct support (Schema definition with validation and constraints, similiar to Ecto.Schema & Ecto.Changeset)
  • Automatically create tables and indexes.
  • Associations (hasOne, belongsTo, hasMany and hasAndBelongsToMany)
  • Life-cycle callbacks
  • Query API must be composable and allow to tap into rethinkdb driver api seamlessly

When you look at the feature list, it's very similar to Ecto so I understand the desire to have an adapter for it. It seems however that a dedicated query DSL would be needed to properly leverage RethinkDB.

tlvenn avatar Nov 05 '15 21:11 tlvenn

I'm currently exploring Ecto without Ecto.Query, will report on findings.

On Thu, Nov 5, 2015 at 1:23 PM Tlvenn [email protected] wrote:

As any though been given to create a project on top of this one to provide all the goodies so to speak that avoid some boilerplates ? In my opinion, the driver should try keep its focus rather narrow and its perimeter should be well defined.

Ideally, the project on top of the driver should provide:

  • Struct support (Schema definition with validation and constraints, similiar to Ecto.Schema & Ecto.Changeset)
  • Automatically create tables and indexes.
  • Associations (hasOne, belongsTo, hasMany and hasAndBelongsToMany)
  • Life-cycle callbacks
  • Query API must be composable and allow to tap into rethinkdb driver api seamlessly

When you look at the feature list, it's very similar to Ecto so I understand the desire to have an adapter for it. It seems however that a dedicated query DSL would be needed to properly leverage RethinkDB.

— Reply to this email directly or view it on GitHub https://github.com/hamiltop/rethinkdb-elixir/issues/45#issuecomment-154196828 .

hamiltop avatar Nov 05 '15 21:11 hamiltop

Great to hear ! :+1:

tlvenn avatar Nov 06 '15 07:11 tlvenn

@Tlvenn Can you take a bit of time and play with https://github.com/hamiltop/rethinkdb_ecto ? It's super minimal, but I think it accomplishes what we want. It should support all the callbacks and validation goodness you want.

hamiltop avatar Nov 07 '15 17:11 hamiltop

Hi @hamiltop, sorry for the late feedback.

Thanks for exploring this and I will definitely play with it soon. Regarding the feature set, when I look at what is left there, I think there is still some value that totally apply to Rethinkdb.

  • Repo abstraction is interesting and useful concept.
  • Migration is very important, just because we use a schemaless DB does not mean we dont have to migrate our data when we decide to change the shape of our documents and we do have to create our tables and indexes.
  • Associations is very handy and can only be leveraged by a query DSL, using Reql to do that all the time is tedious especially when you have multiple joins going on.
  • The query API of Ecto is really nice to work with, the fact that it is composable is just awesome and so much to the core of Elixir and what FP is all about. I dont think Reql is so friendly in that regard and when you look at what the Ecto Query API, I bet it would address 90% of the use cases. The important part imho, is that for the remaining 10% you can tap into Reql specifically if you need to.

I started to use rethinkdb from node.js land, and neumino did a fantastic job creating a very performant driver for it and then on top of it, a light very useful ORM (https://thinky.io/) which gets the hassle and ceremony out of the way and yet when you need it, allow you to harness all the power of RethinkDB.

I believe Ecto could be a very valuable platform to do that for RethinkDB given that we find a way to elegantly still have access to Reql should we need it.

In any case, thanks again, will play with rethinkdb_ecto and I will report back.

tlvenn avatar Nov 12 '15 10:11 tlvenn

Migrations are definitely necessary. That can be handled without Ecto.Query.

ReQL is pretty composable itself, though a little bit of capture/curry magic is sometimes necessary.

Thinky looks like a great reference. I'm a little concerned they created a while new ORM instead of hooking into an existing ORM. The analogous approach in Elixir would be to not use Ecto and build something custom tailored. I think I'll reach out to the Thinky creators and get their take, as I'm sure they considered the possibility of using a more general ORM.

Joins/associations are an area of RethinkDB I'm lacking experience in. It's also one of the areas I feel Ecto nailed (specifically forcing eager loading). I'll have to play with it a bit.

Overall, I have some PTSD from ActiveRecord that keeps me from blindly jumping into the Ecto boat. In Rails apps I've worked on, I've pretty much converted anything significant over to raw sql for performance and maintenance reasons. I'm probably biased by those experiences, so I greatly appreciate my view being challenged.

On Thu, Nov 12, 2015, 02:49 Tlvenn [email protected] wrote:

Hi @hamiltop https://github.com/hamiltop, sorry for the late feedback.

Thanks for exploring this and I will definitely play with it soon. Regarding the feature set, when I look at what is left there, I think there is still some value that totally apply to Rethinkdb.

Repo abstraction is interesting and useful concept.

Migration is very important, just because we use a schemaless DB does not mean we dont have to migrate our data when we decide to change the shape of our documents and we do have to create our tables and indexes.

Associations is very handy and can only be leveraged by a query DSL, using Reql to do that all the time is tedious especially when you have multiple joins going on.

The query API of Ecto is really nice to work with, the fact that it is composable is just awesome and so much to the core of Elixir and what FP is all about. I dont think Reql is so friendly in that regard and when you look at what the Ecto Query API, I bet it would address 90% of the use cases. The important part imho, is that for the remaining 10% you can tap into Reql specifically if you need to.

I started to use rethinkdb from node.js land, and neumino did a fantastic job creating a very performant driver for it and then on top of it, a light very useful ORM (https://thinky.io/) which gets the hassle and ceremony out of the way and yet when you need it, allow you to harness all the power of RethinkDB.

I believe Ecto could be a very valuable platform to do that for RethinkDB given that we find a way to elegantly still have access to Reql should we need it.

In any case, thanks again, will play with rethinkdb_ecto and I will report back.

— Reply to this email directly or view it on GitHub https://github.com/hamiltop/rethinkdb-elixir/issues/45#issuecomment-156070485 .

hamiltop avatar Nov 12 '15 15:11 hamiltop

I explored migrations in Ecto and it looks like we have to support Ecto.Query for managing the schema_versions table. Everything else we can handle using the shim.

I reached out to the author of Thinky and he agrees that shoehorning ReQL into SQL is a waste and will cause confusion.

I think I'm close to satisfied with the idea of providing a RethinkDB.Ecto.Repo as a drop in replacement for Ecto.Repo. Just a few warts to sort out.

Anyone interested in becoming actively involved, this is a huge opportunity. I'd love a buddy on this.

hamiltop avatar Nov 16 '15 05:11 hamiltop

Sounds good to me! I wish I had extra time to help, and also my Elxir is prob. not that good yet :smile:

I'd be more than happy to help out with testing. I'm using this in a slow moving greenfield app so i'm not too worried about the stability (worst case I could always fork it dev stops).

Changesets are the biggest feature for me! Such a great idea!

AdamBrodzinski avatar Nov 16 '15 20:11 AdamBrodzinski

@hamiltop I'm very interested in becoming actively involved and being your 'wingman' on this. Based on your presentation, I think that we share a similar interest in helping make the integration between RethinkDB and Elixir/Phoenix as solid as possible. If you think it makes sense, I can start with some documentation, testing and examples? Then I'll aim to gradually get more actively involved from there.

willykaram avatar Jan 04 '16 00:01 willykaram

@willykaram reach out to me via email. I'd love the help!

hamiltop avatar Jan 04 '16 00:01 hamiltop