Skip to content

feat(firestore): [PQ] Add rest of the input stages#12425

Merged
bhshkh merged 3 commits intogoogleapis:feature/fs-pipeline-queriesfrom
bhshkh:feature/fs-pipeline-queries-02
Jun 16, 2025
Merged

feat(firestore): [PQ] Add rest of the input stages#12425
bhshkh merged 3 commits intogoogleapis:feature/fs-pipeline-queriesfrom
bhshkh:feature/fs-pipeline-queries-02

Conversation

@bhshkh
Copy link
Copy Markdown
Contributor

@bhshkh bhshkh commented Jun 9, 2025

b/364927702

Add rest of the pipeline stages: collectionGroup and database

Once the protos are public, kokoro and apidiff checks will pass

More context:

@product-auto-label product-auto-label Bot added the api: firestore Issues related to the Firestore API. label Jun 9, 2025
@bhshkh bhshkh force-pushed the feature/fs-pipeline-queries-02 branch from 045f92b to 2f1dd89 Compare June 9, 2025 15:06
@bhshkh bhshkh marked this pull request as ready for review June 9, 2025 17:30
@bhshkh bhshkh requested review from a team June 9, 2025 17:30
Comment thread firestore/pipeline_result.go
Copy link
Copy Markdown
Member

@hongalex hongalex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm for Go pending vet, want: firestore review

@bhshkh bhshkh requested a review from wu-hui June 9, 2025 19:24
Copy link
Copy Markdown
Contributor

@daniel-sanche daniel-sanche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with a few small comments

Comment thread firestore/pipeline_source.go Outdated
// /continents/NorthAmerica/Canada/Cities/Montreal = {population: 90}
//
// CollectionGroupWithAncestor can be used to query across all "Cities" in "/continents/Europe".
// TODO(bhshkh): Subcollections are not yet supported in enterprise DB. Check with Firestore team whether ancestors should be added.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it feels a little weird merging with this TODO in place. Is there a better way to track this? Maybe as a GitHub issue, in a hotlist linked to this release? Or maybe this feature should be pushed to a follow-up PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the TODO
Confirmed with Andy that collectionGroup will be in the initial launch

