Skip to content
This repository was archived by the owner on Mar 26, 2026. It is now read-only.

fix: fix getLong on NUMERIC#2420

Merged
rayudu3745 merged 2 commits intogoogleapis:mainfrom
rayudu3745:get_long_fix
Mar 23, 2026
Merged

fix: fix getLong on NUMERIC#2420
rayudu3745 merged 2 commits intogoogleapis:mainfrom
rayudu3745:get_long_fix

Conversation

@rayudu3745
Copy link
Copy Markdown
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
Copy Markdown
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?

Copy link
Copy Markdown
Collaborator Author

@rayudu3745 rayudu3745 Mar 23, 2026

Choose a reason for hiding this comment

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

verified pgdriver behaviour, it also throws exception for "NaN" for these methods

assertEquals("NaN", subject.getString(PG_NUMERIC_COLINDEX_NAN));
try {
subject.getByte(PG_NUMERIC_COLINDEX_NAN);
fail("missing expected SQLException");
Copy link
Copy Markdown
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)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

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
Copy Markdown
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?)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

@rayudu3745 rayudu3745 requested a review from olavloite March 23, 2026 06:42
@rayudu3745 rayudu3745 merged commit cb10252 into googleapis:main Mar 23, 2026
26 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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