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

google_bigquery_job does not execute #6875

Closed
dboshardy opened this issue Jul 28, 2020 · 9 comments
Closed

google_bigquery_job does not execute #6875

dboshardy opened this issue Jul 28, 2020 · 9 comments
Assignees
Labels

Comments

@dboshardy
Copy link

dboshardy commented Jul 28, 2020

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment
  • If an issue is assigned to the "modular-magician" user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If an issue is assigned to a user, that user is claiming responsibility for the issue. If an issue is assigned to "hashibot", a community member has claimed the issue already.

Terraform Version

Affected Resource(s)

  • google_bigquery_job

Terraform Configuration Files (if applicable)

$ terraform --version
Terraform v0.12.26
Your version of Terraform is out of date! The latest version
is 0.12.29. You can update by downloading from https://www.terraform.io/downloads.html
resource "google_bigquery_job" "job" {
 job_id = "job_query"
 query {
 use_legacy_sql = false
 query = <<EOT
 DELETE FROM `${local.project}.${google_bigquery_dataset.dataset.dataset_id}.${google_bigquery_table.metrics.table_id}`
 WHERE ...
EOT

 }
 
}

Issue Description

When creating a query job with no destination, the terraform plan and apply creates the job according to tf log output, but the jobs never show up in the project or personal job history in the BQ UI.

@edwardmedia edwardmedia self-assigned this Jul 29, 2020
@edwardmedia
Copy link
Contributor

@dboshardy could you post your full debug log?

@dboshardy
Copy link
Author

I can't post our full log ,but looking at the debug trace, the google api returns a 200 response but the response contains this:

   "status": {
     "errorResult": {
       "reason": "invalidQuery",
       "location": "query",
       "message": "1.19 - 6.41: Unrecognized token DELETE.\n[Try using standard SQL (https://cloud.google.com/bigquery/docs/reference/standard-sql/enabling
-standard-sql)]"

So there's two issues. Looking at the debug trace, while the plan shows use_legacy_sql = false the create job request doesn't have any useLegacySql property in the configuration object.

Despite the 200 response, IMO this should return an error that the job creation was not successful due to the presence of the errorResult object. Additionally, it looks like the provider isn't adhering to the resource specified at least with regards to use_legacy_sql. It looks like the generated tests don't test for legacy sql setting.

@ghost ghost removed the waiting-response label Jul 29, 2020
@dboshardy
Copy link
Author

It looks like I'm not the first to encounter the use_legacy_sql bug, as this issue was closed without resolution in April:
#6231

@dboshardy
Copy link
Author

dboshardy commented Jul 29, 2020

It looks like this line is the culprit [0]. That will always return true for a false boolean value. That means this statement[1] always evaluates to false. So setting use_legacy _sql to true, will result in that field being populated in the configuration sent to the api, but use_legacy_sql false is not added to the transformed config and is thus excluded from the final request to the api.

[0]https://github.com/terraform-providers/terraform-provider-google/blob/master/google/transport.go#L29
[1]https://github.com/terraform-providers/terraform-provider-google/blob/master/google/resource_big_query_job.go#L1867

I'd be happy to contribute a PR for this, I'm just not sure about the purpose/utility of isEmptyValue in this particular case. Perhaps just removing that from the else if in [1] would suffice?

@edwardmedia
Copy link
Contributor

@dboshardy can you create a job without a destination using a different way? If you can, could you post the steps? Thank you

@dboshardy
Copy link
Author

Yes, if you use use_legacy_sql = true or left unspecified (default is true), and use a valid legacy sql query, it works. If you use a standard sql query, and use_legacy_sql = false the legacy sql setting is not honored. When applying, you get an error in the debug level, but not in info. The resource says it was created successfully, but it in fact errorred and was not created. As per my response earlier, debug logs show the job creation request returned a 200 but the response body had a a list of errors that meant the job was not created.

So this is, in fact, 2 issues:

  1. The use_legacy_sql parameter is not honored in the actual job creation just like google_bigquery_job resource ignores use_legacy_sql=false flag #6231 points out. And what [0] specifies as the cause.
  2. Job creation can fail but is treated as a success by terraform.

I would expect that if there was an error creating the BQ Job, the terraform resource creation should fail.

[0] #6875 (comment)

@ghost ghost removed the waiting-response label Aug 3, 2020
@dboshardy
Copy link
Author

dboshardy commented Aug 3, 2020

I have a fork that fixes these issues [0]. However, it opens up some more issues, primarily:

  1. DML statements cannot have a create_disposition or write_disposition set which is always set by default to the default values, CREATE_NEVER, WRITE_EMPTY.

Fixing this would require detecting DML language in the query and unsetting the defaults, removing them from the request.

[0] https://github.com/dboshardy/terraform-provider-google

@dboshardy
Copy link
Author

dboshardy commented Aug 3, 2020

After investigating, specifying create_disposition = "" and write_disposition = "" will properly create DML query jobs. My fork also contains a change to docs to note this.

I've created a PR with my fork @ndmckinley. I've tested and verified that it works for DML statements and that it honors the use_legacy_sql variable.

@github-actions
Copy link

github-actions bot commented Feb 4, 2022

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
3 participants