dmfay / massive-js

A data mapper for Node.js and PostgreSQL.
2.49k stars 158 forks source link

Decompose insert queries #522

Closed mike-engel closed 6 years ago

mike-engel commented 6 years ago

First off, thanks for this awesome project. It's so much better than other SQL/pg ORMs I've used so far.

I'm wondering if it's possible to decompose insert queries, or if I've missed something really obvious. I've tried a couple variations with no luck. Let me know if you need a more in depth code sample or other detail. Thanks!

postgres: 10.1 massive: 4.6.2

Table schema

                                 Table "public.ratings"
    Column    |            Type             | Collation | Nullable |      Default       
--------------+-----------------------------+-----------+----------+--------------------
 id           | uuid                        |           | not null | uuid_generate_v4()
 user_id      | uuid                        |           |          | 
 photo_id     | uuid                        |           | not null | 
 detail       | text                        |           | not null | 
 composition  | numeric                     |           | not null | 
 color        | numeric                     |           | not null | 
 lighting     | numeric                     |           | not null | 
 storytelling | numeric                     |           | not null | 
 needs_review | boolean                     |           | not null | false
 created_at   | timestamp without time zone |           | not null | now()
 updated_at   | timestamp without time zone |           | not null | now()

Code

return db.pool.ratings.insert(
  {
    composition,
    color,
    lighting,
    detail,
    storytelling,
    photo_id: photoId,
    user_id: userId,
    needs_review: needsReview
  },
  {
    decompose: {
      pk: "id",
      columns: {
        composition: "composition",
        color: "color",
        lighting: "lighting",
        storytelling: "storytelling",
        detail: "detail",
        photoId: "photo_id",
        userId: "user_id",
        needsReview: "needs_review"
      }
    }
  }
);

Desired output

{
  id: "11111111-1111-1111-1111-111111111111",
  composition: 5.0,
  color: 5.0,
  lighting: 5.0,
  storytelling: 5.0,
  detail: "Really nice and good",
  photoId: "22222222-2222-2222-2222-222222222222",
  userId: "33333333-3333-3333-3333-333333333333",
  needsReview: false
}

Actual output

{
  id: "11111111-1111-1111-1111-111111111111",
  composition: 5.0,
  color: 5.0,
  lighting: 5.0,
  storytelling: 5.0,
  detail: "Really nice and good",
  photo_id: "22222222-2222-2222-2222-222222222222",
  user_id: "33333333-3333-3333-3333-333333333333",
  needs_review: false
}
dmfay commented 6 years ago

You're using flat decomposition schemas to rename resultset columns? That's creative!

Decomposition is supposed to work for all statements, but it looks like inserts at least don't preserve that key in the options object. I won't be able to get to it until this evening at the earliest but if you want to take a shot at it before then, check out the first few lines of the constructor in lib/statement/insert.js. It should be a one-line fix to preserve options.decompose and then it's just a matter of validating that with a testcase and making sure update and delete (destroy) statements do the same.

mike-engel commented 6 years ago

@dmfay thanks for the quick response! I won't be able to get to it until tonight either, but I'm more than happy to make an attempt. It sounds pretty easy, so I'll see if I can throw a PR together tonight ๐Ÿ˜„

dmfay commented 6 years ago

I took a second look and it turns out it's a little more involved than I was thinking :grimacing: Try pulling the decompose branch and see how that works for you. You'll probably want to include the id in your columns since that also acts as a filter ;)

mike-engel commented 6 years ago

@dmfay Thanks so much for taking the timeโ€”it works perfectly! I, for some reason, had the column's backwards (key was value, value was key) which caused some confusion. Reading the docs again though it makes sense.

As for the bit about needing id in the columns object, it wasn't necessary unless I wanted to return the id through the api, which I don't need to right now. So, leaving it off was actually ok!

If you are ever in need of any eyes on stuff or feedback, let me know. I'm very excited about this project. I'll also keep an eye on issues I think I could tackle to help out where I can ๐Ÿ˜„

dmfay commented 6 years ago

Awesome! I've released 4.6.3 so you should be able to get the update from npm.

Contributions are welcome if you're so inclined. I try to keep Massive relatively minimal but the goal is for it to be useful first and foremost. I have actually wanted to pick users' brains about the documentation for a while, though: too much? Too little? Does the organization make sense? Were any areas in particular difficult?

mike-engel commented 6 years ago

Thanks for the release! I think the amount of documentation is actually good. I have noticed a couple of broken links, so I'll try and go through and fix those. I think the only thing I'd change is have a consistent way to document the project. It seems like it's split between two documentation systems? One is more user friendly and one is more technical. Maybe that's ok though?

dmfay commented 6 years ago

Yeah, I'm kind of conflicted about the split. I wrote the readme to provide a more gradual introduction building on the basics but it still more or less doubles the amount of effort that has to go into documentation.

mike-engel commented 6 years ago

I think it's definitely worth it. Documentation is hard, but has the most impact ๐Ÿ˜‰