Skip to content

Commit ab575d0

Browse files
authored
Avoid listing table data for destination of CREATE VIEW DDL queries. (#3469)
* Avoid listing table data for destination of CREATE VIEW DDL queries. CREATE VIEW DDL queries set a "destination table" field, but that table is actually a view. If you attempt to list rows on the view table, it results in an error. The fix is to avoid listing rows altogether if getQueryResults() says that a query has completed but the number of rows in the result set is undefined. * Applied google-java-format * Correct EmptyTableResult javadoc * Fix BigQueryImpl unit tests. * Fix JobTest unit tests. * Remove unused TableResult.
1 parent d900124 commit ab575d0

File tree

6 files changed

+238
-55
lines changed

6 files changed

+238
-55
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* Copyright 2018 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.cloud.bigquery;
18+
19+
import com.google.api.core.InternalApi;
20+
import com.google.cloud.PageImpl;
21+
22+
public class EmptyTableResult extends TableResult {
23+
24+
private static final long serialVersionUID = -4831062717210349819L;
25+
26+
/**
27+
* An empty {@code TableResult} to avoid making API requests to unlistable tables.
28+
*/
29+
@InternalApi("Exposed for testing")
30+
public EmptyTableResult() {
31+
super(null, 0, new PageImpl<FieldValueList>(null, "", null));
32+
}
33+
}

google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/Job.java

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import com.google.cloud.bigquery.BigQuery.QueryResultsOption;
2828
import com.google.cloud.bigquery.BigQuery.TableDataListOption;
2929
import com.google.cloud.bigquery.JobConfiguration.Type;
30+
import com.google.common.collect.ImmutableList;
3031
import java.io.IOException;
3132
import java.io.ObjectInputStream;
3233
import java.util.ArrayList;
@@ -154,7 +155,8 @@ public Job build() {
154155
* Checks if this job exists.
155156
*
156157
* <p>Example of checking that a job exists.
157-
* <pre> {@code
158+
*
159+
* <pre>{@code
158160
* if (!job.exists()) {
159161
* // job doesn't exist
160162
* }
@@ -173,7 +175,8 @@ public boolean exists() {
173175
* not exist this method returns {@code true}.
174176
*
175177
* <p>Example of waiting for a job until it reports that it is done.
176-
* <pre> {@code
178+
*
179+
* <pre>{@code
177180
* while (!job.isDone()) {
178181
* Thread.sleep(1000L);
179182
* }
@@ -196,7 +199,8 @@ public boolean isDone() {
196199
* 12 hours as a total timeout and unlimited number of attempts.
197200
*
198201
* <p>Example usage of {@code waitFor()}.
199-
* <pre> {@code
202+
*
203+
* <pre>{@code
200204
* Job completedJob = job.waitFor();
201205
* if (completedJob == null) {
202206
* // job no longer exists
@@ -208,7 +212,8 @@ public boolean isDone() {
208212
* }</pre>
209213
*
210214
* <p>Example usage of {@code waitFor()} with checking period and timeout.
211-
* <pre> {@code
215+
*
216+
* <pre>{@code
212217
* Job completedJob =
213218
* job.waitFor(
214219
* RetryOption.initialRetryDelay(Duration.ofSeconds(1)),
@@ -285,10 +290,24 @@ public TableResult getQueryResults(QueryResultsOption... options)
285290
QueryResponse response =
286291
waitForQueryResults(
287292
DEFAULT_JOB_WAIT_SETTINGS, waitOptions.toArray(new QueryResultsOption[0]));
288-
if (response.getSchema() == null) {
289-
throw new JobException(getJobId(), response.getErrors());
293+
294+
// Get the job resource to determine if it has errored.
295+
Job job = this;
296+
if (job.getStatus() == null || job.getStatus().getState() != JobStatus.State.DONE) {
297+
job = reload();
298+
}
299+
if (job.getStatus() != null && job.getStatus().getError() != null) {
300+
throw new JobException(
301+
getJobId(), ImmutableList.copyOf(job.getStatus().getExecutionErrors()));
290302
}
291-
303+
304+
// If there are no rows in the result, this may have been a DDL query.
305+
// Listing table data might fail, such as with CREATE VIEW queries.
306+
// Avoid a tabledata.list API request by returning an empty TableResult.
307+
if (response.getTotalRows() == 0) {
308+
return new EmptyTableResult();
309+
}
310+
292311
TableId table = ((QueryJobConfiguration) getConfiguration()).getDestinationTable();
293312
return bigquery.listTableData(
294313
table, response.getSchema(), listOptions.toArray(new TableDataListOption[0]));
@@ -356,15 +375,17 @@ public boolean shouldRetry(Throwable prevThrowable, Job prevResponse) {
356375
* Fetches current job's latest information. Returns {@code null} if the job does not exist.
357376
*
358377
* <p>Example of reloading all fields until job status is DONE.
359-
* <pre> {@code
378+
*
379+
* <pre>{@code
360380
* while (job.getStatus().getState() != JobStatus.State.DONE) {
361381
* Thread.sleep(1000L);
362382
* job = job.reload();
363383
* }
364384
* }</pre>
365385
*
366386
* <p>Example of reloading status field until job status is DONE.
367-
* <pre> {@code
387+
*
388+
* <pre>{@code
368389
* while (job.getStatus().getState() != JobStatus.State.DONE) {
369390
* Thread.sleep(1000L);
370391
* job = job.reload(BigQuery.JobOption.fields(BigQuery.JobField.STATUS));
@@ -384,7 +405,8 @@ public Job reload(JobOption... options) {
384405
* Sends a job cancel request.
385406
*
386407
* <p>Example of cancelling a job.
387-
* <pre> {@code
408+
*
409+
* <pre>{@code
388410
* if (job.cancel()) {
389411
* return true; // job successfully cancelled
390412
* } else {

google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/JobStatus.java

Lines changed: 21 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@
2222
import com.google.common.base.MoreObjects;
2323
import com.google.common.collect.ImmutableList;
2424
import com.google.common.collect.Lists;
25-
2625
import java.io.Serializable;
2726
import java.util.List;
2827
import java.util.Objects;
28+
import javax.annotation.Nullable;
2929

3030
/**
3131
* A Google BigQuery Job status. Objects of this class can be examined when polling an asynchronous
@@ -35,9 +35,7 @@ public class JobStatus implements Serializable {
3535

3636
private static final long serialVersionUID = -714976456815445365L;
3737

38-
/**
39-
* Possible states that a BigQuery Job can assume.
40-
*/
38+
/** Possible states that a BigQuery Job can assume. */
4139
public static final class State extends StringEnumValue {
4240
private static final long serialVersionUID = 818920627219751204L;
4341

@@ -49,18 +47,12 @@ public State apply(String constant) {
4947
}
5048
};
5149

52-
private static final StringEnumType<State> type = new StringEnumType(
53-
State.class,
54-
CONSTRUCTOR);
50+
private static final StringEnumType<State> type = new StringEnumType(State.class, CONSTRUCTOR);
5551

56-
/**
57-
* The BigQuery Job is waiting to be executed.
58-
*/
52+
/** The BigQuery Job is waiting to be executed. */
5953
public static final State PENDING = type.createAndRegister("PENDING");
6054

61-
/**
62-
* The BigQuery Job is being executed.
63-
*/
55+
/** The BigQuery Job is being executed. */
6456
public static final State RUNNING = type.createAndRegister("RUNNING");
6557

6658
/**
@@ -74,23 +66,19 @@ private State(String constant) {
7466
}
7567

7668
/**
77-
* Get the State for the given String constant, and throw an exception if the constant is
78-
* not recognized.
69+
* Get the State for the given String constant, and throw an exception if the constant is not
70+
* recognized.
7971
*/
8072
public static State valueOfStrict(String constant) {
8173
return type.valueOfStrict(constant);
8274
}
8375

84-
/**
85-
* Get the State for the given String constant, and allow unrecognized values.
86-
*/
76+
/** Get the State for the given String constant, and allow unrecognized values. */
8777
public static State valueOf(String constant) {
8878
return type.valueOf(constant);
8979
}
9080

91-
/**
92-
* Return the known values for State.
93-
*/
81+
/** Return the known values for State. */
9482
public static State[] values() {
9583
return type.values();
9684
}
@@ -112,35 +100,33 @@ public static State[] values() {
112100
this.executionErrors = executionErrors != null ? ImmutableList.copyOf(executionErrors) : null;
113101
}
114102

115-
116103
/**
117-
* Returns the state of the job. A {@link State#PENDING} job is waiting to be executed. A
118-
* {@link State#RUNNING} is being executed. A {@link State#DONE} job has completed either
119-
* succeeding or failing. If failed {@link #getError()} will be non-null.
104+
* Returns the state of the job. A {@link State#PENDING} job is waiting to be executed. A {@link
105+
* State#RUNNING} is being executed. A {@link State#DONE} job has completed either succeeding or
106+
* failing. If failed {@link #getError()} will be non-null.
120107
*/
121108
public State getState() {
122109
return state;
123110
}
124111

125-
126112
/**
127-
* Returns the final error result of the job. If present, indicates that the job has completed
128-
* and was unsuccessful.
113+
* Returns the final error result of the job. If present, indicates that the job has completed and
114+
* was unsuccessful.
129115
*
130-
* @see <a href="https://cloud.google.com/bigquery/troubleshooting-errors">
131-
* Troubleshooting Errors</a>
116+
* @see <a href="https://cloud.google.com/bigquery/troubleshooting-errors">Troubleshooting
117+
* Errors</a>
132118
*/
119+
@Nullable
133120
public BigQueryError getError() {
134121
return error;
135122
}
136123

137-
138124
/**
139125
* Returns all errors encountered during the running of the job. Errors here do not necessarily
140126
* mean that the job has completed or was unsuccessful.
141127
*
142-
* @see <a href="https://cloud.google.com/bigquery/troubleshooting-errors">
143-
* Troubleshooting Errors</a>
128+
* @see <a href="https://cloud.google.com/bigquery/troubleshooting-errors">Troubleshooting
129+
* Errors</a>
144130
*/
145131
public List<BigQueryError> getExecutionErrors() {
146132
return executionErrors;
@@ -164,8 +150,8 @@ public final int hashCode() {
164150
public final boolean equals(Object obj) {
165151
return obj == this
166152
|| obj != null
167-
&& obj.getClass().equals(JobStatus.class)
168-
&& Objects.equals(toPb(), ((JobStatus) obj).toPb());
153+
&& obj.getClass().equals(JobStatus.class)
154+
&& Objects.equals(toPb(), ((JobStatus) obj).toPb());
169155
}
170156

171157
com.google.api.services.bigquery.model.JobStatus toPb() {

google-cloud-clients/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/BigQueryImplTest.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
package com.google.cloud.bigquery;
1818

1919
import static com.google.common.truth.Truth.assertThat;
20+
import static org.easymock.EasyMock.anyObject;
21+
import static org.easymock.EasyMock.anyString;
2022
import static org.easymock.EasyMock.capture;
2123
import static org.easymock.EasyMock.eq;
2224
import static org.junit.Assert.assertArrayEquals;
@@ -962,7 +964,7 @@ public JobId get() {
962964
.andThrow(new BigQueryException(409, "already exists, for some reason"));
963965
EasyMock.expect(
964966
bigqueryRpcMock.getJob(
965-
EasyMock.anyString(),
967+
anyString(),
966968
EasyMock.eq(id),
967969
EasyMock.eq((String) null),
968970
EasyMock.eq(EMPTY_RPC_OPTIONS)))
@@ -1270,6 +1272,9 @@ public void testQueryRequestCompletedOnSecondAttempt() throws InterruptedExcepti
12701272
bigqueryRpcMock.create(
12711273
JOB_INFO.toPb(), Collections.<BigQueryRpc.Option, Object>emptyMap()))
12721274
.andReturn(jobResponsePb1);
1275+
EasyMock.expect(
1276+
bigqueryRpcMock.getJob(eq(PROJECT), eq(JOB), anyString(), anyObject(Map.class)))
1277+
.andReturn(jobResponsePb1);
12731278

12741279
EasyMock.expect(
12751280
bigqueryRpcMock.getQueryResults(

0 commit comments

Comments
 (0)