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.

Sign up for GitHub

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

Jump to bottom

Support writing timestamp with timezone in BigQuery #17793

Merged
merged 1 commit into from
Jul 10, 2023
Merged

Support writing timestamp with timezone in BigQuery #17793

merged 1 commit into from
Jul 10, 2023

Conversation

regadas
Copy link
Contributor

@regadas regadas commented Jun 7, 2023 •
edited by ebyhr
Loading

Description

Relates to #13741

Release notes

(x) Release notes are required, with the following suggested text:

# BigQuery
* Add support for writing `timestamp with time zone` type. ({issue}`17793`)

@cla-bot cla-bot bot added the cla-signed label Jun 7, 2023
@regadas regadas requested review from ebyhr and wendigo June 7, 2023 20:40
@github-actions github-actions bot added the bigquery BigQuery connector label Jun 7, 2023
@regadas regadas force-pushed the 13741_timestamp_tz branch from 97124ac to 8568983 Compare June 9, 2023 03:19
@ebyhr ebyhr self-requested a review June 9, 2023 07:02
@ebyhr ebyhr dismissed their stale review June 9, 2023 07:02

Addressed.

@hashhar hashhar mentioned this pull request Jun 9, 2023
Open
7 tasks
@regadas regadas force-pushed the 13741_timestamp_tz branch 3 times, most recently from 7551a05 to 6d60de1 Compare June 9, 2023 18:37
@regadas regadas force-pushed the 13741_timestamp_tz branch from 6d60de1 to f69ff05 Compare June 12, 2023 18:18
@regadas regadas requested a review from ebyhr June 13, 2023 17:51
@regadas regadas force-pushed the 13741_timestamp_tz branch from f69ff05 to 24243d4 Compare June 14, 2023 00:50
@regadas regadas requested a review from ebyhr June 14, 2023 00:53
@regadas regadas force-pushed the 13741_timestamp_tz branch 2 times, most recently from b51b35c to 6674fe9 Compare June 14, 2023 19:02
@regadas regadas requested a review from ebyhr June 19, 2023 19:06
@regadas
Copy link
Contributor Author

regadas commented Jun 19, 2023

Hi @ebyhr can you take another look? thank you

@regadas regadas force-pushed the 13741_timestamp_tz branch from 6674fe9 to 4fdb7eb Compare June 19, 2023 23:30
@ebyhr
Copy link
Member

ebyhr commented Jun 19, 2023

/test-with-secrets sha=4fdb7eb72775cb8a3de06c0a243b78b9b6913a30

@github-actions
Copy link

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/5316983873

@ebyhr
Copy link
Member

ebyhr commented Jun 20, 2023 •
edited
Loading

@regadas Please confirm the above CI failure. Some failure come from BigQuery SLA, but TestBigQueryAvroConnectorTest.testDataMappingSmokeTest failure looks related to this change.

@regadas
Copy link
Contributor Author

regadas commented Jun 20, 2023

@ebyhr hm this seems a BQ related issue (probably due to resource availability for the project used). I'll rerun the tests on my side to confirm.

@regadas
Copy link
Contributor Author

regadas commented Jun 20, 2023

yup all good

[INFO] Tests run: 41, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 355.769 s - in io.trino.plugin.bigquery.TestBigQueryAvroTypeMapping
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 41, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  06:08 min (Wall Clock)
[INFO] Finished at: 2023-06-19T21:23:31-04:00
[INFO] ------------------------------------------------------------------------
[WARNING] 
[WARNING] Plugin validation issues were detected in 4 plugin(s)
[WARNING] 
[WARNING]  * com.hubspot.maven.plugins:dependency-scope-maven-plugin:0.10
[WARNING]  * org.jacoco:jacoco-maven-plugin:0.8.10
[WARNING]  * org.apache.maven.plugins:maven-dependency-plugin:3.5.0
[WARNING]  * org.codehaus.mojo:build-helper-maven-plugin:3.3.0
[WARNING] 
[WARNING] For more or less details, use 'maven.plugin.validation' property with one of the values (case insensitive): [BRIEF, DEFAULT, VERBOSE]
[WARNING]

@ebyhr
Copy link
Member

ebyhr commented Jun 20, 2023

this seems a BQ related issue

No, testDataMappingSmokeTest failure is related to this change.

@ebyhr
Copy link
Member

ebyhr commented Jun 20, 2023

[INFO] Tests run: 41, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 355.769 s - in io.trino.plugin.bigquery.TestBigQueryAvroTypeMapping

The test is wrong. Please confirm TestBigQueryAvroConnectorTest.

@regadas
Copy link
Contributor Author

regadas commented Jun 20, 2023

No, testDataMappingSmokeTest failure is related to this change.

unsure really because the logs do have

Caused by: com.google.cloud.bigquery.BigQueryException: 502 Bad Gateway

@ebyhr
Copy link
Member

ebyhr commented Jun 20, 2023

Please take a look at:

