bigpresh / Dancer-Plugin-Database

Dancer::Plugin::Database - easy database support for Dancer applications
http://search.cpan.org/dist/Dancer-Plugin-Database
37 stars 36 forks source link

Fetch specific columns using quick_select instead of entire row #12

Closed jadeallenx closed 12 years ago

jadeallenx commented 12 years ago

It was useful to me to fetch only specific columns using quick_select so I added the ability to specify an arrayref of column names to the end of the method. I also added some syntactic sugar called quick_lookup which returns a single scalar value (not an array/hashref of a row) which is handy for converting a username to a userid or something.

Thanks for this module. It's been really useful for my lil side project. Hope you like these changes.

By the way, I've added some flexibility to WHERE clauses by inspecting the 1st character of the value.

For example a first character of

'%" means "WHERE key LIKE %value",

'>' means WHERE key > value,

'<' means WHERE key < value

'!' means WHERE key IS NOT value

I have retained all of the meanings of {} where clauses and undef values to signify NULL. I also added the ability to use an arrayref as a WHERE value to signify a set operator as in

{ foo => ['bar', 'baz'] } which would mean

WHERE foo IN ('bar', 'baz')

but I haven't written the test cases for those extensions yet, so I haven't sent you a pull request yet, but if you want a sneak preview, they're in my fork of your lib. I will try to finish the test cases in the next couple days and will send a pull request when they pass.

Mark

bigpresh commented 12 years ago

Thanks for the pull request - at a quick glance these additions look excellent!

Part of me wants to avoid adding a new positional param to quick_select and allow it to be called with named params instead, but I think it's acceptable still in this case.

The additional operators for WHERE clauses look good - although I'm not entirely sure I like including the operator in with the data itself; I may re-design this slightly so you could pass a hashref stating the operator and a negation, maybe something like:

{ foo => { ge => 42 }                    # WHERE foo >= 42
{ foo => { like => '%bar%', not => 1 }   # WHERE foo NOT LIKE '%bar%'

I'll also have a look at how e.g. SQL::Abstract handles this.

bigpresh commented 12 years ago

Edited example above - if you're using LIKE, you probably want to be able to put % placeholders where you want, e.g. '%foo%', 'foo%', '%foo' etc.

Thanks to @jamesronan for pointing this out.

jadeallenx commented 12 years ago

Ultimately, using a hashref as the value is a cleaner way to go, I agree. I was just aiming to fit in some flexibility while keeping the idea of a simple scalar value as the way to pass in data. I'll go ahead and recode for a hashref value using keys of 'like' 'is' 'gt' 'lt' 'ge' 'le' and have 'not' as a bool for negation. I think we should also keep in the happy path of scalar value means '=', undef value means 'IS NULL', and an empty hashref means no WHERE clause. That way we won't break any existing code that might be out there. (Hopefully)

jadeallenx commented 12 years ago

Well, looking at SQL::Abstract just now makes me just want to outsource the whole business of building SQL from data structures lol - what do you think about that?

jadeallenx commented 12 years ago

I implemented my own logic, but SQL::Abstract still looks like a winner to me, in terms of power and flexibility. On the other hand, using this code avoids a lot of SQL::Abstract dependencies which may be attractive depending on your goal for this module - should it generally 'standalone' or rely on something like SQL::Abstract?

I kind of feel like where I'm at with this code is more than sufficient for my own personal needs / desires. I think it will cover a lot of the 'typical case' queries one might be tempted to toss in a Dancer app. I guess if you need more power, you're going to use something like DBIx::Class anyway?

bigpresh commented 12 years ago

Only been able to review it briefly, but it looks good - thanks for all your work!

I have been considering SQL::Abstract, but I would rather avoid the additional dependencies ideally, and I'm not a big fan of its interface (hash-like-arrays and references to arrays etc seem ugly to me, although I understand the reasoning behind the choices).

I think I need to have more of a think about this, and consider whether SQL::Abstract could be fitted in whilst maintaining backwards compatibility and a sane interface.

jadeallenx commented 12 years ago

I suppose you could make whether to use SQL::Abstract a configurable option for where processing. Like I said I'm pretty happy with where things are for my own needs - but I am hopeful you'll take some or all of my changes into your tree.

Thanks again for this module. It's saved me a lot of effort.

bigpresh commented 12 years ago

Just wanted to make it clear I haven't forgotten about this - I was away for a short holiday and am just catching up, but plan to test & merge this soon and get a new version out.

jadeallenx commented 12 years ago

That's cool - thanks for letting me know! :)