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

Tvp schema error fix. #38

Closed
wants to merge 1 commit into from
Closed

Conversation

gstojsic
Copy link
Contributor

This is in reference to issue (#37). Essentially the getParameterTypeName(int param) method returns only the type name. This causes the error when the TVP is in a schema different than dbo. I have taken the SS_TYPE_SCHEMA_NAME parameter returned by the sp_sproc_columns_100 stored procedure for the schema name and built the tvp name from that.

@xiangyushawn
Copy link
Contributor

xiangyushawn commented Nov 28, 2016

@gstojsic thank you very much for your contribution. As a design decision, we decided to support TVP with setStructured() method only. So, TVP does not work with setObject()

The fix that you provided is a real fix for the case with stored procedure. We can get the TVP schema from stored procedure. However, TVP should work with Prepared Statement (without stored procedure) as well. With Prepared Statement, setObject() does not work with ResultSet.

@gstojsic
Copy link
Contributor Author

I did not really understand the point of your comment. The fix is really for the case with the stored procedure which now does not work properly because it takes only the type name into account without the schema. The fix does not affect the Prepared statement case. Did you want me to provide the fix for the Prepared statement also?

Also I did not understand the first part. TVP's are handled in setObject methods as I can see from the source code. For example:
com.microsoft.sqlserver.jdbc.SQLServerPreparedStatement#setObject(int, java.lang.Object)
which is an implementation of java.sql.PreparedStatement#setObject(int, java.lang.Object) calls:
com.microsoft.sqlserver.jdbc.SQLServerPreparedStatement#setObjectNoType
which has a line:
if (JavaType.TVP == javaType)
that is obviously meant to handle TVP's. Did you mean you will be removing TVP handling from setObject() methods?

@xiangyushawn
Copy link
Contributor

@gstojsic I have talked with our team here, we all think the fix that you provided is great and we really appreciate it. After testing is done on our side, we will accept your PR and make TVP support setObject() with stored procedure. Also, thanks for pointing out we have implemented setObject() to handle TVP in the source code, we will make some changes there, to throw exception explicitly when setObject() is used with ResultSet.

@xiangyushawn xiangyushawn added Under Review Used for pull requests under review under testing labels Nov 29, 2016
@xiangyushawn
Copy link
Contributor

hello @gstojsic we very appreciate your idea on how to fix this issue, it is awesome. however, the fix is causing regression failures on our test lab. We will look into those regression failures and provide another fix based on your idea. For now, we are going to close this PR. Thank you again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Under Review Used for pull requests under review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants