dmfay / massive-js

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

Updatable Views should be "executable" and not only "queryable" #528

Closed terehov closed 6 years ago

terehov commented 6 years ago

Awesome library! Exactly what we were looking for, however... ;-)

We are building a Postgres-centric framework and would like to use massive.js as the underlying database access layer on the server (clients directly talk through our GraphQL query builder).

A major part of our security layer is however backed into PostgreSQL views. No user should have permission to access the underlying tables directly and only use (updatable) views for manipulations. Even-though Postgres has a concept of "updatable views", massive-js treats all views as read only ( queryable). Unfortunately thats a show-stopper for us. :-(

What would it take to make updatable views "executable"? Or what were your thought when you decided to design it that way. Thanks a lot!

cheers,

Eugene

dmfay commented 6 years ago

Easy answer on the last question -- Massive doesn't have updatable views because I've never needed updatable views, and this is the first time someone else has brought them up! :smile:

I'd absolutely be interested in that feature. My first thought is to simply make them Tables (probably renaming that class to Writable or some such), or if there's some functionality that doesn't work for views, to split the class. But before we get too far into that, you may want to evaluate PostGraphile as well since you're already building queries with GraphQL.

terehov commented 6 years ago

Hi Dean,

that sounds great. afaik there is no substantial difference between tables and updatable views. Would that be a big rewrite, or more or less just renaming and throwing together (tables and views)?

Thanks for mentioning PostGraphile. We were following PostgREST, PostGraphQL (PostGraphile) for quite a while and evaluated them for our use cases. They are both great frameworks and I admire what this guys are putting together. In fact, they gave us the initial idea to think this way, rather than keeping everything in Node, like pretty much every other framework out there. However there is a number of things that we need / decided to do differently (from an architectural point of view). And one of them is exactly the reason, why we need a lightweight ORM-like API on the server, that is not an ORM (I feel the same way about ORMs as you do). So even-though we think they are doing an amazing job, we decided to do some things differently and are quite far already. So it was probably a similar decision, that made you create massive instead of using one of the other great ORMs like TypeORM out there :-)

dmfay commented 6 years ago

I don't think it'd be too bad. The main issue is that reload in lib/database.js couples the object type with the loader (starting around line 226), so anything coming out of the "views" loader is a Queryable. Assuming all Table functionality is more or less meaningful for updatable views, the "views" loader SQL scripts (lib/scripts) could simply ignore them in the WHERE clause and then a new loader (lib/loaders) could bind them to the Table constructor. Once it's working and tested, then think about renaming Table; and the listViews and listTables functions should be updated, of course.

I am a little busy at the moment, but if you'd like to take point on this I'd be happy to answer questions and review pull requests!