Skip to content

Commit

Permalink
fix(datastore): PKG:datastore TYPE:datastoreClient FUNC:RunAggregatio…
Browse files Browse the repository at this point in the history
…nQuery (#7803)

Bug:
#7800
Calling `func (c *datastore.Client) RunAggregationQuery` panics at line `789` when calling `c.client.RunAggregationQuery`. `c.client` is defined `type pb.DatastoreClient interface` and it should have `RunAggregationQuery(ctx context.Context, in *RunAggregationQueryRequest, opts ...grpc.CallOption) (*RunAggregationQueryResponse, error)` but, `c.client` is implemented as a `type datastoreClient struct` and is missing `func (dc *datastoreClient) RunAggregationQuery(ctx context.Context, in *pb.RunAggregationQueryRequest, opts ...grpc.CallOption) (res *pb.RunAggregationQueryResponse, err error)`.

This bug was covered by the fact that `datastoreClient` complies to `type pb.DatastoreClient interface` because the interface is part of it struct declaration:
```go
// From /datastore/client.go, line 36
...
type datastoreClient struct {
	// Embed so we still implement the DatastoreClient interface,
	// if the interface adds more methods.
	pb.DatastoreClient
...
```

Also(Bonus):
`func (c *datastore.Client) RunAggregationQuery(ctx context.Context, aq *AggregationQuery) (AggregationResult, error)` returning an `type AggregationResult map[string]interface{}` but the content of type `interface{}` are `*datastore.Value` from `google.golang.org/genproto/googleapis/datastore/v1/entity.pb.go`. Not very useful.

Solution:
Add `func (dc *datastoreClient) RunAggregationQuery(ctx context.Context, in *pb.RunAggregationQueryRequest, opts ...grpc.CallOption) (res *pb.RunAggregationQueryResponse, err error)`
Add a `switch` statement to use concrete values from the `GetXXXValue() YYY` of `google.golang.org/genproto/googleapis/datastore/v1 :: datastore.Value`

Co-authored-by: Mario Gravel <info@MarioGravel.dev>
Co-authored-by: meredithslota <meredithslota@google.com>
  • Loading branch information
3 people committed Jun 21, 2023
1 parent 93d6a1a commit 1f050ea
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 1 deletion.
11 changes: 11 additions & 0 deletions datastore/client.go
Expand Up @@ -73,6 +73,17 @@ func (dc *datastoreClient) RunQuery(ctx context.Context, in *pb.RunQueryRequest,
return res, err
}

func (dc *datastoreClient) RunAggregationQuery(ctx context.Context, in *pb.RunAggregationQueryRequest, opts ...grpc.CallOption) (res *pb.RunAggregationQueryResponse, err error) {
ctx = trace.StartSpan(ctx, "cloud.google.com/go/datastore.datastoreClient.RunAggregationQuery")
defer func() { trace.EndSpan(ctx, err) }()

err = dc.invoke(ctx, func(ctx context.Context) error {
res, err = dc.c.RunAggregationQuery(ctx, in, opts...)
return err
})
return res, nil
}

func (dc *datastoreClient) BeginTransaction(ctx context.Context, in *pb.BeginTransactionRequest, opts ...grpc.CallOption) (res *pb.BeginTransactionResponse, err error) {
ctx = trace.StartSpan(ctx, "cloud.google.com/go/datastore.datastoreClient.BeginTransaction")
defer func() { trace.EndSpan(ctx, err) }()
Expand Down
27 changes: 26 additions & 1 deletion datastore/query.go
Expand Up @@ -796,7 +796,32 @@ func (c *Client) RunAggregationQuery(ctx context.Context, aq *AggregationQuery)
// TODO(developer): change batch parsing logic if other aggregations are supported.
for _, a := range res.Batch.AggregationResults {
for k, v := range a.AggregateProperties {
ar[k] = v
switch v.ValueType.(type) {
case *pb.Value_NullValue:
ar[k] = v.GetNullValue()
case *pb.Value_BooleanValue:
ar[k] = v.GetBooleanValue()
case *pb.Value_IntegerValue:
ar[k] = v.GetIntegerValue()
case *pb.Value_DoubleValue:
ar[k] = v.GetDoubleValue()
case *pb.Value_TimestampValue:
ar[k] = v.GetTimestampValue()
case *pb.Value_KeyValue:
ar[k] = v.GetKeyValue()
case *pb.Value_StringValue:
ar[k] = v.GetStringValue()
case *pb.Value_BlobValue:
ar[k] = v.GetBlobValue()
case *pb.Value_GeoPointValue:
ar[k] = v.GetGeoPointValue()
case *pb.Value_EntityValue:
ar[k] = v.GetEntityValue()
case *pb.Value_ArrayValue:
ar[k] = v.GetArrayValue()
default:
ar[k] = v
}
}
}

Expand Down

0 comments on commit 1f050ea

Please sign in to comment.