feat(firestore): [PQ] general, key, type, logical and object functions by bhshkh · Pull Request #13283 · googleapis/google-cloud-go

bhshkh · 2025-11-01T03:30:12Z

Changes in this PR:

firestore_client.go : Updated generated client as per fix(gengapic): Ensure stable order of dynamic routing header parameters gapic-generator-go#1661 . Removed retries from tests since the headers have now been fixed.
Remove Equivalent since it was removed from backend.
Add/update comments
Add timestamp truncate (pending from feat(firestore): [PQ] Add all Aggregation and Timestamp functions #13194) and string split (pending from feat(firestore): [PQ] add all array, string and vector functions #13245) functions.
add all the private preview general, key, logical (except iferror), type and object functions.
See "Firestore Features (Pipeline)" sheet in 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

bhshkh · 2025-11-01T03:30:31Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a comprehensive set of new pipeline functions for Firestore, including general, key, logical, and object manipulation functions. The changes are well-structured, with corresponding updates to interfaces, implementations, and extensive integration tests. The code quality is high, but I've identified a couple of areas for improvement: one related to a potentially leftover testing change in the integration test setup, and another concerning code duplication that could be refactored for better maintainability. Overall, this is a solid contribution that significantly enhances the pipeline query capabilities.

bhshkh · 2025-11-04T03:38:42Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces several new pipeline functions for Firestore, including general, key, logical, and object functions, along with corresponding integration tests. The changes are a good addition to the feature set. My review focuses on improving performance, ensuring deterministic behavior, adhering to Go conventions, and fixing some minor issues in the tests. I've identified a couple of high-severity issues related to performance and potential bugs that should be addressed. Other comments are of medium severity and aim to improve code quality and maintainability.

bhshkh · 2025-11-05T18:45:41Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a significant number of new pipeline functions for Firestore, including general, key, logical, and object functions, along with their corresponding integration tests. The changes are extensive and well-tested. My review focuses on improving code efficiency, fixing a critical compilation error, and addressing a bug in the test suite. Overall, this is a great addition, and with a few adjustments, it will be even better.

daniel-sanche

LGTM

hongalex · 2025-11-06T01:25:00Z

+func typeFuncs(t *testing.T) {
+	t.Parallel()
+	h := testHelper{t}
+	client := integrationClient(t)


consider adding defer client.Close line in tests

All the tests use the same client

google-cloud-go/firestore/integration_test.go

Lines 396 to 407 in 3584537

// integrationClient should be called by integration tests to get a valid client. It will never

// return nil. If integrationClient returns, an integration test can proceed without

// further checks.

func integrationClient(t *testing.T) *Client {

if testing.Short() {

t.Skip("Integration tests skipped in short mode")

}

if iClient == nil {

t.SkipNow() // log message printed in initIntegrationTest

}

return iClient

}

The client is closed here after all the tests run:

google-cloud-go/firestore/integration_test.go

Line 388 in 3584537

iClient.Close()

Add ExecuteOptions similar to query run options which was introduced in #10164. GetRawData and GetText implementation similar to Java https://github.com/googleapis/java-firestore/blob/742fab6583c9a6f9c47cf0496124c3c9b05fe0ee/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java https://github.com/googleapis/java-firestore/blob/742fab6583c9a6f9c47cf0496124c3c9b05fe0ee/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/ExplainOptions.java https://github.com/googleapis/java-firestore/blob/742fab6583c9a6f9c47cf0496124c3c9b05fe0ee/google-cloud-firestore/src/main/java/com/google/cloud/firestore/ExplainStats.java#L42-L71 https://github.com/googleapis/java-firestore/blob/742fab6583c9a6f9c47cf0496124c3c9b05fe0ee/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java#L2474-L2498 Previous pull requests - #12217 - #12425 - #12538 - #13147 - #13199 - #13218 - #13194 - #13245 - #13270 - #13271 - #13279 - #13280 - #13281 - #13282 - googleapis/gapic-generator-go#1661 - #13283

Add CreateFrom() and Pipeline() similar to Java. https://github.com/googleapis/java-firestore/blob/742fab6583c9a6f9c47cf0496124c3c9b05fe0ee/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java#L140-L164 Previous pull requests - #12217 - #12425 - #12538 - #13147 - #13199 - #13218 - #13194 - #13245 - #13270 - #13271 - #13279 - #13280 - #13281 - #13282 - googleapis/gapic-generator-go#1661 - #13283 - #13274 - #13338

product-auto-label Bot added the api: firestore Issues related to the Firestore API. label Nov 1, 2025

gemini-code-assist Bot reviewed Nov 1, 2025

View reviewed changes

Comment thread firestore/integration_test.go Outdated

Comment thread firestore/pipeline_function.go Outdated

bhshkh force-pushed the feature/fs-pipeline-queries branch 2 times, most recently from 75ef6c1 to a2fb94c Compare November 3, 2025 22:14

bhshkh force-pushed the feat/fspq-general-key-logical-object branch from 48bb423 to 5c23efa Compare November 3, 2025 22:18

feat(firestore): [PQ] general, key, logical and object functions

b865ed6

bhshkh force-pushed the feat/fspq-general-key-logical-object branch from 5c23efa to b865ed6 Compare November 3, 2025 22:33

remove IfError

56ce014

gemini-code-assist Bot reviewed Nov 4, 2025

View reviewed changes

bhshkh added 3 commits November 4, 2025 17:45

fix vet failures

aaa6383

add truncate, type and split

13c51ea

remove commented aunthentication options

bede60f

gemini-code-assist Bot reviewed Nov 5, 2025

View reviewed changes

Comment thread firestore/pipeline_stage.go Outdated

Comment thread firestore/pipeline_integration_test.go Outdated

Comment thread firestore/apiv1/firestore_client.go

Comment thread firestore/pipeline_function.go Outdated

Comment thread firestore/pipeline_integration_test.go Outdated

add ifError

3c4917e

bhshkh changed the title ~~feat(firestore): [PQ] general, key, logical and object functions~~ feat(firestore): [PQ] general, key, type, logical and object functions Nov 5, 2025

bhshkh marked this pull request as ready for review November 5, 2025 19:55

bhshkh requested review from a team November 5, 2025 19:55

daniel-sanche approved these changes Nov 5, 2025

View reviewed changes

hongalex reviewed Nov 6, 2025

View reviewed changes

bhshkh requested a review from hongalex November 6, 2025 08:38

bhshkh mentioned this pull request Nov 6, 2025

feat(firestore): [PQ] add executeoptions #13274

Merged

hongalex approved these changes Nov 7, 2025

View reviewed changes

bhshkh merged commit b80712a into googleapis:feature/fs-pipeline-queries Nov 10, 2025
9 of 10 checks passed

bhshkh deleted the feat/fspq-general-key-logical-object branch November 10, 2025 19:50

bhshkh mentioned this pull request Nov 10, 2025

refactor(firestore): [PQ] Rename Expr to Expression #13338

Merged

bhshkh mentioned this pull request Nov 11, 2025

feat(firestore): [PQ] create pipeline from query #13339

Merged

bhshkh mentioned this pull request Jan 13, 2026

feat(firestore): [PQ] Support pipeline queries #12347

Merged

	// integrationClient should be called by integration tests to get a valid client. It will never
	// return nil. If integrationClient returns, an integration test can proceed without
	// further checks.
	func integrationClient(t testing.T) Client {
	if testing.Short() {
	t.Skip("Integration tests skipped in short mode")
	}
	if iClient == nil {
	t.SkipNow() // log message printed in initIntegrationTest
	}
	return iClient
	}

Conversation

bhshkh commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bhshkh commented Nov 1, 2025

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

bhshkh commented Nov 4, 2025

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bhshkh commented Nov 5, 2025

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daniel-sanche left a comment

Choose a reason for hiding this comment

Uh oh!

hongalex Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

bhshkh Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bhshkh commented Nov 1, 2025 •

edited

Loading