Skip to content

Commit 46ebfdc

Browse files
kotharironakkotharironak
authored
feat: add support for orderby clause for new query api (#96)
* adds integration test and fixed unit tests * reverted the accidental change * Fixed few comments * removes the usage of data accessor from filter clause * removes commented code * fixed the string cases after removal of data accessor pattern * feat : add support for sotring expression * addressed comments * added test for covering where and aggregation filter * fixed build error after merging to main * fixed spotless issue * added comments and refactored exception
1 parent bf03510 commit 46ebfdc

File tree

6 files changed

+180
-29
lines changed

6 files changed

+180
-29
lines changed

‎document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreTest.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@
1414
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.GTE;
1515
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.LTE;
1616
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.NEQ;
17+
import static org.hypertrace.core.documentstore.expression.operators.SortOrder.DESC;
1718
import static org.hypertrace.core.documentstore.utils.CreateUpdateTestThread.FAILURE;
1819
import static org.hypertrace.core.documentstore.utils.CreateUpdateTestThread.SUCCESS;
20+
import static org.hypertrace.core.documentstore.utils.Utils.assertDocsAndSizeEqual;
1921
import static org.hypertrace.core.documentstore.utils.Utils.convertDocumentToMap;
2022
import static org.hypertrace.core.documentstore.utils.Utils.convertJsonToMap;
2123
import static org.hypertrace.core.documentstore.utils.Utils.createDocumentsFromResource;
@@ -1806,6 +1808,35 @@ public void testQueryQ1AggregationFilterAlongWithNonAliasFields(String dataStore
18061808
dataStoreName, resultDocs, 4, "mongo/test_aggr_alias_distinct_count_response.json");
18071809
}
18081810

