GODRIVER-2443 Make Distinct return a decodable struct

[prestonvasquez]

GODRIVER-2443

Summary

The Distinct collection method should return a struct that can be decoded, similar to FindOne.

Background & Motivation

Providing a decodable type will improve user experience. Instead of iterating through an any slice, users can decode same-type data using conventional Go syntax:

res, err := coll.Distinct(ctx, "myField", filter)
if err != nil {
	return err
}

var arr []string
res.Decode(&arr)

Alternatively, if the data returned is not same-type a user can iterate it directly as a bson.RawArray type:

res, err := coll.Distinct(ctx, "myField", filter)
if err != nil {
	return err
}

val, err := res.Values()
if err != nil {
	return err
}

for _, rawValue := range val {
	switch rawValue.Type {
		case bson.TypeDouble:
			var f64 float64 
			
			rawValue.Unmarshal(&f64)
			fmt.Println("f64: ", f64)
		case bson.TypeString:
			var str string 
			
			rawValue.Unmarshal(&str)
			fmt.Println("str: ", str)
	}
}

[mongodb-drivers-pr-bot]

API Change Report

./mongo

incompatible changes

##(Collection).Distinct: changed from func(context.Context, string, interface{}, ..../mongo/options.DistinctOptions) ([]interface{}, error) to func(context.Context, string, interface{}, ...*./mongo/options.DistinctOptions) *DistinctResult

compatible changes

DistinctResult: added

[prestonvasquez]

T defines the return type of the SingleResult.Raw method.

[matthewdale]

I'm concerned about the scope of the change required to add this type parameter. One of the challenges with generic types is that they require parameterizing all functions and types that use them, unless only a single type is ever used (see verifySingleResult).

An alternate approach that doesn't impact any existing types is to add a new type DistinctResult that holds a SingleResult and mostly reproduces the API of SingleResult, changing the return type of Raw. That creates fewer parameterized type changes in existing code and provides the ability to independently extend the APIs of DistinctResult and SingleResult.

E.g.

type DistinctResult struct {
	sr *SingleResult
}

func (dr *DistinctResult) Decode(v interface{}) error { return dr.sr.Decode(v) }
func (dr *DistinctResult) Err() error                 { return dr.sr.Err() }

func (dr *DistinctResult) Raw() (bson.RawArray, error) {
	r, err := dr.sr.Raw()
	return bson.RawArray(r), err
}

[matthewdale]

Here are some examples of user code that would be impacted by adding a type parameter to SingleResult. They seem to fall in a few categories:

• Database client wrappers that return a *mongo.SingleResult.
• Interfaces used for mocking the Collection API.
• Adding helper functions for using a *mongo.SingleResult.

[prestonvasquez]

Ah I see what you mean. It's worth noting that most of these examples will require updates due to GODRIVER-2696. However, I believe it's reasonable to minimize the impact where possible

[matthewdale]

Overall, the API of SingleResult seems to work great for decoding results from Distinct. I have concerns about the parameterized type.

[matthewdale]

We need to use the configured Registry and BSONOptions when unmarshaling the RawValue here.

[prestonvasquez]

It makes sense to decode distinct data with a registry, but the BSONOptions are specific to bson.Decoder which expects the value to decode into to be a map / struct. E.g., the following would throw a runtime error:

var i32s []int32
distincResult.Decode(i32s)

[matthewdale]

You're correct, there's currently no way to do that. However, when combined with the changes in #1584, the following code using a bson.Decoder to decode a value works:

func (dr *DistinctResult) Decode(v any) error {
	doc := bsoncore.NewDocumentBuilder().
		AppendValue("Arr", bsoncore.Value{
			Type: bsoncore.TypeArray,
			Data: dr.arr,
		}).Build()

	dec := getDecoder(doc, dr.opts, dr.reg)

	s := struct {
		Arr any
	}{
		Arr: v,
	}
	return dec.Decode(&s)
}

I created GODRIVER-3205 and added it to the Go Driver 2.0 epic to track that improvement, so we can consider that is resolved for now.

[matthewdale]

I'm concerned about the scope of the change required to add this type parameter. One of the challenges with generic types is that they require parameterizing all functions and types that use them, unless only a single type is ever used (see verifySingleResult).

An alternate approach that doesn't impact any existing types is to add a new type DistinctResult that holds a SingleResult and mostly reproduces the API of SingleResult, changing the return type of Raw. That creates fewer parameterized type changes in existing code and provides the ability to independently extend the APIs of DistinctResult and SingleResult.

E.g.

type DistinctResult struct {
	sr *SingleResult
}

func (dr *DistinctResult) Decode(v interface{}) error { return dr.sr.Decode(v) }
func (dr *DistinctResult) Err() error                 { return dr.sr.Err() }

func (dr *DistinctResult) Raw() (bson.RawArray, error) {
	r, err := dr.sr.Raw()
	return bson.RawArray(r), err
}

[prestonvasquez]

@matthewdale would you mind expanding on this concern: #1603 (comment) ? Genericizing SingleResult shouldn't require any user updates for existing methods that return SingleResult. That is, the only user code that would require changes are the usages of collection.Distinct().

[matthewdale]

@prestonvasquez I added some Sourcegraph search results for code that references mongo.SingleResult and would be impacted by the change to a comment here.

[matthewdale]

Looks good! 👍

[qingyang-hu]

👍