lib/collection/src/operations/mod.rs (1)

443-446: Exercise both presence/absence of update_filter in proptests

Right now, the arbitrary UpdateVectors case always uses update_filter: None. Generating cases with Some(Filter) too will better cover serialization/deserialization and downstream logic.

-            let update = Self::UpdateVectors(UpdateVectorsOp {
-                points: Vec::new(),
-                update_filter: None,
-            });
+            let update = any::<Option<Filter>>().prop_map(|update_filter| {
+                Self::UpdateVectors(UpdateVectorsOp {
+                    points: Vec::new(),
+                    update_filter,
+                })
+            });
@@
-            prop_oneof![Just(update), Just(delete), Just(delete_by_filter),].boxed()
+            prop_oneof![update, Just(delete), Just(delete_by_filter),].boxed()

lib/api/src/rest/schema.rs (2)

1192-1195: Document behavior and accept update_if alias for UpdateVectors

For vector updates, non-matching points must be skipped (no inserts happen for vector ops). Add docs and alias = "update_if" for a smooth transition.

-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    #[validate(nested)]
-    pub update_filter: Option<Filter>,
+    /// If specified, only points that match this filter will be updated.
+    /// Points that do not match will be skipped (vector updates never insert).
+    #[serde(default, skip_serializing_if = "Option::is_none", alias = "update_if")]
+    #[validate(nested)]
+    pub update_filter: Option<Filter>,

---

1203-1206: Clarify upsert wording and add update_if alias for PointsList

Refine the wording to avoid implying inserts for existing non-matching IDs and add a serde alias for backward compatibility.

-    /// If specified, only points that match this filter will be updated, others will be inserted
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    #[validate(nested)]
-    pub update_filter: Option<Filter>,
+    /// If specified, existing points that match this filter will be updated.
+    /// Existing points that do not match are left unchanged.
+    /// Non-existing points in the request will be inserted.
+    #[serde(default, skip_serializing_if = "Option::is_none", alias = "update_if")]
+    #[validate(nested)]
+    pub update_filter: Option<Filter>,

lib/collection/src/operations/vector_ops.rs (2)

42-45: Add rustdoc clarifying vector-update semantics

Helpful to spell out that non-matching points are skipped (no inserts for vector updates).

 pub struct UpdateVectorsOp {
     /// Points with named vectors
     pub points: Vec<PointVectorsPersisted>,
-    /// Condition to check before updating vectors
+    /// Condition to check before updating vectors.
+    /// Points that do not match the filter will be skipped (vector updates never insert).
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub update_filter: Option<Filter>,
 }

---

97-125: Preserve update_filter (correct) and simplify sharding logic

Current logic correctly clones update_filter per shard. You can reduce duplication by reusing split_iter_by_shard for PointVectorsPersisted.

Add this impl:

+impl SplitByShard for Vec<PointVectorsPersisted> {
+    fn split_by_shard(self, ring: &HashRingRouter) -> OperationToShard<Self> {
+        split_iter_by_shard(self, |point| point.id, ring)
+    }
+}

Then simplify the match arm:

-            VectorOperations::UpdateVectors(UpdateVectorsOp {
-                points,
-                update_filter,
-            }) => {
-                let shard_points = points
-                    .into_iter()
-                    .flat_map(|point| {
-                        point_to_shards(&point.id, ring)
-                            .into_iter()
-                            .map(move |shard_id| (shard_id, point.clone()))
-                    })
-                    .fold(
-                        AHashMap::new(),
-                        |mut map: AHashMap<u32, Vec<PointVectorsPersisted>>, (shard_id, points)| {
-                            map.entry(shard_id).or_default().push(points);
-                            map
-                        },
-                    );
-                let shard_ops = shard_points.into_iter().map(|(shard_id, points)| {
-                    (
-                        shard_id,
-                        VectorOperations::UpdateVectors(UpdateVectorsOp {
-                            points,
-                            update_filter: update_filter.clone(),
-                        }),
-                    )
-                });
-                OperationToShard::by_shard(shard_ops)
-            }
+            VectorOperations::UpdateVectors(UpdateVectorsOp { points, update_filter }) => {
+                points
+                    .split_by_shard(ring)
+                    .map(|points| {
+                        VectorOperations::UpdateVectors(UpdateVectorsOp {
+                            points,
+                            update_filter: update_filter.clone(),
+                        })
+                    })
+            }

