Feat: Support Interval Type #3416
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
product-auto-label
bot
added
size: xl
sagarwaal
force-pushed
the
interval_support
branch
from
5733d5f to
3f5bd80
Compare
sagarwaal
force-pushed
the
interval_support
branch
from
3f5bd80 to
df6ebe7
Compare
sagarwaal
changed the title
sagarwaal
force-pushed
the
interval_support
branch
from
d366c1c to
f9517a0
Compare
sagarwaal
force-pushed
the
interval_support
branch
from
f9517a0 to
f16ae42
Compare
rahul2393
reviewed
Oct 24, 2024
google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITIntervalTest.java
Show resolved
Hide resolved
sagarwaal
force-pushed
the
interval_support
branch
from
c201180 to
2b50235
Compare
sagarwaal
force-pushed
the
interval_support
branch
2 times, most recently
from
bea6426 to
b136a58
Compare
olavloite
reviewed
Nov 8, 2024
google-cloud-spanner/src/main/java/com/google/cloud/spanner/GrpcStruct.java
Show resolved
Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/Interval.java
Outdated
Show resolved
Hide resolved
Comment on lines
72
to
105
| /** Returns the nanoseconds component of the interval. */ | ||
| public BigInteger nanos() { | ||
| return BigInteger.valueOf(micros()) | ||
| .multiply(BigInteger.valueOf(NANOS_PER_MICRO)) | ||
| .add(BigInteger.valueOf(nanoFractions())); | ||
| } |
There was a problem hiding this comment.
Shouldn't this just return the nano fraction? (and then also be renamed?)
Currently, it seems that this method does the exact same as getAsNanos().
There was a problem hiding this comment.
nanoFractions() function returns fractional part of microseconds which is in range [-999, 999].
nanos() returns microsecond and nanoFractions() combined.
getAsNanos() returns all the components of Interval - months, days and nanos() combined as nanoseconds.
google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntervalTest.java
Outdated
Show resolved
Hide resolved
Comment on lines
175
to
179
| public void testEquals() { | ||
| Interval interval1 = Interval.fromMonthsDaysMicros(10, 20, 30); | ||
| Interval interval2 = Interval.fromMonthsDaysMicros(10, 20, 30); | ||
| assertEquals(interval1, interval2); | ||
| } | ||
|
|
||
| @Test | ||
| public void testHashCode() { | ||
| Interval interval1 = Interval.fromMonthsDaysMicros(10, 20, 30); | ||
| Interval interval2 = Interval.fromMonthsDaysMicros(10, 20, 30); | ||
| assertEquals(interval1.hashCode(), interval2.hashCode()); | ||
| } |
There was a problem hiding this comment.
These should cover more variations, and also include verifications for non-equality
There was a problem hiding this comment.
Updated tests for Equality and HashCode.
google-cloud-spanner/src/test/java/com/google/cloud/spanner/RandomResultSetGenerator.java
Outdated
Show resolved
Hide resolved
| import com.google.cloud.spanner.AbortedException; | ||
| import com.google.cloud.spanner.ResultSet; | ||
| import com.google.cloud.spanner.ResultSets; | ||
| import com.google.cloud.spanner.*; |
There was a problem hiding this comment.
nit: Google code style forbids the use of wildcard imports (https://google.github.io/styleguide/javaguide.html#s3.3.1-wildcard-imports)
google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITQueryTest.java
Outdated
Show resolved
Hide resolved
sagarwaal
force-pushed
the
interval_support
branch
from
b136a58 to
41b396c
Compare
sagarwaal
force-pushed
the
interval_support
branch
from
41b396c to
f9e6383
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> ☕️
If you write sample code, please follow the samples format.