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

Fix count_all_results on SQLSRV with query cache #5168

Closed
wants to merge 2 commits into from
Closed

Fix count_all_results on SQLSRV with query cache #5168

wants to merge 2 commits into from

Conversation

addbee
Copy link

@addbee addbee commented Jun 30, 2017

Fix for count_all_results on MSSQL when using start_cache

  • usage scenario, pagination

Fix for count_all_results on MSSQL when using start_cache
- usage scenario, pagination
@narfbg
Copy link
Contributor

narfbg commented Jul 3, 2017

Explain what's broken please.

Required change when reset_select is called but cache is on
@addbee
Copy link
Author

addbee commented Jul 3, 2017

Using MSSQL / SQLSRV Driver with SQL Azure

Previously:
When using start_cache() with a query having an order_by() clause, and then performing count_all_results() this resulted in an SQL error (MSSQL specific, due to the order by in a count(*))

This is worked around for regular queries with the qb_orderby being overridden in count_all_results but the compile select uses the qb_cache_orderby when cache is enabled

After the change:
qb_cache_orderby is emptied while the count is performed, then reinstated.

narfbg added a commit that referenced this pull request Jul 3, 2017
@narfbg narfbg added the Bug label Jul 3, 2017
@narfbg
Copy link
Contributor

narfbg commented Jul 3, 2017

Hope you don't mind me hacking the fix myself ... I wanted to simplify the code.

Also, your patch isn't following our styleguide - please read it if you ever want to contribute again.

@narfbg
Copy link
Contributor

narfbg commented Oct 3, 2017

Well, as evident by #5282, both of our patches are erroneous.

While it is my fault that I didn't test mine, yours should've never been submitted if you haven't solved your own problem with it ... please don't do that again.

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

Successfully merging this pull request may close these issues.

2 participants