Comment thread firestore/pipeline_result.go Outdated
return nil
func (p *PipelineResult) Data() (map[string]interface{}, error) {
if p == nil {
return nil, status.Errorf(codes.NotFound, "document does not exist")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will the data always be related to a document? Some stages aggregate data across documents, but I'm not too sure if that applies here or not

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the wording to "result doe snot exist"

@bhshkh bhshkh merged commit 5806663 into googleapis:feature/fs-pipeline-queries Jun 16, 2025
6 of 7 checks passed
@bhshkh bhshkh deleted the feature/fs-pipeline-queries-02 branch June 16, 2025 20:29
Copy link
Copy Markdown

@wu-hui wu-hui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, FWIW.

// /continents/NorthAmerica/Canada/Cities/Montreal = {population: 90}
//
// CollectionGroupWithAncestor can be used to query across all "Cities" in "/continents/Europe".
func (ps *PipelineSource) CollectionGroupWithAncestor(ancestor, collectionID string) *Pipeline {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were not planning on shipping the withAncestor(...) variant near-term so its likely better to just skip this for now.

func (ps *PipelineSource) Collection(path string) *Pipeline {
return newPipeline(ps.client, newInputStageCollection(path))
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is also missing the documents(...) stage?

bhshkh added a commit that referenced this pull request Jun 24, 2025
* feat(firestore): Add rest of the input stages

* correct the method comment to resolve vet failure

* address review comments
bhshkh added a commit that referenced this pull request Jul 1, 2025
* feat(firestore): Add rest of the input stages

* correct the method comment to resolve vet failure

* address review comments
bhshkh added a commit that referenced this pull request Oct 21, 2025
…tions (#13147)

b/364927702

Add rest of the arithmetics and comparison functions from
[go/firestore-query-tracker](http://go/firestore-query-tracker)


Previous pull requests:
- #12217
- #12425
- #12538
bhshkh added a commit that referenced this pull request Oct 23, 2025
)

b/364927702

1. Fixes test
```go
--- FAIL: TestPipelineResult_NoResults (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered, repanicked]
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x10d40b6]

goroutine 4633 [running]:
testing.tRunner.func1.2({0x13c5240, 0x1ec2150})
	/usr/local/go/src/testing/testing.go:1872 +0x419
testing.tRunner.func1()
	/usr/local/go/src/testing/testing.go:1875 +0x683
panic({0x13c5240?, 0x1ec2150?})
	/usr/local/go/src/runtime/panic.go:783 +0x132
cloud.google.com/go/firestore.(*PipelineResult).Data(0xc00046f380)
	/tmpfs/src/google-cloud-go/firestore/pipeline_result.go:95 +0x96
cloud.google.com/go/firestore.TestPipelineResult_NoResults(0xc000455500)
	/tmpfs/src/google-cloud-go/firestore/pipeline_result_test.go:359 +0x312
testing.tRunner(0xc000455500, 0x157dca0)
	/usr/local/go/src/testing/testing.go:1934 +0x21d
created by testing.(*T).Run in goroutine 1
	/usr/local/go/src/testing/testing.go:1997 +0x9d3
FAIL	cloud.google.com/go/firestore	2.337s
```

```go
=== RUN   TestPipelineResultIterator_GetAll
    pipeline_result_test.go:249: second result id: got 1, want: 2
--- FAIL: TestPipelineResultIterator_GetAll (0.00s)
```
2. Add enterprise database env variable


Previous pull requests:

- #12217
- #12425
- #12538
- #13147
@bhshkh bhshkh changed the title feat(firestore): Add rest of the input stages feat(firestore): [PQ] Add rest of the input stages Oct 27, 2025
bhshkh added a commit that referenced this pull request Oct 30, 2025
…3194)

b/364927702

1. add all the remaining private preview aggregate functions. Merging
this PR completes the implementation of all the **type: "Function"
subType : "Accumulators (Aggregation)"** private preview features.
See "Firestore Features (Pipeline)" sheet in
[go/firestore-query-tracker](http://go/firestore-query-tracker) for the
list of features.

    Java reference:
-
https://github.com/googleapis/java-firestore/blob/ccaf9d4fac5bd87a4da3d37493ca66fdc7681bc3/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/expressions/AggregateFunction.java#L43-L71
-
https://github.com/googleapis/java-firestore/blob/ccaf9d4fac5bd87a4da3d37493ca66fdc7681bc3/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/expressions/AggregateFunction.java#L93-L111

2. add all the remaining private preview timestamp functions. Merging
this PR completes the implementation of all the **type: "Function"
subType: "Date / Timestamp"** private preview features. (except
timestamp_trunc function which is not yet inmplemented in any of the
SDKs. Requires additonal approvals from Firestore team and will be added
to separate PR).
See "Firestore Features (Pipeline)" sheet in
[go/firestore-query-tracker](http://go/firestore-query-tracker) for the
list of functions.

    Java reference:
-
https://github.com/googleapis/java-firestore/blob/ccaf9d4fac5bd87a4da3d37493ca66fdc7681bc3/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/expressions/Expression.java#L2262-L2517

3. Add integration tests for all functions.
4. Remove Rand function since it is not targeted for private preview.
5. Renamed numericExprOrField to numericExprOrFieldPath since field is a
separate type/expression.

https://github.com/googleapis/google-cloud-go/blob/a3ee1f19068c6d3fb77ad797e29884a90d6402a2/firestore/pipeline_field.go#L21-L41



Previous pull requests

- #12217
- #12425
- #12538
- #13147
- #13199
- #13218
bhshkh added a commit that referenced this pull request Oct 30, 2025
…13218)

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](http://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

https://github.com/googleapis/google-cloud-go/blob/9a4cb31f4d34948404d91123fb560a43aeebe83e/firestore/document.go#L64-L127



Previous pull requests

- #12217
- #12425
- #12538
- #13147
- #13199
bhshkh added a commit that referenced this pull request Oct 30, 2025
)

1. add all the private preview 'array' functions. 
Merging this PR completes the implementation of all the **type:
"Function" subType : "Array"** private preview features (except
'maximum' and 'minimum' which are not yet implemented in any of the
SDKs. Requires additonal approvals from Firestore team and will be added
to separate PR). See "Firestore Features (Pipeline)" sheet in
[go/firestore-query-tracker](http://go/firestore-query-tracker) for the
list of features.
    Java reference: 
-
https://github.com/googleapis/java-firestore/blob/wuandy/JavaPplPP/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/expressions/Expression.java
2. add all the private preview 'string' functions. (except
'string_split' which is not yet implemented in any of the SDKs. Requires
additonal approvals from Firestore team and will be added to separate
PR)

3. add all the private preview 'vector' functions. 
4. add remaining types to ConstantOf to match Java's implementation.
    Java reference: 
-
https://github.com/googleapis/java-firestore/blob/ccaf9d4fac5bd87a4da3d37493ca66fdc7681bc3/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/expressions/Expression.java#L70-L211

Previous pull requests

- #12217
- #12425
- #12538
- #13147
- #13199
- #13218
- #13194
bhshkh added a commit that referenced this pull request Oct 30, 2025
b/364927702

- toExprOrField was renamed to asFieldExpr in
https://github.com/googleapis/google-cloud-go/pull/13194/files#diff-4a55211f7d38a1f0599e2f4cc92795073f138b2c56b846c933bda19e26bc3a7a
. There were a few call locations were rename was missed while resolving
merge conflicts which caused build failures. Fixing those failures in
this PR.
- the function signature of Data was changed in
#13218. It no longer
returns err as second argument. Fixing this in this PR.
- remove duplicate asInt64Expr and asStringExpr
- Move pipeline tests to their own file


Previous pull requests

- #12217
- #12425
- #12538
- #13147
- #13199
- #13218
- #13194
- #13245
bhshkh added a commit that referenced this pull request Oct 30, 2025
b/364927702

- Combine FieldOf and FieldOfPath to avoid verbose name FieldOfPath


Previous pull requests

- #12217
- #12425
- #12538
- #13147
- #13199
- #13218
- #13194
- #13245
- #13270

---------

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
bhshkh added a commit that referenced this pull request Oct 31, 2025
1. Move PipelineStages integration tests.
2. Remove IsNaN, IsNotNaN, IsNull, IsNotNull, Equivalent as they are no
longer supported by backend
3. Remove examples as commented here

#13245 (comment)

Previous pull requests

- #12217
- #12425
- #12538
- #13147
- #13199
- #13218
- #13194
- #13245
- #13270
- #13271
bhshkh added a commit that referenced this pull request Nov 3, 2025
Add raw stage similar to Java

https://github.com/googleapis/java-firestore/blob/742fab6583c9a6f9c47cf0496124c3c9b05fe0ee/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java#L997-L1021


The raw stage is an escape hatch to allow customers to consume new
stages supported by the backend without having to update their SDK to a
version that adds the stage.



Previous pull requests

- #12217
- #12425
- #12538
- #13147
- #13199
- #13218
- #13194
- #13245
- #13270
- #13271
- #13279
bhshkh added a commit that referenced this pull request Nov 3, 2025
1. Move PipelineStages integration tests.
2. Remove IsNaN, IsNotNaN, IsNull, IsNotNull, Equivalent as they are no
longer supported by backend
3. Remove examples as commented here

#13245 (comment)

Previous pull requests

- #12217
- #12425
- #12538
- #13147
- #13199
- #13218
- #13194
- #13245
- #13270
- #13271
bhshkh added a commit that referenced this pull request Nov 3, 2025
Add raw stage similar to Java

https://github.com/googleapis/java-firestore/blob/742fab6583c9a6f9c47cf0496124c3c9b05fe0ee/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java#L997-L1021


The raw stage is an escape hatch to allow customers to consume new
stages supported by the backend without having to update their SDK to a
version that adds the stage.



Previous pull requests

- #12217
- #12425
- #12538
- #13147
- #13199
- #13218
- #13194
- #13245
- #13270
- #13271
- #13279
bhshkh added a commit that referenced this pull request Nov 10, 2025
#13283)

Changes in this PR:

1. firestore_client.go : Updated generated client as per
googleapis/gapic-generator-go#1661 . Removed
retries from tests since the headers have now been fixed.
2. Remove Equivalent since it was removed from backend.
3. Add/update comments
4. Add timestamp truncate (pending from
#13194) and string
split (pending from
#13245) functions.
5. add all the private preview general, key, logical (except iferror),
type and object functions.
See "Firestore Features (Pipeline)" sheet in
[go/firestore-query-tracker](http://go/firestore-query-tracker) for the
list of functions.
    Java reference:
-
https://github.com/googleapis/java-firestore/blob/ccaf9d4fac5bd87a4da3d37493ca66fdc7681bc3/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/expressions/Expression.java



Previous pull requests

- #12217
- #12425
- #12538
- #13147
- #13199
- #13218
- #13194
- #13245
- #13270
- #13271
- #13279
- #13280
- #13281
- #13282
- googleapis/gapic-generator-go#1661
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: firestore Issues related to the Firestore API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants