karamaan / karamaan-opaleye

Other
11 stars 7 forks source link

Implement 'limit'. #64

Closed hesselink closed 10 years ago

hesselink commented 10 years ago

Seems to be as simple as the TODO promised :)

hesselink commented 10 years ago

I just realized that since this is small, and also useless without ordering, it might as well live in that module. If you agree, please hold off on this until ordering is merged. I'll then rebase this on top of that.

tomjaguarpaw commented 10 years ago

Makes sense to me.

hesselink commented 10 years ago

One thing I've noticed is that haskelldb allows you to limit multiple times, and consequently so does my simple implementation of limit in opaleye. This means that if you do limit 1 $ limit 2 someQuery you get sql like ... LIMIT 1 LIMIT 2, which is invalid. This should probably be fixed in haskelldb, right?

tomjaguarpaw commented 10 years ago

Well spotted. I agree it should be fixed in HaskellDB.

tomjaguarpaw commented 10 years ago

Ready for this now, although on reflection I don't mind creating a new module just for this and offset. I'll leave it up to you.

hesselink commented 10 years ago

I still think it makes sense for these things to all be in the same module. Limiting or offsets without ordering is not really possible, since it's undefined what records will be returned. By putting them in separate modules, we'd just force people to import two modules when they want to limit.

I've rebased this on top of the current dev branch.

tomjaguarpaw commented 10 years ago

Looks great, and your rationale for putting them in the same module is a good one.