feat(firestore): Support for Firestore pipelines API #8931
feat(firestore): Support for Firestore pipelines API #8931russellwheatley wants to merge 148 commits intomainfrom
Conversation
… and option types - Add arithmetic/constant helpers: constant, add, subtract, divide, multiply, documentId - Add aggregate helpers: sum, count, average, arrayAgg, countDistinct, first, last - Add math/conditional helpers: abs, ceil, floor, mod, round, conditional, sqrt, not, ifAbsent, ifError - Add string helpers: toLower, toUpper, trim, substring - Add concat, currentTimestamp - Add option/type exports: StageOptions, AliasedAggregate, AliasedExpression, AddFieldsStageOptions, AggregateStageOptions, ExpressionType - Match add() to SDK (two-arg only, no rest params)
… helpers - Declare ceil/round with (string) before (Expression) to match JS SDK - Add arrayAggDistinct, arrayConcat, arrayGet, arrayLength, arraySum - Reduces firestore-pipelines missing count (86→81) and different shape (4→2)
…g, pow, split, etc.). stubbed
…sType, stringConcat, etc.). Stubbed
…ns, stringReplaceAll, etc.). stubbed
…TimeGranularity. stubbed.
…re:types)
- Add length(fieldName) and length(expression) to pipelines stage API
- Export stage option types from pipelines index (OneOf, *StageOptions, PipelineExecuteOptions)
- Document 47 firestore-pipelines differences in compare-types config (21 extra in RN, 26 different shape)
- Restore DatabaseStageOptions as StageOptions & {} to match SDK
…ract, unixMillisToTimestamp to ios unsupported
mikehardy
left a comment
mikehardy
left a comment
There was a problem hiding this comment.
I still haven't reviewed everything but have enough comments to start thinking about I believe - some with some known solutions but the general theme on the unknown ones is fear of stack and/or heap memory exhaustion
...ative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestorePipelineExecutor.java
Show resolved
Hide resolved
...ative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestorePipelineExecutor.java
Show resolved
Hide resolved
...ve/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestorePipelineNodeBuilder.java
Outdated
Show resolved
Hide resolved
...ve/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestorePipelineNodeBuilder.java
Outdated
Show resolved
Hide resolved
| "pipelineExecute() expected \(fieldName).args to include left and right operands.") | ||
| } | ||
|
|
||
| let left = try coerceExpression(args[0], fieldName: "\(fieldName).args[0]") |
There was a problem hiding this comment.
coerceExpression may call coerceFunctionExpression which can then recur here, not sure on this specific provenance/size for data tho, maybe not an issue
There was a problem hiding this comment.
still a recursive cycle here, however, while data may be arbitrarily nested, perhaps there is a limit to depth of pipeline operations? Such that stack exhaustion isn't a worry ?
There was a problem hiding this comment.
Not quite finished on android yet, I've been doing it piecemeal and rebuilding/testing so that it's easier to catch
packages/firestore/ios/RNFBFirestore/RNFBFirestorePipelineNodeBuilder.swift
Outdated
Show resolved
Hide resolved
packages/firestore/ios/RNFBFirestore/RNFBFirestorePipelineSnapshotSerializer.swift
Show resolved
Hide resolved
…e instead of recursive
…nd parseValueNode
…dExpression are now iterative
…o iterative process
…ean lowering now flattened. part of moving away from recursion
mikehardy
left a comment
mikehardy
left a comment
There was a problem hiding this comment.
whew! still a couple comments from prior to address - specifically I know the iOS header include thing will break builds already (a fix for same was released on stable branch) and it appears there may be a couple more recursive->iterative transforms to complete
an outstanding question of whether we're reading all results in to memory and passing through - but a possible "clean" resolution of that is simply to lean on what firebase-js-sdk settled on - if they're not streaming then the heap exhaustion risk must be acceptable yes?
in general I will assume the comments I left will be addressed - you're nothing if not diligent :-), so going to plus-1 this one as it seems ready to go modulo current comments
this is an incredible / incredibly large feature!
| import type { FilterSpec, OrderSpec, QueryOptions } from '../query'; | ||
|
|
||
| const FIRESTORE_LITE_UNSUPPORTED_SUFFIX = | ||
| ' This operation is unavailable because the web runtime uses Firestore Lite.'; |
There was a problem hiding this comment.
Just an idle thought - related to an issue in the tracker I'm still thinking on and maybe you have an idea - it may be possible to optionally depend on firestore, and let the user specify their own firebase-js-sdk dependency. We have currently non-optionally chosen for users that they get firestore-lite but there are people that want full firestore. If it were possible to alter the dependency at the app package.json level and for us to tolerate both cases, that would be ideal and a change I hope to / wish to make
How would it affect this area? (looks like it's just a couple places here - would need to be a conditional vs a hard-throw-error type thing I guess - not sure what we'd test to see if it was lite or not 🤔 )
| ] as const; | ||
|
|
||
| export const PIPELINE_UNSUPPORTED_BASE_MESSAGE = | ||
| 'Firestore pipelines are not supported by this native implementation yet.'; |
There was a problem hiding this comment.
base message seems conclusive and "all not supported"-like to me, perhaps slightly softer / less binary-sounding "not supported" base message to flow better with the suffixes that are tacked on about specific stages / sources ?
| 'Firestore pipelines are not supported by this native implementation yet.'; | |
| 'Some Firestore pipeline features are not supported by this native implementation yet.'; |
2 participants
Description
Support for Firestore pipelines API
@react-native-firebase/firestore/pipelinesso consumers can build pipelines withdb.pipeline()and run them withexecute(...).pipelineExecute(...)bridge call at execution time.Public JS & runtime shape
import '@react-native-firebase/firestore/pipelines'installspipeline()onto the Firestore runtime prototype as a side effect. Similar to how it works on firebase-js-sdk.collection,collectionGroup,database,documents, andcreateFrom(query).documents(...)inputsunion(...)self-cycles or cross-instance pipelinespipelineExecute()JS -> native contractpackages/firestore/lib/types/internal.tsis the source of truth for the data sent over the wire to native.pipeline.source: one ofcollection | collectionGroup | database | documents | querypipeline.stages: ordered{ stage, options }[]options: currentlyindexMode?: 'recommended'andrawOptions?: Record<string, unknown>path,queryType,filters,orders,options) rather than as an already-built native query.executionTimeplusresults[], which are rehydrated back intoPipelineSnapshot/PipelineResult.Native architecture by platform
parse -> build native SDK objects -> execute -> serialize snapshotflow.parse -> bridge factory/node builder -> PipelineBridge.execute -> snapshot serializer.Parsing & query building
createFrom(query)can be serialized in JS and rebuilt natively/web later.pipelineSource.createFrom(...).Known limitations & current guards
execute({ indexMode, rawOptions })is rejected in JS on all platforms, because the execute-option surface is not available yet (It just throws an error on all platforms).pipeline.source.rawOptionsfor source builders because the linked iOS pipeline bridge does not currently expose source options.Snapshot & result handling
PipelineSnapshot/PipelineResultobjects.Tests
createFrom(query), documents source, stage execution, expression behavior, unhappy paths, and platform-specific option failures.Why the compare-types script changed
firestore-pipelinesSDK snapshot/config was added so CI can detect drift between RNFB pipeline types and the Firestore JS SDK pipeline types.Related issues
Release Summary
Checklist
AndroidiOSOther(macOS, web)e2etests added or updated inpackages/\*\*/e2ejesttests added or updated inpackages/\*\*/__tests__Test Plan
Think
react-native-firebaseis great? Please consider supporting the project with any of the below:React Native FirebaseandInvertaseon Twitter