Skip to content
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

feat: changes to support data, timestamp and arrays in IT tests #1840

Merged
merged 8 commits into from
Jul 5, 2022

Conversation

asthamohta
Copy link
Contributor

No description provided.

@asthamohta asthamohta requested a review from a team as a code owner April 20, 2022 10:30
@product-auto-label product-auto-label bot added size: l Pull request size is large. api: spanner Issues related to the googleapis/java-spanner API. labels Apr 20, 2022
@asthamohta asthamohta added the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 21, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 21, 2022
@asthamohta asthamohta added the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 21, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 21, 2022
Copy link
Collaborator

@olavloite olavloite left a comment

Choose a reason for hiding this comment

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

Looks good to me, with a couple of minor questions.

@@ -771,7 +767,7 @@ public void bindNumericArray_doesNotPreservePrecision() {

Struct row =
execute(
Statement.newBuilder("SELECT @v").bind("v").toNumericArray(asList(b1, b2, null)),
Statement.newBuilder(selectValueQuery).bind("p1").toNumericArray(asList(b1, b2, null)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test case seem to be disabled for PostgreSQL still. Is that intentional?
The same also seems to be the case for other numeric array tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

missed those, adding it

@@ -429,6 +522,59 @@ public void testReadNullValuesInArrays() {
}
}

@Test
public void testReadNullValuesInArraysPostgreSQL() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need separate tests for GoogleSQL and PostgreSQL in this case? They seem to be equal. Or am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

structs and json are not supported and column names are different as PG requires explicit quotes for case

@@ -615,9 +793,16 @@ public void testReadFloat64LiteralsGoogleStandardSQL() {
public void testReadFloat64LiteralsPostgreSQL() {
Assume.assumeTrue(dialect.dialect == Dialect.POSTGRESQL);
try (ResultSet resultSet =
databaseClient.singleUse().executeQuery(Statement.of("SELECT 10.0 AS float64"))) {
databaseClient
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this need a separate test case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as above

@asthamohta asthamohta added the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 25, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 25, 2022
@asthamohta asthamohta requested a review from a team as a code owner April 25, 2022 04:44
@asthamohta asthamohta added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 25, 2022
Copy link
Contributor

@ansh0l ansh0l left a comment

Choose a reason for hiding this comment

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

lgtm

@asthamohta asthamohta added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Jul 5, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jul 5, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 5, 2022
@asthamohta asthamohta merged commit c667653 into googleapis:main Jul 5, 2022
gcf-merge-on-green bot pushed a commit that referenced this pull request Jul 14, 2022
🤖 I have created a release *beep* *boop*
---


## [6.26.0](v6.25.7...v6.26.0) (2022-07-13)


### Features

* Adding two new fields for Instance create_time and update_time ([#1908](#1908)) ([00b3817](00b3817))
* changes to support data, timestamp and arrays in IT tests ([#1840](#1840)) ([c667653](c667653))
* Error Details Improvement ([c8a2184](c8a2184))
* Error Details Improvement ([#1929](#1929)) ([c8a2184](c8a2184))


### Bug Fixes

* enable longpaths support for windows test ([#1485](#1485)) ([#1946](#1946)) ([fd0b845](fd0b845))


### Dependencies

* update dependency com.google.cloud:google-cloud-trace to v2.3.0 ([#1934](#1934)) ([2813eb2](2813eb2))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
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 API. do not merge Indicates a pull request not ready for merge, due to either quality or timing. size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants