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

PR - Fix for #69 #70

Closed
wants to merge 2 commits into from
Closed

Conversation

akrock
Copy link
Contributor

@akrock akrock commented Oct 3, 2018

I searched but I was not able to figure out the recommended driver behavior for this case.

I opted to return -1 if more than int.MaxValue rows were mutated and an OverflowException was generated. This may be non-standard behavior and you should probably confirm what the expected behavior for a .NET driver is in this case.

Handle the OverflowExcpetion and return -1.
@akrock
Copy link
Contributor Author

akrock commented Oct 3, 2018

This PR addresses #69

@akrock akrock changed the title Fix for #69 PR - Fix for #69 Oct 3, 2018
@manigandham
Copy link
Contributor

@akrock Do you have a copy of the HTTP/JSON response that came back? If it's just a large number then the response parsing can (and should) be updated to parse that number as a long value instead.

@akrock
Copy link
Contributor Author

akrock commented Oct 3, 2018

Yeah you could also just request GetValue<long>

However you will still need to figure out what should be returned by the method in the case that it exceeds int.MaxValue since you are forced by the interface to return an int for ExecuteNonQuery

@howryu
Copy link
Contributor

howryu commented Oct 4, 2018

Seems that should be the way. This actually happens after update/delete/insert succeed in the server side. Exception is not a good idea, which will lead to user thinking that their insertion failed. But I was wondering if there is a better to inform user. Like in JDBC, there is a class call SqlWarning. https://docs.oracle.com/javase/7/docs/api/java/sql/SQLWarning.html I did not find anything similar in .net

@akrock
Copy link
Contributor Author

akrock commented Oct 5, 2018

Rewrote this PR to use GetValue instead of handling the overflow exceptions from GetValue, still returns -1 in the case that more than int.MaxValue rows were modified.

@ikudjoi
Copy link
Contributor

ikudjoi commented Nov 15, 2018

Hi, we collided into this problem last night, please accept this PR or find alternative solution, pretty please.

@ikudjoi
Copy link
Contributor

ikudjoi commented Nov 22, 2018

Hi, any comment on this @howryu?

@howryu
Copy link
Contributor

howryu commented Nov 22, 2018

@ikudjoi ThanksGiving now. I will fix it next Monday since I don't have access to my dev machine now. Sorry for that.

@ikudjoi
Copy link
Contributor

ikudjoi commented Nov 23, 2018

Fair enough! :)

@ikudjoi
Copy link
Contributor

ikudjoi commented Nov 28, 2018

Ping

@howryu
Copy link
Contributor

howryu commented Nov 28, 2018

Working on it now.

@howryu
Copy link
Contributor

howryu commented Nov 28, 2018

Merged in #95

@howryu howryu closed this Nov 28, 2018
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.

4 participants