v4: Refactor tiny_tds to avoid sharing DBPROCESS

[andyundso]

Background

In order for tiny_tds to communicate with a MSSQL server using FreeTDS, they provide a DBPROCESS struct to do so in C land. The interaction with DBPROCESS require to follow an exact sequence:

• 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 dbresults.
• Fetch each row using dbnextrow or cancel the running results (dbcancel)
• Close the client using dbclose if not cancelled.

Our insert and do method, currently implemented on the Result class, perform the entire sequence. However, with execute, this is intentionally not done to allow lazy-loading of results from the server. This can lead to errors, some intended, others not:

1. You get an error message if you try to make another query without requesting all results first (intended error). Although it can have unintended side-effects. For example, if you call find on Result, it will abort the each early, therefore not all results are consumed and you cannot start a new query - you have to initialise a new Client.

2. There is a scenario with threads where you force a crash, see the following Ruby code:

it 'raises error when query cannot be sent' do
  client = new_connection
  assert_client_works(client)
    
  thread1 = Thread.new do
    client.execute("WaitFor Delay '00:00:10'")
  end

  assert thread1.alive?    
  thread2 = Thread.new do
    assert_raises(TinyTds::Error) { client.execute("WaitFor Delay '00:00:02'") }
  end
    
  thread1.join
  thread2.join
end

This will result in the following crash:

ruby: mem.c:1202: tds_free_connection: Assertion `conn->in_net_tds == NULL' failed.
Aborted (core dumped)
rake aborted!

3. Technically, DBPROCESS as well as some metadata of ours (like dbsqlsent) is part of the client instance in C land. If the garbage collector decided to sweep away the Client instance, the results can no longer be consumed. A reproduction of this is provided in Segementation Fault when reading from a closed connection #435.

results = TinyTds::Client.new(opts).execute('SELECT 42 as answer_to_life')
GC.start
puts results.to_a

This will yield a segmentation fault, since the Client instance is unreachable from the point of view of the garbage collector, and it gets deallocated.

4. You can force a segmentation fault by closing the client before consuming the result. Closing the client deallocates the DBPROCESS as well as the metadata. See Segementation Fault when reading from a closed connection #435 for the reproduction script.

Proposed solution

The proposed solution in this PR removes the lazy-loading functionality of tiny_tds. insert, do and execute are moved to the Client class. The C code for the Result class is removed entirely, thus leading the Client class to have sole control over all C data structures. Result is now a PORO holding the results rows as well as couple of metadata, like fields.

Point 2 from the list above is only partially solved - you won't get the coredump anymore, but still a DBPROCESS dead alert by FreeTDS. My proposal in #563 also addressed this by just cancelling the running query. But now, since we no longer have the sharing of DBPROCESS, I think we can add a check and raise an exception if a query is already in progress (sqlsent == 1). Realistically, this could only happen if you do some funny stuff with Threads as I did in point 2.

[Copilot]

Pull Request Overview

This PR removes lazy-loading functionality from tiny_tds by refactoring the execute, do, and insert methods to be part of the Client class instead of the Result class. The Result class is now a pure Ruby object (PORO) that holds pre-computed results data, addressing several critical issues including thread safety problems, garbage collection segfaults, and connection state management issues.

• Moves query execution methods (execute, do, insert) from Result to Client class
• Removes all C code from the Result class, making it a pure Ruby object
• Changes execute method to use keyword arguments instead of options hash

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/thread_test.rb	Updates test calls to use new do method instead of execute().do pattern
test/test_helper.rb	Converts helper method calls from chained pattern to direct do/execute calls
test/schema_test.rb	Updates test to use new insert method on client
test/result_test.rb	Major refactor removing lazy-loading tests and updating to new eager-loading API
test/client_test.rb	Adds new tests for insert method and updates timeout tests
lib/tiny_tds/result.rb	Simplified to pure Ruby class with just attr_readers and enumerable interface
lib/tiny_tds/client.rb	Removes query_options instance variable as execution is now handled in C
ext/tiny_tds/tiny_tds_ext.h	Removes result.h include as Result class no longer has C implementation
ext/tiny_tds/tiny_tds_ext.c	Removes result initialization call
ext/tiny_tds/result.h	Empty file - C Result implementation removed
ext/tiny_tds/result.c	Empty file - C Result implementation removed
ext/tiny_tds/client.c	Major refactor adding result processing, value casting, and new method implementations
README.md	Updates documentation to reflect new API and removal of lazy-loading
CHANGELOG.md	Documents breaking changes in new version

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

The client state assertions are inconsistent with the new eager-loading behavior. After execute completes, both sqlsent? and canceled? should be false since the query is fully processed, but line 149 expects canceled? to be true.

[Copilot]

After each completes with the new eager-loading implementation, canceled? should be true (as results are automatically canceled after processing), but sqlsent? should be false. The assertion on line 158 incorrectly expects sqlsent? to be false when it should remain consistent with the canceled state.

Suggested change

	_(@client.sqlsent?).must_equal false
	_(@client.sqlsent?).must_equal true

[Copilot]

Use the standard PRId64 macro from inttypes.h instead of platform-specific format strings for better portability and maintainability.