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

Add failing testParameterMetaDataProcMarkers and update #1490

Closed

Conversation

garydgregory
Copy link

@garydgregory garydgregory commented Dec 22, 2020

Add failing testParameterMetaDataProcMarkers and update testParameterMetaDataProc.

  • ParameterMetaData methods don't throw exceptions on bad index input 0.
  • ParameterMetaData#getParameterCount() does not count all parameters when a call defines a return
    value.
  • ParameterMetaData return the wrong values when a call defines a return
    value.

If we can agree that these are indeed bugs, I can work on fixes.

These bugs makes it very hard to write JDBC compliant code. The other drivers we use: MySQL, JT400 (DB2), H2 do not fail these kinds of test.

@ghost
Copy link

ghost commented Dec 22, 2020

CLA assistant check
All CLA requirements met.

testParameterMetaDataProc.

- ParameterMetaData methods don't throw exceptions on bad index input.
- ParameterMetaData#getParameterCount() does not count all parameters.
- ParameterMetaData return the wrong values when a call defines a return
value.
@peterbae
Copy link
Contributor

peterbae commented Jan 4, 2021

Hi @garydgregory, we haven't had other users complain about it yet, but if our methods aren't JDBC compliant, please add the changes to the PR and we'll be sure to review them.

@lilgreenbird
Copy link
Contributor

hi @garydgregory

Apologies this one fell through the cracks.

The JDBC specs is a bit vague about this it just says that an exception will be thrown if a database access error occurs. However we agree that this is confusing an error should be returned if an invalid index is provided. This affects all the APIs with index params I have marked this as an enhancement we will look into addressing this in future releases thanks for bringing this to our attention.

@lilgreenbird lilgreenbird added the Enhancement An enhancement to the driver. Lower priority than bugs. label Sep 12, 2023
@garydgregory
Copy link
Author

hi @garydgregory

Apologies this one fell through the cracks.

The JDBC specs is a bit vague about this it just says that an exception will be thrown if a database access error occurs. However we agree that this is confusing an error should be returned if an invalid index is provided. This affects all the APIs with index params I have marked this as an enhancement we will look into addressing this in future releases thanks for bringing this to our attention.

Ty!

@lilgreenbird
Copy link
Contributor

lilgreenbird commented Sep 13, 2023

hi @garydgregory

The index issue was an easy fix and I can get that in for upcoming release. The other issues relating to return params looks to require a bit more work so I would have to leave that in the backlog as it will have to be triaged along with other bugs/features and to be honest with you I don't think this would make the cut any time soon as this doesn't look to be very high priority given that this behavior had been around since early days of the driver (I checked back as far as version 6.2.2).

The issue is that we call sp_spro_columns_100 here to determine the number of params and it's not counting the return param. Looking at the specs it's a bit vague just says "Retrieves the number of parameters in the PreparedStatement object for which this ParameterMetaData object contains information" doesn't mention anything about return params. Was curious so asked Bing "does ParamterMetadata#getParameterCount() include return parameters" it returned:

ParameterMetadata#getParameterCount() returns the number of input parameters in a prepared statement 1. It does not include the return parameters 2.`

I couldn't find any examples of this though most examples for getParameterCount don't have return params but I did find links to definitions of this API that it returns number of "formal" parameters so looks like this is open to interpretation.

You mentioned that other drivers have different behaviors curious if you know Oracle's behavior?

As mentioned above this behavior had been in place for the driver for quite a while so is potentially a breaking change and given the low priority it would unlikely be given much time allocated for this. However if you would like to contribute we will definitely review and considering including in the driver. I promise we will take a look at your PR and not let it slip next time :)

For the time being, I am going to close this PR as this PR as is isn't going to be merged however I've opened up a new issue #2216 to track this in our backlog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement An enhancement to the driver. Lower priority than bugs.
Projects
Status: Closed/Merged PRs
Development

Successfully merging this pull request may close these issues.

3 participants