1811+
@ParameterizedTest
1812+
@MethodSource("databaseContextProvider")
1813+
public void testQueryV1DistinctCountWithSortingSpecs(String dataStoreName) throws IOException {
1814+
Map<Key, Document> documents = createDocumentsFromResource("mongo/collection_data.json");
1815+
Datastore datastore = datastoreMap.get(dataStoreName);
1816+
Collection collection = datastore.getCollection(COLLECTION_NAME);
1817+
1818+
// add docs
1819+
boolean result = collection.bulkUpsert(documents);
1820+
Assertions.assertTrue(result);
1821+
1822+
org.hypertrace.core.documentstore.query.Query query =
1823+
org.hypertrace.core.documentstore.query.Query.builder()
1824+
.addSelection(
1825+
AggregateExpression.of(DISTINCT_COUNT, IdentifierExpression.of("quantity")),
1826+
"qty_count")
1827+
.addSelection(IdentifierExpression.of("item"))
1828+
.addAggregation(IdentifierExpression.of("item"))
1829+
.setAggregationFilter(
1830+
RelationalExpression.of(
1831+
IdentifierExpression.of("qty_count"), LTE, ConstantExpression.of(1000)))
1832+
.addSort(IdentifierExpression.of("qty_count"), DESC)
1833+
.addSort(IdentifierExpression.of("item"), DESC)
1834+
.build();
1835+
1836+
Iterator<Document> resultDocs = collection.aggregate(query);
1837+
assertDocsAndSizeEqual(resultDocs, "mongo/distinct_count_response.json", 4);
1838+
}
1839+
18091840
@ParameterizedTest
18101841
@MethodSource("databaseContextProvider")
18111842
public void whenBulkUpdatingNonExistentRecords_thenExpectNothingToBeUpdatedOrCreated(

‎document-store/src/integrationTest/java/org/hypertrace/core/documentstore/mongo/MongoQueryExecutorIntegrationTest.java‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import static org.hypertrace.core.documentstore.expression.operators.AggregationOperator.COUNT;
44
import static org.hypertrace.core.documentstore.expression.operators.AggregationOperator.DISTINCT;
5-
import static org.hypertrace.core.documentstore.expression.operators.AggregationOperator.DISTINCT_COUNT;
65
import static org.hypertrace.core.documentstore.expression.operators.AggregationOperator.SUM;
76
import static org.hypertrace.core.documentstore.expression.operators.FunctionOperator.LENGTH;
87
import static org.hypertrace.core.documentstore.expression.operators.FunctionOperator.MULTIPLY;
@@ -498,27 +497,6 @@ public void testAggregateWithMultipleGroupingLevels() throws IOException {
498497
assertSizeEqual(query, "mongo/multi_level_grouping_response.json");
499498
}
500499

501-
@Test
502-
public void testDistinctCount() throws IOException {
503-
Query query =
504-
Query.builder()
505-
.addSelection(
506-
AggregateExpression.of(DISTINCT_COUNT, IdentifierExpression.of("quantity")),
507-
"qty_count")
508-
.addSelection(IdentifierExpression.of("item"))
509-
.addAggregation(IdentifierExpression.of("item"))
510-
.setAggregationFilter(
511-
RelationalExpression.of(
512-
IdentifierExpression.of("qty_count"), LTE, ConstantExpression.of(1000)))
513-
.addSort(IdentifierExpression.of("qty_count"), DESC)
514-
.addSort(IdentifierExpression.of("item"), DESC)
515-
.build();
516-
517-
Iterator<Document> resultDocs = collection.aggregate(query);
518-
assertDocsEqual(resultDocs, "mongo/distinct_count_response.json");
519-
assertSizeEqual(query, "mongo/distinct_count_response.json");
520-
}
521-
522500
@Test
523501
public void testUnnestAndAggregate() throws IOException {
524502
org.hypertrace.core.documentstore.query.Query query =

‎document-store/src/integrationTest/java/org/hypertrace/core/documentstore/utils/Utils.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
package org.hypertrace.core.documentstore.utils;
22

3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
35
import com.fasterxml.jackson.core.JsonProcessingException;
46
import com.fasterxml.jackson.core.type.TypeReference;
57
import com.fasterxml.jackson.databind.ObjectMapper;
68
import com.fasterxml.jackson.databind.node.ObjectNode;
79
import java.io.IOException;
810
import java.io.InputStream;
911
import java.nio.charset.StandardCharsets;
12+
import java.util.ArrayList;
1013
import java.util.HashMap;
14+
import java.util.Iterator;
1115
import java.util.List;
1216
import java.util.Map;
1317
import java.util.Optional;
@@ -92,4 +96,20 @@ public static Map<String, Object> convertDocumentToMap(Document document)
9296
throws JsonProcessingException {
9397
return OBJECT_MAPPER.readValue(document.toJson(), new TypeReference<>() {});
9498
}
99+
100+
public static void assertDocsAndSizeEqual(
101+
Iterator<Document> documents, String filePath, int expectedSize) throws IOException {
102+
String fileContent = readFileFromResource(filePath).orElseThrow();
103+
List<Map<String, Object>> expected = convertJsonToMap(fileContent);
104+
105+
int actualSize = 0;
106+
List<Map<String, Object>> actual = new ArrayList<>();
107+
while (documents.hasNext()) {
108+
actual.add(convertDocumentToMap(documents.next()));
109+
actualSize++;
110+
}
111+
112+
assertEquals(expected, actual);
113+
assertEquals(expectedSize, actualSize);
114+
}
95115
}

‎document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/PostgresQueryParser.java‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package org.hypertrace.core.documentstore.postgres.query.v1;
22

33
import java.util.HashMap;
4-
import java.util.List;
54
import java.util.Map;
65
import java.util.Optional;
76
import lombok.Getter;
@@ -10,9 +9,9 @@
109
import org.hypertrace.core.documentstore.postgres.query.v1.vistors.PostgresFilterTypeExpressionVisitor;
1110
import org.hypertrace.core.documentstore.postgres.query.v1.vistors.PostgresGroupTypeExpressionVisitor;
1211
import org.hypertrace.core.documentstore.postgres.query.v1.vistors.PostgresSelectTypeExpressionVisitor;
12+
import org.hypertrace.core.documentstore.postgres.query.v1.vistors.PostgresSortTypeExpressionVisitor;
1313
import org.hypertrace.core.documentstore.query.Pagination;
1414
import org.hypertrace.core.documentstore.query.Query;
15-
import org.hypertrace.core.documentstore.query.SortingSpec;
1615

1716
public class PostgresQueryParser {
1817
private static String NOT_YET_SUPPORTED = "Not yet supported %s";
@@ -94,11 +93,7 @@ private Optional<String> parseHaving() {
9493
}
9594

9695
private Optional<String> parseOrderBy() {
97-
List<SortingSpec> sortingSpecs = this.query.getSorts();
98-
if (sortingSpecs.size() > 0) {
99-
throw new UnsupportedOperationException(String.format(NOT_YET_SUPPORTED, "order by clause"));
100-
}
101-
return Optional.empty();
96+
return PostgresSortTypeExpressionVisitor.getOrderByClause(this);
10297
}
10398

10499
private Optional<String> parsePagination() {

‎document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/vistors/PostgresSortTypeExpressionVisitor.java‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package org.hypertrace.core.documentstore.postgres.query.v1.vistors;
2+
3+
import java.util.List;
4+
import java.util.Optional;
5+
import java.util.stream.Collectors;
6+
import org.apache.commons.lang3.StringUtils;
7+
import org.hypertrace.core.documentstore.expression.impl.AggregateExpression;
8+
import org.hypertrace.core.documentstore.expression.impl.FunctionExpression;
9+
import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression;
10+
import org.hypertrace.core.documentstore.parser.SortTypeExpressionVisitor;
11+
import org.hypertrace.core.documentstore.postgres.query.v1.PostgresQueryParser;
12+
import org.hypertrace.core.documentstore.query.SortingSpec;
13+
14+
public class PostgresSortTypeExpressionVisitor implements SortTypeExpressionVisitor {
15+
16+
private PostgresQueryParser postgresQueryParser;
17+
private PostgresIdentifierExpressionVisitor identifierExpressionVisitor;
18+
PostgresFieldIdentifierExpressionVisitor fieldIdentifierExpressionVisitor;
19+
20+
public PostgresSortTypeExpressionVisitor(PostgresQueryParser postgresQueryParser) {
21+
this.postgresQueryParser = postgresQueryParser;
22+
identifierExpressionVisitor = new PostgresIdentifierExpressionVisitor();
23+
fieldIdentifierExpressionVisitor = new PostgresFieldIdentifierExpressionVisitor();
24+
}
25+
26+
@Override
27+
public String visit(AggregateExpression expression) {
28+
throw new UnsupportedOperationException(
29+
"Sorting using aggregation expression is not yet supported."
30+
+ "Use alias in selection for aggregation expression for sorting");
31+
}
32+
33+
@Override
34+
public String visit(FunctionExpression expression) {
35+
throw new UnsupportedOperationException(
36+
"Sorting using function expression is not yet supported."
37+
+ "Use alias in selection for functional expression for sorting");
38+
}
39+
40+
@Override
41+
public String visit(IdentifierExpression expression) {
42+
// NOTE: SQL supports alias as part of ORDER BY clause.
43+
// So, if we have already found any user-defined alias, we will use it.
44+
// Otherwise, we are using a field accessor pattern.
45+
String fieldName = identifierExpressionVisitor.visit(expression);
46+
47+
return postgresQueryParser.getPgSelections().containsKey(fieldName)
48+
? fieldName
49+
: fieldIdentifierExpressionVisitor.visit(expression);
50+
}
51+
52+
public static Optional<String> getOrderByClause(PostgresQueryParser postgresQueryParser) {
53+
PostgresSortTypeExpressionVisitor sortTypeExpressionVisitor =
54+
new PostgresSortTypeExpressionVisitor(postgresQueryParser);
55+
56+
List<SortingSpec> sortingSpecs = postgresQueryParser.getQuery().getSorts();
57+
58+
String childList =
59+
sortingSpecs.stream()
60+
.map(
61+
sortingSpec -> {
62+
String sortingExp = sortingSpec.getExpression().accept(sortTypeExpressionVisitor);
63+
return String.format("%s %s", sortingExp, sortingSpec.getOrder().toString());
64+
})
65+
.collect(Collectors.joining(","));
66+
67+
return StringUtils.isNotEmpty(childList) ? Optional.of(childList) : Optional.empty();
68+
}
69+
}

‎document-store/src/test/java/org/hypertrace/core/documentstore/postgres/query/v1/PostgresQueryParserTest.java‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.GTE;
1616
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.LTE;
1717
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.NEQ;
18+
import static org.hypertrace.core.documentstore.expression.operators.SortOrder.ASC;
19+
import static org.hypertrace.core.documentstore.expression.operators.SortOrder.DESC;
1820

1921
import org.hypertrace.core.documentstore.expression.impl.AggregateExpression;
2022
import org.hypertrace.core.documentstore.expression.impl.ConstantExpression;
@@ -431,4 +433,60 @@ void testAggregationFilterAlongWithNonAliasFields() {
431433
Assertions.assertEquals(10, params.getObjectParams().get(1));
432434
Assertions.assertEquals(5, params.getObjectParams().get(2));
433435
}
436+
437+
@Test
438+
void testSimpleOrderByClause() {
439+
Query query =
440+
Query.builder()
441+
.addSelection(IdentifierExpression.of("item"))
442+
.addSelection(IdentifierExpression.of("price"))
443+
.addSort(IdentifierExpression.of("price"), ASC)
444+
.addSort(IdentifierExpression.of("item"), DESC)
445+
.build();
446+
447+
PostgresQueryParser postgresQueryParser = new PostgresQueryParser(TEST_COLLECTION, query);
448+
String sql = postgresQueryParser.parse();
449+
450+
Assertions.assertEquals(
451+
"SELECT document->'item' AS item, document->'price' AS price "
452+
+ "FROM testCollection "
453+
+ "ORDER BY document->'price' ASC,document->'item' DESC",
454+
sql);
455+
456+
Params params = postgresQueryParser.getParamsBuilder().build();
457+
Assertions.assertEquals(0, params.getObjectParams().size());
458+
}
459+
460+
@Test
461+
void testOrderByClauseWithAlias() {
462+
Query query =
463+
Query.builder()
464+
.addSelection(
465+
AggregateExpression.of(DISTINCT_COUNT, IdentifierExpression.of("quantity")),
466+
"qty_count")
467+
.addSelection(IdentifierExpression.of("item"))
468+
.addAggregation(IdentifierExpression.of("item"))
469+
.setAggregationFilter(
470+
RelationalExpression.of(
471+
IdentifierExpression.of("qty_count"), LTE, ConstantExpression.of(1000)))
472+
.addSort(IdentifierExpression.of("qty_count"), DESC)
473+
.addSort(IdentifierExpression.of("item"), DESC)
474+
.build();
475+
476+
PostgresQueryParser postgresQueryParser = new PostgresQueryParser(TEST_COLLECTION, query);
477+
String sql = postgresQueryParser.parse();
478+
479+
Assertions.assertEquals(
480+
"SELECT COUNT(DISTINCT CAST (document->>'quantity' AS NUMERIC) ) AS qty_count, "
481+
+ "document->'item' AS item "
482+
+ "FROM testCollection "
483+
+ "GROUP BY document->'item' "
484+
+ "HAVING COUNT(DISTINCT CAST (document->>'quantity' AS NUMERIC) ) <= ? "
485+
+ "ORDER BY qty_count DESC,document->'item' DESC",
486+
sql);
487+
488+
Params params = postgresQueryParser.getParamsBuilder().build();
489+
Assertions.assertEquals(1, params.getObjectParams().size());
490+
Assertions.assertEquals(1000, params.getObjectParams().get(1));
491+
}
434492
}

0 commit comments

Comments
 (0)