nherment / seneca-postgresql-store

PostgreSQL plugin for Seneca MVP
MIT License
3 stars 11 forks source link

Changes to default limit & ability to select only the fields you want returned #18

Open dberesford opened 8 years ago

dberesford commented 8 years ago

@nherment @cristianianto @piccoloaiutante please review :-)

nherment commented 8 years ago

A bit of context on the limit$ param. It was put in place to prevent devs from creating bigger queries than they intended (eg: list of all users in the system displayed in a dropdown in the UI) and have the system just stop working when in production. It has happened a few times.

Putting the nolimit flag at the module level would unlock the limit for all queries regardless of their intent. I can understand the intent and reason for it though.

Cc'ing @AdrianRossouw

AdrianRossouw commented 8 years ago

I would personally remove the default limit$, increment the api to a breaking version and document how to set the default limit to the old value. The current mechanism seems to second guess the developer's intent, and behaves in the opposite manner to how postgres is expected to behave.

we've had bugs that end up being circumvented by adding limit$: 'all' to our .list$ calls. It happens fairly often, especially when dealing with migration scripts and the like.

Another thing to consider is the consistency between other store plugins.

If the limit$ behaviour is considered desireable, it should be implemented for all stores and not just this one. There are tests for all stores, and if it's necessary, it should be tested for.

That said, elasticsearch also has a 'soft' limit of 10 rows, but that's to do with it's default behaviour and not the store.

dberesford commented 8 years ago

@nherment yes I get the reasoning, just couldn't disagree more ;-) +1 on @AdrianRossouw's comments

piccoloaiutante commented 8 years ago

I agree with @AdrianRossouw, for the same two main reasons: I would too give a chance to developer to bypass the default limit and I would like to have a consistent behaviour across all storage.