Skip to content

fix: fix getLong on NUMERIC#2420

Open
rayudu3745 wants to merge 1 commit intogoogleapis:mainfrom
rayudu3745:get_long_fix
Open

fix: fix getLong on NUMERIC#2420
rayudu3745 wants to merge 1 commit intogoogleapis:mainfrom
rayudu3745:get_long_fix

Conversation

@rayudu3745
Copy link
Collaborator

@rayudu3745 rayudu3745 commented Mar 18, 2026

Updated the getLong getter in JdbcResultSet for GoogleSQL NUMERIC columns to use spanner.getBigDecimal(spannerIndex) instead of getString(). This prevents an IllegalStateException generated by the underlying Spanner library

we call .toBigInteger() on the retrieved BigDecimal value to truncate fractional parts (e.g., 3.14 becomes 3).

Other integer getters (getByte, getShort, getInt) in JdbcResultSet and generic type conversions in JdbcTypeConverter have been updated accordingly

@rayudu3745 rayudu3745 requested a review from a team as a code owner March 18, 2026 11:54
@product-auto-label product-auto-label bot added size: s Pull request size is small. api: spanner Issues related to the googleapis/java-spanner-jdbc API. labels Mar 18, 2026
@rayudu3745 rayudu3745 force-pushed the get_long_fix branch 3 times, most recently from bc31c62 to ba09078 Compare March 18, 2026 13:18
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Mar 18, 2026
@rayudu3745 rayudu3745 requested a review from olavloite March 18, 2026 13:24
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Mar 20, 2026
}

try {
subject.getInt(PG_NUMERIC_COLINDEX_NAN);
Copy link
Collaborator

Choose a reason for hiding this comment

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

So one interesting thing regarding NaN is that the following assertion actually passes:

assertEquals(0, (int) Float.NaN)

Do you know what the PostgreSQL JDBC driver returns in a case like this:

  1. Does it return zero (which would be consistent with how Java casts a NaN to int)?
  2. Or does it throw an error like here?

assertEquals("NaN", subject.getString(PG_NUMERIC_COLINDEX_NAN));
try {
subject.getByte(PG_NUMERIC_COLINDEX_NAN);
fail("missing expected SQLException");
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: can we use assertThrows(...) for these assertions instead of this try-fail construct. I know that there are many of them in this project, but that is because the original JDBC driver was written when Java 7 was the minimum supported version, which did not support assertThrows(..) (as it did not support lambda expressions)

Comment on lines +62 to +66
try {
Thread.sleep(1500); // Give gRPC server time to fully initialize
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this really needed? I don't remember having seen any issues with this in the past for this repository. But it might be that recent versions of the emulator are more sensitive to this. (Could we maybe make it a bit more dynamic than always sleeping for 1.5 seconds?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: spanner Issues related to the googleapis/java-spanner-jdbc API. size: l Pull request size is large.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants