Skip to content

java: Fix handling of SQL NULL in Row class.#1461

Merged
enisoc merged 1 commit into
vitessio:masterfrom
enisoc:java-row-null
Jan 25, 2016
Merged

java: Fix handling of SQL NULL in Row class.#1461
enisoc merged 1 commit into
vitessio:masterfrom
enisoc:java-row-null

Conversation

@enisoc
Copy link
Copy Markdown
Contributor

@enisoc enisoc commented Jan 24, 2016

@erzel

Previously, getters that convert to primitive types
(like long getLong()) would throw NullPointerException if the value
was actually SQL NULL. To better match the expectations set by the JDBC
ResultSet interface, which we intend to mimic, we now will instead
return 0 when asked to convert SQL NULL to a primitive type.

To distinguish between 0 and SQL NULL, the JDBC ResultSet interface
provides two mechanisms, which we now support as well:

  1. Call wasNull() after getLong() returns 0.
  2. Call getObject(..., Long.class) instead of getLong()
    to get a (possibly-null) Long instead of a long.

See also: https://docs.oracle.com/javase/7/docs/api/java/sql/ResultSet.html

@erzel
Copy link
Copy Markdown
Contributor

erzel commented Jan 25, 2016

It seems strange to do it this way. For one thing, it's not thread-safe to ask about the "last" call.
Why not have the API getters return Native type wrappers (i.e. Integer instead of int) which can be null?
Then the client can test whether these are null or not before converting to a primitive type.

@enisoc
Copy link
Copy Markdown
Contributor Author

enisoc commented Jan 25, 2016

I agree it's strange, but it's the way JDBC does it. If we don't do it that way, we would have type-conversion methods that are 99% similar to JDBC. I think going that extra 1% is worth it for two reasons: (1) it's immediately familiar to anyone who's used JDBC, and (2) we are planning to wrap this in a JDBC-compatible interface anyway, and recommend that over our low-level interface whenever possible.

For users who want getters that return wrapped types (e.g. Long), they can get something equivalent to that by calling getObject(..., Long.class), which is the same way they would do it in JDBC. So I don't think we really lose anything by doing this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be 0 if null, too, to be consistent?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about that, but in my opinion the least ambiguous convention is "return null for any getter where it is possible to return null". This seems to be the convention followed by JDBC, which returns null for BigDecimal getBigDecimal().

@erzel
Copy link
Copy Markdown
Contributor

erzel commented Jan 25, 2016

Ok, I still think it's strange but it's up to you. I just have two comments which you can resolve as you wish. LGTM.

Previously, getters that convert to primitive types
(like `long getLong()`) would throw NullPointerException if the value
was actually SQL NULL. To better match the expectations set by the JDBC
ResultSet interface, which we intend to mimic, we now will instead
return 0 when asked to convert SQL NULL to a primitive type.

To distinguish between 0 and SQL NULL, the JDBC ResultSet interface
provides two mechanisms, which we now support as well:

1. Call `wasNull()` after `getLong()` returns 0.
2. Call `getObject(..., Long.class)` instead of `getLong()`
   to get a (possibly-null) `Long` instead of a `long`.
enisoc pushed a commit that referenced this pull request Jan 25, 2016
java: Fix handling of SQL NULL in Row class.
@enisoc enisoc merged commit ddac90d into vitessio:master Jan 25, 2016
@enisoc enisoc deleted the java-row-null branch January 25, 2016 23:04
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.

2 participants