Skip to content

Commit

Permalink
refactor(bigquery): cleanup unused params (googleapis#11149)
Browse files Browse the repository at this point in the history
This PR revisits a few of the internal method signatures and simplifies them in the cases where a param is effectively unused or returns only a single possible value.  Fixes here are scoped only to the main bigquery package, a subsequent PR will introduce similar refactors for managedwriter
  • Loading branch information
shollyman authored Nov 19, 2024
1 parent 8d58b78 commit 771aa46
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 28 deletions.
4 changes: 2 additions & 2 deletions bigquery/iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ type fetchPageResult struct {
// then dispatches to either the appropriate job or table-based backend mechanism
// as needed.
func fetchPage(ctx context.Context, src *rowSource, schema Schema, startIndex uint64, pageSize int64, pageToken string) (*fetchPageResult, error) {
result, err := fetchCachedPage(ctx, src, schema, startIndex, pageSize, pageToken)
result, err := fetchCachedPage(src, schema, startIndex, pageSize, pageToken)
if err != nil {
if err != errNoCacheData {
// This likely means something more severe, like a problem with schema.
Expand Down Expand Up @@ -371,7 +371,7 @@ var errNoCacheData = errors.New("no rows in rowSource cache")
// fetchCachedPage attempts to service the first page of results. For the jobs path specifically, we have an
// opportunity to fetch rows before the iterator is constructed, and thus serve that data as the first request
// without an unnecessary network round trip.
func fetchCachedPage(ctx context.Context, src *rowSource, schema Schema, startIndex uint64, pageSize int64, pageToken string) (*fetchPageResult, error) {
func fetchCachedPage(src *rowSource, schema Schema, startIndex uint64, pageSize int64, pageToken string) (*fetchPageResult, error) {
// we have no cached data
if src.cachedRows == nil {
return nil, errNoCacheData
Expand Down
2 changes: 1 addition & 1 deletion bigquery/iterator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func TestRowIteratorCacheBehavior(t *testing.T) {
},
}
for _, tc := range testCases {
gotResp, gotErr := fetchCachedPage(context.Background(), tc.inSource, tc.inSchema, tc.inStartIndex, tc.inPageSize, tc.inPageToken)
gotResp, gotErr := fetchCachedPage(tc.inSource, tc.inSchema, tc.inStartIndex, tc.inPageSize, tc.inPageToken)
if gotErr != tc.wantErr {
t.Errorf("err mismatch. got %v, want %v", gotErr, tc.wantErr)
} else {
Expand Down
9 changes: 4 additions & 5 deletions bigquery/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (m *Model) Metadata(ctx context.Context) (mm *ModelMetadata, err error) {
if err != nil {
return nil, err
}
return bqToModelMetadata(model)
return bqToModelMetadata(model), nil
}

// Update updates mutable fields in an ML model.
Expand All @@ -120,7 +120,7 @@ func (m *Model) Update(ctx context.Context, mm ModelMetadataToUpdate, etag strin
}); err != nil {
return nil, err
}
return bqToModelMetadata(res)
return bqToModelMetadata(res), nil
}

// Delete deletes an ML model.
Expand Down Expand Up @@ -229,8 +229,8 @@ func bqToModelCols(s []*bq.StandardSqlField) ([]*StandardSQLField, error) {
return cols, nil
}

func bqToModelMetadata(m *bq.Model) (*ModelMetadata, error) {
md := &ModelMetadata{
func bqToModelMetadata(m *bq.Model) *ModelMetadata {
return &ModelMetadata{
Description: m.Description,
Name: m.FriendlyName,
Type: m.ModelType,
Expand All @@ -245,7 +245,6 @@ func bqToModelMetadata(m *bq.Model) (*ModelMetadata, error) {
trainingRuns: m.TrainingRuns,
ETag: m.Etag,
}
return md, nil
}

// ModelMetadataToUpdate is used when updating an ML model's metadata.
Expand Down
5 changes: 1 addition & 4 deletions bigquery/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,7 @@ func TestBQToModelMetadata(t *testing.T) {
},
},
} {
got, err := bqToModelMetadata(test.in)
if err != nil {
t.Fatal(err)
}
got := bqToModelMetadata(test.in)
if diff := testutil.Diff(got, test.want, cmpopts.IgnoreUnexported(ModelMetadata{})); diff != "" {
t.Errorf("%+v:\n, -got, +want:\n%s", test.in, diff)
}
Expand Down
24 changes: 8 additions & 16 deletions bigquery/routine.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,9 @@ type RemoteFunctionOptions struct {
UserDefinedContext map[string]string
}

func bqToRemoteFunctionOptions(in *bq.RemoteFunctionOptions) (*RemoteFunctionOptions, error) {
func bqToRemoteFunctionOptions(in *bq.RemoteFunctionOptions) *RemoteFunctionOptions {
if in == nil {
return nil, nil
return nil
}
rfo := &RemoteFunctionOptions{
Connection: in.Connection,
Expand All @@ -252,12 +252,12 @@ func bqToRemoteFunctionOptions(in *bq.RemoteFunctionOptions) (*RemoteFunctionOpt
rfo.UserDefinedContext[k] = v
}
}
return rfo, nil
return rfo
}

func (rfo *RemoteFunctionOptions) toBQ() (*bq.RemoteFunctionOptions, error) {
func (rfo *RemoteFunctionOptions) toBQ() *bq.RemoteFunctionOptions {
if rfo == nil {
return nil, nil
return nil
}
r := &bq.RemoteFunctionOptions{
Connection: rfo.Connection,
Expand All @@ -270,7 +270,7 @@ func (rfo *RemoteFunctionOptions) toBQ() (*bq.RemoteFunctionOptions, error) {
r.UserDefinedContext[k] = v
}
}
return r, nil
return r
}

func (rm *RoutineMetadata) toBQ() (*bq.Routine, error) {
Expand Down Expand Up @@ -307,11 +307,7 @@ func (rm *RoutineMetadata) toBQ() (*bq.Routine, error) {
r.Arguments = args
r.ImportedLibraries = rm.ImportedLibraries
if rm.RemoteFunctionOptions != nil {
rfo, err := rm.RemoteFunctionOptions.toBQ()
if err != nil {
return nil, err
}
r.RemoteFunctionOptions = rfo
r.RemoteFunctionOptions = rm.RemoteFunctionOptions.toBQ()
}
if !rm.CreationTime.IsZero() {
return nil, errors.New("cannot set CreationTime on create")
Expand Down Expand Up @@ -528,11 +524,7 @@ func bqToRoutineMetadata(r *bq.Routine) (*RoutineMetadata, error) {
return nil, err
}
meta.ReturnType = ret
rfo, err := bqToRemoteFunctionOptions(r.RemoteFunctionOptions)
if err != nil {
return nil, err
}
meta.RemoteFunctionOptions = rfo
meta.RemoteFunctionOptions = bqToRemoteFunctionOptions(r.RemoteFunctionOptions)
tt, err := bqToStandardSQLTableType(r.ReturnTableType)
if err != nil {
return nil, err
Expand Down

0 comments on commit 771aa46

Please sign in to comment.