From 8a1dd03aa62b6598ca6ebc355c340ebb631e3117 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 3 Jun 2025 15:06:21 -0700 Subject: [PATCH 1/3] Fix #2109: allow 1/-1 sort on `$lexical` --- .../command/builders/SortClauseBuilder.java | 34 +++++++++++-------- ...dCollectionWithLexicalIntegrationTest.java | 30 +++++++++++++--- 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java index 293c6934e1..9a31251fec 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java @@ -73,21 +73,27 @@ protected SortClause defaultBuildAndValidate(ObjectNode sortNode) { // $lexical is only allowed alone: handle first JsonNode lexicalValue = sortNode.get(DocumentConstants.Fields.LEXICAL_CONTENT_FIELD); if (lexicalValue != null) { - if (sortNode.size() > 1) { - throw ErrorCodeV1.INVALID_SORT_CLAUSE.toApiException( - "if sorting by '%s' no other sort expressions allowed", - DocumentConstants.Fields.LEXICAL_CONTENT_FIELD); - } - if (!lexicalValue.isTextual()) { - throw ErrorCodeV1.INVALID_SORT_CLAUSE.toApiException( - "if sorting by '%s' value must be String, not %s", - DocumentConstants.Fields.LEXICAL_CONTENT_FIELD, - JsonUtil.nodeTypeAsString(lexicalValue)); + // Typical we need a String, but "old" 1/-1 still allowed: so bail out + // if we got a Number + if (lexicalValue.isNumber()) { + ; // do nothing, yet, fall-through to next block + } else { + if (!lexicalValue.isTextual()) { + throw ErrorCodeV1.INVALID_SORT_CLAUSE.toApiException( + "if sorting by '%s' value must be String, not %s", + DocumentConstants.Fields.LEXICAL_CONTENT_FIELD, + JsonUtil.nodeTypeAsString(lexicalValue)); + } + if (sortNode.size() > 1) { + throw ErrorCodeV1.INVALID_SORT_CLAUSE.toApiException( + "if sorting by '%s' no other sort expressions allowed", + DocumentConstants.Fields.LEXICAL_CONTENT_FIELD); + } + // We cannot yet determine if lexical sort supported by the collection, just + // construct clause + return new SortClause( + Collections.singletonList(SortExpression.bm25Search(lexicalValue.textValue()))); } - // We cannot yet determine if lexical sort supported by the collection, just - // construct clause - return new SortClause( - Collections.singletonList(SortExpression.bm25Search(lexicalValue.textValue()))); } while (fieldIter.hasNext()) { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindCollectionWithLexicalIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindCollectionWithLexicalIntegrationTest.java index 30fa4b0a39..cb947987ca 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindCollectionWithLexicalIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindCollectionWithLexicalIntegrationTest.java @@ -158,11 +158,33 @@ void findManyWithLexicalAndOtherFilter() { .body("data.documents", hasSize(1)) .body("data.documents[0]._id", is("lexical-4")); } + + // [data-api#2109] Non-lexical sort on $lexical column should also work + @Test + void findManyWithNonLexicalSort() { + givenHeadersPostJsonThenOkNoErrors( + keyspaceName, + COLLECTION_WITH_LEXICAL, + """ + { + "find": { + "sort" : {"$lexical": 1 } + } + } + """) + .body("$", responseIsFindSuccess()) + .body("data.documents", hasSize(5)) + .body("data.documents[0]._id", is("lexical-4")) + .body("data.documents[1]._id", is("lexical-3")) + .body("data.documents[2]._id", is("lexical-5")) + .body("data.documents[3]._id", is("lexical-2")) + .body("data.documents[4]._id", is("lexical-1")); + } } @DisabledIfSystemProperty(named = TEST_PROP_LEXICAL_DISABLED, matches = "true") @Nested - @Order(2) + @Order(3) class HappyCasesFindOne { @Test void findOneWithLexicalSortBiking() { @@ -221,7 +243,7 @@ void findOneWithOnlyLexicalFilter() { @DisabledIfSystemProperty(named = TEST_PROP_LEXICAL_DISABLED, matches = "true") @Nested - @Order(3) + @Order(4) class FailingCasesFindMany { @Test void failSortIfLexicalDisabledForCollection() { @@ -265,7 +287,7 @@ void failForBadLexicalSortValueType() { """ { "find": { - "sort" : {"$lexical": -1 } + "sort" : {"$lexical": false } } } """) @@ -273,7 +295,7 @@ void failForBadLexicalSortValueType() { .body("errors[0].errorCode", is("INVALID_SORT_CLAUSE")) .body( "errors[0].message", - containsString("if sorting by '$lexical' value must be String, not Number")); + containsString("if sorting by '$lexical' value must be String, not Boolean")); } @Test From 23466c784c1d721341c7fdbfd59051d08926f14c Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 3 Jun 2025 17:00:38 -0700 Subject: [PATCH 2/3] ... --- .../api/model/command/builders/SortClauseBuilder.java | 10 +++++----- .../operation/collections/FindCollectionOperation.java | 1 - .../v1/FindCollectionWithLexicalIntegrationTest.java | 1 + 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java index 9a31251fec..02ba3db76a 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/SortClauseBuilder.java @@ -65,10 +65,7 @@ public SortClause build(JsonNode node) { protected abstract SortClause buildAndValidate(ObjectNode sortNode); protected SortClause defaultBuildAndValidate(ObjectNode sortNode) { - // safe to iterate, we know it's an Object - Iterator> fieldIter = sortNode.fields(); - int totalFields = sortNode.size(); - List sortExpressions = new ArrayList<>(sortNode.size()); + final int totalFields = sortNode.size(); // $lexical is only allowed alone: handle first JsonNode lexicalValue = sortNode.get(DocumentConstants.Fields.LEXICAL_CONTENT_FIELD); @@ -84,7 +81,7 @@ protected SortClause defaultBuildAndValidate(ObjectNode sortNode) { DocumentConstants.Fields.LEXICAL_CONTENT_FIELD, JsonUtil.nodeTypeAsString(lexicalValue)); } - if (sortNode.size() > 1) { + if (totalFields > 1) { throw ErrorCodeV1.INVALID_SORT_CLAUSE.toApiException( "if sorting by '%s' no other sort expressions allowed", DocumentConstants.Fields.LEXICAL_CONTENT_FIELD); @@ -96,6 +93,9 @@ protected SortClause defaultBuildAndValidate(ObjectNode sortNode) { } } + // safe to iterate, we know it's an Object + Iterator> fieldIter = sortNode.fields(); + List sortExpressions = new ArrayList<>(sortNode.size()); while (fieldIter.hasNext()) { Map.Entry inner = fieldIter.next(); final String path = inner.getKey().trim(); 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 36c0f3844d..093f56d788 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 @@ -402,7 +402,6 @@ public Uni getDocuments( QueryExecutor queryExecutor, String pageState, IDCollectionFilter additionalIdFilter) { - // ensure we pass failure down if read type is not DOCUMENT or KEY // COUNT is not supported switch (readType) { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindCollectionWithLexicalIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindCollectionWithLexicalIntegrationTest.java index cb947987ca..f280d8bd42 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindCollectionWithLexicalIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindCollectionWithLexicalIntegrationTest.java @@ -168,6 +168,7 @@ void findManyWithNonLexicalSort() { """ { "find": { + "projection": {"$lexical": 1 }, "sort" : {"$lexical": 1 } } } From 7df4bc3cebb089827cb91056750d82176a293c41 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 16 Jun 2025 13:57:11 -0700 Subject: [PATCH 3/3] Re-do check for non-lexical sort on $lexical (lost due to preceding merge) --- .../builders/CollectionSortClauseBuilder.java | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/CollectionSortClauseBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/CollectionSortClauseBuilder.java index 9e188fefd9..abcbf69f15 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/CollectionSortClauseBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/builders/CollectionSortClauseBuilder.java @@ -25,21 +25,28 @@ public SortClause buildAndValidate(ObjectNode sortNode) { // $lexical is only special for Collections: handle first JsonNode lexicalNode = sortNode.get(DocumentConstants.Fields.LEXICAL_CONTENT_FIELD); if (lexicalNode != null) { - // We can also check if lexical sort supported by the collection: - if (!schema.lexicalConfig().enabled()) { - throw SchemaException.Code.LEXICAL_NOT_ENABLED_FOR_COLLECTION.get(errVars(schema)); - } - if (sortNode.size() > 1) { - throw ErrorCodeV1.INVALID_SORT_CLAUSE.toApiException( - "if sorting by '%s' no other sort expressions allowed", - DocumentConstants.Fields.LEXICAL_CONTENT_FIELD); - } - if (!lexicalNode.isTextual()) { - throw ErrorCodeV1.INVALID_SORT_CLAUSE.toApiException( - "if sorting by '%s' value must be String, not %s", - DocumentConstants.Fields.LEXICAL_CONTENT_FIELD, JsonUtil.nodeTypeAsString(lexicalNode)); + // Typical we need a String, but "old" 1/-1 still allowed: so bail out + // if we got a Number + if (lexicalNode.isNumber()) { + ; // do nothing, yet, fall-through to next block + } else { + // We can also check if lexical sort supported by the collection: + if (!schema.lexicalConfig().enabled()) { + throw SchemaException.Code.LEXICAL_NOT_ENABLED_FOR_COLLECTION.get(errVars(schema)); + } + if (sortNode.size() > 1) { + throw ErrorCodeV1.INVALID_SORT_CLAUSE.toApiException( + "if sorting by '%s' no other sort expressions allowed", + DocumentConstants.Fields.LEXICAL_CONTENT_FIELD); + } + if (!lexicalNode.isTextual()) { + throw ErrorCodeV1.INVALID_SORT_CLAUSE.toApiException( + "if sorting by '%s' value must be String, not %s", + DocumentConstants.Fields.LEXICAL_CONTENT_FIELD, + JsonUtil.nodeTypeAsString(lexicalNode)); + } + return SortClause.immutable(SortExpression.collectionLexicalSort(lexicalNode.textValue())); } - return SortClause.immutable(SortExpression.collectionLexicalSort(lexicalNode.textValue())); } JsonNode vectorNode = sortNode.get(DocumentConstants.Fields.VECTOR_EMBEDDING_FIELD); if (vectorNode != null) {