Implement $eq support for sub document #98

maheshrajamani · 2023-02-10T16:32:58Z

Fix for #96

sub_doc_equals field in table changed to

# Conflicts: # src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperation.java # src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterableResolver.java # src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/DocValueHasher.java # src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java # src/test/java/io/stargate/sgv2/jsonapi/service/shredding/model/DocValueHasherTest.java

src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperation.java

src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/DocValueHasher.java

…92)

tatu-at-datastax · 2023-02-10T18:38:40Z

src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java

@@ -93,6 +93,28 @@ public void setUp() {
          .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName)
          .then()
          .statusCode(200);
+
+      json =


Ok, my changes will conflict here (sorry!)

tatu-at-datastax · 2023-02-10T18:42:07Z

src/test/java/io/stargate/sgv2/jsonapi/service/shredding/model/DocValueHasherTest.java

+      assertThat(hash).isNotNull();
+      Map<String, Object> values = new LinkedHashMap<>();
+      values.put("a1", new BigDecimal(5));
+      Map<String, Object> innerValues = new LinkedHashMap<>();


One idea: would be good to verify that innerValues hash is also identical to equivalent sub-tree (ObjectNode of document at JsonPointer.compile("/b1").

Not sure how to do this the hash method calls in recursive.

You can just get JsonNode out of document (document.get("b1")), calculate it separately.
And calculate similarly for innerValues.

.../java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java

tatu-at-datastax

Looks good: added some suggestions and one question on exception.

sub_doc_equals field in table changed to Rebased to main # Conflicts: # src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java

… sub_doc_eq # Conflicts: # src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/DocValueHasher.java # src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java

maheshrajamani · 2023-02-10T19:17:51Z

Not sure why it shows the changes from merged code when i did a rebase

vkarpov15 · 2023-02-10T21:45:45Z

src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java

@@ -412,24 +557,101 @@ public void findWithSizeOperatorNoMatch() {
          .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName)
          .then()
          .statusCode(200)
-          .body("data.count", is(0));
+          .body("data.count", is(1))
+          .body("data.docs[0]", jsonEquals(expected));


Great that this works! Small question though: what about key order? If I swap key order by changing { "c": "v1", "d": false } to { "d": false, "c": "v1" } would I still get a result?

@vkarpov15 The key order has to be same to return result. Do we need to make it work for any order?

@maheshrajamani As far as I know, MongoDB retains and checks field order so filter must match that too. From that I think the suggestion is that maybe there should be negative test to ensure different ordering does NOT match the row?

Another minor point: jsonEquals() probably does order-insensitive comparison (at least Jackson JsonNode is order-insensitive and most JSON packages are similarly). I think this is fine for most tests but good to keep in mind for small number where we do want to check that order is retained. I think it's fine here too since the focus is not on ensuring storage of doc, retaining.

@tatu-at-datastax is right, key order matters for filtering. If you swap key order and change { "c": "v1", "d": false } to { "d": false, "c": "v1" } you should not get a result in this test. Would be nice to add a test case for checking key order just in case.

Will add a test case for this

maheshrajamani added 5 commits February 9, 2023 13:00

Changes for $eq support for array data type

b2b4f87

Changes for $eq support for sub doc data type

f9c5330

sub_doc_equals field in table changed to

Changes for Find Integration Test

a89008e

Changes for Find Integration Test

4745db2

maheshrajamani marked this pull request as ready for review February 10, 2023 17:12

maheshrajamani requested a review from a team as a code owner February 10, 2023 17:12

tatu-at-datastax reviewed Feb 10, 2023

View reviewed changes

src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperation.java Outdated Show resolved Hide resolved

tatu-at-datastax reviewed Feb 10, 2023

View reviewed changes

src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperation.java Outdated Show resolved Hide resolved

tatu-at-datastax reviewed Feb 10, 2023

View reviewed changes

src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/DocValueHasher.java Outdated Show resolved Hide resolved

Fix #91: remove escaping/decorating of dotted path Shredder produces (#…

c4ff8a5

…92)

tatu-at-datastax reviewed Feb 10, 2023

View reviewed changes

.../java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java Show resolved Hide resolved

tatu-at-datastax approved these changes Feb 10, 2023

View reviewed changes

maheshrajamani added 7 commits February 10, 2023 13:53

Changes for $eq support for sub doc data type

70c1bcb

sub_doc_equals field in table changed to Rebased to main # Conflicts: # src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java

Changes for Find Integration Test

d4772d6

Changes for Find Integration Test

13655a4

Removed comments

58e4621

Changes for $eq support for sub doc data type

82c03a4

sub_doc_equals field in table changed to Rebased to main # Conflicts: # src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java

Merge branch 'sub_doc_eq' of https://github.com/stargate/jsonapi into…

0c55bb4

… sub_doc_eq # Conflicts: # src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/DocValueHasher.java # src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java

Updated comments

8075092

maheshrajamani merged commit 75514ec into main Feb 10, 2023

maheshrajamani deleted the sub_doc_eq branch February 10, 2023 19:21

vkarpov15 reviewed Feb 10, 2023

View reviewed changes

maheshrajamani mentioned this pull request Feb 13, 2023

$eq test case for unmatched sub doc order query #103

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement $eq support for sub document #98

Implement $eq support for sub document #98

maheshrajamani commented Feb 10, 2023

tatu-at-datastax Feb 10, 2023

tatu-at-datastax Feb 10, 2023

maheshrajamani Feb 10, 2023

tatu-at-datastax Feb 10, 2023

tatu-at-datastax left a comment

maheshrajamani commented Feb 10, 2023

vkarpov15 Feb 10, 2023

maheshrajamani Feb 13, 2023 •

edited

Loading

tatu-at-datastax Feb 13, 2023

vkarpov15 Feb 13, 2023

maheshrajamani Feb 13, 2023

Implement $eq support for sub document #98

Implement $eq support for sub document #98

Conversation

maheshrajamani commented Feb 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tatu-at-datastax left a comment

Choose a reason for hiding this comment

maheshrajamani commented Feb 10, 2023

Choose a reason for hiding this comment

maheshrajamani Feb 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maheshrajamani Feb 13, 2023 •

edited

Loading