lib/segment/src/types.rs (3)

3298-3324: Guard empty ids and make the builder harder to misuse

• If ids is empty, this produces an unsatisfiable filter. Prefer a no-op early return to avoid surprising behavior.
• Add #[must_use] so callers don't accidentally drop the returned Filter.

Apply this diff:

-    /// Create an extended filtering condition, which would also include filter by given list of IDs.
-    pub fn merge_with_ids(self, ids: impl IntoIterator<Item = PointIdType>) -> Filter {
+    /// Create an extended filtering condition, which would also include filter by given list of IDs.
+    #[must_use]
+    pub fn merge_with_ids(self, ids: impl IntoIterator<Item = PointIdType>) -> Filter {
         let has_id_condition: HasIdCondition = ids.into_iter().collect();
+        // No IDs supplied — leave the filter unchanged
+        if has_id_condition.has_id.is_empty() {
+            return self;
+        }
 
         let Filter {
             should,
             min_should,
             must,
             must_not,
         } = self;

---

3298-3324: Avoid duplicating HasId in must by intersecting with existing one (optional)

If must already contains a Condition::HasId, consider intersecting the existing set with the new ids instead of pushing another condition. This:

• keeps condition count smaller (helps filter_max_conditions limits),
• reduces planner work, and
• makes intent explicit (restrict to IDs ∩ new_ids).

If implementing, ensure:

• Intersection is computed on the underlying AHashSet,
• Large sets retain Arc via MaybeArc threshold,
• If the intersection is empty, the filter becomes unsatisfiable (or consider returning a known-empty sentinel if you have one).

Would you like me to draft a concrete patch that performs the intersection while preserving MaybeArc semantics?

---

3298-3324: Add unit tests for merge_with_ids semantics

Cover at least:

• Adds HasId to must while preserving should/min_should/must_not.
• No-op when ids is empty (after the early-return change).
• Optional: Intersects with existing HasId if you adopt the refactor.

Here is a test scaffold you can drop into the existing tests module in this file:

#[test]
fn test_merge_with_ids_adds_must() {
    let base = Filter {
        should: Some(vec![Condition::Field(FieldCondition::new_match(
            JsonPath::new("k"),
            Match::new_value(ValueVariants::String("v".into())),
        ))]),
        min_should: None,
        must: None,
        must_not: Some(vec![Condition::IsNull(JsonPath::new("n").into())]),
    };

    let merged = base.clone().merge_with_ids([ExtendedPointId::from(1_u64), ExtendedPointId::from(2_u64)]);
    assert!(merged.must.is_some());
    assert_eq!(merged.must.as_ref().unwrap().len(), 1);
    assert!(matches!(merged.must.as_ref().unwrap()[0], Condition::HasId(_)));

    // Preserve others
    assert_eq!(merged.should.as_ref().unwrap().len(), base.should.as_ref().unwrap().len());
    assert_eq!(merged.must_not.as_ref().unwrap().len(), base.must_not.as_ref().unwrap().len());
}

#[test]
fn test_merge_with_ids_empty_is_noop() {
    let base = Filter::new_must(Condition::Field(FieldCondition::new_is_empty(JsonPath::new("f"), true)));
    let merged = base.clone().merge_with_ids(std::iter::empty());
    assert_eq!(merged, base);
}

lib/api/src/grpc/proto/points.proto (2)

138-139: Clarify semantics for existing IDs that fail the filter

The comment says “others will be inserted”, which can be misread when the point already exists but doesn’t match the filter. Consider clarifying that:

• existing points not matching the filter are left unchanged (not re-inserted),
• non-existing points are inserted.

This helps avoid ambiguity across clients.

---

799-800: Mirror the upsert comment clarification

Same nit as above: explicitly document behavior for existing IDs that don’t satisfy the filter to avoid misinterpretation by client implementers.

tests/openapi/test_conditional_update.py (3)

1-1: Remove unused import

itemgetter is imported but unused. Please remove to keep tests clean.

-from operator import itemgetter

---

19-20: Fix misleading comments

Comments say “Upsert and delete points”, but these blocks perform conditional upserts/updates. Reword to something like “Conditionally upsert point” / “Conditionally update vectors”.

Also applies to: 34-35, 77-78, 119-121

---

144-155: Make vector presence explicit in GET

To avoid relying on default behavior, consider requesting vectors explicitly in the GET (e.g., query param to include vectors) so the assertion on result["vector"] is robust across configs.

lib/collection/src/shards/remote_shard.rs (1)

246-257: Optional: de-duplicate upsert code paths

Upsert and conditional upsert branches are identical except for the converter. Consider extracting a small helper that builds the request from either variant, reducing duplication and potential drift.

Also applies to: 498-511

lib/storage/src/rbac/ops_checks.rs (1)

1278-1279: Vector Update test updated for new field

Initializing update_filter: None keeps tests compiling against the expanded struct. Consider adding a positive test with a non-None filter later.

src/common/update.rs (2)

248-269: Propagate update_filter: OK; consider renaming locals to reduce shadowing

Logic looks correct and preserves shard_key/inference usage. Minor readability tweak: avoid reusing the name operation for multiple different types in this function.

Suggested rename across these lines and the subsequent construction below:

-    let (operation, shard_key, usage, update_filter) = match operation {
+    let (points_op, shard_key, usage, update_filter) = match operation {
@@
-            let operation = PointInsertOperationsInternal::PointsBatch(batch);
-            (operation, shard_key, usage, update_filter)
+            let points_op = PointInsertOperationsInternal::PointsBatch(batch);
+            (points_op, shard_key, usage, update_filter)
@@
-            let operation = PointInsertOperationsInternal::PointsList(list);
-            (operation, shard_key, usage, update_filter)
+            let points_op = PointInsertOperationsInternal::PointsList(list);
+            (points_op, shard_key, usage, update_filter)

And in the block below (Lines 272-281):

-    let operation = if let Some(condition) = update_filter {
+    let collection_op = if let Some(condition) = update_filter {
         CollectionUpdateOperations::PointOperation(PointOperations::UpsertPointsConditional(
             ConditionalInsertOperationInternal {
-                points_op: operation,
+                points_op,
                 condition,
             },
         ))
     } else {
-        CollectionUpdateOperations::PointOperation(PointOperations::UpsertPoints(operation))
+        CollectionUpdateOperations::PointOperation(PointOperations::UpsertPoints(points_op))
     };

Then pass collection_op to update().

---

360-364: Carry update_filter into UpdateVectorsOp: LGTM; optional naming nit

Correct propagation into UpdateVectorsOp. Optional: rename local operation to vector_op or collection_op for clarity when reading alongside other functions.

-    let operation = CollectionUpdateOperations::VectorOperation(VectorOperations::UpdateVectors(
+    let collection_op = CollectionUpdateOperations::VectorOperation(VectorOperations::UpdateVectors(
         UpdateVectorsOp {
             points,
             update_filter,
         },
     ));
@@
-    let result = update(
+    let result = update(
         toc,
         &collection_name,
-        operation,
+        collection_op,
         internal_params,
         params,
         shard_key,
         access,
         hw_measurement_acc,
     )

docs/grpc/docs.md (4)

3880-3881: Add explicit semantics example for upsert update_filter

Text is clear. Consider adding a short example clarifying that for upsert:

• existing points matching the filter are updated,
• existing points not matching the filter are left unchanged,
• non-existing points are inserted regardless of filter.

---

3931-3932: Clarify interaction between UpdateVectors points list and update_filter

State explicitly that update_filter is ANDed with the provided point IDs, i.e., only those point IDs in the request that also match the filter will be updated; others are skipped.

---

4910-4910: Mirror UpdateVectors semantics on the RPC request

Repeat the AND semantics with provided point IDs here to avoid ambiguity at the service layer. This helps client authors avoid assuming the filter is applied collection-wide.

---

4946-4946: Mention strict-mode/indexing implications for update_filter

Consider a short note: when strict mode enforces unindexed_filtering_update=false, update_filter conditions should target indexed fields; otherwise, the request may be rejected. This helps set user expectations.

src/tonic/api/update_common.rs (3)

62-65: Filter conversion duplicated across handlers — extract helper to reduce repetition

The TryFrom + transpose pattern is correct. It’s duplicated in multiple places (upsert, update_vectors, batch branches). Consider a small helper to centralize conversion.

Add helper (outside the changed range):

fn try_convert_filter(
    f: Option<grpc::qdrant::Filter>,
) -> Result<Option<segment::types::Filter>, Status> {
    f.map(segment::types::Filter::try_from).transpose()
}

Then here replace with:

-        update_filter: update_filter
-            .map(segment::types::Filter::try_from)
-            .transpose()?,
+        update_filter: try_convert_filter(update_filter)?,

Repeat similarly in update_vectors and update_batch branches below.

---

160-163: Use the same helper for update_filter conversion

Same duplication here; apply the try_convert_filter helper if you adopt it.

-        update_filter: update_filter
-            .map(segment::types::Filter::try_from)
-            .transpose()?,
+        update_filter: try_convert_filter(update_filter)?,

---

558-562: Batch UpdateVectors branch: correct pass-through of update_filter

Looks good and consistent with the non-batch handler. Consider adding an integration test that:

• sends multiple UpdateVectors ops where only a subset meets the filter,
• verifies only those are updated.

I can draft these tests if helpful.

Also applies to: 572-573

lib/api/src/grpc/qdrant.rs (2)

6028-6031: Clarify nested upsert semantics; consider enabling validation here too

• Suggest revising the doc to clarify behavior for existing non-matching points (left unchanged) vs inserts.
• For consistency with top-level messages, consider deriving validator::Validate for this struct in the proto and annotating update_filter with #[validate(nested)].

Example doc tweak:

-        /// If specified, only points that match this filter will be updated, others will be inserted
+        /// Conditional upsert. Update existing points that match this filter.
+        /// Insert points that don't exist. Existing non-matching points are left unchanged.

If you decide to validate here as well, update the proto so this message derives validator::Validate and the field is marked for nested validation (then re-generate).

---

6093-6096: Make nested vector-update semantics explicit; optionally enable nested validation

• Clarify that non-matching points are ignored (no insert).
• For parity with top-level UpdatePointVectors, consider deriving validator::Validate for this struct in the proto and adding #[validate(nested)] to the field.

Suggested doc tweak:

-        /// If specified, only points that match this filter will be updated
+        /// Conditional vector update. Only existing points matching this filter will be updated; others are ignored.

lib/collection/src/shards/conversions.rs (1)

88-119: Reduce duplication between upsert builders

internal_conditional_upsert_points largely duplicates internal_upsert_points construction logic except for update_filter. Consider factoring a small helper that builds UpsertPoints from common parts and takes an Option<Filter>.

This will centralize conversions and reduce maintenance cost.

lib/collection/src/operations/point_ops.rs (3)

486-492: New ConditionalInsertOperationInternal: aligns with PR goals

Structure is clear and minimal. Consider adding a brief doc comment clarifying semantics: inserts always; updates only if condition matches existing point.

---

639-656: Payload updates: filter + id merge is the right semantics

Using update_filter.merge_with_ids([id]) ensures the payload update applies only when the condition holds for that specific point. This matches the conditional update intent.

Optional micro-optimization: avoid cloning the whole Filter per point by holding let base = &update_filter; and cloning only once inside merge_with_ids.

Also applies to: 681-695

---

1128-1151: Tests updated for new field: good baseline

Including update_filter: None in REST tests keeps schema alignment. Consider adding tests that exercise non-None update_filter in the appropriate suite (as planned).

I can add a focused unit/integration test that validates conditional upsert behavior for both batch and list variants. Want me to draft it?

lib/collection/src/collection_manager/segments_updater.rs (2)

600-618: Filter inversion to find excluded IDs is correct and efficient

must = HasId(ids) ∧ must_not = Filter(condition) yields exactly the set of provided IDs that do not satisfy the condition. Good reuse of points_by_filter.

Minor nit: rename closure argument in later usage from idx to id for readability.

---

619-642: Conditional upsert semantics implemented as intended

• Only existing points are tested against the condition.
• Non-matching existing points are dropped from the op.
• Non-existing points are inserted unconditionally.

This aligns with the described behavior in the PR summary. One small readability tweak:

-    points_op.retain_point_ids(|idx| !points_to_exclude.contains(idx));
+    points_op.retain_point_ids(|id| !points_to_exclude.contains(id));

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: generall
PR: qdrant/qdrant#7006
File: lib/collection/src/shards/remote_shard.rs:258-268
Timestamp: 2025-08-10T18:22:33.034Z
Learning: In Qdrant, the conditional upsert feature with `update_filter` is designed to be rolled out only after a full cluster upgrade to the next minor release. Version compatibility guards in the code are not required because the feature deployment strategy ensures all nodes will support the feature before it's used, and it doesn't change existing functionality for users not using the conditional updates.

Learnt from: generall
PR: qdrant/qdrant#7006
File: src/common/update.rs:272-281
Timestamp: 2025-08-10T18:20:42.203Z
Learning: In Qdrant, conditional updates with `update_filter` always apply the condition check to a small, pre-determined set of point IDs (those being updated) and never perform a full collection scan. Therefore, strict mode validation for unindexed filtering is not necessary for conditional update operations, as the performance concerns that strict mode addresses don't apply to these targeted operations.

Learnt from: generall
PR: qdrant/qdrant#7006
File: docs/redoc/master/openapi.json:13645-13652
Timestamp: 2025-08-10T18:26:12.421Z
Learning: In qdrant, docs/redoc/master/openapi.json is auto-generated from the REST schemas. To change field docs, edit lib/api/src/rest/schema.rs (e.g., add doc comments or #[schemars(description = ...)]). Specifically, UpdateVectors.update_filter lacked a description and should state: "If specified, only update vectors for points that match this filter; points not matching the filter are left unchanged."

Learnt from: generall
PR: qdrant/qdrant#7006
File: docs/redoc/master/openapi.json:13645-13652
Timestamp: 2025-08-10T18:26:12.421Z
Learning: In qdrant, docs/redoc/master/openapi.json is auto-generated from the REST schemas. To change field docs, edit lib/api/src/rest/schema.rs (e.g., add doc comments or #[schemars(description = ...)]). Specifically, UpdateVectors.update_filter lacked a description and should state: "If specified, only update vectors for points that match this filter; points not matching the filter are left unchanged."

lib/api/src/grpc/qdrant.rs (2)

6021-6035: Batched upsert: add explicit per-id semantics to update_filter doc

Good call adding Validate + nested. The doc mirrors the ambiguity from the non-batch variant. Clarify behavior for existing-but-non-matching points.

Regenerate with this proto doc:

-        /// If specified, only points that match this filter will be updated, others will be inserted
+        /// Conditional upsert per point id:
+        /// - If an existing point with the same id matches this filter, it will be updated.
+        /// - If no point with the id exists, it will be inserted.
+        /// - If an existing point with the same id does not match this filter, it will be left unchanged.
+        /// An empty filter matches all points.

---

6088-6102: Batched vector update: specify non-matching and non-existing behavior

Validation looks good. The doc should explicitly state that non-matching existing points are untouched and non-existing points are not inserted.

Update proto comment so the generated docs read:

-        /// If specified, only points that match this filter will be updated
+        /// Conditional vector update. For each provided point:
+        /// - If an existing point with the same id matches this filter, its vectors will be updated.
+        /// - If an existing point with the same id does not match this filter, it will be left unchanged.
+        /// - Points that do not exist will not be inserted.
+        /// An empty filter matches all points.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: generall
PR: qdrant/qdrant#7006
File: lib/collection/src/shards/remote_shard.rs:258-268
Timestamp: 2025-08-10T18:22:33.034Z
Learning: In Qdrant, the conditional upsert feature with `update_filter` is designed to be rolled out only after a full cluster upgrade to the next minor release. Version compatibility guards in the code are not required because the feature deployment strategy ensures all nodes will support the feature before it's used, and it doesn't change existing functionality for users not using the conditional updates.

Learnt from: generall
PR: qdrant/qdrant#7006
File: src/common/update.rs:272-281
Timestamp: 2025-08-10T18:20:42.203Z
Learning: In Qdrant, conditional updates with `update_filter` always apply the condition check to a small, pre-determined set of point IDs (those being updated) and never perform a full collection scan. Therefore, strict mode validation for unindexed filtering is not necessary for conditional update operations, as the performance concerns that strict mode addresses don't apply to these targeted operations.

Learnt from: generall
PR: qdrant/qdrant#7006
File: docs/redoc/master/openapi.json:13645-13652
Timestamp: 2025-08-10T18:26:12.421Z
Learning: In qdrant, docs/redoc/master/openapi.json is auto-generated from the REST schemas. To change field docs, edit lib/api/src/rest/schema.rs (e.g., add doc comments or #[schemars(description = ...)]). Specifically, UpdateVectors.update_filter lacked a description and should state: "If specified, only update vectors for points that match this filter; points not matching the filter are left unchanged."

Learnt from: generall
PR: qdrant/qdrant#7006
File: lib/collection/src/operations/verification/update.rs:158-174
Timestamp: 2025-08-10T18:30:02.965Z
Learning: In Qdrant's strict mode verification code (lib/collection/src/operations/verification/update.rs), exhaustive pattern matching without `..` is intentionally used for structs like PointsBatch and PointsList. This design pattern ensures compilation fails when new fields are added, forcing developers to explicitly consider how new fields should be handled in the indexed_filter_write method. This provides visibility and compile-time safety for struct evolution.

Learnt from: generall
PR: qdrant/qdrant#7006
File: lib/collection/src/shards/remote_shard.rs:258-268
Timestamp: 2025-08-10T18:22:33.034Z
Learning: In Qdrant, the conditional upsert feature with `update_filter` is designed to be rolled out only after a full cluster upgrade to the next minor release. Version compatibility guards in the code are not required because the feature deployment strategy ensures all nodes will support the feature before it's used, and it doesn't change existing functionality for users not using the conditional updates.

Learnt from: generall
PR: qdrant/qdrant#7006
File: src/common/update.rs:272-281
Timestamp: 2025-08-10T18:20:42.203Z
Learning: In Qdrant, conditional updates with `update_filter` always apply the condition check to a small, pre-determined set of point IDs (those being updated) and never perform a full collection scan. Therefore, strict mode validation for unindexed filtering is not necessary for conditional update operations, as the performance concerns that strict mode addresses don't apply to these targeted operations.