Repository navigation
feat(firestore): DML - #14703
Merged
Merged
feat(firestore): DML#14703
Conversation
… option to Java SDK pipelines - Implement Insert.java and Upsert.java stages in com.google.cloud.firestore.pipeline.stages - Add insert() and upsert() builder methods to Pipeline.java - Add withAtomic() option to PipelineExecuteOptions.java and wire proto request (newTransaction + autoCommitTransaction) - Add proto serialization unit tests in PipelineProtoTest.java - Add system integration test suite in ITPipelineTest.java covering insert, upsert, atomic execution, and transaction execution BUG=b/545136773 TAG=agy
The backend evaluates expressions only in the top-level fields of each literal document. Previously, an Expression nested inside a Map or List value was sent as a raw function_value inside a map_value/array_value and the request was rejected with `INVALID_ARGUMENT: Value type is not supported: FUNCTION_VALUE`. A top-level Map or List value that contains an Expression at any depth is now converted into a map(...) / array(...) expression, so nested expressions are evaluated. Values that contain no expressions keep their existing encoding, so constant-only data (including maps shaped like a proto Value) is unaffected. literals() shipped in 3.40.0, so the conversion is applied only to inputs that failed before. Adds a proto unit test and an integration test with expressions nested in a map (two levels deep) and in a list. Proposal P-3.
…t-side
- literals() now requires at least one document and fails fast with
IllegalArgumentException "Function literals() requires at least one
document." The backend previously rejected an empty literals stage
with INVALID_ARGUMENT. The varargs signature shipped in 3.40.0, so
this is a runtime check rather than a signature change.
- A null document now fails with a clear NullPointerException
("Function literals() called with invalid data. Document 0 is null.")
instead of an NPE from Map.entrySet().
- FieldValue sentinels anywhere in a literal value (including inside
nested maps and lists) are rejected with literals() wording and the
real field path, for example: "Function literals() called with invalid
data. FieldValue.serverTimestamp() can only be used with update() and
set() (found in field a.b)". Previously the message ended with
"at field ''". Values are still rejected before any RPC is sent, now
when literals() is called rather than when the pipeline is executed.
- null field values remain allowed.
POJO values are unchanged: they still fail with "Cannot convert ... to
Firestore Value". Whether literals() should map POJOs the way set()
does is left as an open decision (proposal P-4e).
Proposal P-4 (a, b, c, d).
…e.withTransformedFields() Add PipelineSource.literals(List<Map<String, Object>>) and Update.withTransformedFields(List<Selectable>) to mirror the existing varargs overloads and the List overloads offered elsewhere in the pipeline API. Note: with both overloads present, an untyped literals(null) call is now ambiguous at compile time (edge case; null was already rejected). Proposal P-2/P-6 (minor).
…xpose DML stage classes Add the missing convenience overloads so every DML stage has both a no-arg form and a form taking a pre-configured stage: - Pipeline.insert() (equivalent to insert(new Insert())) - Pipeline.delete(Delete) (equivalent to delete()) Remove the class-level @internalapi annotation from the user-constructed stage classes Insert, Upsert, Update and Delete. They are passed to public Pipeline methods (insert(Insert), upsert(Upsert), update(Update), delete(Delete)) and must be constructed by users, so marking them internal contradicted the public API. Literals keeps @internalapi: it is only created by PipelineSource.literals() and no public method accepts it. Proposal P-7 + P-12.
…Upsert Remove the duplicate alias methods so each option has a single name across the pipeline DML stages: - Insert.withDocumentId(Expression) -> use withDocumentIdExpression - Upsert.withDocumentId(Expression) -> use withDocumentIdExpression - Upsert.withTransformedFields(Selectable...) and (List<Selectable>) -> use withAdditionalFields These aliases (and the Insert/Upsert classes themselves) exist only on this feature branch and were never released (absent from origin/main), so removing them breaks no released API. Update.withTransformedFields is released and unchanged. The "backward compatibility" blocks in PipelineProtoTest that exercised the aliases are removed. Proposal P-9.
… or at a read time PipelineExecuteOptions.withAtomic(true) makes the backend start and auto-commit its own read-write transaction. Combining it with an enclosing transaction (Transaction.execute(pipeline, options)) or with a read time (read-only transactions) was silently ignored: the transaction ID took precedence and the atomic flag was dropped. Throw IllegalArgumentException from the request builder instead, before any RPC is sent: - atomic + transaction: "Atomic pipeline execution cannot be used inside a transaction. ..." - atomic + read time: "Atomic pipeline execution cannot be combined with a read time." Non-atomic execution in transactions and at a read time is unchanged. Adds a PipelineProtoTest unit test and the ITPipelineTest testAtomicExecutionInsideTransactionIsRejected integration test. (proposal P-14; pending team decision)
…ecution - Added Javadoc for the Insert and Upsert classes, their constructors and builder methods, Pipeline.insert(), insert(Insert), upsert(Upsert), upsert(Selectable...), and PipelineExecuteOptions.withAtomic()/isAtomic(). - Documented the insert/upsert target rules: with neither a collection nor a document ID expression the input path is reused (so inserting documents read from the database fails with ALREADY_EXISTS); with only a collection the input document ID is reused (generated if the input has none); with only a document ID expression the input document's parent collection is used. The document ID expression must evaluate to an ID, not a path. Upsert replaces the target document; stored fields are not merged. - Documented atomic execution: default false; when true, all writes are applied or none are; not allowed in Transaction.execute() or with a read time (throws IllegalArgumentException). - Documented the literals() contract on both overloads: at least one document, expressions (including nested ones) are evaluated, FieldValue sentinels are rejected (use currentTimestamp()), and __name__ sets the document path. - Extended the delete() and update() Javadoc (result shape, replace semantics, no document creation, atomicity). - Removed the stray "Performs an insert operation..." first sentence from the rawStage() Javadoc. CHANGELOG.md is generated by release-please and is not edited. Proposal P-16 (and the P-10 target rules).
Remove the client-side "at least one document" check from Literals
(and therefore from both PipelineSource.literals() overloads). The
backend already rejects an empty literals stage with INVALID_ARGUMENT
("... must have at least one document"), and the SDK should not
duplicate checks the backend enforces. The error now surfaces from
execute() instead of at pipeline build time.
Kept client-side:
- checkNotNull for a null documents array or list and for a null
document, since the SDK would otherwise fail with a bare
NullPointerException while encoding. The null-array message is now
"Function literals() called with invalid data. Documents must not be
null."
- FieldValue sentinel rejection, because sentinels cannot be encoded
in a pipeline, so the backend never sees them.
The literals() Javadoc now says that an empty set of documents is
rejected by the backend at execute time. The unit tests assert that an
empty literals() builds a stage with no arguments, and that
literals(emptyList()) encodes the same way. A new IT,
testLiteralsWithNoDocumentsIsRejectedByBackend, checks the backend
error.
Follows the decision not to duplicate backend-enforced checks
(proposal D-1 / P-4a).
Tighten the Javadoc and comments added for the pipeline DML stages and literals so they describe behavior or explain non-obvious code, without narrating how the change was made: - Document that insert() and upsert() without a target collection use the input document's __name__ path, and that a document without __name__ fails with INVALID_ARGUMENT. - State the empty literals() behavior plainly (execution fails with INVALID_ARGUMENT) and drop an internal comment that described why no client-side check exists. - Reword the sentinel validation Javadoc to say why sentinels are rejected. - Remove test comments that only restate the assertions, and reword the remaining ones as statements of behavior. No code changes.
…ction or at a read time This reverts commit 74d47c4. Atomic execution inside Transaction.execute is redundant but harmless, and atomic execution with a read time is rejected by the backend, so the SDK doesn't need dedicated checks for either.
…UserDataConverter Literal values are already encoded with UserDataConverter, which rejects FieldValue sentinels in pipeline arguments. Drop the separate sentinel walk and the literals-specific message, and use plain null-check messages. Sentinels are now reported when the pipeline is encoded rather than when literals() is called.
Document atomic execution once, on PipelineExecuteOptions.withAtomic, instead of on every DML method. Describe the insert and upsert target rules only on the Insert and Upsert classes, and remove the Javadoc for the rejected-atomic-in-transaction behavior that is no longer thrown.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces new 'Insert' and 'Upsert' stages to the Firestore pipeline, enabling document insertion and upsert operations. It also adds support for atomic pipeline execution and improves literal expression handling. My review identified potential issues where 'collection' and 'document_id' options are not correctly cleared when set to null in the 'Insert' and 'Upsert' stages, as well as opportunities to simplify the 'withAdditionalFields' methods in the 'Upsert' class by removing redundant null checks.
MarkDuckworth
approved these changes
Oct 21, 2026
MarkDuckworth
left a comment
MarkDuckworth
left a comment
Contributor
There was a problem hiding this comment.
lgtm with minor documentation nits
dlarocque
enabled auto-merge (squash)
October 22, 2026 16:51
jinseopkim0
pushed a commit
that referenced
this pull request
Oct 23, 2026
🤖 I have created a release *beep* *boop* --- <details><summary>1.94.0</summary> ## [1.94.0](v1.93.0...v1.94.0) (2026-10-22) ### Features * **auth:** request certificate-bound tokens for agent identities by ([01eaba1](01eaba1)) * **bigquery-jdbc:** Add clearcut telemetry client ([#14607](#14607)) ([c702438](c702438)) * **bigquery-jdbc:** expose QueryStatistics for jobless queries ([#14596](#14596)) ([15f472c](15f472c)) * **bigquery:** expose QueryStatistics from TableResult ([#14588](#14588)) ([40b2b1a](40b2b1a)) * **bigtable:** add per-method session diversion configuration ([#14681](#14681)) ([042dd3e](042dd3e)) * **bigtable:** support SessionCheckAndMutateRow on the session client ([#14565](#14565)) ([86de9c1](86de9c1)) * **bigtable:** support SessionReadModifyWriteRow on the session client ([#14585](#14585)) ([8e653b2](8e653b2)) * **firestore:** DML ([#14703](#14703)) ([11bc071](11bc071)) * **gax:** implement operation-level client request spans (T3) ([#14549](#14549)) ([275063b](275063b)) * **gax:** link attempt spans to parent context and clean up on operation completion ([#14459](#14459)) ([0c481bd](0c481bd)) * **gax:** standardize mapping terminal resumable upload exceptions to ApiException ([#14624](#14624)) ([d4961f0](d4961f0)) * **google/devicesandservices/health/v4beta:** add health v4beta ([#14600](#14600)) ([95f863c](95f863c)) * **google/shopping/merchant/reviews/v1:** add shopping-merchant-reviews ([#14724](#14724)) ([13310c7](13310c7)) * include writerId as part of trace id ([#13488](#13488)) ([ffa8063](ffa8063)) * log actionable errors on operation failure in LoggingTracer ([#14460](#14460)) ([27559f4](27559f4)) * populate client_stats in AppendRowsRequest ([#14614](#14614)) ([a7a0098](a7a0098)) * **spanner:** add option to create GcpFallbackChannel per channel instead of per pool ([#14501](#14501)) ([74f7591](74f7591)) ### Bug Fixes * **api-common:** validate path template variables against dot segments ([#14641](#14641)) ([4ad59e2](4ad59e2)) * **auth:** fix serialization gaps in DownscopedCredentials and UrlIdentityPoolSubjectTokenSupplier ([#14341](#14341)) ([a3e3324](a3e3324)) * **bigquery-jdbc:** ensure JDBC uses PQC-compliant algorithm by default ([#14560](#14560)) ([b0ffe3d](b0ffe3d)) * **bigquery-jdbc:** fix Exception handling ([#14566](#14566)) ([3e7a206](3e7a206)) * **bigquery-jdbc:** fix missing logger instances ([#14733](#14733)) ([0e822dc](0e822dc)) * **bigquery-jdbc:** fix pqc working in proxy env ([#14599](#14599)) ([ceb45b4](ceb45b4)) * **bigquery-jdbc:** format nested ARRAY fields as valid JSON in Struct.toString() ([#14720](#14720)) ([bc0126a](bc0126a)) * **bigquery-jdbc:** format TIMESTAMP as UTC in getString and add multi-zone TIMESTAMP(12) tests ([#14666](#14666)) ([e666612](e666612)) * **bigquery-jdbc:** honor `Calendar` timezone and prevent `ClassCastException` in ResultSet temporal getters ([#14657](#14657)) ([7dd2b37](7dd2b37)) * **bigquery-jdbc:** limit Storage Read API Arrow batch queue to 32 batches ([#14709](#14709)) ([d339c41](d339c41)) * **bigquery-jdbc:** normalize `setTime` from Calendar zone to JVM zone ([#14660](#14660)) ([54d2bc6](54d2bc6)) * **bigquery-jdbc:** poll the JSON reference queue and release processed finalizers ([#14719](#14719)) ([1b7d00d](1b7d00d)) * **bigquery-jdbc:** preserve `Timestamp` instant in `PreparedStatement` and `CallableStatement` Calendar overloads ([#14640](#14640)) ([3b02a44](3b02a44)) * **bigquery-jdbc:** preserve the instant on the TIMESTAMP write path and bind LocalDateTime as STRING ([#14647](#14647)) ([2106e26](2106e26)) * **bigquery-jdbc:** stop per-ResultSet logger registration leak ([#14718](#14718)) ([e83c768](e83c768)) * **bigquery-jdbc:** stop producer and release resources on ResultSet.close ([#14712](#14712)) ([6b99452](6b99452)) * **bigquery:** fix invalid mappings in domain models ([#14646](#14646)) ([17048ed](17048ed)) * **bigquery:** remove duplicate google-cloud-storage dependency ([#14586](#14586)) ([ab6173e](ab6173e)) * **datastore:** allow any gRPC TransportChannelProvider in DatastoreOptions ([#14478](#14478)) ([d9a8db5](d9a8db5)) * **datastore:** update emulator zip checksum for 2.3.1 ([#14592](#14592)) ([fb73ed5](fb73ed5)) * **deps:** update dependency com.google.api-client:google-api-client-bom to v2.9.2 ([#14725](#14725)) ([1ffa467](1ffa467)) * **gax-httpjson:** gracefully handle error details with unrecognized types ([#14612](#14612)) ([7295640](7295640)) * revise health metrics inflightBytes to become inflightBytesMax ([#14613](#14613)) ([44a12ef](44a12ef)) * **showcase:** update parent context retry tests for operation span in ITOtelTracing ([#14673](#14673)) ([312b5ea](312b5ea)) * **spanner:** increase max inbound message size from 256MB to 300MB ([#14551](#14551)) ([70c7439](70c7439)) * **spanner:** increase max inbound message size from 256MB to 300MB as ([70c7439](70c7439)) * **spanner:** prevent stream leaks and race conditions on result set close ([#14665](#14665)) ([013b49b](013b49b)) * **spanner:** support autoscaling config, edition, and positive capacity in cloud instance actions ([#14574](#14574)) ([004f0fd](004f0fd)) ### Performance Improvements * **bigquery:** defer blocking jobs.get RPC in QueryPageFetcher until getNextPage() ([#14650](#14650)) ([6183b63](6183b63)) ### Dependencies * Upgrade jackson-bom to v2.22.4 ([#14701](#14701)) ([09ce4c5](09ce4c5)) ### Documentation * **auth:** clarify quota project behavior on ImpersonatedCredentials ([#14597](#14597)) ([4e5a0d0](4e5a0d0)) * **pubsub:** Create a Cloud Pub/Sub sample for publishing with hedging ([#14623](#14623)) ([9de611f](9de611f)) * **pubsub:** regenerate README.md sample table ([#14643](#14643)) ([5394618](5394618)), refs [#14543](#14543) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.