From e916fe163810e176e1a368e7271b077659ee38e1 Mon Sep 17 00:00:00 2001 From: toptobes Date: Mon, 24 Aug 2026 23:03:36 -0500 Subject: [PATCH 1/7] Allow upsert document reconstruction to recognize $ands --- .../jsonapi/exception/UpdateException.java | 1 + .../collections/FindCollectionOperation.java | 76 ++++++++++++------- src/main/resources/errors.yaml | 10 +++ 3 files changed, 61 insertions(+), 26 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/UpdateException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/UpdateException.java index b1ead8f10b..7124cef9dd 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/UpdateException.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/UpdateException.java @@ -20,6 +20,7 @@ public enum Code implements ErrorCode { UNKNOWN_TABLE_COLUMNS, UNSUPPORTED_COLUMN_TYPES, UNSUPPORTED_OVERLAPPING_UPDATE_OPERATIONS, + UNSUPPORTED_OVERLAPPING_UPSERT_PATHS, UNSUPPORTED_UPDATE_DATA_TYPE, // from ErrorCodeV1 UNSUPPORTED_UPDATE_FOR_PRIMARY_KEY_COLUMNS, UNSUPPORTED_UPDATE_OPERATIONS_FOR_TABLE, diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java index 2c74be73cc..fc80469da0 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java @@ -10,8 +10,10 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression; +import io.stargate.sgv2.jsonapi.api.model.command.clause.update.ActionWithLocator; import io.stargate.sgv2.jsonapi.api.request.RequestContext; import io.stargate.sgv2.jsonapi.exception.SchemaException; +import io.stargate.sgv2.jsonapi.exception.UpdateException; import io.stargate.sgv2.jsonapi.service.cql.builder.Query; import io.stargate.sgv2.jsonapi.service.cql.builder.QueryBuilder; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.QueryExecutor; @@ -19,12 +21,11 @@ import io.stargate.sgv2.jsonapi.service.operation.builder.BuiltCondition; import io.stargate.sgv2.jsonapi.service.operation.filters.collection.CollectionFilter; import io.stargate.sgv2.jsonapi.service.operation.filters.collection.IDCollectionFilter; -import io.stargate.sgv2.jsonapi.service.operation.query.DBFilterBase; import io.stargate.sgv2.jsonapi.service.operation.query.DBLogicalExpression; import io.stargate.sgv2.jsonapi.service.projection.DocumentProjector; import io.stargate.sgv2.jsonapi.service.schema.collections.CollectionSchemaObject; import io.stargate.sgv2.jsonapi.service.schema.collections.spec.SuperShreddingMetadata; -import io.stargate.sgv2.jsonapi.service.shredding.collections.DocumentId; +import io.stargate.sgv2.jsonapi.util.PathMatchLocator; import java.util.*; import java.util.function.Supplier; @@ -453,42 +454,65 @@ public Uni getDocuments( } } - /** - * An operation method which can return ReadDocument with an empty document, if the filter - * condition has _id filter it will return document with this field added. - */ + /** An operation method which can return ReadDocument reconstructed from the search filter. */ public ReadDocument getNewDocument() { - final var rootNode = objectMapper().createObjectNode(); - DocumentId documentId = null; + final List paths = new ArrayList<>(); final var stack = new Stack(); stack.push(dbLogicalExpression); while (!stack.empty()) { var currentDbLogicalExpression = stack.pop(); - for (DBFilterBase filter : dbLogicalExpression.filters()) { - // every filter must be a collection filter, because we are making a new document, - // and we only do this for docs - if (filter instanceof IDCollectionFilter idFilter) { - documentId = idFilter.getSingularDocumentId(); - idFilter - .updateForNewDocument(objectMapper().getNodeFactory()) - .ifPresent(setOperation -> setOperation.updateDocument(rootNode)); - } else if (filter instanceof CollectionFilter collectionFilter) { - collectionFilter - .updateForNewDocument(objectMapper().getNodeFactory()) - .ifPresent(setOperation -> setOperation.updateDocument(rootNode)); - } else { - throw new IllegalArgumentException( - "Unsupported filter type in getNewDocument: %s" - .formatted(filter.getClass().getName())); + for (var filter : currentDbLogicalExpression.filters()) { + switch (filter) { + case CollectionFilter cf -> + cf.updateForNewDocument(objectMapper().getNodeFactory()) + .ifPresent( + op -> { + op.updateDocument(rootNode); + for (ActionWithLocator action : op.actions()) { + paths.add(action.locator()); + } + }); + default -> + throw new IllegalArgumentException( + "Unsupported filter type in getNewDocument: %s" + .formatted(filter.getClass().getName())); } } - currentDbLogicalExpression.subExpressions().forEach(stack::push); + if (currentDbLogicalExpression.operator() == DBLogicalExpression.DBLogicalOperator.AND) { + currentDbLogicalExpression.subExpressions().forEach(stack::push); + } } - return ReadDocument.from(documentId, null, rootNode); + + validateUpsertPaths(paths); + return ReadDocument.from(null, null, rootNode); + } + + private void validateUpsertPaths(List paths) { + Collections.sort(paths); + + for (var i = 0; i < paths.size() - 1; i++) { + final var current = paths.get(i); + final var next = paths.get(i + 1); + + if (current.compareTo(next) == 0) { + throwUnsupportedOverlappingUpsertPaths( + "Path '%s' is matched more than once".formatted(current)); + } + + if (next.isSubPathOf(current)) { + throwUnsupportedOverlappingUpsertPaths( + "Both paths '%s' and '%s' are matched".formatted(current, next)); + } + } + } + + private void throwUnsupportedOverlappingUpsertPaths(String message) { + throw UpdateException.Code.UNSUPPORTED_OVERLAPPING_UPSERT_PATHS.get( + errVars(commandContext.schemaObject(), map -> map.put("errorMessage", message))); } /** diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index 38906b8fe8..709da192f3 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -925,6 +925,16 @@ request-errors: Resend the command using a single update operation for each column. + - scope: UPDATE + code: UNSUPPORTED_OVERLAPPING_UPSERT_PATHS + title: Paths cannot be inferred for upsert due to overlap + body: |- + The command included overlapping paths, making it impossible to infer the document structure for the upsert. + + ${errorMessage} + + Resend the command ensuring paths are not contradictory or overlapping. + # Note UNSUPPORTED_VECTORIZE_WHEN_MISSING_VECTORIZE_DEFINITION is a duplicate for Document scope, this one is used for Update scope. - scope: UPDATE code: UNSUPPORTED_VECTORIZE_WHEN_MISSING_VECTORIZE_DEFINITION From 6a0a17c1bdd9e11a18767a2d0ad24c32a4b1aede Mon Sep 17 00:00:00 2001 From: toptobes Date: Tue, 25 Aug 2026 01:42:48 -0500 Subject: [PATCH 2/7] Evoultion which solves bifurcation of update reconstruction + improves behavior w/ findOneAndReplace --- .../clause/update/UpdateOperation.java | 2 +- .../collections/FindCollectionOperation.java | 22 ++++++-- .../ReadAndUpdateCollectionOperation.java | 15 ++--- .../service/updater/DocumentUpdater.java | 55 ++++++++++++------- 4 files changed, 57 insertions(+), 37 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperation.java index 1df0da2abd..dc2ae2d109 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperation.java @@ -20,7 +20,7 @@ protected UpdateOperation(List actions) { this.actions = actions; } - public List actions() { + public List actions() { return actions; } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java index fc80469da0..968e9cbe3c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java @@ -5,6 +5,7 @@ import com.bpodgursky.jbool_expressions.Expression; import com.datastax.oss.driver.api.core.cql.SimpleStatement; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.collect.Lists; import io.smallrye.mutiny.Uni; import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; @@ -27,6 +28,7 @@ import io.stargate.sgv2.jsonapi.service.schema.collections.spec.SuperShreddingMetadata; import io.stargate.sgv2.jsonapi.util.PathMatchLocator; import java.util.*; +import java.util.function.Predicate; import java.util.function.Supplier; /** Operation that returns the documents or its key based on the filter condition. */ @@ -454,8 +456,11 @@ public Uni getDocuments( } } - /** An operation method which can return ReadDocument reconstructed from the search filter. */ - public ReadDocument getNewDocument() { + public ReadDocument newEmptyDocument() { + return ReadDocument.from(null, null, objectMapper().createObjectNode()); + } + + public ObjectNode buildBaseDocument(Predicate pathFilter) { final var rootNode = objectMapper().createObjectNode(); final List paths = new ArrayList<>(); final var stack = new Stack(); @@ -470,9 +475,14 @@ public ReadDocument getNewDocument() { cf.updateForNewDocument(objectMapper().getNodeFactory()) .ifPresent( op -> { - op.updateDocument(rootNode); - for (ActionWithLocator action : op.actions()) { - paths.add(action.locator()); + var filtered = + op.actions().stream() + .map(ActionWithLocator::locator) + .filter(l -> pathFilter.test(l.path())) + .toList(); + + if (paths.addAll(filtered)) { + op.updateDocument(rootNode); } }); default -> @@ -488,7 +498,7 @@ public ReadDocument getNewDocument() { } validateUpsertPaths(paths); - return ReadDocument.from(null, null, rootNode); + return rootNode; } private void validateUpsertPaths(List paths) { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ReadAndUpdateCollectionOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ReadAndUpdateCollectionOperation.java index 9cb22c9469..97c9277e52 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ReadAndUpdateCollectionOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ReadAndUpdateCollectionOperation.java @@ -77,14 +77,7 @@ public Uni> execute( pageStateReference.set(findResponse.pageState()); final List docs = findResponse.docs(); if (upsert() && docs.isEmpty() && matchedCount.get() == 0) { - // TODO: creating the new document here, with the defaults from the filter, makes it - // harder because the new document created here may nto have an _id if there was - // none in the filter. A better approach may be to have the documentUpdater create - // the upsert document totally in once place. - // Currently creating to upsert document is in multiple places. To do this we would - // create UpdateOperations from the filter and give them to the document updated - // when it is created. - return Multi.createFrom().item(findCollectionOperation().getNewDocument()); + return Multi.createFrom().item(findCollectionOperation().newEmptyDocument()); } else { matchedCount.addAndGet(docs.size()); return Multi.createFrom().items(docs.stream()); @@ -182,7 +175,11 @@ private Uni processUpdate( JsonNode originalDocument = upsert ? null : readDocument.get(); DocumentUpdater.DocumentUpdaterResponse documentUpdaterResponse = - documentUpdater().apply(readDocument.get().deepCopy(), upsert); + documentUpdater() + .apply( + readDocument.get().deepCopy(), + upsert, + findCollectionOperation()::buildBaseDocument); return documentUpdaterResponse .updateEmbeddingVector( diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdater.java b/src/main/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdater.java index de60019225..a6154a6b02 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdater.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdater.java @@ -17,6 +17,8 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.function.Function; +import java.util.function.Predicate; /** Updates the document read from the database with the updates came as part of the request. */ public record DocumentUpdater( @@ -24,6 +26,9 @@ public record DocumentUpdater( ObjectNode replaceDocument, JsonNode replaceDocumentId, UpdateType updateType) { + + public interface DocumentReconstructor extends Function, ObjectNode> {} + /** * Construct to create updater using update clause * @@ -51,49 +56,53 @@ public static DocumentUpdater construct(ObjectNode replaceDocument) { * to do the following embedding update. * * @param readDocument Document to update - * @param docInserted True if document was just created (inserted); false if updating existing - * document + * @param isNew True if document was just created (inserted); false if updating existing document */ - public DocumentUpdaterResponse apply(JsonNode readDocument, boolean docInserted) { - ObjectNode docToUpdate = (ObjectNode) readDocument; + public DocumentUpdaterResponse apply( + JsonNode readDocument, boolean isNew, DocumentReconstructor reconstructor) { + final var docToUpdate = (ObjectNode) readDocument; + if (UpdateType.UPDATE == updateType) { - return update(docToUpdate, docInserted); + return update(docToUpdate, isNew, reconstructor); } else { - return replace(docToUpdate, docInserted); + return replace(docToUpdate, isNew, reconstructor); } } /** * Will be used for update commands. This is first level replace. This method will replace the * document, but won't re-vectorize yet(detail in updateEmbeddingVector method) - * - * @param docToUpdate - * @param docInserted - * @return */ - private DocumentUpdaterResponse update(ObjectNode docToUpdate, boolean docInserted) { - boolean modified = false; + private DocumentUpdaterResponse update( + ObjectNode docToUpdate, boolean isNew, DocumentReconstructor reconstructor) { + if (isNew) { + docToUpdate = reconstructor.apply(path -> true); + } + + boolean modified = isNew; List embeddingUpdateOperationList = new ArrayList<>(); - for (UpdateOperation updateOperation : updateOperations) { - if (updateOperation.shouldApplyIf(docInserted)) { - final UpdateOperation.UpdateOperationResult updateOperationResult = - updateOperation.updateDocument(docToUpdate); + + for (var updateOperation : updateOperations) { + if (updateOperation.shouldApplyIf(isNew)) { + final var updateOperationResult = updateOperation.updateDocument(docToUpdate); modified |= updateOperationResult.modified(); embeddingUpdateOperationList.addAll(updateOperationResult.embeddingUpdateOperations()); } } + return new DocumentUpdaterResponse(docToUpdate, modified, embeddingUpdateOperationList); } /** * Will be used for findOneAndReplace. This is first level replace. This method will replace the * document, but won't re-vectorize yet(detail in updateEmbeddingVector method) - * - * @param docToUpdate - * @param docInserted - * @return */ - private DocumentUpdaterResponse replace(ObjectNode docToUpdate, boolean docInserted) { + private DocumentUpdaterResponse replace( + ObjectNode docToUpdate, boolean isNew, DocumentReconstructor reconstructor) { + if (isNew) { + docToUpdate = reconstructor.apply(DocumentConstants.Fields.DOC_ID::equals); + } + // Do deep clone so we can remove _id field and check ObjectNode compareDoc = docToUpdate.deepCopy(); @@ -102,6 +111,10 @@ private DocumentUpdaterResponse replace(ObjectNode docToUpdate, boolean docInser // from the replacement document to use later, future work needed to so we can // reliably go back and forth between JsonNode and DocumentId without losing the benefits of // both. + // + // addendum (toptobes) - instead of removing the field from the doc we can just overwrite the + // _id in the replaceDocument (if idNode != null) as they're proven to be already equivalent + // (i.e. no compareDoc, no deep clone, and use .get() instead of .remove()) JsonNode idNode = compareDoc.remove(DocumentConstants.Fields.DOC_ID); // The replace document cannot specify an _id value that differs from the replaced document. From b6343a6591e4b2f7b81010ebf4d512fb9e733a14 Mon Sep 17 00:00:00 2001 From: toptobes Date: Tue, 25 Aug 2026 02:11:57 -0500 Subject: [PATCH 3/7] fix test compilation --- .../service/updater/DocumentUpdaterTest.java | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdaterTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdaterTest.java index 97401a1bab..46a87065f4 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdaterTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdaterTest.java @@ -67,7 +67,7 @@ public void setUpdateCondition() throws Exception { UpdateOperator.SET, objectMapper.getNodeFactory().objectNode().put("location", "New York"))); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( @@ -96,7 +96,7 @@ public void setUpdateNewData() throws Exception { UpdateOperator.SET, objectMapper.getNodeFactory().objectNode().put("new_data", "data"))); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( @@ -125,7 +125,7 @@ public void setUpdateVector() throws Exception { UpdateOperator.SET, objectMapper.getNodeFactory().objectNode().put("new_data", "data"))); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( @@ -157,7 +157,7 @@ public void setVectorData() throws Exception { DocumentUpdaterUtils.updateClause( UpdateOperator.SET, (ObjectNode) objectMapper.readTree(vectorData))); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( @@ -177,7 +177,7 @@ public void unsetVectorData() throws Exception { UpdateOperator.UNSET, objectMapper.getNodeFactory().objectNode().put("$vector", ""))); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( @@ -206,7 +206,7 @@ public void setUpdateNumberData() throws Exception { UpdateOperator.SET, objectMapper.getNodeFactory().objectNode().put("new_data", 40))); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( @@ -234,7 +234,7 @@ public void unsetUpdateData() throws Exception { DocumentUpdaterUtils.updateClause( UpdateOperator.UNSET, objectMapper.getNodeFactory().objectNode().put("col", 1))); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( @@ -430,7 +430,7 @@ public void replaceDocument() throws Exception { } """)); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( @@ -465,7 +465,7 @@ public void replaceDocumentSameId() throws Exception { } """)); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( @@ -493,7 +493,7 @@ public void replaceWithDifferentId() throws Exception { catchThrowable( () -> { DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); }); assertThat(t) .isNotNull() @@ -518,7 +518,7 @@ public void replaceEmpty() throws Exception { DocumentUpdater documentUpdater = DocumentUpdater.construct((ObjectNode) objectMapper.readTree("{ }")); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( @@ -555,7 +555,7 @@ public void updateVectorize() throws Exception { JsonNode baseData = objectMapper.readTree(BASE_DOC_JSON); // location as London JsonNode expectedData1 = objectMapper.readTree(expected1); DocumentUpdater.DocumentUpdaterResponse firstResponse = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(firstResponse) .isNotNull() .satisfies( @@ -626,7 +626,7 @@ public void update_noVectorize() throws Exception { JsonNode baseData = objectMapper.readTree(BASE_DOC_JSON); // location as London JsonNode expectedData1 = objectMapper.readTree(expected1); DocumentUpdater.DocumentUpdaterResponse firstResponse = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(firstResponse) .isNotNull() .satisfies( @@ -660,7 +660,7 @@ public void update_notModified() throws Exception { JsonNode baseData = objectMapper.readTree(BASE_DOC_JSON); // location as London JsonNode expectedData1 = objectMapper.readTree(expected1); DocumentUpdater.DocumentUpdaterResponse firstResponse = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(firstResponse) .isNotNull() .satisfies( @@ -696,7 +696,7 @@ public void update_notModifiedVectorize() throws Exception { JsonNode baseData = objectMapper.readTree(BASE_DOC_JSON_VECTOR); // location as London JsonNode expectedData1 = objectMapper.readTree(expected1); DocumentUpdater.DocumentUpdaterResponse firstResponse = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(firstResponse) .isNotNull() .satisfies( @@ -732,7 +732,7 @@ public void update_modifiedVector() throws Exception { JsonNode baseData = objectMapper.readTree(BASE_DOC_JSON_VECTOR); // location as London JsonNode expectedData1 = objectMapper.readTree(expected1); DocumentUpdater.DocumentUpdaterResponse firstResponse = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(firstResponse) .isNotNull() .satisfies( @@ -769,7 +769,7 @@ public void update_vectorizeOverwriteVector() throws Exception { JsonNode baseData = objectMapper.readTree(BASE_DOC_JSON_VECTOR); // location as London JsonNode expectedData1 = objectMapper.readTree(expected1); DocumentUpdater.DocumentUpdaterResponse firstResponse = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(firstResponse) .isNotNull() .satisfies( @@ -839,7 +839,7 @@ public void update_vectorizeBlank() throws JsonProcessingException { JsonNode baseData = objectMapper.readTree(BASE_DOC_JSON_VECTOR); // location as London JsonNode expectedData1 = objectMapper.readTree(expected1); DocumentUpdater.DocumentUpdaterResponse firstResponse = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(firstResponse) .isNotNull() .satisfies( @@ -876,7 +876,7 @@ public void update_vectorizeNullValue() throws JsonProcessingException { JsonNode baseData = objectMapper.readTree(BASE_DOC_JSON_VECTOR); // location as London JsonNode expectedData1 = objectMapper.readTree(expected1); DocumentUpdater.DocumentUpdaterResponse firstResponse = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(firstResponse) .isNotNull() .satisfies( @@ -905,7 +905,7 @@ public void update_vectorizeNonTextualFailure() throws JsonProcessingException { catchThrowable( () -> { DocumentUpdater.DocumentUpdaterResponse firstResponse = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); }); assertThat(failure) .isInstanceOf(DocumentException.class) @@ -940,7 +940,7 @@ public void replaceDocument() throws Exception { } """)); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( @@ -1008,7 +1008,7 @@ public void replaceDocument_only_replace_vector() throws Exception { } """)); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( @@ -1042,7 +1042,7 @@ public void replaceDocument_vectorizeBlankTest() throws Exception { } """)); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( @@ -1079,7 +1079,7 @@ public void replaceDocument_vectorizeNonTextFailure() throws Exception { catchThrowable( () -> { DocumentUpdater.DocumentUpdaterResponse firstResponse = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); }); assertThat(failure) .isInstanceOf(DocumentException.class) @@ -1111,7 +1111,7 @@ public void replaceDocument_vectorizeNullValue() throws Exception { } """)); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( @@ -1154,7 +1154,7 @@ public void replaceDocument_allNull() throws Exception { } """)); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( @@ -1189,7 +1189,7 @@ public void replaceDocument_willVectorizeEvenVectorizeHasNoDiff() throws Excepti } """)); DocumentUpdater.DocumentUpdaterResponse updatedDocument = - documentUpdater.apply(baseData, false); + documentUpdater.apply(baseData, false, null); assertThat(updatedDocument) .isNotNull() .satisfies( From 3d2f9730a5e113da2c16bbfcbe0a6e1699619688 Mon Sep 17 00:00:00 2001 From: toptobes Date: Tue, 25 Aug 2026 03:47:25 -0500 Subject: [PATCH 4/7] Tests for FindCollectionOperationTest.java --- .../collections/FindCollectionOperation.java | 17 +- .../FindCollectionOperationTest.java | 170 ++++++++++++++++++ 2 files changed, 182 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java index 968e9cbe3c..7a211ddd3f 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java @@ -12,6 +12,7 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression; import io.stargate.sgv2.jsonapi.api.model.command.clause.update.ActionWithLocator; +import io.stargate.sgv2.jsonapi.api.model.command.clause.update.SetOperation; import io.stargate.sgv2.jsonapi.api.request.RequestContext; import io.stargate.sgv2.jsonapi.exception.SchemaException; import io.stargate.sgv2.jsonapi.exception.UpdateException; @@ -462,13 +463,19 @@ public ReadDocument newEmptyDocument() { public ObjectNode buildBaseDocument(Predicate pathFilter) { final var rootNode = objectMapper().createObjectNode(); - final List paths = new ArrayList<>(); + final var paths = new ArrayList(); + final var ops = new ArrayList(); + final var stack = new Stack(); stack.push(dbLogicalExpression); while (!stack.empty()) { var currentDbLogicalExpression = stack.pop(); + if (currentDbLogicalExpression.operator() != DBLogicalExpression.DBLogicalOperator.AND) { + continue; + } + for (var filter : currentDbLogicalExpression.filters()) { switch (filter) { case CollectionFilter cf -> @@ -482,7 +489,7 @@ public ObjectNode buildBaseDocument(Predicate pathFilter) { .toList(); if (paths.addAll(filtered)) { - op.updateDocument(rootNode); + ops.add(op); } }); default -> @@ -492,12 +499,12 @@ public ObjectNode buildBaseDocument(Predicate pathFilter) { } } - if (currentDbLogicalExpression.operator() == DBLogicalExpression.DBLogicalOperator.AND) { - currentDbLogicalExpression.subExpressions().forEach(stack::push); - } + currentDbLogicalExpression.subExpressions().forEach(stack::push); } validateUpsertPaths(paths); + ops.forEach(op -> op.updateDocument(rootNode)); + return rootNode; } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java index 18dde25f11..9ca566aaa6 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java @@ -2,6 +2,7 @@ import static io.restassured.RestAssured.given; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; @@ -29,6 +30,7 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; import io.stargate.sgv2.jsonapi.config.constants.DocumentConstants; import io.stargate.sgv2.jsonapi.exception.DatabaseException; +import io.stargate.sgv2.jsonapi.exception.UpdateException; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.QueryExecutor; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.VectorColumnDefinition; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.VectorConfig; @@ -2994,6 +2996,174 @@ public void readFailureException() throws UnknownHostException { } } + @Nested + class BuildBaseDocument { + @Test + public void buildsFullDocumentWhenPredicateIsTrue() throws Exception { + var andExpr = new DBLogicalExpression(DBLogicalExpression.DBLogicalOperator.AND); + + andExpr.addFilter( + new IDCollectionFilter(IDCollectionFilter.Operator.EQ, DocumentId.fromString("user-1"))); + andExpr.addFilter( + new TextCollectionFilter("address.city", MapCollectionFilter.Operator.EQ, "NYC")); + andExpr.addFilter( + new TextCollectionFilter("address.post.code", MapCollectionFilter.Operator.EQ, "BTSME")); + andExpr.addFilter( + new BoolCollectionFilter("ok", MapCollectionFilter.Operator.EQ, true)); + + var operation = mkUnsortedOperation(andExpr); + var result = operation.buildBaseDocument(path -> true); + + var expected = + """ + { + "_id": "user-1", + "address": { + "city": "NYC", + "post": { + "code": "BTSME" + } + }, + "ok": true + } + """; + assertThat(result).isEqualTo(objectMapper.readTree(expected)); + } + + @Test + public void filtersPathsBasedOnPredicate() throws Exception { + var andExpr = new DBLogicalExpression(DBLogicalExpression.DBLogicalOperator.AND); + + andExpr.addFilter( + new IDCollectionFilter(IDCollectionFilter.Operator.EQ, DocumentId.fromString("user-1"))); + andExpr.addFilter( + new TextCollectionFilter("address.city", MapCollectionFilter.Operator.EQ, "London")); + andExpr.addFilter( + new TextCollectionFilter("preferences.email", MapCollectionFilter.Operator.EQ, "true")); + + var operation = mkUnsortedOperation(andExpr); + var result = + operation.buildBaseDocument(path -> path.equals("_id") || path.startsWith("preferences")); + + var expected = + """ + { + "_id": "user-1", + "preferences": { + "email": "true" + } + } + """; + assertThat(result).isEqualTo(objectMapper.readTree(expected)); + } + + @Test + public void throwsExceptionOnOverlappingPaths() { + var andExpr = new DBLogicalExpression(DBLogicalExpression.DBLogicalOperator.AND); + + andExpr.addFilter( + new IDCollectionFilter(IDCollectionFilter.Operator.EQ, DocumentId.fromString("1"))); + andExpr.addFilter( + new TextCollectionFilter("conf.net", MapCollectionFilter.Operator.EQ, "wifi")); + andExpr.addFilter( + new TextCollectionFilter("conf.net.ip", MapCollectionFilter.Operator.EQ, "10.0.0.1")); + + var operation = mkUnsortedOperation(andExpr); + + assertThatThrownBy(() -> operation.buildBaseDocument(path -> true)) + .isInstanceOf(UpdateException.class) + .hasMessageContaining("Both paths 'conf.net' and 'conf.net.ip' are matched"); + } + + @Test + public void bypassesExceptionIfOverlappingPathsFilteredOut() throws Exception { + var andExpr = new DBLogicalExpression(DBLogicalExpression.DBLogicalOperator.AND); + + andExpr.addFilter( + new IDCollectionFilter(IDCollectionFilter.Operator.EQ, DocumentId.fromString("1"))); + andExpr.addFilter( + new TextCollectionFilter("conf.net", MapCollectionFilter.Operator.EQ, "wifi")); + andExpr.addFilter( + new TextCollectionFilter("conf.net.ip", MapCollectionFilter.Operator.EQ, "10.0.0.1")); + + var operation = mkUnsortedOperation(andExpr); + var result = operation.buildBaseDocument(path -> path.equals("_id")); + + var expected = + """ + { + "_id": "1" + } + """; + assertThat(result).isEqualTo(objectMapper.readTree(expected)); + } + + @Test + public void handlesArbitrarilyNestedAndExpressions() { + var depth = 50; + var top = new DBLogicalExpression(DBLogicalExpression.DBLogicalOperator.AND); + top.addFilter( + new IDCollectionFilter(IDCollectionFilter.Operator.EQ, DocumentId.fromString("user-1"))); + + var current = top; + for (int i = 1; i <= depth; i++) { + var nested = new DBLogicalExpression(DBLogicalExpression.DBLogicalOperator.AND); + nested.addFilter( + new TextCollectionFilter("field" + i, MapCollectionFilter.Operator.EQ, "value" + i)); + current.addSubExpressionReturnSub(nested); + current = nested; + } + + var operation = mkUnsortedOperation(top); + var result = operation.buildBaseDocument(path -> true); + + var expected = objectMapper.createObjectNode(); + expected.put("_id", "user-1"); + for (int i = 1; i <= depth; i++) { + expected.put("field" + i, "value" + i); + } + assertThat(result).isEqualTo(expected); + } + + @Test + public void ignoresFiltersUnderOrSubExpressions() throws Exception { + var top = new DBLogicalExpression(DBLogicalExpression.DBLogicalOperator.AND); + top.addFilter( + new IDCollectionFilter(IDCollectionFilter.Operator.EQ, DocumentId.fromString("user-1"))); + + var orExpr = new DBLogicalExpression(DBLogicalExpression.DBLogicalOperator.OR); + orExpr.addFilter( + new TextCollectionFilter("address.city", MapCollectionFilter.Operator.EQ, "NYC")); + orExpr.addFilter( + new TextCollectionFilter("address.city", MapCollectionFilter.Operator.EQ, "LA")); + top.addSubExpressionReturnSub(orExpr); + + var operation = mkUnsortedOperation(top); + var result = operation.buildBaseDocument(path -> true); + + var expected = + """ + { + "_id": "user-1" + } + """; + assertThat(result).isEqualTo(objectMapper.readTree(expected)); + } + + private FindCollectionOperation mkUnsortedOperation(DBLogicalExpression expression) { + return FindCollectionOperation.unsorted( + COMMAND_CONTEXT, + expression, + DocumentProjector.defaultProjector(), + null, + 20, + 20, + CollectionReadType.DOCUMENT, + objectMapper, + false); + } + } + MockRow resultRow(int index, String key, UUID txId, String doc) { return new MockRow( KEY_TXID_JSON_COLUMNS, From baf920d02ffe5ef23575f9b53474f0d73ff07955 Mon Sep 17 00:00:00 2001 From: toptobes Date: Tue, 25 Aug 2026 16:13:33 -0500 Subject: [PATCH 5/7] tiny stuff --- .../collections/FindCollectionOperation.java | 13 +++++++------ .../collections/FindCollectionOperationTest.java | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java index 7a211ddd3f..e818c93a17 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java @@ -482,13 +482,14 @@ public ObjectNode buildBaseDocument(Predicate pathFilter) { cf.updateForNewDocument(objectMapper().getNodeFactory()) .ifPresent( op -> { - var filtered = - op.actions().stream() - .map(ActionWithLocator::locator) - .filter(l -> pathFilter.test(l.path())) - .toList(); + var beforeSize = paths.size(); - if (paths.addAll(filtered)) { + op.actions().stream() + .map(ActionWithLocator::locator) + .filter(l -> pathFilter.test(l.path())) + .forEach(paths::add); + + if (paths.size() > beforeSize) { ops.add(op); } }); diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java index 9ca566aaa6..6e6be0c97e 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java @@ -3009,7 +3009,7 @@ public void buildsFullDocumentWhenPredicateIsTrue() throws Exception { andExpr.addFilter( new TextCollectionFilter("address.post.code", MapCollectionFilter.Operator.EQ, "BTSME")); andExpr.addFilter( - new BoolCollectionFilter("ok", MapCollectionFilter.Operator.EQ, true)); + new BoolCollectionFilter("is_alive", MapCollectionFilter.Operator.EQ, true)); var operation = mkUnsortedOperation(andExpr); var result = operation.buildBaseDocument(path -> true); @@ -3024,7 +3024,7 @@ public void buildsFullDocumentWhenPredicateIsTrue() throws Exception { "code": "BTSME" } }, - "ok": true + "is_alive": true } """; assertThat(result).isEqualTo(objectMapper.readTree(expected)); From defd52de6b637bbf9b4b5af6447ec0d276f9a3a6 Mon Sep 17 00:00:00 2001 From: toptobes Date: Tue, 25 Aug 2026 16:53:40 -0500 Subject: [PATCH 6/7] add documentation --- .../clause/update/UpdateOperation.java | 6 +++++ .../collections/FindCollectionOperation.java | 25 ++++++++++++++++++- .../ReadAndUpdateCollectionOperation.java | 2 +- .../service/updater/DocumentUpdater.java | 13 +++++++--- .../FindCollectionOperationTest.java | 13 +++++----- 5 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperation.java index dc2ae2d109..c34641230e 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperation.java @@ -20,6 +20,12 @@ protected UpdateOperation(List actions) { this.actions = actions; } + /** + * @apiNote Doesn't return {@code List}, otherwise an operation like {@code + * actions().map(ActionWithLocator::locator)} may error trying to call a package-private + * implementation (e.g. {@code SetOperation.Action::locator}) + * @return List of actions that this update operation will apply to document + */ public List actions() { return actions; } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java index e818c93a17..5ccb3a3c80 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperation.java @@ -457,11 +457,27 @@ public Uni getDocuments( } } + /** + * Creates a new empty document with no fields. Intended as a placeholder for when the find + * operation does not return any documents, but the caller still needs a document to work with + * (e.g. an upsert). + * + * @return A new empty document with no fields. + */ public ReadDocument newEmptyDocument() { return ReadDocument.from(null, null, objectMapper().createObjectNode()); } - public ObjectNode buildBaseDocument(Predicate pathFilter) { + /** + * Reconstructs a document from the filter used in the find operation, intended for upserts. + * + *

Contradictory filters (e.g. {@code {"a": 1, "a": 2}}) will cause an error to be thrown. + * + * @param pathFilter A predicate to filter which paths should be included in the reconstructed + * document. Contradictory paths not matched in the document won't cause an error. + * @return The reconstructed document as an {@code ObjectNode}. + */ + public ObjectNode reconstructDocumentFromFilter(Predicate pathFilter) { final var rootNode = objectMapper().createObjectNode(); final var paths = new ArrayList(); final var ops = new ArrayList(); @@ -509,6 +525,13 @@ public ObjectNode buildBaseDocument(Predicate pathFilter) { return rootNode; } + /** + * Validates that the given paths do not overlap or contradict each other. + * + *

Throws an {@code UNSUPPORTED_OVERLAPPING_UPSERT_PATHS} on error. + * + * @param paths The list of paths to validate. + */ private void validateUpsertPaths(List paths) { Collections.sort(paths); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ReadAndUpdateCollectionOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ReadAndUpdateCollectionOperation.java index 97c9277e52..dc7bae66b4 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ReadAndUpdateCollectionOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ReadAndUpdateCollectionOperation.java @@ -179,7 +179,7 @@ private Uni processUpdate( .apply( readDocument.get().deepCopy(), upsert, - findCollectionOperation()::buildBaseDocument); + findCollectionOperation()::reconstructDocumentFromFilter); return documentUpdaterResponse .updateEmbeddingVector( diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdater.java b/src/main/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdater.java index a6154a6b02..b31079f6fd 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdater.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdater.java @@ -26,9 +26,6 @@ public record DocumentUpdater( ObjectNode replaceDocument, JsonNode replaceDocumentId, UpdateType updateType) { - - public interface DocumentReconstructor extends Function, ObjectNode> {} - /** * Construct to create updater using update clause * @@ -50,6 +47,15 @@ public static DocumentUpdater construct(ObjectNode replaceDocument) { return new DocumentUpdater(null, replaceDocument, replaceDocumentId, UpdateType.REPLACE); } + /** + * Some function which creates a document given a predicate to filter the fields to include. + * + *

Intended to be satisfied by {@link + * io.stargate.sgv2.jsonapi.service.operation.collections.FindCollectionOperation#reconstructDocumentFromFilter} + */ + @FunctionalInterface + public interface DocumentReconstructor extends Function, ObjectNode> {} + /** * This method is the entrance for first level update or replace. First level means it won't * vectorize if needed, but will warp an EmbeddingUpdateOperation in the DocumentUpdaterResponse @@ -57,6 +63,7 @@ public static DocumentUpdater construct(ObjectNode replaceDocument) { * * @param readDocument Document to update * @param isNew True if document was just created (inserted); false if updating existing document + * @param reconstructor A function which creates a reconstructed document from the search filter */ public DocumentUpdaterResponse apply( JsonNode readDocument, boolean isNew, DocumentReconstructor reconstructor) { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java index 6e6be0c97e..b264ea0b96 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java @@ -3012,7 +3012,7 @@ public void buildsFullDocumentWhenPredicateIsTrue() throws Exception { new BoolCollectionFilter("is_alive", MapCollectionFilter.Operator.EQ, true)); var operation = mkUnsortedOperation(andExpr); - var result = operation.buildBaseDocument(path -> true); + var result = operation.reconstructDocumentFromFilter(path -> true); var expected = """ @@ -3043,7 +3043,8 @@ public void filtersPathsBasedOnPredicate() throws Exception { var operation = mkUnsortedOperation(andExpr); var result = - operation.buildBaseDocument(path -> path.equals("_id") || path.startsWith("preferences")); + operation.reconstructDocumentFromFilter( + path -> path.equals("_id") || path.startsWith("preferences")); var expected = """ @@ -3070,7 +3071,7 @@ public void throwsExceptionOnOverlappingPaths() { var operation = mkUnsortedOperation(andExpr); - assertThatThrownBy(() -> operation.buildBaseDocument(path -> true)) + assertThatThrownBy(() -> operation.reconstructDocumentFromFilter(path -> true)) .isInstanceOf(UpdateException.class) .hasMessageContaining("Both paths 'conf.net' and 'conf.net.ip' are matched"); } @@ -3087,7 +3088,7 @@ public void bypassesExceptionIfOverlappingPathsFilteredOut() throws Exception { new TextCollectionFilter("conf.net.ip", MapCollectionFilter.Operator.EQ, "10.0.0.1")); var operation = mkUnsortedOperation(andExpr); - var result = operation.buildBaseDocument(path -> path.equals("_id")); + var result = operation.reconstructDocumentFromFilter(path -> path.equals("_id")); var expected = """ @@ -3115,7 +3116,7 @@ public void handlesArbitrarilyNestedAndExpressions() { } var operation = mkUnsortedOperation(top); - var result = operation.buildBaseDocument(path -> true); + var result = operation.reconstructDocumentFromFilter(path -> true); var expected = objectMapper.createObjectNode(); expected.put("_id", "user-1"); @@ -3139,7 +3140,7 @@ public void ignoresFiltersUnderOrSubExpressions() throws Exception { top.addSubExpressionReturnSub(orExpr); var operation = mkUnsortedOperation(top); - var result = operation.buildBaseDocument(path -> true); + var result = operation.reconstructDocumentFromFilter(path -> true); var expected = """ From 373fd29a14c07275b6926f52cd4852eb2155e4a6 Mon Sep 17 00:00:00 2001 From: toptobes Date: Tue, 25 Aug 2026 17:39:14 -0500 Subject: [PATCH 7/7] fix #2572 --- .../filters/collection/ArrayEqualsCollectionFilter.java | 5 ++++- .../operation/filters/collection/IsNullCollectionFilter.java | 5 ++++- .../operation/filters/collection/MatchCollectionFilter.java | 2 +- .../filters/collection/SubDocEqualsCollectionFilter.java | 5 ++++- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/ArrayEqualsCollectionFilter.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/ArrayEqualsCollectionFilter.java index 0b152c7205..5ff1b9637c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/ArrayEqualsCollectionFilter.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/ArrayEqualsCollectionFilter.java @@ -19,7 +19,10 @@ public ArrayEqualsCollectionFilter( @Override protected Optional jsonNodeForNewDocument(JsonNodeFactory nodeFactory) { - return Optional.of(toJsonNode(nodeFactory, arrayValue)); + if (Operator.MAP_EQUALS.equals(operator)) { + return Optional.of(toJsonNode(nodeFactory, arrayValue)); + } + return Optional.empty(); } // @Override diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/IsNullCollectionFilter.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/IsNullCollectionFilter.java index ab2850eee0..c36628ceee 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/IsNullCollectionFilter.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/IsNullCollectionFilter.java @@ -17,7 +17,10 @@ public IsNullCollectionFilter(String path, Operator operator) { @Override protected Optional jsonNodeForNewDocument(JsonNodeFactory nodeFactory) { - return Optional.of(toJsonNode(nodeFactory)); + if (Operator.CONTAINS.equals(operator)) { + return Optional.of(toJsonNode(nodeFactory)); + } + return Optional.empty(); } // @Override diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/MatchCollectionFilter.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/MatchCollectionFilter.java index 64a007cf42..576f9ede27 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/MatchCollectionFilter.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/MatchCollectionFilter.java @@ -29,7 +29,7 @@ public BuiltCondition get() { } protected Optional jsonNodeForNewDocument(JsonNodeFactory nodeFactory) { - return Optional.of(toJsonNode(nodeFactory, value)); + return Optional.empty(); } @Override diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/SubDocEqualsCollectionFilter.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/SubDocEqualsCollectionFilter.java index c197aeaae2..cfb0c46228 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/SubDocEqualsCollectionFilter.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/filters/collection/SubDocEqualsCollectionFilter.java @@ -28,7 +28,10 @@ public SubDocEqualsCollectionFilter( */ @Override protected Optional jsonNodeForNewDocument(JsonNodeFactory nodeFactory) { - return Optional.of(toJsonNode(nodeFactory, subDocValue)); + if (Operator.MAP_EQUALS.equals(operator)) { + return Optional.of(toJsonNode(nodeFactory, subDocValue)); + } + return Optional.empty(); } // @Override