Skip to content

feat(firestore): refine Pipelines API, error handling, and tests#14429

Merged
bhshkh merged 4 commits into
googleapis:mainfrom
bhshkh:fspq-minor-fixes
Apr 14, 2026
Merged

feat(firestore): refine Pipelines API, error handling, and tests#14429
bhshkh merged 4 commits into
googleapis:mainfrom
bhshkh:fspq-minor-fixes

Conversation

@bhshkh
Copy link
Copy Markdown
Contributor

@bhshkh bhshkh commented Apr 14, 2026

This PR introduces several refinements to the Firestore Pipelines experimental feature, aimed at improving API consistency, error reporting, and the reliability of integration tests.

Key Changes:

  • API Refinement:
    • Renamed ArraySliceWithLength to ArraySliceToEnd to better reflect its behavior similar to Java.
    • Enabled the ArrayFilter expression (previously commented out).
    • Updated TimestampTruncateWithTimezone to accept any for the timezone parameter, allowing for dynamic timezone expressions. TimestampTruncateWithTimezone in Java accepts string or expression src.
  • Error Handling:
    • Introduced typed errors ErrPipelineWithoutDatabase and ErrUnionNotSupportRelativeScope to replace generic fmt.Errorf calls feat(firestore): pipeline subqueries #14365 (comment).
    • Added internal nil checks across various pipeline stages (inputStageDocuments, findNearestStage, removeFieldsStage, etc.) to provide more descriptive error messages during proto conversion.
  • Integration Test Enhancements:
    • Replaced skipIfEnterprise with a more flexible skipIfEdition helper to handle feature support across different Firestore editions.
    • Improved resource cleanup in tests by moving t.Cleanup calls earlier, ensuring documents are deleted even if a test fails prematurely.
  • Bug Fixes & Robustness:
    • Added nil checks when processing results in TestIntegration_Query_Pipeline.
    • Standardized test skipping logic across all pipeline-related integration tests.

@bhshkh bhshkh requested review from a team as code owners April 14, 2026 01:30
@product-auto-label product-auto-label Bot added the api: firestore Issues related to the Firestore API. label Apr 14, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request improves the reliability of Firestore integration tests by standardizing resource cleanup using t.Cleanup and ensuring clients and bulk writers are properly closed. It also introduces named errors for pipeline execution, enables the ArrayFilter expression, and refines the ArraySlice and TimestampTruncateWithTimezone APIs. Feedback identifies a naming inconsistency with the Java SDK regarding array slicing, points out redundant cleanup registrations in tests, and suggests removing commented-out code and improving internal error messages for better clarity.

Comment thread firestore/pipeline_expression.go Outdated
Comment thread firestore/integration_test.go Outdated
Comment thread firestore/pipeline_integration_test.go Outdated
Comment thread firestore/pipeline_stage.go Outdated
@bhshkh bhshkh force-pushed the fspq-minor-fixes branch from 3e9884e to bd75ed6 Compare April 14, 2026 03:40
@bhshkh bhshkh enabled auto-merge (squash) April 14, 2026 03:41
Comment thread firestore/pipeline.go Outdated
// ErrPipelineWithoutDatabase is returned when a pipeline is executed without a database such as a subcollection pipeline.
ErrPipelineWithoutDatabase = errors.New("firestore: pipeline without a database cannot be executed directly, only as part of another pipeline")
// ErrUnionNotSupportRelativeScope is returned when a union is used with a relative scope pipeline.
ErrUnionNotSupportRelativeScope = errors.New("firestore: union only supports combining root pipelines; relative scope pipelines (like subcollection pipelines) are not supported")
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: maybe some naming improvement here? ErrRelativeScopeUnionUnsupported or something?

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.

Fixed

@bhshkh bhshkh requested a review from shollyman April 14, 2026 17:21
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

@bhshkh bhshkh merged commit 1ceb42e into googleapis:main Apr 14, 2026
11 checks passed
@bhshkh bhshkh deleted the fspq-minor-fixes branch April 14, 2026 18:56
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.

3 participants