Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

quick_count convenience method added #42

Merged
merged 3 commits into from Aug 7, 2013
Merged

quick_count convenience method added #42

merged 3 commits into from Aug 7, 2013

Conversation

Casao
Copy link
Contributor

@Casao Casao commented Aug 6, 2013

Added a quick_count feature.

I believe this was wishlisted in #13

* Added new _quick_query type, as the auto-quoting on a select was breaking the system.
* Hard coded to count(*) -- change to count a specific column?
@ambs
Copy link
Collaborator

ambs commented Aug 7, 2013

Hey. Was just preparing to implement it in the next few days.
I will review your pull request shortly.
Thank you
Alberto

@ambs
Copy link
Collaborator

ambs commented Aug 7, 2013

Seems good.
@bigpresh, do you mind to give it a quick look? You are more aware of the main code.
If you could just comment one 👍 I would be happy to merge it later.

@bigpresh
Copy link
Owner

bigpresh commented Aug 7, 2013

I'm a bit cautious whether it's currently too MySQL-specific - do PostgreSQL and SQLite also both return the result with a column named COUNT(*)?

A quick test confirms that MySQL and SQLite do; assuming PostgreSQL does too, I think it's good to merge.

Thanks @Casao for your contribution!

@ambs
Copy link
Collaborator

ambs commented Aug 7, 2013

Thank you, @bigpresh. Later tonight I'll do the merge.

@@ -336,6 +356,8 @@ sub _quick_query {
return $self->selectrow_hashref($sql, undef, @bind_params);
}

} elsif ($type eq 'COUNT') {
return $self->selectrow_hashref($sql, undef, @bind_params);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be safer (in case the column name returned isn't exactly COUNT(*)) and potentially faster, to just say return $self->selectrow_array($sql, undef, @bind_params); possibly?

(Using it in a scalar context means it returns undef on failure, just like your code is already doing.)

@ambs ambs merged commit 87b7b9c into bigpresh:master Aug 7, 2013
@ambs
Copy link
Collaborator

ambs commented Aug 7, 2013

Changed the code a little to use selectrow_array and merged (5812912). Thank you, @Casao.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants