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

Cancel results for user automatically #563

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

andyundso
Copy link
Member

@andyundso andyundso commented Oct 16, 2024

Relates to #539

FreeTDS enforces a strict sequence of how queries are sent and results are retrieved. To summarize it very briefly:

  • Open a connection using dbopen.
  • Prepare a command buffer using dbcmd.
  • Send it to the server using dbsqlsend.
  • Acknowledge that the server acknowledge using dbsqlok.
  • Gather metadata using dbdbresults.
  • Fetch each row using dbnextrow or cancel the running results (dbcancel)
  • Close the client using dbclose if not cancelled.

You do not have to serialize / bind each retrieved result, but you still need to fetch them. This is mentioned in the sample code as well:

DB-Library doesn't insist every column — or even any column — be bound or otherwise retrieved into the application's variables. There is, however, one absolutely crucial, inflexible, unalterable requirement: the application must process all rows produced by the query.

The current implementation of do and insert perform the entire sequence. However, each works different: Since each lazy-loads each result from the server, it cannot perform the entire sequence as we do not want to fetch all results at once and the user is free to cancel the iterator early.

As mentioned on #539, the error Assertion conn->in_net_tds == NULL' likely occured when people did not call cancel on the result object, as the query is technically still pending from FreeTDS point-of-view.

FreeTDS also mentions:

Before the DBPROCESS can be used for another query, the application must either fetch all rows, or cancel the results and receive an acknowledgement from the server.

With this PR, in #execute, I check if we have to still acknowledge and fetch results prior to running another query, essentially doing the job of cancel automatically for the user.

This is not my initially proposed solution on #539. I have coded out that proposed solution as well in a different branch, but during that development, I imagined that people could run into issues when a query will produce a large result set. But it helped me to understand the request lifecycle of FreeTDS better in order to develop this solution.

@andyundso andyundso changed the title Refactor do, insert and each Cancel results for user automatically Oct 16, 2024
@andyundso andyundso force-pushed the acknowledge-all-results branch from dc5083a to 30f0541 Compare October 23, 2024 08:50
@andyundso andyundso marked this pull request as ready for review November 11, 2024 13:43
@andyundso
Copy link
Member Author

@aharpervc even if I am still waiting for a reply on the original issue, I think this PR is good to have look.

@andyundso andyundso requested a review from aharpervc November 11, 2024 13:44
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.

1 participant