diff --git a/src/integrationTest/java/com/mongodb/hibernate/query/select/Book.java b/src/integrationTest/java/com/mongodb/hibernate/query/select/Book.java index 1b2fd9b9..765ccacd 100644 --- a/src/integrationTest/java/com/mongodb/hibernate/query/select/Book.java +++ b/src/integrationTest/java/com/mongodb/hibernate/query/select/Book.java @@ -43,6 +43,20 @@ class Book { this.outOfStock = outOfStock; } + public Book( + int id, + String title, + Integer publishYear, + Boolean outOfStock, + Long isbn13, + Double discount, + BigDecimal price) { + this(id, title, publishYear, outOfStock); + this.isbn13 = isbn13; + this.discount = discount; + this.price = price; + } + @Override public String toString() { return "Book{" + "id=" + id + '}'; diff --git a/src/integrationTest/java/com/mongodb/hibernate/query/select/BooleanExpressionWhereClauseIntegrationTests.java b/src/integrationTest/java/com/mongodb/hibernate/query/select/BooleanExpressionWhereClauseIntegrationTests.java index 3eb51677..9fa0f35e 100644 --- a/src/integrationTest/java/com/mongodb/hibernate/query/select/BooleanExpressionWhereClauseIntegrationTests.java +++ b/src/integrationTest/java/com/mongodb/hibernate/query/select/BooleanExpressionWhereClauseIntegrationTests.java @@ -54,9 +54,41 @@ void testBooleanFieldPathExpression(boolean negated) { assertSelectionQuery( "from Book where" + (negated ? " not " : " ") + "outOfStock", Book.class, - "{'aggregate': 'books', 'pipeline': [{'$match': {'outOfStock': {'$eq': " - + (negated ? "false" : "true") - + "}}}, {'$project': {'_id': true, 'discount': true, 'isbn13': true, 'outOfStock': true, 'price': true, 'publishYear': true, 'title': true}}]}", + """ + { + "aggregate": "books", + "pipeline": [ + { + "$match": { + "$and": [ + { + "outOfStock": { + "$eq": %s + } + }, + { + "outOfStock": { + "$ne": null + } + } + ] + } + }, + { + "$project": { + "_id": true, + "discount": true, + "isbn13": true, + "outOfStock": true, + "price": true, + "publishYear": true, + "title": true + } + } + ] + } + """ + .formatted(negated ? "false" : "true"), negated ? singletonList(bookInStock) : singletonList(bookOutOfStock)); } diff --git a/src/integrationTest/java/com/mongodb/hibernate/query/select/SimpleSelectQueryIntegrationTests.java b/src/integrationTest/java/com/mongodb/hibernate/query/select/SimpleSelectQueryIntegrationTests.java index 19d8c731..f9b7d4e8 100644 --- a/src/integrationTest/java/com/mongodb/hibernate/query/select/SimpleSelectQueryIntegrationTests.java +++ b/src/integrationTest/java/com/mongodb/hibernate/query/select/SimpleSelectQueryIntegrationTests.java @@ -16,7 +16,8 @@ package com.mongodb.hibernate.query.select; -import static java.util.Collections.singletonList; +import static java.lang.String.format; +import static java.util.Collections.emptyList; import static org.assertj.core.api.Assertions.assertThatCode; import com.mongodb.hibernate.internal.FeatureNotSupportedException; @@ -34,7 +35,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; -@DomainModel(annotatedClasses = {SimpleSelectQueryIntegrationTests.Contact.class, Book.class}) +@DomainModel(annotatedClasses = {Book.class, SimpleSelectQueryIntegrationTests.Contact.class}) class SimpleSelectQueryIntegrationTests extends AbstractSelectionQueryIntegrationTests { @Nested @@ -50,7 +51,10 @@ void beforeEach() { new Contact(2, "Mary", 35, Country.CANADA), new Contact(3, "Dylan", 7, Country.CANADA), new Contact(4, "Lucy", 78, Country.CANADA), - new Contact(5, "John", 25, Country.USA)); + new Contact(5, "John", 25, Country.USA), + new Contact(6, "Alice", 40, Country.USA), + new Contact(7, "Eve", null, null), + new Contact(8, "Eve", null, Country.CANADA)); private static List getTestingContacts(int... ids) { return Arrays.stream(ids) @@ -63,7 +67,7 @@ private static List getTestingContacts(int... ids) { @ParameterizedTest @ValueSource(booleans = {true, false}) - void testComparisonByEq(boolean fieldAsLhs) { + void testComparisonByEqToNonNull(boolean fieldAsLhs) { assertSelectionQuery( "from Contact where " + (fieldAsLhs ? "country = :country" : ":country = country"), Contact.class, @@ -74,10 +78,19 @@ void testComparisonByEq(boolean fieldAsLhs) { "pipeline": [ { "$match": { - "country": { - "$eq": "USA" - } - } + "$and": [ + { + "country": { + "$eq": "USA" + } + }, + { + "country": { + "$ne": null + } + } + ] + } }, { "$project": { @@ -89,26 +102,35 @@ void testComparisonByEq(boolean fieldAsLhs) { } ] }""", - getTestingContacts(1, 5)); + getTestingContacts(1, 5, 6)); } @ParameterizedTest @ValueSource(booleans = {true, false}) - void testComparisonByNe(boolean fieldAsLhs) { + void testComparisonByEqToNull(boolean fieldAsLhs) { assertSelectionQuery( - "from Contact where " + (fieldAsLhs ? "country != ?1" : "?1 != country"), + "from Contact where " + (fieldAsLhs ? "country = :country" : ":country = country"), Contact.class, - q -> q.setParameter(1, Country.USA.name()), + q -> q.setParameter("country", null), """ { "aggregate": "contacts", "pipeline": [ { "$match": { - "country": { - "$ne": "USA" - } - } + "$and": [ + { + "country": { + "$eq": null + } + }, + { + "country": { + "$ne": null + } + } + ] + } }, { "$project": { @@ -120,25 +142,34 @@ void testComparisonByNe(boolean fieldAsLhs) { } ] }""", - getTestingContacts(2, 3, 4)); + emptyList()); } @ParameterizedTest @ValueSource(booleans = {true, false}) - void testComparisonByLt(boolean fieldAsLhs) { + void testComparisonByNeToNonNull(boolean fieldAsLhs) { assertSelectionQuery( - "from Contact where " + (fieldAsLhs ? "age < :age" : ":age > age"), + "from Contact where " + (fieldAsLhs ? "country != ?1" : "?1 != country"), Contact.class, - q -> q.setParameter("age", 35), + q -> q.setParameter(1, Country.USA.name()), """ { "aggregate": "contacts", "pipeline": [ { "$match": { - "age": { - "$lt": 35 - } + "$and": [ + { + "country": { + "$ne": "USA" + } + }, + { + "country": { + "$ne": null + } + } + ] } }, { @@ -151,25 +182,34 @@ void testComparisonByLt(boolean fieldAsLhs) { } ] }""", - getTestingContacts(1, 3, 5)); + getTestingContacts(2, 3, 4, 8)); } @ParameterizedTest @ValueSource(booleans = {true, false}) - void testComparisonByLte(boolean fieldAsLhs) { + void testComparisonByNeToNull(boolean fieldAsLhs) { assertSelectionQuery( - "from Contact where " + (fieldAsLhs ? "age <= ?1" : "?1 >= age"), + "from Contact where " + (fieldAsLhs ? "country != ?1" : "?1 != country"), Contact.class, - q -> q.setParameter(1, 35), + q -> q.setParameter(1, null), """ { "aggregate": "contacts", "pipeline": [ { "$match": { - "age": { - "$lte": 35 - } + "$and": [ + { + "country": { + "$ne": null + } + }, + { + "country": { + "$ne": null + } + } + ] } }, { @@ -182,25 +222,34 @@ void testComparisonByLte(boolean fieldAsLhs) { } ] }""", - getTestingContacts(1, 2, 3, 5)); + getTestingContacts(1, 2, 3, 4, 5, 6, 8)); } @ParameterizedTest @ValueSource(booleans = {true, false}) - void testComparisonByGt(boolean fieldAsLhs) { + void testComparisonByLtNonNull(boolean fieldAsLhs) { assertSelectionQuery( - "from Contact where " + (fieldAsLhs ? "age > :age" : ":age < age"), + "from Contact where " + (fieldAsLhs ? "age < :age" : ":age > age"), Contact.class, - q -> q.setParameter("age", 18), + q -> q.setParameter("age", 35), """ { "aggregate": "contacts", "pipeline": [ { "$match": { - "age": { - "$gt": 18 - } + "$and": [ + { + "age": { + "$lt": 35 + } + }, + { + "age": { + "$ne": null + } + } + ] } }, { @@ -213,25 +262,34 @@ void testComparisonByGt(boolean fieldAsLhs) { } ] }""", - getTestingContacts(2, 4, 5)); + getTestingContacts(1, 3, 5)); } @ParameterizedTest @ValueSource(booleans = {true, false}) - void testComparisonByGte(boolean fieldAsLhs) { + void testComparisonByLtNull(boolean fieldAsLhs) { assertSelectionQuery( - "from Contact where " + (fieldAsLhs ? "age >= :age" : ":age <= age"), + "from Contact where " + (fieldAsLhs ? "age < :age" : ":age > age"), Contact.class, - q -> q.setParameter("age", 18), + q -> q.setParameter("age", null), """ { "aggregate": "contacts", "pipeline": [ { "$match": { - "age": { - "$gte": 18 - } + "$and": [ + { + "age": { + "$lt": null + } + }, + { + "age": { + "$ne": null + } + } + ] } }, { @@ -244,15 +302,16 @@ void testComparisonByGte(boolean fieldAsLhs) { } ] }""", - getTestingContacts(1, 2, 4, 5)); + emptyList()); } - @Test - void testAndFilter() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testComparisonByLteNonNull(boolean fieldAsLhs) { assertSelectionQuery( - "from Contact where country = ?1 and age > ?2", + "from Contact where " + (fieldAsLhs ? "age <= ?1" : "?1 >= age"), Contact.class, - q -> q.setParameter(1, Country.CANADA.name()).setParameter(2, 18), + q -> q.setParameter(1, 35), """ { "aggregate": "contacts", @@ -261,13 +320,13 @@ void testAndFilter() { "$match": { "$and": [ { - "country": { - "$eq": "CANADA" + "age": { + "$lte": 35 } }, { "age": { - "$gt": 18 + "$ne": null } } ] @@ -283,30 +342,31 @@ void testAndFilter() { } ] }""", - getTestingContacts(2, 4)); + getTestingContacts(1, 2, 3, 5)); } - @Test - void testOrFilter() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testComparisonByLteNull(boolean fieldAsLhs) { assertSelectionQuery( - "from Contact where country = :country or age > :age", + "from Contact where " + (fieldAsLhs ? "age <= ?1" : "?1 >= age"), Contact.class, - q -> q.setParameter("country", Country.CANADA.name()).setParameter("age", 18), + q -> q.setParameter(1, null), """ { "aggregate": "contacts", "pipeline": [ { "$match": { - "$or": [ + "$and": [ { - "country": { - "$eq": "CANADA" + "age": { + "$lte": null } }, { "age": { - "$gt": 18 + "$ne": null } } ] @@ -322,14 +382,16 @@ void testOrFilter() { } ] }""", - getTestingContacts(2, 3, 4, 5)); + emptyList()); } - @Test - void testSingleNegation() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testComparisonByGtNonNull(boolean fieldAsLhs) { assertSelectionQuery( - "from Contact where age > 18 and not (country = 'USA')", + "from Contact where " + (fieldAsLhs ? "age > :age" : ":age < age"), Contact.class, + q -> q.setParameter("age", 18), """ { "aggregate": "contacts", @@ -343,13 +405,9 @@ void testSingleNegation() { } }, { - "$nor": [ - { - "country": { - "$eq": "USA" - } - } - ] + "age": { + "$ne": null + } } ] } @@ -364,36 +422,32 @@ void testSingleNegation() { } ] }""", - getTestingContacts(2, 4)); + getTestingContacts(2, 4, 5, 6)); } - @Test - void testSingleNegationWithAnd() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testComparisonByGtNull(boolean fieldAsLhs) { assertSelectionQuery( - "from Contact where not (country = 'USA' and age > 18)", + "from Contact where " + (fieldAsLhs ? "age > :age" : ":age < age"), Contact.class, + q -> q.setParameter("age", null), """ { "aggregate": "contacts", "pipeline": [ { "$match": { - "$nor": [ + "$and": [ { - "$and": [ - { - "country": { - "$eq": "USA" - } - }, - { - "age": { - "$gt": { - "$numberInt": "18" - } - } - } - ] + "age": { + "$gt": null + } + }, + { + "age": { + "$ne": null + } } ] } @@ -408,36 +462,32 @@ void testSingleNegationWithAnd() { } ] }""", - getTestingContacts(1, 2, 3, 4)); + emptyList()); } - @Test - void testSingleNegationWithOr() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testComparisonByGteNonNull(boolean fieldAsLhs) { assertSelectionQuery( - "from Contact where not (country = 'USA' or age > 18)", + "from Contact where " + (fieldAsLhs ? "age >= :age" : ":age <= age"), Contact.class, + q -> q.setParameter("age", 18), """ { "aggregate": "contacts", "pipeline": [ { "$match": { - "$nor": [ + "$and": [ { - "$or": [ - { - "country": { - "$eq": "USA" - } - }, - { - "age": { - "$gt": { - "$numberInt": "18" - } - } - } - ] + "age": { + "$gte": 18 + } + }, + { + "age": { + "$ne": null + } } ] } @@ -452,47 +502,32 @@ void testSingleNegationWithOr() { } ] }""", - getTestingContacts(3)); + getTestingContacts(1, 2, 4, 5, 6)); } - @Test - void testSingleNegationWithAndOr() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testComparisonByGteNull(boolean fieldAsLhs) { assertSelectionQuery( - "from Contact where not (country = 'USA' and age > 18 or age < 25)", + "from Contact where " + (fieldAsLhs ? "age >= :age" : ":age <= age"), Contact.class, + q -> q.setParameter("age", null), """ { "aggregate": "contacts", "pipeline": [ { "$match": { - "$nor": [ + "$and": [ { - "$or": [ - { - "$and": [ - { - "country": { - "$eq": "USA" - } - }, - { - "age": { - "$gt": { - "$numberInt": "18" - } - } - } - ] - }, - { - "age": { - "$lt": { - "$numberInt": "25" - } - } - } - ] + "age": { + "$gte": null + } + }, + { + "age": { + "$ne": null + } } ] } @@ -507,14 +542,15 @@ void testSingleNegationWithAndOr() { } ] }""", - getTestingContacts(2, 4)); + emptyList()); } @Test - void testDoubleNegation() { + void testAndFilter() { assertSelectionQuery( - "from Contact where age > 18 and not ( not (country = 'USA') )", + "from Contact where country = ?1 and age > ?2", Contact.class, + q -> q.setParameter(1, Country.CANADA.name()).setParameter(2, 18), """ { "aggregate": "contacts", @@ -523,20 +559,30 @@ void testDoubleNegation() { "$match": { "$and": [ { - "age": { - "$gt": 18 - } + "$and": [ + { + "country": { + "$eq": "CANADA" + } + }, + { + "country": { + "$ne": null + } + } + ] }, { - "$nor": [ + "$and": [ { - "$nor": [ - { - "country": { - "$eq": "USA" - } - } - ] + "age": { + "$gt": 18 + } + }, + { + "age": { + "$ne": null + } } ] } @@ -553,35 +599,504 @@ void testDoubleNegation() { } ] }""", - getTestingContacts(5)); + getTestingContacts(2, 4)); } @Test - void testProjectWithoutAlias() { + void testOrFilter() { assertSelectionQuery( - "select name, age from Contact where country = :country", - Object[].class, - q -> q.setParameter("country", Country.CANADA.name()), + "from Contact where country = :country or age > :age", + Contact.class, + q -> q.setParameter("country", Country.CANADA.name()).setParameter("age", 18), """ { "aggregate": "contacts", "pipeline": [ { "$match": { - "country": { - "$eq": "CANADA" - } - } - }, - { - "$project": { - "name": true, - "age": true - } - } + "$or": [ + { + "$and": [ + { + "country": { + "$eq": "CANADA" + } + }, + { + "country": { + "$ne": null + } + } + ], + }, + { + "$and": [ + { + "age": { + "$gt": 18 + } + }, + { + "age": { + "$ne": null + } + } + ] + } + ] + } + }, + { + "$project": { + "_id": true, + "age": true, + "country": true, + "name": true + } + } ] }""", - List.of(new Object[] {"Mary", 35}, new Object[] {"Dylan", 7}, new Object[] {"Lucy", 78})); + getTestingContacts(2, 3, 4, 5, 6, 8)); + } + + @Test + void testSingleNegation() { + assertSelectionQuery( + "from Contact where age > 18 and not (country = 'USA')", + Contact.class, + """ + { + "aggregate": "contacts", + "pipeline": [ + { + "$match": { + "$and": [ + { + "$and": [ + { + "age": { + "$gt": { + "$numberInt": "18" + } + } + }, + { + "age": { + "$ne": null + } + } + ] + }, + { + "$nor": [ + { + "$and": [ + { + "country": { + "$eq": "USA" + } + }, + { + "country": { + "$ne": null + } + } + ] + } + ] + } + ] + } + }, + { + "$project": { + "_id": true, + "age": true, + "country": true, + "name": true + } + } + ] + }""", + getTestingContacts(2, 4)); + } + + @Test + void testSingleNegationWithAnd() { + assertSelectionQuery( + "from Contact where not (country = 'USA' and age > 18)", + Contact.class, + """ + { + "aggregate": "contacts", + "pipeline": [ + { + "$match": { + "$nor": [ + { + "$and": [ + { + "$and": [ + { + "country": { + "$eq": "USA" + } + }, + { + "country": { + "$ne": null + } + } + ] + }, + { + "$and": [ + { + "age": { + "$gt": { + "$numberInt": "18" + } + } + }, + { + "age": { + "$ne": null + } + } + ] + } + ] + } + ] + } + }, + { + "$project": { + "_id": true, + "age": true, + "country": true, + "name": true + } + } + ] + }""", + getTestingContacts(1, 2, 3, 4, 7, 8)); + } + + @Test + void testSingleNegationWithOr() { + assertSelectionQuery( + "from Contact where not (country = 'USA' or age > 18)", + Contact.class, + """ + { + "aggregate": "contacts", + "pipeline": [ + { + "$match": { + "$nor": [ + { + "$or": [ + { + "$and": [ + { + "country": { + "$eq": "USA" + } + }, + { + "country": { + "$ne": null + } + } + ] + }, + { + "$and": [ + { + "age": { + "$gt": { + "$numberInt": "18" + } + } + }, + { + "age": { + "$ne": null + } + } + ] + } + ] + } + ] + } + }, + { + "$project": { + "_id": true, + "age": true, + "country": true, + "name": true + } + } + ] + }""", + getTestingContacts(3, 7, 8)); + } + + @Test + void testSingleNegationWithAndOr() { + assertSelectionQuery( + "from Contact where not (country = 'USA' and age > 18 or age < 25)", + Contact.class, + """ + { + "aggregate": "contacts", + "pipeline": [ + { + "$match": { + "$nor": [ + { + "$or": [ + { + "$and": [ + { + "$and": [ + { + "country": { + "$eq": "USA" + } + }, + { + "country": { + "$ne": null + } + } + ] + }, + { + "$and": [ + { + "age": { + "$gt": { + "$numberInt": "18" + } + } + }, + { + "age": { + "$ne": null + } + } + ] + } + ] + }, + { + "$and": [ + { + "age": { + "$lt": { + "$numberInt": "25" + } + } + }, + { + "age": { + "$ne": null + } + } + ] + } + ] + } + ] + } + }, + { + "$project": { + "_id": true, + "age": true, + "country": true, + "name": true + } + } + ] + }""", + getTestingContacts(2, 4, 7, 8)); + } + + @Test + void testDoubleNegation() { + assertSelectionQuery( + "from Contact where age > 18 and not ( not (country = 'USA') )", + Contact.class, + """ + { + "aggregate": "contacts", + "pipeline": [ + { + "$match": { + "$and": [ + { + "$and": [ + { + "age": { + "$gt": { + "$numberInt": "18" + } + } + }, + { + "age": { + "$ne": null + } + } + ] + }, + { + "$nor": [ + { + "$nor": [ + { + "$and": [ + { + "country": { + "$eq": "USA" + } + }, + { + "country": { + "$ne": null + } + } + ] + } + ] + } + ] + } + ] + } + }, + { + "$project": { + "_id": true, + "age": true, + "country": true, + "name": true + } + } + ] + }""", + getTestingContacts(5, 6)); + } + + @Test + void testDoubleNegationWithOr() { + assertSelectionQuery( + "from Contact where not(not((country = :country) OR (age = :age)))", + Contact.class, + q -> q.setParameter("country", "CANADA").setParameter("age", null), + """ + { + "aggregate": "contacts", + "pipeline": [ + { + "$match": { + "$nor": [ + { + "$nor": [ + { + "$or": [ + { + "$and": [ + { + "country": { + "$eq": "CANADA" + } + }, + { + "country": { + "$ne": null + } + } + ] + }, + { + "$and": [ + { + "age": { + "$eq": null + } + }, + { + "age": { + "$ne": null + } + } + ] + } + ] + } + ] + } + ] + } + }, + { + "$project": { + "_id": true, + "age": true, + "country": true, + "name": true + } + } + ] + }""", + getTestingContacts(2, 3, 4, 8)); + } + + @Test + void testProjectWithoutAlias() { + assertSelectionQuery( + "select name, age from Contact where country = :country", + Object[].class, + q -> q.setParameter("country", Country.CANADA.name()), + """ + { + "aggregate": "contacts", + "pipeline": [ + { + "$match": { + "$and": [ + { + "country": { + "$eq": "CANADA" + } + }, + { + "country": { + "$ne": null + } + } + ] + } + }, + { + "$project": { + "name": true, + "age": true + } + } + ] + }""", + List.of( + new Object[] {"Mary", 35}, + new Object[] {"Dylan", 7}, + new Object[] {"Lucy", 78}, + new Object[] {"Eve", null})); } @Test @@ -596,9 +1111,18 @@ void testProjectUsingAlias() { "pipeline": [ { "$match": { - "country": { - "$eq": "CANADA" - } + "$and": [ + { + "country": { + "$eq": "CANADA" + } + }, + { + "country": { + "$ne": null + } + } + ] } }, { @@ -609,7 +1133,11 @@ void testProjectUsingAlias() { } ] }""", - List.of(new Object[] {"Mary", 35}, new Object[] {"Dylan", 7}, new Object[] {"Lucy", 78})); + List.of( + new Object[] {"Mary", 35}, + new Object[] {"Dylan", 7}, + new Object[] {"Lucy", 78}, + new Object[] {"Eve", null})); } @Test @@ -672,16 +1200,6 @@ void testComparisonBetweenParametersNotSupported() { "Only the following comparisons are supported: field vs literal, field vs parameter"); } - @Test - void testNullParameterNotSupported() { - assertSelectQueryFailure( - "from Contact where country = :country", - Contact.class, - q -> q.setParameter("country", null), - FeatureNotSupportedException.class, - "TODO-HIBERNATE-74 https://jira.mongodb.org/browse/HIBERNATE-74"); - } - @Test void testQueryPlanCacheIsSupported() { getSessionFactoryScope().inTransaction(session -> assertThatCode( @@ -692,218 +1210,291 @@ void testQueryPlanCacheIsSupported() { } } - @Nested - class QueryLiteralTests { - - private Book testingBook; + private static final List testingBooks = List.of( + new Book(1, "War and Peace", null, true, null, 0.2, new BigDecimal("123.50")), + new Book(2, "Crime and Punishment", 1866, null), + new Book(3, "Anna Karenina", null, false, 9780310904168L, 0.8, null), + new Book(4, "The Brothers Karamazov", null, null, null, 0.7, null), + new Book(5, "War and Peace", 2025, false), + new Book(6, null, null, null)); - @BeforeEach - void beforeEach() { - testingBook = new Book(); - testingBook.title = "Holy Bible"; - testingBook.outOfStock = true; - testingBook.publishYear = 1995; - testingBook.isbn13 = 9780310904168L; - testingBook.discount = 0.25; - testingBook.price = new BigDecimal("123.50"); - getSessionFactoryScope().inTransaction(session -> session.persist(testingBook)); + private static List getBooksByIds(int... ids) { + return Arrays.stream(ids) + .mapToObj(id -> testingBooks.stream() + .filter(c -> c.id == id) + .findFirst() + .orElseThrow(() -> new IllegalArgumentException("id does not exist: " + id))) + .toList(); + } - getTestCommandListener().clear(); - } + @BeforeEach + void beforeEach() { + getSessionFactoryScope().inTransaction(session -> testingBooks.forEach(session::persist)); + getTestCommandListener().clear(); + } - @Test - void testBoolean() { - assertSelectionQuery( - "from Book where outOfStock = true", - Book.class, - """ + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testBoolean(boolean isNullLiteral) { + var nonNullBooleanLiteralStr = "true"; + assertSelectionQuery( + format("from Book where outOfStock = %s", isNullLiteral ? null : nonNullBooleanLiteralStr), + Book.class, + """ + { + "aggregate": "books", + "pipeline": [ { - "aggregate": "books", - "pipeline": [ - { - "$match": { + "$match": { + "$and": [ + { "outOfStock": { - "$eq": true + "$eq": %s + } + }, + { + "outOfStock": { + "$ne": null } } - }, - { - "$project": { - "_id": true, - "discount": true, - "isbn13": true, - "outOfStock": true, - "price": true, - "publishYear": true, - "title": true - } - } - ] - }""", - singletonList(testingBook)); - } + ] + } + }, + { + "$project": { + "_id": true, + "discount": true, + "isbn13": true, + "outOfStock": true, + "price": true, + "publishYear": true, + "title": true + } + } + ] + }""" + .formatted(isNullLiteral ? null : nonNullBooleanLiteralStr), + isNullLiteral ? emptyList() : getBooksByIds(1)); + } - @Test - void testInteger() { - assertSelectionQuery( - "from Book where publishYear = 1995", - Book.class, - """ + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testInteger(boolean isNullLiteral) { + var nonNullIntegerLiteralStr = "1866"; + assertSelectionQuery( + format("from Book where publishYear = %s", isNullLiteral ? null : nonNullIntegerLiteralStr), + Book.class, + """ + { + "aggregate": "books", + "pipeline": [ { - "aggregate": "books", - "pipeline": [ - { - "$match": { + "$match": { + "$and": [ + { "publishYear": { - "$eq": 1995 + "$eq": %s + } + }, + { + "publishYear": { + "$ne": null } } - }, - { - "$project": { - "_id": true, - "discount": true, - "isbn13": true, - "outOfStock": true, - "price": true, - "publishYear": true, - "title": true - } - } - ] - }""", - singletonList(testingBook)); - } + ] + } + }, + { + "$project": { + "_id": true, + "discount": true, + "isbn13": true, + "outOfStock": true, + "price": true, + "publishYear": true, + "title": true + } + } + ] + }""" + .formatted(isNullLiteral ? null : nonNullIntegerLiteralStr), + isNullLiteral ? emptyList() : getBooksByIds(2)); + } - @Test - void testLong() { - assertSelectionQuery( - "from Book where isbn13 = 9780310904168L", - Book.class, - """ + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testLong(boolean isNullLiteral) { + var nonNullLongLiteralStr = "9780310904168"; + assertSelectionQuery( + format("from Book where isbn13 = %s", isNullLiteral ? null : (nonNullLongLiteralStr + "L")), + Book.class, + """ + { + "aggregate": "books", + "pipeline": [ { - "aggregate": "books", - "pipeline": [ - { - "$match": { + "$match": { + "$and": [ + { "isbn13": { - "$eq": 9780310904168 + "$eq": %s + } + }, + { + "isbn13": { + "$ne": null } } - }, - { - "$project": { - "_id": true, - "discount": true, - "isbn13": true, - "outOfStock": true, - "price": true, - "publishYear": true, - "title": true - } - } - ] - }""", - singletonList(testingBook)); - } + ] + } + }, + { + "$project": { + "_id": true, + "discount": true, + "isbn13": true, + "outOfStock": true, + "price": true, + "publishYear": true, + "title": true + } + } + ] + }""" + .formatted(isNullLiteral ? null : nonNullLongLiteralStr), + isNullLiteral ? emptyList() : getBooksByIds(3)); + } - @Test - void testDouble() { - assertSelectionQuery( - "from Book where discount = 0.25D", - Book.class, - """ + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testDouble(boolean isNullLiteral) { + var nonNullLiteralStr = "0.5"; + assertSelectionQuery( + format("from Book where discount > %s", isNullLiteral ? null : (nonNullLiteralStr + "D")), + Book.class, + """ + { + "aggregate": "books", + "pipeline": [ { - "aggregate": "books", - "pipeline": [ - { - "$match": { + "$match": { + "$and": [ + { "discount": { - "$eq": 0.25 + "$gt": %s + } + }, + { + "discount": { + "$ne": null } } - }, - { - "$project": { - "_id": true, - "discount": true, - "isbn13": true, - "outOfStock": true, - "price": true, - "publishYear": true, - "title": true - } - } - ] - }""", - singletonList(testingBook)); - } + ] + } + }, + { + "$project": { + "_id": true, + "discount": true, + "isbn13": true, + "outOfStock": true, + "price": true, + "publishYear": true, + "title": true + } + } + ] + }""" + .formatted(isNullLiteral ? null : nonNullLiteralStr), + isNullLiteral ? emptyList() : getBooksByIds(3, 4)); + } - @Test - void testString() { - assertSelectionQuery( - "from Book where title = 'Holy Bible'", - Book.class, - """ + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testString(boolean isNullLiteral) { + var nonNullLiteralStr = "War and Peace"; + assertSelectionQuery( + format("from Book where title = %s", isNullLiteral ? null : ("\"" + nonNullLiteralStr + "\"")), + Book.class, + """ + { + "aggregate": "books", + "pipeline": [ { - "aggregate": "books", - "pipeline": [ - { - "$match": { + "$match": { + "$and": [ + { "title": { - "$eq": "Holy Bible" + "$eq": %s + } + }, + { + "title": { + "$ne": null } } - }, - { - "$project": { - "_id": true, - "discount": true, - "isbn13": true, - "outOfStock": true, - "price": true, - "publishYear": true, - "title": true - } - } - ] - }""", - singletonList(testingBook)); - } + ] + } + }, + { + "$project": { + "_id": true, + "discount": true, + "isbn13": true, + "outOfStock": true, + "price": true, + "publishYear": true, + "title": true + } + } + ] + }""" + .formatted(isNullLiteral ? null : ("\"" + nonNullLiteralStr + "\"")), + isNullLiteral ? emptyList() : getBooksByIds(1, 5)); + } - @Test - void testBigDecimal() { - assertSelectionQuery( - "from Book where price = 123.50BD", - Book.class, - """ + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testBigDecimal(boolean isNullLiteral) { + var nonNullLiteralStr = "123.50"; + assertSelectionQuery( + format("from Book where price = %s", isNullLiteral ? null : (nonNullLiteralStr + "BD")), + Book.class, + """ + { + "aggregate": "books", + "pipeline": [ { - "aggregate": "books", - "pipeline": [ - { - "$match": { + "$match": { + "$and": [ + { "price": { - "$eq": { - "$numberDecimal": "123.50" - } + "$eq": %s + } + }, + { + "price": { + "$ne": null } } - }, - { - "$project": { - "_id": true, - "discount": true, - "isbn13": true, - "outOfStock": true, - "price": true, - "publishYear": true, - "title": true - } - } - ] - }""", - singletonList(testingBook)); - } + ] + } + }, + { + "$project": { + "_id": true, + "discount": true, + "isbn13": true, + "outOfStock": true, + "price": true, + "publishYear": true, + "title": true + } + } + ] + }""" + .formatted(isNullLiteral ? null : "{\"$numberDecimal\": \"123.50\"}"), + isNullLiteral ? emptyList() : getBooksByIds(1)); } @Entity(name = "Contact") @@ -913,16 +1504,21 @@ static class Contact { int id; String name; - int age; + Integer age; String country; Contact() {} - Contact(int id, String name, int age, Country country) { + Contact(int id, String name, Integer age, Country country) { this.id = id; this.name = name; this.age = age; - this.country = country.name(); + this.country = country == null ? null : country.name(); + } + + @Override + public String toString() { + return "Contact{" + "id=" + id + '}'; } } diff --git a/src/integrationTest/java/com/mongodb/hibernate/query/select/SortingSelectQueryIntegrationTests.java b/src/integrationTest/java/com/mongodb/hibernate/query/select/SortingSelectQueryIntegrationTests.java index 4d7527b3..105b5d42 100644 --- a/src/integrationTest/java/com/mongodb/hibernate/query/select/SortingSelectQueryIntegrationTests.java +++ b/src/integrationTest/java/com/mongodb/hibernate/query/select/SortingSelectQueryIntegrationTests.java @@ -146,9 +146,18 @@ void testOrderByMultipleFieldsWithoutTies() { "pipeline": [ { "$match": { - "outOfStock": { - "$eq": false - } + "$and": [ + { + "outOfStock": { + "$eq": false + } + }, + { + "outOfStock": { + "$ne": null + } + } + ] } }, { diff --git a/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java b/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java index 43b140b4..dc042045 100644 --- a/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java +++ b/src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java @@ -518,7 +518,7 @@ public void visitRelationalPredicate(ComparisonPredicate comparisonPredicate) { var astComparisonFilterOperator = getAstComparisonFilterOperator(operator); var astFilterOperation = new AstComparisonFilterOperation(astComparisonFilterOperator, comparisonValue); - var filter = new AstFieldOperationFilter(fieldPath, astFilterOperation); + var filter = new AstFieldOperationFilter(fieldPath, astFilterOperation).withTernaryNullnessLogicEnforced(); astVisitorValueHolder.yield(FILTER, filter); } @@ -566,9 +566,6 @@ public void visitColumnReference(ColumnReference columnReference) { @Override public void visitQueryLiteral(QueryLiteral queryLiteral) { var literalValue = queryLiteral.getLiteralValue(); - if (literalValue == null) { - throw new FeatureNotSupportedException("TODO-HIBERNATE-74 https://jira.mongodb.org/browse/HIBERNATE-74"); - } astVisitorValueHolder.yield(VALUE, new AstLiteralValue(toLiteralBsonValue(literalValue))); } @@ -600,7 +597,7 @@ public void visitBooleanExpressionPredicate(BooleanExpressionPredicate booleanEx var fieldPath = acceptAndYield(booleanExpressionPredicate.getExpression(), FIELD_PATH); var astFilterOperation = new AstComparisonFilterOperation(EQ, booleanExpressionPredicate.isNegated() ? FALSE : TRUE); - var filter = new AstFieldOperationFilter(fieldPath, astFilterOperation); + var filter = new AstFieldOperationFilter(fieldPath, astFilterOperation).withTernaryNullnessLogicEnforced(); astVisitorValueHolder.yield(FILTER, filter); } @@ -1020,8 +1017,7 @@ private static boolean isComparingFieldWithValue(ComparisonPredicate comparisonP || (isFieldPathExpression(rhs) && isValueExpression(lhs)); } - private static BsonValue toLiteralBsonValue(Object value) { - // TODO-HIBERNATE-74 decide if `value` is nullable + private static BsonValue toLiteralBsonValue(@Nullable Object value) { try { return ValueConversions.toBsonValue(value); } catch (SQLFeatureNotSupportedException e) { diff --git a/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/AstLiteralValue.java b/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/AstLiteralValue.java index 7c45ea9e..36a0a312 100644 --- a/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/AstLiteralValue.java +++ b/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/AstLiteralValue.java @@ -17,6 +17,7 @@ package com.mongodb.hibernate.internal.translate.mongoast; import org.bson.BsonBoolean; +import org.bson.BsonNull; import org.bson.BsonValue; import org.bson.BsonWriter; import org.bson.codecs.BsonValueCodec; @@ -30,6 +31,7 @@ public record AstLiteralValue(BsonValue literalValue) implements AstValue { public static final AstLiteralValue TRUE = new AstLiteralValue(BsonBoolean.TRUE); public static final AstLiteralValue FALSE = new AstLiteralValue(BsonBoolean.FALSE); + public static final AstLiteralValue NULL = new AstLiteralValue(BsonNull.VALUE); @Override public void render(BsonWriter writer) { diff --git a/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/filter/AstFieldOperationFilter.java b/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/filter/AstFieldOperationFilter.java index 437bf126..df957d6e 100644 --- a/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/filter/AstFieldOperationFilter.java +++ b/src/main/java/com/mongodb/hibernate/internal/translate/mongoast/filter/AstFieldOperationFilter.java @@ -16,9 +16,15 @@ package com.mongodb.hibernate.internal.translate.mongoast.filter; +import static com.mongodb.hibernate.internal.translate.mongoast.filter.AstComparisonFilterOperator.NE; +import static com.mongodb.hibernate.internal.translate.mongoast.filter.AstLogicalFilterOperator.AND; + +import com.mongodb.hibernate.internal.translate.mongoast.AstLiteralValue; +import java.util.List; import org.bson.BsonWriter; public record AstFieldOperationFilter(String fieldPath, AstFilterOperation filterOperation) implements AstFilter { + @Override public void render(BsonWriter writer) { writer.writeStartDocument(); @@ -28,4 +34,10 @@ public void render(BsonWriter writer) { } writer.writeEndDocument(); } + + public AstFilter withTernaryNullnessLogicEnforced() { + var nullFieldExclusionFilter = + new AstFieldOperationFilter(fieldPath, new AstComparisonFilterOperation(NE, AstLiteralValue.NULL)); + return new AstLogicalFilter(AND, List.of(this, nullFieldExclusionFilter)); + } } diff --git a/src/main/java/com/mongodb/hibernate/jdbc/MongoPreparedStatement.java b/src/main/java/com/mongodb/hibernate/jdbc/MongoPreparedStatement.java index 74fbbabd..bfab16e4 100644 --- a/src/main/java/com/mongodb/hibernate/jdbc/MongoPreparedStatement.java +++ b/src/main/java/com/mongodb/hibernate/jdbc/MongoPreparedStatement.java @@ -23,7 +23,6 @@ import com.mongodb.client.ClientSession; import com.mongodb.client.MongoDatabase; -import com.mongodb.hibernate.internal.FeatureNotSupportedException; import com.mongodb.hibernate.internal.type.MongoStructJdbcType; import com.mongodb.hibernate.internal.type.ObjectIdJdbcType; import java.math.BigDecimal; @@ -41,7 +40,6 @@ import java.util.ArrayList; import java.util.Calendar; import java.util.List; -import java.util.Set; import java.util.function.Consumer; import org.bson.BsonArray; import org.bson.BsonDocument; @@ -86,41 +84,8 @@ private void checkAllParametersSet() throws SQLException { throw new SQLException(format("Parameter with index [%d] is not set", i + 1)); } } - checkComparatorNotComparingWithNullValues(); } - /** - * Temporary method to ensure exception is thrown when comparison query operators are comparing with {@code null} - * values. - * - *

Note that only find expression is involved before HIBERNATE-74. TODO-HIBERNATE-74 delete this temporary method - */ - private void checkComparatorNotComparingWithNullValues() { - doCheckComparatorNotComparingWithNullValues(command); - } - - private void doCheckComparatorNotComparingWithNullValues(BsonDocument document) { - for (var entry : document.entrySet()) { - if (COMPARISON_OPERATOR_NAMES_SUPPORTED.contains(entry.getKey()) - && entry.getValue().isNull()) { - throw new FeatureNotSupportedException( - "TODO-HIBERNATE-74 https://jira.mongodb.org/browse/HIBERNATE-74"); - } - if (entry.getValue().isDocument()) { - doCheckComparatorNotComparingWithNullValues(entry.getValue().asDocument()); - } else if (entry.getValue().isArray()) { - for (var bsonValue : entry.getValue().asArray()) { - if (bsonValue.isDocument()) { - doCheckComparatorNotComparingWithNullValues(bsonValue.asDocument()); - } - } - } - } - } - - private static final Set COMPARISON_OPERATOR_NAMES_SUPPORTED = - Set.of("$eq", "$ne", "$gt", "$gte", "$lt", "$lte"); - @Override public void setNull(int parameterIndex, int sqlType) throws SQLException { checkClosed();