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

fix: Handling of array ResultSet MetaData when the array is empty and binary protocol is being used. Fixes issue #421 #422

Merged
merged 5 commits into from Nov 13, 2015

Conversation

@davecramer
Copy link
Member

@davecramer davecramer commented Nov 10, 2015

No description provided.

davecramer added 3 commits Nov 10, 2015
The type of the empty array was not properly read as a result
getting the type of the array caused an NPE
fixes Issue #421 reported by Juha Komulainen
}
catch (NullPointerException ex)
{
assertTrue("should not get here",true);

This comment has been minimized.

@vlsi

vlsi Nov 10, 2015
Member

I would prefer more descriptive error messages.
Think how that would look like for the developer who would observe this error message: "should not get here".

Even though test name puts some context, it would make more sense to use throw new IllegalStateException("ars.getMetaData().getColumnType(1) failed for empty array in binary mode, however it should return ...", ex)

Note how your assertTrue hides original exception, making it hard to identify the root cause if any.

ResultSet ars = array.getResultSet();
try
{
ars.getMetaData().getColumnType(1); // this throws NPE

This comment has been minimized.

@vlsi

vlsi Nov 10, 2015
Member

Can you test the result value as well?

@vlsi
Copy link
Member

@vlsi vlsi commented Nov 10, 2015

I would be nice if you could rename PR

@davecramer davecramer changed the title fix: Issue #421 fix: Handling of array ResultSet MetaData when the array is empty and binary protocol is being used. Nov 10, 2015
@davecramer davecramer changed the title fix: Handling of array ResultSet MetaData when the array is empty and binary protocol is being used. fix: Handling of array ResultSet MetaData when the array is empty and binary protocol is being used. Fixes issue #421 Nov 10, 2015
don't mask the exception in the event that it is thrown
@davecramer
Copy link
Member Author

@davecramer davecramer commented Nov 10, 2015

Thanks for the comments

Dave Cramer

On 10 November 2015 at 11:19, Vladimir Sitnikov notifications@github.com
wrote:

I would be nice if you could rename PR


Reply to this email directly or view it on GitHub
#422 (comment).

if (!rs.wasNull())
{
ResultSet ars = array.getResultSet();
assertEquals(java.sql.Types.INTEGER,ars.getMetaData().getColumnType(1));

This comment has been minimized.

@vlsi

vlsi Nov 10, 2015
Member

Having explicit message in assertEquals is a big plus as it would explain failure without any need to look up the code: ars.getMetaData().getColumnType(1) should return Types.INTEGER, expected: X, actual: Y is much more readable than expected: X, actual: Y

davecramer added a commit that referenced this pull request Nov 13, 2015
fix: Handling of array ResultSet MetaData when the array is empty and binary protocol is being used. Fixes issue #421
@davecramer davecramer merged commit f71150e into pgjdbc:master Nov 13, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.