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: Add fallback to setObject(int, Object) for Number #812

Merged
merged 1 commit into from May 19, 2017

Conversation

Projects
None yet
4 participants
@RobertZenz
Contributor

RobertZenz commented Apr 14, 2017

This pull request adds the fallback in question in #810.

It adds the following changes:

  • There is a new (private) setNumber(int, Number) method in PgPreparedStatement with the same functionality as setBigDecimal(int, BigDecimal).
  • setBigDecimal(int, BigDecimal) does invoke setNumber(int, Number).
  • A case has been added in setObject(int, Object) for when a Number of an unknown/unhandled type is given.
  • Unittests for this.

I've opted for the least intrusive way of adding this by simply adding another check "at the end" to see if the given Object is of the type Number. If yes, it is forwarded to setNumber(int, Number).

@vlsi

This comment has been minimized.

Member

vlsi commented Apr 14, 2017

Why did you implement autoboxing-related changes?
Can you elaborate?

@RobertZenz RobertZenz changed the title from Pgpreparedstatement setobject fix to fix: Add fallback to setObject(int, Object) for Number Apr 14, 2017

@RobertZenz

This comment has been minimized.

Contributor

RobertZenz commented Apr 14, 2017

@vlsi Should I purge that commit?

@codecov-io

This comment has been minimized.

codecov-io commented Apr 14, 2017

Codecov Report

Merging #812 into master will increase coverage by 0.02%.
The diff coverage is 75%.

@@             Coverage Diff             @@
##             master    #812      +/-   ##
===========================================
+ Coverage     65.27%   65.3%   +0.02%     
- Complexity     3520    3524       +4     
===========================================
  Files           166     166              
  Lines         15228   15232       +4     
  Branches       2466    2467       +1     
===========================================
+ Hits           9940    9947       +7     
+ Misses         4099    4098       -1     
+ Partials       1189    1187       -2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ab5ccb...1759c07. Read the comment docs.

@vlsi

This comment has been minimized.

Member

vlsi commented Apr 14, 2017

Should I purge that commit?

I would love to see just those changes that are required, tested, etc.
Please remove boxing-related changes and/or split them to a separate PR along with tests proving the need.

@RobertZenz RobertZenz force-pushed the sibvisions:pgpreparedstatement-setobject-fix branch from 7aac037 to ea71c42 Apr 14, 2017

@RobertZenz

This comment has been minimized.

Contributor

RobertZenz commented Apr 14, 2017

@vlsi I've purged said commit.

assertEquals(new BigDecimal("733"), rs.getBigDecimal(1));
pstruncate.execute();
// Test BigInteger

This comment has been minimized.

@vlsi

vlsi Apr 14, 2017

Member

Can you please split the test method to several ones and use relevant assert message? (see https://github.com/pgjdbc/pgjdbc/blob/master/CONTRIBUTING.md#test )

This comment has been minimized.

@RobertZenz

RobertZenz Apr 14, 2017

Contributor

I've split it into three methods, though I'm not sure if the two fallback tests shouldn't be one.

fix: Add fallback to setObject(int, Object) for Number
Add a fallback to the PgPreparedStatement.setObject(int, Object) method
which handles Number types. This allows support for BigInteger and
similar types (f.e. AtomicLong) which are not explicitly mentioned by
JDBC.

This fallback operates by invoking toString() on the Number, which is
the same method used by setBigDecimal(int, BigDecimal). Also,
setBigDecimal(int, BigDecimal) does now invoke setNumber(int, Number)
because they work the same way.

The tracking bug is #810.

@RobertZenz RobertZenz force-pushed the sibvisions:pgpreparedstatement-setobject-fix branch from ea71c42 to 1759c07 Apr 14, 2017

@RobertZenz

This comment has been minimized.

Contributor

RobertZenz commented Apr 28, 2017

Is there something missing in the request or can this be pulled as is?

@RobertZenz

This comment has been minimized.

Contributor

RobertZenz commented May 18, 2017

Ping

@davecramer

This comment has been minimized.

Member

davecramer commented May 18, 2017

LGTM, barring any objections I'll merge this tomorrow

@davecramer davecramer merged commit 5b9edb7 into pgjdbc:master May 19, 2017

2 checks passed

codecov/project 65.3% (+0.02%) compared to 4ab5ccb
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@vlsi vlsi added this to the 42.1.2 milestone May 19, 2017

davecramer added a commit to davecramer/pgjdbc that referenced this pull request Sep 19, 2017

fix: Add fallback to setObject(int, Object) for Number (pgjdbc#812)
Add a fallback to the PgPreparedStatement.setObject(int, Object) method
which handles Number types. This allows support for BigInteger and
similar types (f.e. AtomicLong) which are not explicitly mentioned by
JDBC.

This fallback operates by invoking toString() on the Number, which is
the same method used by setBigDecimal(int, BigDecimal). Also,
setBigDecimal(int, BigDecimal) does now invoke setNumber(int, Number)
because they work the same way.

The tracking bug is pgjdbc#810.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment