Separate `new` and `connect`
(this is just an idea for now)
When creating a new instance of a TinyTds::Client, it immediately tries to connect to the database. It might be a better pattern to separate the connect part to a new method that needs to be called separately.
For one, you could call escape on the Client without requiring a live connection to the server.
Second, we could investigate to hand off the GVL when calling dbopen and dbuse. Both of these functions communicate with the server. In case the connection is slow / bad, it blocks the current Ruby thread. I personally would feel more comfortable handing off the GVL within a method instead of when we initialize a new object in Ruby space.
Technically, the connect method on the client is already public. It would be more about removing it from initialize.
For one, you could call
escapeon theClientwithout requiring a live connection to the server.
Yup, completely agree. This has always been a massive pain point for me. Another approach would be to make escape a class method, so it doesn't even need an instance to be called.
Additionally... does it even need to be in C space? This seems extremely peculiar and not nearly as robust as you'd hope/assume. What if escape was just pure Ruby?
https://github.com/rails-sqlserver/tiny_tds/blob/25ab0bdbd4092fc24230f12e6ece6bcc3caaebff/ext/tiny_tds/client.c#L488-L491
I was wondering the same - I think it can be pure Ruby since it does not call anything that requires it to be in C space. And class method would also make sense. It could even be it's own separate small class (TinyTds::Encoder?)
had another thought that might make this separation possible without requiring a breaking change: we already track in the C code if the client is closed (two times actually .. sure this has a reason but need to look into it in more details). When calling execute, we could check if the client is closed, and call the connect code automatically. This would achieve what has been discussed above: escape does not require a live connection to the server.
I am still in favor of moving escape into it's separate function and out of C. But it is a first step in the right direction.