feat(firestore): [PQ] add all the remaining private preview stages

[bhshkh]

b/364927702

1. add all the remaining private preview stages. Merging this PR completes the implementation of all the type "Stage" subType "General" private preview features. (except literals stage which is not yet inmplemented in Java and Node. Requires additonal approvals from Firestore team and will be added to separate PR).
   See "Firestore Features (Pipeline)" sheet in go/firestore-query-tracker
2. Add integration and unit tests.
3. Refactor existing stages code to remove duplicated code and rearrange in alphabetical order.
4. Modify behaviour of Data and DataTo to match existing implementation in document.go
   

   google-cloud-go/firestore/document.go

   Lines 64 to 127 in 9a4cb31

   	// Exists reports whether the DocumentSnapshot represents an existing document.
   	// Even if Exists returns false, the Ref and ReadTime fields of the DocumentSnapshot
   	// are valid.
   	func (d *DocumentSnapshot) Exists() bool {
   	return d.proto != nil
   	}
   	// Data returns the DocumentSnapshot's fields as a map.
   	// It is equivalent to
   	//
   	// var m map[string]interface{}
   	// d.DataTo(&m)
   	//
   	// except that it returns nil if the document does not exist.
   	func (d *DocumentSnapshot) Data() map[string]interface{} {
   	if !d.Exists() {
   	return nil
   	}
   	m, err := createMapFromValueMap(d.proto.Fields, d.c)
   	// Any error here is a bug in the client.
   	if err != nil {
   	panic(fmt.Sprintf("firestore: %v", err))
   	}
   	return m
   	}
   	// DataTo uses the document's fields to populate p, which can be a pointer to a
   	// map[string]interface{} or a pointer to a struct.
   	//
   	// Firestore field values are converted to Go values as follows:
   	// - Null converts to nil.
   	// - Bool converts to bool.
   	// - String converts to string.
   	// - Integer converts int64. When setting a struct field, any signed or unsigned
   	// integer type is permitted except uint, uint64 or uintptr. Overflow is detected
   	// and results in an error.
   	// - Double converts to float64. When setting a struct field, float32 is permitted.
   	// Overflow is detected and results in an error.
   	// - Bytes is converted to []byte.
   	// - Timestamp converts to time.Time.
   	// - GeoPoint converts to *latlng.LatLng, where latlng is the package
   	// "google.golang.org/genproto/googleapis/type/latlng".
   	// - Arrays convert to []interface{}. When setting a struct field, the field
   	// may be a slice or array of any type and is populated recursively.
   	// Slices are resized to the incoming value's size, while arrays that are too
   	// long have excess elements filled with zero values. If the array is too short,
   	// excess incoming values will be dropped.
   	// - Vectors convert to []float64
   	// - Maps convert to map[string]interface{}. When setting a struct field,
   	// maps of key type string and any value type are permitted, and are populated
   	// recursively.
   	// - References are converted to *firestore.DocumentRefs.
   	//
   	// Field names given by struct field tags are observed, as described in
   	// DocumentRef.Create.
   	//
   	// Only the fields actually present in the document are used to populate p. Other fields
   	// of p are left unchanged.
   	//
   	// If the document does not exist, DataTo returns a NotFound error.
   	func (d *DocumentSnapshot) DataTo(p interface{}) error {
   	if !d.Exists() {
   	return status.Errorf(codes.NotFound, "document %s does not exist", d.Ref.Path)
   	}

Previous pull requests

• feat(firestore): [PQ] add pipeline queries #12217
• feat(firestore): [PQ] Add rest of the input stages #12425
• feat(firestore): [PQ] Add pipeline expressions basic structure #12538
• feat(firestore): [PQ] add all the remaining arithmetic and comparison functions #13147
• test(firestore): [PQ] Modify integration test to run on enterprise db #13199

[bhshkh]

/gemini review

[gemini-code-assist]

Code Review

This pull request is a substantial contribution, adding the remaining private preview stages for Firestore pipelines. The changes include implementing numerous new pipeline stages, refactoring existing pipeline code for better structure and consistency, and adding comprehensive unit and integration tests. The refactoring of PipelineResult.Data() and DataTo() to align with DocumentSnapshot is a good consistency improvement.

I've found a couple of critical issues in the new integration tests that will cause compilation errors due to leftover code. I've also pointed out some minor issues, including leftover debug code and an inconsistently named function parameter. Once these are addressed, this will be a solid addition to the library.

[gemini-code-assist]

This if err != nil block will cause a compilation error because the err variable is not defined in this scope. The Data() method on PipelineResult was changed in this PR to no longer return an error. This block appears to be a leftover from the previous implementation and should be removed.

[hongalex]

err is defined in the scope but it was already checked in L3777. This comment still seems relevant though.

[bhshkh]

Done

[gemini-code-assist]

Similar to a previous finding, this if err != nil block will cause a compilation error as err is not defined here. The Data() method no longer returns an error. This check should be removed.

[bhshkh]

Done

[gemini-code-assist]

The parameter name fieldpathsOrSelectables is plural, but the function accepts a single any argument, not a variadic one. This is inconsistent with other functions like Replace and can be confusing. For clarity and consistency, please consider renaming it to the singular fieldpathOrSelectable and updating its usage within the function.

Suggested change

	func (p *Pipeline) Unnest(fieldpathsOrSelectables any) *Pipeline {
	if p.err != nil {
	return p
	}
	stage, err := newUnnestStageFromAny(fieldpathsOrSelectables)
	func (p *Pipeline) Unnest(fieldpathOrSelectable any) *Pipeline {
	if p.err != nil {
	return p
	}
	stage, err := newUnnestStageFromAny(fieldpathOrSelectable)

[bhshkh]

Done

[gemini-code-assist]

This commented-out debugging code should be removed before merging to keep the codebase clean.

[hongalex]

please fix

[bhshkh]

Fixed

[hongalex]

err is defined in the scope but it was already checked in L3777. This comment still seems relevant though.

[hongalex]

panic here seems undesirable. Why was Data() changed to not return an error?

[bhshkh]

To be inline with existing Data() method

google-cloud-go/firestore/document.go

Lines 78 to 88 in 9a4cb31

	func (d *DocumentSnapshot) Data() map[string]interface{} {
	if !d.Exists() {
	return nil
	}
	m, err := createMapFromValueMap(d.proto.Fields, d.c)
	// Any error here is a bug in the client.
	if err != nil {
	panic(fmt.Sprintf("firestore: %v", err))
	}
	return m
	}

[hongalex]

please fix

[daniel-sanche]

Left some initial comments, I'll try to take another look soon

[daniel-sanche]

Is this todo still valid? If it's meant to be long term, maybe add some more context

[bhshkh]

Yes, it is still valid. I'll resolve it in next PR.

[daniel-sanche]

I noticed inputStageCollection names are handled in a different way, with the name strings hard-coded in-line. Should those be added to this constant list for consistency?

[bhshkh]

Updated

[daniel-sanche]

Other sdks have a Generic stage (which seems to be renamed to RawStage after the latest updates?). Do you have anything like that here?

[bhshkh]

Will add in next PR.

[daniel-sanche]

Something I was meaning to look into that you might have an answer to:

For the CollectionGroup above this, you're taking in an ancestor and passing it in as the first argument. Is that still valid? The other implementations I've seen seem to just be passing an empty string as the first argument, but I haven't dug into that yet

[bhshkh]

Removed the function to match other implementations.

[shollyman]

I'm a little slow to review given the chaining builder pattern.

Things that throw me:

• the differing behavior for error for happy path. One embeds an error in the existing entity and ignores the current spec in the chain of pipeline directives. The other returns a copy/clone of the pipeline?
• Field namings. I'm having trouble tracking down the corresponding protos. I see this is being built in a feature branch, where can I see the source protos for pipelines?

[shollyman]

How much of the naming is coming from a common spec here? I'd expect more of an option pattern due to the WithGroups naming, but this appears to just set the Groups field of the AggregateSpec. Should it just be Groups()?

Feel free to ignore, I'm still coming to terms with this particular builder pattern.

[bhshkh]

I'm a little slow to review given the chaining builder pattern.

Things that throw me:

• the differing behavior for error for happy path. One embeds an error in the existing entity and ignores the current spec in the chain of pipeline directives. The other returns a copy/clone of the pipeline?
• Field namings. I'm having trouble tracking down the corresponding protos. I see this is being built in a feature branch, where can I see the source protos for pipelines?

The protos are here:

google-cloud-go/firestore/apiv1/firestorepb/pipeline.pb.go

Lines 43 to 52 in a3ee1f1

	type StructuredPipeline struct {
	state protoimpl.MessageState
	sizeCache protoimpl.SizeCache
	unknownFields protoimpl.UnknownFields
	// Required. The pipeline query to execute.
	Pipeline *Pipeline `protobuf:"bytes,1,opt,name=pipeline,proto3" json:"pipeline,omitempty"`
	// Optional. Optional query-level arguments.
	Options map[string]*Value `protobuf:"bytes,2,rep,name=options,proto3" json:"options,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"`
	}

google-cloud-go/firestore/apiv1/firestorepb/document.pb.go

Lines 621 to 629 in a3ee1f1

	// A Firestore query represented as an ordered list of operations / stages.
	type Pipeline struct {
	state protoimpl.MessageState
	sizeCache protoimpl.SizeCache
	unknownFields protoimpl.UnknownFields
	// Required. Ordered list of stages to evaluate.
	Stages []*Pipeline_Stage `protobuf:"bytes,1,rep,name=stages,proto3" json:"stages,omitempty"`
	}

google-cloud-go/firestore/apiv1/firestorepb/document.pb.go

Lines 668 to 704 in a3ee1f1

	// A single operation within a pipeline.
	//
	// A stage is made up of a unique name, and a list of arguments. The exact
	// number of arguments & types is dependent on the stage type.
	//
	// To give an example, the stage `filter(state = "MD")` would be encoded as:
	//
	// ```
	// name: "filter"
	//
	// args {
	// function_value {
	// name: "eq"
	// args { field_reference_value: "state" }
	// args { string_value: "MD" }
	// }
	// }
	//
	// ```
	//
	// See public documentation for the full list.
	type Pipeline_Stage struct {
	state protoimpl.MessageState
	sizeCache protoimpl.SizeCache
	unknownFields protoimpl.UnknownFields
	// Required. The name of the stage to evaluate.
	//
	// **Requires:**
	//
	// * must be in snake case (lower case with underscore separator).
	Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"`
	// Optional. Ordered list of arguments the given stage expects.
	Args []*Value `protobuf:"bytes,2,rep,name=args,proto3" json:"args,omitempty"`
	// Optional. Optional named arguments that certain functions may support.
	Options map[string]*Value `protobuf:"bytes,3,rep,name=options,proto3" json:"options,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"`
	}

[shollyman]

After offline discussion about this, approving this PR.