Skip to content
This repository was archived by the owner on Aug 13, 2021. It is now read-only.

Change function count#65

Open
grin0c wants to merge 1 commit intobalderdashy:masterfrom
grin0c:master
Open

Change function count#65
grin0c wants to merge 1 commit intobalderdashy:masterfrom
grin0c:master

Conversation

@grin0c
Copy link

@grin0c grin0c commented Oct 27, 2015

If no conditions LIMIT and OFFSET, your request will be like this:
SELECT COUNT(*) as count FROM (SELECT * FROM "tablename" ) AS "tablename".

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this comment is out of date now

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, in the former case at least.

@devinivy
Copy link
Contributor

Looks good to me! Can we do better with just a little more effort? I think we can use the new strategy whenever there's no limit/skip/sort. Basically, the new case you added should be able to withstand a where clause.

@grin0c
Copy link
Author

grin0c commented Oct 28, 2015

What you mean to do better with less effort? These changes have done better and minimal changes.

@devinivy
Copy link
Contributor

Didn't mean to make it sound like this change was unappreciated, so I hope it didn't come off that way! We're happy to have it. I just meant that we may be able to use the optimized query (the one you added) in more cases by putting in a little more time to write code that covers those cases. Looking into it, that might be a more complicated change than I had anticipated. This looks good. @particlebanana mind peekin'?

@anotherpit
Copy link
Contributor

Bump!

@devinivy devinivy mentioned this pull request Dec 14, 2015
@particlebanana
Copy link
Contributor

Can we get a test added for this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Development

Successfully merging this pull request may close these issues.

5 participants