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

Adding ScanAllSets to the api #115

Merged
merged 7 commits into from
Jan 6, 2024
Merged

Conversation

kmpm
Copy link
Contributor

@kmpm kmpm commented Sep 1, 2023

I made a new function in the API for getting all result sets from sql.Rows.
My use case for this is a 3rd party application that uses mssql and have stored procedures returning multiple different results.
Possibly also a solution for #114

Tests can be run if you have a Sql Server up an running somewhere and run the tests with the tag with_mssql.

$ go test -tags with_mssql ./...

# if you need a custom connection string for the server you can use the MSSQL_URL environment variable
$ MSSQL_URL="sqlserver://user:pwd@dbhost" go test -tags with_mssql ./...

@codecov
Copy link

codecov bot commented Sep 3, 2023

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (80c2cc6) 82.74% compared to head (40f2666) 80.41%.

Files Patch % Lines
dbscan/dbscan.go 33.33% 13 Missing and 1 partial ⚠️
pgxscan/pgxscan.go 0.00% 3 Missing ⚠️
sqlscan/sqlscan.go 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #115      +/-   ##
==========================================
- Coverage   82.74%   80.41%   -2.34%     
==========================================
  Files           5        5              
  Lines         510      531      +21     
==========================================
+ Hits          422      427       +5     
- Misses         73       89      +16     
  Partials       15       15              
Flag Coverage Δ
unittests 80.41% <32.14%> (-2.34%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dbscan/dbscan.go Outdated Show resolved Hide resolved
@georgysavva
Copy link
Owner

Hi @kmpm, thank you for adding this new feature to the library. The code and tests look great.

Let me think about it from the design point of view for a bit, though. If I have any questions, I will let you know.

@kmpm
Copy link
Contributor Author

kmpm commented Nov 22, 2023

@georgysavva, you closed the issue I was refering to as completed so I kind of hoped you would have accepted the PR, but no.
How can I be of assistance to get this PR merged, or can I help to fix something similar, compatible with multiple result sets, with another design.

@georgysavva georgysavva merged commit d60e58c into georgysavva:master Jan 6, 2024
5 checks passed
@georgysavva
Copy link
Owner

@kmpm, thank you for your work, and sorry for taking so much time to merge your PR. Here is the new release containing this feature: https://github.com/georgysavva/scany/releases/tag/v2.1.0

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.

2 participants