sqlx
sqlx copied to clipboard
added NextResultSet implementation
Trying to revive the solution for issue #380
Shamelessly used the stuff that @basvanbeek created in pr #453 but added some testing with postgres. This would also work with mssql but I didn't want to mess with the test environment.
@jmoiron, what is the preferred way of getting something like this included?
This project appears to have been abandoned I've merged this into my own fork here https://github.com/BenKnigge/sqly If anyone else would be interested in going through this pr black log and merging them into my fork the help would be welcomed.
This project appears to have been abandoned I've merged this into my own fork here https://github.com/BenKnigge/sqly If anyone else would be interested in going through this pr black log and merging them into my fork the help would be welcomed.
@BenKnigge Should we use your fork or https://github.com/go-sqlx/sqlx? I recall seeing your name link to that fork as well in another issue or PR comment.
@dlsniper, what kind of testing do you have in mind? I have used it with sqlserver alot, but I don't know how much work it would be to add it to the test environment.
Hi @kmpm I've tagged this as needs testing because it's something that would benefit having the automated CI tests in place before merging. I plan to integrate that in the extremely near future and then I think it will be good to go. I'll take a closer look at it in the next couple of weeks or so, there are plenty PRs to get to and this is more complex than some of the other ones.
Thank you for being so patient about it and for your contribution.
The most complex thing with this PR is actually the test. The funtional part of the code is actually quite straight forward .
Hi @dsniper, what kind of tests are you missing. Preferably there should be some tests for mssql but that is not one of the database engines in the default test environment for this project.
I recently ran into this problem (and I'm using MSSQL) and it'd be nice if NextResultSet invalidated the cache. sqlx lacks testing for MSSQL across the board so holding up this PR for MSSQL tests doesn't make much sense and would be nice to add those in a subsequent PR.
I have been dabbling with adding mssql into sqlx_test.go but I keep having issues with general database syntax differences like.
- mssql uses
SELECT TOP(1)...instead of ending with... LIMIT 1 - mssql cant compare a column with type
textwith string that's get cast into mssqlnvarchar - mssql does not have
natural join - probably more
Changes to the test framework would be far bigger than the changes to implement NextResultSet. It's not optimal but I do not have time or the "urge" to rewrite everything necessary to add mssql tests, I would rather not be using sqlx since that would actually save time in comparison to change the test framework.
If creating some non generic tests just for this that uses mssql I would like to have some directions on how you would like to have that done.
Is there anything that we are waiting for to merge this change? Changes to the test framework could be done as separated issue/task.
As for me this PR is ready to be merged. I'll gladly write some more tests later when and if the testing framework support mssql.