Skip to content

Commit 12a9902

Browse files
NathanQingyangXuNathanQingyangXu
committed
resolve code review comments
1 parent d5f27cd commit 12a9902

File tree

4 files changed

+111
-37
lines changed

4 files changed

+111
-37
lines changed

‎src/integrationTest/java/com/mongodb/hibernate/query/select/AbstractSelectionQueryIntegrationTests.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,21 @@ <T> void assertSelectQueryFailure(
9999
.hasMessage(expectedExceptionMessage, expectedExceptionMessageParameters));
100100
}
101101

102+
<T> void assertSelectQueryFailure(
103+
String hql,
104+
Class<T> resultType,
105+
Class<? extends Exception> expectedExceptionType,
106+
String expectedExceptionMessage,
107+
Object... expectedExceptionMessageParameters) {
108+
assertSelectQueryFailure(
109+
hql,
110+
resultType,
111+
null,
112+
expectedExceptionType,
113+
expectedExceptionMessage,
114+
expectedExceptionMessageParameters);
115+
}
116+
102117
void assertActualCommand(BsonDocument expectedCommand) {
103118
var capturedCommands = testCommandListener.getStartedCommands();
104119

‎src/integrationTest/java/com/mongodb/hibernate/query/select/SortingIntegrationTests.java renamed to ‎src/integrationTest/java/com/mongodb/hibernate/query/select/SortingSelectQueryIntegrationTests.java

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,13 @@
2626
import java.util.List;
2727
import org.hibernate.testing.orm.junit.DomainModel;
2828
import org.junit.jupiter.api.BeforeEach;
29+
import org.junit.jupiter.api.Nested;
2930
import org.junit.jupiter.api.Test;
3031
import org.junit.jupiter.params.ParameterizedTest;
3132
import org.junit.jupiter.params.provider.EnumSource;
3233

3334
@DomainModel(annotatedClasses = Book.class)
34-
class SortingIntegrationTests extends AbstractSelectionQueryIntegrationTests {
35+
class SortingSelectQueryIntegrationTests extends AbstractSelectionQueryIntegrationTests {
3536

3637
private static final List<Book> testingBooks = List.of(
3738
new Book(1, "War and Peace", 1869, true),
@@ -145,7 +146,6 @@ void testSortFieldNotFieldPathExpression() {
145146
assertSelectQueryFailure(
146147
"from Book ORDER BY length(title)",
147148
Book.class,
148-
null,
149149
FeatureNotSupportedException.class,
150150
"%s does not support sort key not of field path type",
151151
MONGO_DBMS_NAME);
@@ -156,9 +156,32 @@ void testNullPrecedenceFeatureNotSupported() {
156156
assertSelectQueryFailure(
157157
"from Book ORDER BY publishYear NULLS LAST",
158158
Book.class,
159-
null,
160159
FeatureNotSupportedException.class,
161160
"%s does not support nulls precedence: NULLS LAST",
162161
MONGO_DBMS_NAME);
163162
}
163+
164+
@Nested
165+
class SqlTupleTests {
166+
167+
@Test
168+
void testOrderBySimpleTuple() {
169+
assertSelectionQuery(
170+
"from Book ORDER BY (publishYear, title) ASC",
171+
Book.class,
172+
null,
173+
"{ 'aggregate': 'books', 'pipeline': [ { '$sort': { 'publishYear': 1, 'title': 1 } }, {'$project': {'_id': true, 'discount': true, 'isbn13': true, 'outOfStock': true, 'price': true, 'publishYear': true, 'title': true} } ] }",
174+
getBooksByIds(2, 1, 3, 4, 5));
175+
}
176+
177+
@Test
178+
void testOrderByNestedTuple() {
179+
assertSelectionQuery(
180+
"from Book ORDER BY (title, (id, publishYear)) DESC",
181+
Book.class,
182+
null,
183+
"{ 'aggregate': 'books', 'pipeline': [ { '$sort': { 'title': -1, '_id': -1, 'publishYear': -1 } }, {'$project': {'_id': true, 'discount': true, 'isbn13': true, 'outOfStock': true, 'price': true, 'publishYear': true, 'title': true} } ] }",
184+
getBooksByIds(5, 1, 4, 2, 3));
185+
}
186+
}
164187
}

‎src/main/java/com/mongodb/hibernate/internal/translate/AbstractMqlTranslator.java

Lines changed: 67 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,11 @@
2424
import static com.mongodb.hibernate.internal.translate.AstVisitorValueDescriptor.COLLECTION_MUTATION;
2525
import static com.mongodb.hibernate.internal.translate.AstVisitorValueDescriptor.COLLECTION_NAME;
2626
import static com.mongodb.hibernate.internal.translate.AstVisitorValueDescriptor.FIELD_PATH;
27+
import static com.mongodb.hibernate.internal.translate.AstVisitorValueDescriptor.FIELD_PATHS;
2728
import static com.mongodb.hibernate.internal.translate.AstVisitorValueDescriptor.FIELD_VALUE;
2829
import static com.mongodb.hibernate.internal.translate.AstVisitorValueDescriptor.FILTER;
2930
import static com.mongodb.hibernate.internal.translate.AstVisitorValueDescriptor.PROJECT_STAGE_SPECIFICATIONS;
30-
import static com.mongodb.hibernate.internal.translate.AstVisitorValueDescriptor.SORT_FIELD;
31+
import static com.mongodb.hibernate.internal.translate.AstVisitorValueDescriptor.SORT_FIELDS;
3132
import static com.mongodb.hibernate.internal.translate.mongoast.command.aggregate.AstSortOrder.ASC;
3233
import static com.mongodb.hibernate.internal.translate.mongoast.command.aggregate.AstSortOrder.DESC;
3334
import static com.mongodb.hibernate.internal.translate.mongoast.filter.AstComparisonFilterOperator.EQ;
@@ -40,6 +41,7 @@
4041
import static com.mongodb.hibernate.internal.translate.mongoast.filter.AstLogicalFilterOperator.NOR;
4142
import static com.mongodb.hibernate.internal.translate.mongoast.filter.AstLogicalFilterOperator.OR;
4243
import static java.lang.String.format;
44+
import static java.util.Collections.singletonList;
4345

4446
import com.mongodb.hibernate.internal.FeatureNotSupportedException;
4547
import com.mongodb.hibernate.internal.extension.service.StandardServiceRegistryScopedState;
@@ -378,14 +380,10 @@ public void visitQuerySpec(QuerySpec querySpec) {
378380
var stages = new ArrayList<AstStage>(3);
379381

380382
// $match stage
381-
var whereClauseRestrictions = querySpec.getWhereClauseRestrictions();
382-
if (whereClauseRestrictions != null && !whereClauseRestrictions.isEmpty()) {
383-
var filter = acceptAndYield(whereClauseRestrictions, FILTER);
384-
stages.add(new AstMatchStage(filter));
385-
}
383+
getAstMatchStage(querySpec).ifPresent(stages::add);
386384

387385
// $sort stage
388-
getOptionalAstSortStage(querySpec).ifPresent(stages::add);
386+
getAstSortStage(querySpec).ifPresent(stages::add);
389387

390388
// $project stage
391389
var projectStageSpecifications = acceptAndYield(querySpec.getSelectClause(), PROJECT_STAGE_SPECIFICATIONS);
@@ -394,26 +392,22 @@ public void visitQuerySpec(QuerySpec querySpec) {
394392
astVisitorValueHolder.yield(COLLECTION_AGGREGATE, new AstAggregateCommand(collection, stages));
395393
}
396394

397-
private Optional<AstSortStage> getOptionalAstSortStage(QuerySpec querySpec) {
395+
private Optional<AstMatchStage> getAstMatchStage(QuerySpec querySpec) {
396+
var whereClauseRestrictions = querySpec.getWhereClauseRestrictions();
397+
if (whereClauseRestrictions != null && !whereClauseRestrictions.isEmpty()) {
398+
var filter = acceptAndYield(whereClauseRestrictions, FILTER);
399+
return Optional.of(new AstMatchStage(filter));
400+
} else {
401+
return Optional.empty();
402+
}
403+
}
404+
405+
private Optional<AstSortStage> getAstSortStage(QuerySpec querySpec) {
398406
if (querySpec.hasSortSpecifications()) {
399407
var sortFields = new ArrayList<AstSortField>(
400408
querySpec.getSortSpecifications().size());
401-
for (SortSpecification sortSpecification : querySpec.getSortSpecifications()) {
402-
if (!isFieldPathExpression(sortSpecification.getSortExpression())) {
403-
throw new FeatureNotSupportedException(
404-
format("%s does not support sort key not of field path type", MONGO_DBMS_NAME));
405-
}
406-
if (sortSpecification.getNullPrecedence() != null
407-
&& sortSpecification.getNullPrecedence() != NullPrecedence.NONE) {
408-
throw new FeatureNotSupportedException(format(
409-
"%s does not support nulls precedence: NULLS %s",
410-
MONGO_DBMS_NAME, sortSpecification.getNullPrecedence()));
411-
}
412-
if (sortSpecification.isIgnoreCase()) {
413-
throw new FeatureNotSupportedException(
414-
format("%s does not support case-insensitive sort key", MONGO_DBMS_NAME));
415-
}
416-
sortFields.add(acceptAndYield(sortSpecification, SORT_FIELD));
409+
for (var sortSpecification : querySpec.getSortSpecifications()) {
410+
sortFields.addAll(acceptAndYield(sortSpecification, SORT_FIELDS));
417411
}
418412
return Optional.of(new AstSortStage(sortFields));
419413
}
@@ -541,12 +535,57 @@ public void visitSqlSelectionExpression(SqlSelectionExpression sqlSelectionExpre
541535
sqlSelectionExpression.getSelection().getExpression().accept(this);
542536
}
543537

538+
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
539+
// ORDER BY clause
540+
544541
@Override
545542
public void visitSortSpecification(SortSpecification sortSpecification) {
546-
var sortFieldPath = acceptAndYield(sortSpecification.getSortExpression(), FIELD_PATH);
547-
var astSortField = new AstSortField(
548-
sortFieldPath, sortSpecification.getSortOrder() == SortDirection.ASCENDING ? ASC : DESC);
549-
astVisitorValueHolder.yield(SORT_FIELD, astSortField);
543+
if (sortSpecification.getNullPrecedence() != null
544+
&& sortSpecification.getNullPrecedence() != NullPrecedence.NONE) {
545+
throw new FeatureNotSupportedException(format(
546+
"%s does not support nulls precedence: NULLS %s",
547+
MONGO_DBMS_NAME, sortSpecification.getNullPrecedence()));
548+
}
549+
if (sortSpecification.isIgnoreCase()) {
550+
throw new FeatureNotSupportedException(
551+
format("%s does not support case-insensitive sort key [%s]", MONGO_DBMS_NAME, sortSpecification));
552+
}
553+
var sortExpression = sortSpecification.getSortExpression();
554+
final List<String> fieldPaths;
555+
if (sortExpression instanceof SqlTuple sqlTuple) {
556+
fieldPaths = acceptAndYield(sqlTuple, FIELD_PATHS);
557+
} else {
558+
if (!isFieldPathExpression(sortExpression)) {
559+
throw new FeatureNotSupportedException(
560+
format("%s does not support sort key not of field path type", MONGO_DBMS_NAME));
561+
}
562+
var sortFieldPath = acceptAndYield(sortExpression, FIELD_PATH);
563+
fieldPaths = singletonList(sortFieldPath);
564+
}
565+
566+
var astSortFields = new ArrayList<AstSortField>(fieldPaths.size());
567+
for (var fieldPath : fieldPaths) {
568+
var astSortField = new AstSortField(
569+
fieldPath, sortSpecification.getSortOrder() == SortDirection.ASCENDING ? ASC : DESC);
570+
astSortFields.add(astSortField);
571+
}
572+
astVisitorValueHolder.yield(SORT_FIELDS, astSortFields);
573+
}
574+
575+
@Override
576+
public void visitTuple(SqlTuple sqlTuple) {
577+
List<String> fieldPaths = new ArrayList<>(sqlTuple.getExpressions().size());
578+
for (var expression : sqlTuple.getExpressions()) {
579+
if (expression instanceof SqlTuple) {
580+
fieldPaths.addAll(acceptAndYield(expression, FIELD_PATHS));
581+
} else {
582+
if (!isFieldPathExpression(expression)) {
583+
throw new FeatureNotSupportedException();
584+
}
585+
fieldPaths.add(acceptAndYield(expression, FIELD_PATH));
586+
}
587+
}
588+
astVisitorValueHolder.yield(FIELD_PATHS, fieldPaths);
550589
}
551590

552591
@Override
@@ -709,11 +748,6 @@ public void visitEmbeddableTypeLiteral(EmbeddableTypeLiteral embeddableTypeLiter
709748
throw new FeatureNotSupportedException();
710749
}
711750

712-
@Override
713-
public void visitTuple(SqlTuple sqlTuple) {
714-
throw new FeatureNotSupportedException();
715-
}
716-
717751
@Override
718752
public void visitCollation(Collation collation) {
719753
throw new FeatureNotSupportedException();

‎src/main/java/com/mongodb/hibernate/internal/translate/AstVisitorValueDescriptor.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ final class AstVisitorValueDescriptor<T> {
4545
new AstVisitorValueDescriptor<>();
4646
static final AstVisitorValueDescriptor<AstFilter> FILTER = new AstVisitorValueDescriptor<>();
4747

48-
static final AstVisitorValueDescriptor<AstSortField> SORT_FIELD = new AstVisitorValueDescriptor<>();
48+
static final AstVisitorValueDescriptor<List<AstSortField>> SORT_FIELDS = new AstVisitorValueDescriptor<>();
49+
50+
static final AstVisitorValueDescriptor<List<String>> FIELD_PATHS = new AstVisitorValueDescriptor<>();
4951

5052
private static final Map<AstVisitorValueDescriptor<?>, String> CONSTANT_TOSTRING_CONTENT_MAP;
5153

0 commit comments

Comments
 (0)