Skip to content

Commit

Permalink
chore: Refactor TableResult using builder pattern (#3130)
Browse files Browse the repository at this point in the history 
* chore: Refactor TableResult using builder pattern

This also removed the redundant EmptyTableResult. These changes are fine as TableResult and EmptyTableResult are internal API only.

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* Remove typo comment in TableResult

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
  • Loading branch information
@PhongChuong @gcf-owl-bot
PhongChuong and gcf-owl-bot[bot] committed Feb 22, 2024
1 parent 678b6ce commit 253f6c9
Show file tree
Hide file tree
Showing 9 changed files with 159 additions and 132 deletions.
25 changes: 25 additions & 0 deletions google-cloud-bigquery/clirr-ignored-differences.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,17 @@
<!-- see http://www.mojohaus.org/clirr-maven-plugin/examples/ignored-differences.html -->
<differences>
<!-- TODO: REMOVE AFTER RELEASE -->
<difference>
<differenceType>3005</differenceType>
<className>com/google/cloud/bigquery/TableResult*</className>
<justification>TableResult is an internal API and it should be fine to update</justification>
</difference>
<difference>
<differenceType>7002</differenceType>
<className>com/google/cloud/bigquery/TableResult*</className>
<method>*TableResult(*)</method>
<justification>TableResult is an internal API and it should be fine to update</justification>
</difference>
<difference>
<differenceType>7004</differenceType>
<className>com/google/cloud/bigquery/spi/v2/BigQueryRpc</className>
Expand Down Expand Up @@ -47,6 +58,16 @@
<className>com/google/cloud/bigquery/TableInfo*</className>
<method>*ResourceTags(*)</method>
</difference>
<difference>
<differenceType>7013</differenceType>
<className>com/google/cloud/bigquery/TableResult*</className>
<method>*getPageNoSchema(*)</method>
</difference>
<difference>
<differenceType>7013</differenceType>
<className>com/google/cloud/bigquery/TableResult*</className>
<method>*toBuilder(*)</method>
</difference>
<difference>
<differenceType>7012</differenceType>
<className>com/google/cloud/bigquery/Connection</className>
Expand Down Expand Up @@ -142,4 +163,8 @@
<className>com/google/cloud/bigquery/StandardTableDefinition*</className>
<method>*BigLakeConfiguration(*)</method>
</difference>
<difference>
<differenceType>8001</differenceType>
<className>com/google/cloud/bigquery/EmptyTableResult*</className>
</difference>
</differences>
51 changes: 29 additions & 22 deletions google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1156,7 +1156,11 @@ public TableResult listTableData(
public TableResult listTableData(TableId tableId, Schema schema, TableDataListOption... options) {
Tuple<? extends Page<FieldValueList>, Long> data =
listTableData(tableId, schema, getOptions(), optionMap(options));
return new TableResult(schema, data.y(), data.x(), null);
return TableResult.newBuilder()
.setSchema(schema)
.setTotalRows(data.y())
.setPageNoSchema(data.x())
.build();
}

private static Tuple<? extends Page<FieldValueList>, Long> listTableData(
Expand Down Expand Up @@ -1400,30 +1404,33 @@ public com.google.api.services.bigquery.model.QueryResponse call() {
if (results.getPageToken() != null) {
JobId jobId = JobId.fromPb(results.getJobReference());
String cursor = results.getPageToken();
return new TableResult(
schema,
numRows,
new PageImpl<>(
// fetch next pages of results
new QueryPageFetcher(jobId, schema, getOptions(), cursor, optionMap(options)),
cursor,
// cache first page of result
transformTableData(results.getRows(), schema)),
// Return the JobID of the successful job
jobId,
results.getQueryId());
return TableResult.newBuilder()
.setSchema(schema)
.setTotalRows(numRows)
.setPageNoSchema(
new PageImpl<>(
// fetch next pages of results
new QueryPageFetcher(jobId, schema, getOptions(), cursor, optionMap(options)),
cursor,
transformTableData(results.getRows(), schema)))
.setJobId(jobId)
.setQueryId(results.getQueryId())
.build();
}
// only 1 page of result
return new TableResult(
schema,
numRows,
new PageImpl<>(
new TableDataPageFetcher(null, schema, getOptions(), null, optionMap(options)),
null,
transformTableData(results.getRows(), schema)),
return TableResult.newBuilder()
.setSchema(schema)
.setTotalRows(numRows)
.setPageNoSchema(
new PageImpl<>(
new TableDataPageFetcher(null, schema, getOptions(), null, optionMap(options)),
null,
transformTableData(results.getRows(), schema)))
// Return the JobID of the successful job
results.getJobReference() != null ? JobId.fromPb(results.getJobReference()) : null,
results.getQueryId());
.setJobId(
results.getJobReference() != null ? JobId.fromPb(results.getJobReference()) : null)
.setQueryId(results.getQueryId())
.build();
}

@Override
Expand Down
32 changes: 0 additions & 32 deletions google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/EmptyTableResult.java
View file

This file was deleted.

14 changes: 10 additions & 4 deletions google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/Job.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.api.gax.retrying.BasicResultRetryAlgorithm;
import com.google.api.gax.retrying.RetrySettings;
import com.google.api.gax.retrying.TimedAttemptSettings;
import com.google.cloud.PageImpl;
import com.google.cloud.RetryHelper;
import com.google.cloud.RetryOption;
import com.google.cloud.bigquery.BigQuery.JobOption;
Expand Down Expand Up @@ -311,8 +312,13 @@ public TableResult getQueryResults(QueryResultsOption... options)
// Listing table data might fail, such as with CREATE VIEW queries.
// Avoid a tabledata.list API request by returning an empty TableResult.
if (response.getTotalRows() == 0) {
TableResult emptyTableResult = new EmptyTableResult(response.getSchema());
emptyTableResult.setJobId(job.getJobId());
TableResult emptyTableResult =
TableResult.newBuilder()
.setSchema(response.getSchema())
.setJobId(job.getJobId())
.setTotalRows(0L)
.setPageNoSchema(new PageImpl<FieldValueList>(null, "", null))
.build();
return emptyTableResult;
}

Expand All @@ -323,8 +329,8 @@ public TableResult getQueryResults(QueryResultsOption... options)
TableResult tableResult =
bigquery.listTableData(
table, response.getSchema(), listOptions.toArray(new TableDataListOption[0]));
tableResult.setJobId(job.getJobId());
return tableResult;
TableResult tableResultWithJobId = tableResult.toBuilder().setJobId(job.getJobId()).build();
return tableResultWithJobId;
}

private QueryResponse waitForQueryResults(
Expand Down
120 changes: 54 additions & 66 deletions google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/TableResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@

package com.google.cloud.bigquery;

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.api.core.InternalApi;
import com.google.api.gax.paging.Page;
import com.google.auto.value.AutoValue;
import com.google.common.base.Function;
import com.google.common.base.MoreObjects;
import com.google.common.collect.Iterables;
Expand All @@ -28,108 +27,95 @@
import java.util.Objects;
import javax.annotation.Nullable;

public class TableResult implements Page<FieldValueList>, Serializable {
@InternalApi
@AutoValue
public abstract class TableResult implements Page<FieldValueList>, Serializable {

private static final long serialVersionUID = -4831062717210349819L;
private static final long serialVersionUID = 1L;

@Nullable private final Schema schema;
private final long totalRows;
private final Page<FieldValueList> pageNoSchema;
@Nullable private JobId jobId = null;
@AutoValue.Builder
public abstract static class Builder {
public abstract TableResult.Builder setSchema(Schema schema);

@Nullable private final String queryId;
/**
* Sets the total number of rows in the complete result set, which can be more than the number
* of rows in the first page of results returned by {@link #getValues()}.
*/
public abstract TableResult.Builder setTotalRows(Long totalRows);

// package-private so job id is not set outside the package.
void setJobId(@Nullable JobId jobId) {
this.jobId = jobId;
}
public abstract TableResult.Builder setJobId(JobId jobId);

public JobId getJobId() {
return jobId;
}
public abstract TableResult.Builder setPageNoSchema(Page<FieldValueList> pageNoSchema);

public String getQueryId() {
return queryId;
}
public abstract TableResult.Builder setQueryId(String queryId);

/**
* If {@code schema} is non-null, {@code TableResult} adds the schema to {@code FieldValueList}s
* when iterating through them. {@code pageNoSchema} must not be null.
*/
@InternalApi("Exposed for testing")
public TableResult(
Schema schema, long totalRows, Page<FieldValueList> pageNoSchema, String queryId) {
this.schema = schema;
this.totalRows = totalRows;
this.pageNoSchema = checkNotNull(pageNoSchema);
this.queryId = queryId;
/** Creates a @code TableResult} object. */
public abstract TableResult build();
}

@InternalApi("Exposed for testing")
public TableResult(
Schema schema,
long totalRows,
Page<FieldValueList> pageNoSchema,
JobId jobId,
String queryId) {
this.schema = schema;
this.totalRows = totalRows;
this.pageNoSchema = checkNotNull(pageNoSchema);
this.jobId = jobId;
this.queryId = queryId;
public abstract Builder toBuilder();

public static Builder newBuilder() {
return new AutoValue_TableResult.Builder();
}

/** Returns the schema of the results. Null if the schema is not supplied. */
public Schema getSchema() {
return schema;
}
@Nullable
public abstract Schema getSchema();

/**
* Returns the total number of rows in the complete result set, which can be more than the number
* of rows in the first page of results returned by {@link #getValues()}.
*/
public long getTotalRows() {
return totalRows;
}
public abstract long getTotalRows();

public abstract Page<FieldValueList> getPageNoSchema();

@Nullable
public abstract JobId getJobId();

@Nullable
public abstract String getQueryId();

@Override
public boolean hasNextPage() {
return pageNoSchema.hasNextPage();
return getPageNoSchema().hasNextPage();
}

@Override
public String getNextPageToken() {
return pageNoSchema.getNextPageToken();
return getPageNoSchema().getNextPageToken();
}

@Override
public TableResult getNextPage() {
if (pageNoSchema.hasNextPage()) {
return new TableResult(schema, totalRows, pageNoSchema.getNextPage(), queryId);
if (getPageNoSchema().hasNextPage()) {
return TableResult.newBuilder()
.setSchema(getSchema())
.setTotalRows(getTotalRows())
.setPageNoSchema(getPageNoSchema().getNextPage())
.setQueryId(getQueryId())
.build();
}
return null;
}

@Override
public Iterable<FieldValueList> iterateAll() {
return addSchema(pageNoSchema.iterateAll());
return addSchema(getPageNoSchema().iterateAll());
}

@Override
public Iterable<FieldValueList> getValues() {
return addSchema(pageNoSchema.getValues());
return addSchema(getPageNoSchema().getValues());
}

private Iterable<FieldValueList> addSchema(Iterable<FieldValueList> iter) {
if (schema == null) {
if (getSchema() == null) {
return iter;
}
return Iterables.transform(
iter,
new Function<FieldValueList, FieldValueList>() {
@Override
public FieldValueList apply(FieldValueList list) {
return list.withSchema(schema.getFields());
return list.withSchema(getSchema().getFields());
}
});
}
Expand All @@ -138,29 +124,31 @@ public FieldValueList apply(FieldValueList list) {
public String toString() {
return MoreObjects.toStringHelper(this)
.add("rows", getValues())
.add("schema", schema)
.add("totalRows", totalRows)
.add("schema", getSchema())
.add("totalRows", getTotalRows())
.add("cursor", getNextPageToken())
.add("queryId", getQueryId())
.toString();
}

@Override
public final int hashCode() {
return Objects.hash(pageNoSchema, schema, totalRows);
return Objects.hash(getPageNoSchema(), getSchema(), getTotalRows(), getQueryId());
}

@Override
public final boolean equals(Object obj) {
if (obj == this) {
return true;
}
if (obj == null || !obj.getClass().equals(TableResult.class)) {
if (obj == null || !obj.getClass().equals(AutoValue_TableResult.class)) {
return false;
}
TableResult response = (TableResult) obj;
return Objects.equals(getNextPageToken(), response.getNextPageToken())
&& Iterators.elementsEqual(getValues().iterator(), response.getValues().iterator())
&& Objects.equals(schema, response.schema)
&& totalRows == response.totalRows;
&& Objects.equals(getSchema(), response.getSchema())
&& getTotalRows() == response.getTotalRows()
&& getQueryId() == response.getQueryId();
}
}
7 changes: 6 additions & 1 deletion google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/JobTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,12 @@ public void testWaitForAndGetQueryResults() throws InterruptedException {
Job completedJob =
expectedJob.toBuilder().setStatus(new JobStatus(JobStatus.State.RUNNING)).build();
Page<FieldValueList> singlePage = Pages.empty();
TableResult result = new TableResult(Schema.of(), 1, singlePage, null);
TableResult result =
TableResult.newBuilder()
.setSchema(Schema.of())
.setTotalRows(1L)
.setPageNoSchema(singlePage)
.build();
QueryResponse completedQuery =
QueryResponse.newBuilder()
.setCompleted(true)
Expand Down

0 comments on commit 253f6c9

Please sign in to comment.