Error:  Failures: 
Error:    TestBigQueryAvroConnectorTest>BaseConnectorTest.testDataMappingSmokeTest:5078->BaseConnectorTest.testDataMapping:5108->BaseConnectorTest.lambda$testDataMapping$67:5101->AbstractTestQueryFramework.assertUpdate:405->AbstractTestQueryFramework.assertUpdate:410 » QueryFailed An internal error occurred and the request could not be completed. This is usually caused by a transient issue. Retrying the job with back-off as described in the BigQuery SLA should solve the problem: https://cloud.google.com/bigquery/sla. If the error continues to occur please contact support at https://cloud.google.com/support.
Error:    TestBigQueryAvroConnectorTest>BaseConnectorTest.testDataMappingSmokeTest:5078->BaseConnectorTest.testDataMapping:5113->AbstractTestQueryFramework.assertQuery:339 For query 20230620_001638_01284_uyiad: 
 SELECT row_id FROM test_data_mapping_smoke_timestamp_6__with_time_zone5njyehw04u WHERE rand() = 42 OR value = TIMESTAMP '1969-12-31 15:03:00.123456 +01:00'
not equal
Actual rows (up to 100 of 0 extra rows shown, 0 rows in total):
    
Expected rows (up to 100 of 1 missing rows shown, 1 rows in total):
    [sample value]

Error:    TestBigQueryAvroConnectorTest>BaseConnectorTest.testDataMappingSmokeTest:5078->BaseConnectorTest.testDataMapping:5113->AbstractTestQueryFramework.assertQuery:339 For query 20230620_001800_01293_uyiad: 
 SELECT row_id FROM test_data_mapping_smoke_timestamp_6__with_time_zone7n85c1xvdz WHERE rand() = 42 OR value = TIMESTAMP '2020-02-12 15:03:00 +01:00'
not equal
Actual rows (up to 100 of 0 extra rows shown, 0 rows in total):
    
Expected rows (up to 100 of 1 missing rows shown, 1 rows in total):
    [sample value]

@regadas
Copy link
Contributor Author

regadas commented Jun 20, 2023

Will do! the 502 could be a red herring.

is there a faster way to run testDataMappingSmokeTest? the other one is still going

@regadas regadas force-pushed the 13741_timestamp_tz branch from 4fdb7eb to e629ec4 Compare June 20, 2023 19:51
@regadas regadas force-pushed the 13741_timestamp_tz branch from e629ec4 to e47f908 Compare June 20, 2023 21:31
@regadas
Copy link
Contributor Author

regadas commented Jun 26, 2023

Hi @ebyhr can you take another look at this? Thank you 🙏

@ebyhr
Copy link
Member

ebyhr commented Jun 26, 2023

/test-with-secrets sha=e47f908d3ac06578266c586cff24d682512f1db3

@github-actions
Copy link

The CI workflow run with tests that require additional secrets finished as : https://github.com/trinodb/trino/actions/runs/5383558308

@wendigo
Copy link
Contributor

wendigo commented Jul 6, 2023

/test-with-secrets sha=e47f908d3ac06578266c586cff24d682512f1db3

@ebyhr
Copy link
Member

ebyhr commented Jul 7, 2023

It seems tests with secret didn't start correctly. I will re-trigger.
cc: @nineinchnick

@ebyhr
Copy link
Member

ebyhr commented Jul 7, 2023

/test-with-secrets sha=e47f908d3ac06578266c586cff24d682512f1db3

@nineinchnick
Copy link
Member

https://github.com/trinodb/trino/actions/runs/5476655936/jobs/9974512729#step:3:16
@wendigo permissions were not correct at that time.

@github-actions
Copy link

github-actions bot commented Jul 7, 2023

The CI workflow run with tests that require additional secrets finished as : https://github.com/trinodb/trino/actions/runs/5482967673

@nineinchnick
Copy link
Member

The workflow with secrets failed because there's an invalid condition, here's a fix: #18165

@wendigo wendigo requested a review from ebyhr July 7, 2023 10:10
@ebyhr ebyhr mentioned this pull request Jul 9, 2023
Closed
ebyhr
ebyhr reviewed Jul 10, 2023
View reviewed changes
plugin/trino-bigquery/src/test/java/io/trino/plugin/bigquery/BaseBigQueryTypeMapping.java
Comment on lines +548 to +549
testTimestampWithTimeZone("TIMESTAMP")
.execute(getQueryRunner(), bigqueryCreateAndInsert("test.timestamp_tz"));
Copy link
Member

Choose a reason for hiding this comment

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

#12346

@ebyhr ebyhr merged commit f2e6ebe into trinodb:master Jul 10, 2023
@github-actions github-actions bot added this to the 422 milestone Jul 10, 2023
@regadas regadas deleted the 13741_timestamp_tz branch July 11, 2023 12:50
@colebow colebow mentioned this pull request Jul 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebyhr ebyhr ebyhr left review comments

@wendigo wendigo Awaiting requested review from wendigo

Assignees
No one assigned
Labels
bigquery BigQuery connector cla-signed
Milestone
422
Development

Successfully merging this pull request may close these issues.
4 participants
@regadas @ebyhr @wendigo @nineinchnick