ITADN

DistributedTransaction (preview): PatchItem cannot accept FilterPredicate — conditional patch is unreachable through DTx

#5934Openjeet1995 创建于 2026-06-09
needs-investigation
J
jeet1995commented
# DistributedTransaction (preview): `PatchItem` cannot accept `FilterPredicate` — conditional patch is unreachable through DTx ## Summary `PatchItemRequestOptions.FilterPredicate` is a public API on regular Cosmos point-patch and on `TransactionalBatchPatchItemRequestOptions` for the per-op options inside a `TransactionalBatch`. It allows the customer to specify a SQL `WHERE` predicate that the server evaluates atomically before applying the patch — the canonical primitive for state-machine transitions, bounded counter decrements, idempotent set-once writes, and per-tenant guards. The new `DistributedWriteTransaction.PatchItem` surface in the preview NuGet **cannot accept a `FilterPredicate`**: * The signature accepts only `DistributedTransactionRequestOptions`, which has no `FilterPredicate` property. * The implementation internally discards any Patch-specific options the customer might have set by instantiating a fresh empty `PatchItemRequestOptions()` on every call. So conditional Patch — one of the primary value propositions of Cosmos Patch — is unreachable through DTx today, forcing customers to fall back to a read-check-write pattern with `IfMatchEtag` that loses atomicity, adds round-trips, and can't be combined with the rest of a DTx commit as a single atomic unit. ## Environment * **Package**: `Microsoft.Azure.Cosmos 3.62.0-preview.0` (DTx surface gated behind `#if PREVIEW`) ## Evidence from SDK source ### 1. `FilterPredicate` is a publicly documented, customer-facing API on regular Patch options `Microsoft.Azure.Cosmos/src/RequestOptions/PatchItemRequestOptions.cs`: ```csharp /// <sample> /// PatchItemRequestOptions patchItemRequestOptions = new PatchItemRequestOptions() /// { /// FilterPredicate = "from c where c.taskNum = 3" /// }; /// </sample> public string FilterPredicate { get; set; } ``` ### 2. `TransactionalBatch` has a dedicated per-op options class for it — clear precedent `Microsoft.Azure.Cosmos/src/Batch/TransactionalBatchPatchItemRequestOptions.cs`: ```csharp /// <sample> /// TransactionalBatchPatchItemRequestOptions options = new TransactionalBatchPatchItemRequestOptions() /// { /// FilterPredicate = "from c where c.taskNum = 3" /// }; /// </sample> public string FilterPredicate { get; set; } ``` The TransactionalBatch team explicitly created a dedicated per-op options class so that `FilterPredicate` could ride along with `PatchItem` calls inside a batch. This is exactly the pattern DTx should follow. ### 3. The wire serializer renders the predicate `Microsoft.Azure.Cosmos/src/Patch/PatchOperationsJsonConverter.cs`: ```csharp if (!String.IsNullOrWhiteSpace(patchRequestOptions.FilterPredicate)) { writer.WritePropertyName(PatchConstants.PatchSpecAttributes.Condition); writer.WriteValue(patchRequestOptions.FilterPredicate); } ``` So when a `PatchSpec` carries Patch options with a non-empty `FilterPredicate`, the serializer emits a `condition` field that the server evaluates atomically. ### 4. DTx Patch discards any Patch-specific options the customer could supply `Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedWriteTransactionCore.cs`: ```csharp public override DistributedWriteTransaction PatchItem( Container container, PartitionKey partitionKey, string id, IReadOnlyList<PatchOperation> patchOperations, DistributedTransactionRequestOptions requestOptions = null) { (string databaseId, string containerId) = DistributedTransactionConstants.ValidateAndUnpackContainer(container, this.clientContext.Client); DistributedWriteTransactionCore.ValidateItemId(id); if (patchOperations == null || !patchOperations.Any()) { throw new ArgumentNullException(nameof(patchOperations)); } // ↓↓↓ Fresh empty PatchItemRequestOptions — any Patch-specific configuration // (FilterPredicate, etc.) the customer might have wanted to attach is // simply unreachable through this surface. PatchSpec patchSpec = new PatchSpec(patchOperations, new PatchItemRequestOptions()); this.operations.Add( new DistributedTransactionOperation<PatchSpec>( operationType: OperationType.Patch, operationIndex: this.operations.Count, databaseId, containerId, partitionKey, id, resource: patchSpec, requestOptions)); return this; } ``` The method signature only accepts `DistributedTransactionRequestOptions`, which has no `FilterPredicate`. The fresh `new PatchItemRequestOptions()` ensures that even if a future refactor smuggled the value in via `RequestOptions`, the serializer would still see an empty predicate. ## Customer impact For each surface, customers can express conditional Patch consistently — except DTx: | Surface | Conditional Patch reachable? | | --- | --- | | `Container.PatchItemAsync` (regular) | ✅ via `PatchItemRequestOptions.FilterPredicate` | | `TransactionalBatch.PatchItem` (per-op) | ✅ via `TransactionalBatchPatchItemRequestOptions.FilterPredicate` | | **`DistributedWriteTransaction.PatchItem` (per-op)** | **❌ no public surface; SDK discards any options the customer could attach** | The customer scenarios this blocks under DTx include: * **Atomic state-machine transitions** — `"WHERE c.status = 'pending'"` to enforce only valid transitions * **Bounded counters / inventory decrement** — `"WHERE c.inventory > 0"` to atomically decrement only if stock exists * **Idempotent set-once writes** — `"WHERE NOT IS_DEFINED(c.processedAt)"` to set a field exactly once * **Per-tenant guards** — `"WHERE c.tenantId = @tenant"` to prevent cross-tenant writes through shared API Today, a customer who wants any of these as part of a multi-op atomic DTx must: 1. Issue a separate read outside the DTx (round-trip 1) 2. Evaluate the predicate client-side 3. Compute the patch and capture the ETag 4. Issue the DTx with the Patch + `IfMatchEtag` (round-trip 2) 5. Handle the retry loop when the ETag drifted between steps 1 and 4 — which defeats the point of using DTx in the first place (atomicity across multiple operations, single round-trip). ## Suggested resolution Mirror the `TransactionalBatch` precedent. Two viable shapes, SDK team's call: 1. **(preferred — matches existing precedent)** Add a `DistributedTransactionPatchItemRequestOptions` class that inherits from `DistributedTransactionRequestOptions` and adds `FilterPredicate`. Add a `PatchItem` overload (or change the parameter type) so it accepts the Patch-specific variant. Then plumb the predicate through `PatchSpec` so `PatchOperationsJsonConverter` emits it on the wire. This matches `TransactionalBatchPatchItemRequestOptions` exactly. 2. **(simpler but bleeds concerns)** Add `FilterPredicate` directly to `DistributedTransactionRequestOptions`. Less ideal because the property would be meaningless for non-Patch ops, but avoids introducing a new type. Document the no-op behavior on non-Patch operation types. Either way, `DistributedWriteTransactionCore.PatchItem` should stop instantiating a fresh empty `PatchItemRequestOptions()` and instead forward the customer-supplied predicate. ## Related * #5932 — broader DTx response-header gaps (`SessionToken` / `RequestCharge` / `SubStatusCode` / `RetryAfter`) * #5933 — `IfNoneMatchEtag` silently dropped for DTx write operations All three issues are gated behind `#if PREVIEW` and have a clean fix window before GA. Thanks!
0 条评论