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

fix: throw jobexception for invalid multiple statements query #732

Merged
merged 2 commits into from Sep 17, 2020

Conversation

stephaniewang526
Copy link
Contributor

Fixes b/163347694

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 15, 2020
@stephaniewang526 stephaniewang526 changed the title Walmart fix: throw jobexception for invalid multiple statements query Sep 15, 2020
add another test case

add exception checking Job class

update reload() method
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/java-bigquery API. label Sep 16, 2020
@codecov
Copy link

codecov bot commented Sep 16, 2020

Codecov Report

Merging #732 into master will increase coverage by 0.08%.
The diff coverage is 87.50%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #732      +/-   ##
============================================
+ Coverage     81.07%   81.15%   +0.08%     
- Complexity     1236     1240       +4     
============================================
  Files            78       78              
  Lines          6334     6341       +7     
  Branches        705      708       +3     
============================================
+ Hits           5135     5146      +11     
+ Misses          833      828       -5     
- Partials        366      367       +1     
Impacted Files Coverage Δ Complexity Δ
...y/src/main/java/com/google/cloud/bigquery/Job.java 85.90% <87.50%> (-0.01%) 34.00 <0.00> (+2.00) ⬇️
...n/java/com/google/cloud/bigquery/JobException.java 83.33% <0.00%> (+83.33%) 2.00% <0.00%> (+2.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2ca177...ba23ef6. Read the comment docs.

@stephaniewang526 stephaniewang526 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 16, 2020
Copy link
Contributor

@shollyman shollyman left a comment

Choose a reason for hiding this comment

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

Could you expand on what's going on here in the PR description? I see you're adding a new raise to the reload, is this to match behavior from something else?

@stephaniewang526
Copy link
Contributor Author

Could you expand on what's going on here in the PR description? I see you're adding a new raise to the reload, is this to match behavior from something else?

Indeed! Customer noticed that we are throwing Exception in invalid single-statement scenario but not invalid Multi-statement scenario. We are trying to make the behavior consistent here.

@stephaniewang526 stephaniewang526 merged commit 2a0d86d into googleapis:master Sep 17, 2020
gcf-merge-on-green bot pushed a commit that referenced this pull request Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the googleapis/java-bigquery API. cla: yes This human has signed the Contributor License Agreement. do not merge Indicates a pull request not ready for merge, due to either quality or timing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants