Skip to content

Commit f0b4e81

Browse files
mbhaskarmbhaskar
authored
Crosspartition Orderby mixed types (Azure#19458)
* Enabling orderby on mixed and undefined types * Test cleanup * Adding null values for test Fixing multiorderby test as it supports mixed values with nulls now. * cleanup * Improving the test to check for proper grouping and sorting of items inside groups
1 parent 7e7710b commit f0b4e81

File tree

4 files changed

+185
-23
lines changed

4 files changed

+185
-23
lines changed

‎sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/query/orderbyquery/OrderbyRowComparer.java‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,6 @@ public int compare(OrderByRowResult<T> r1, OrderByRowResult<T> r2) {
5656
}
5757
}
5858

59-
this.checkOrderByItemType(result1);
60-
this.checkOrderByItemType(result2);
61-
6259
for (int i = 0; i < result1.size(); ++i) {
6360
int cmp = ItemComparator.getInstance().compare(result1.get(i).getItem(), result2.get(i).getItem());
6461
if (cmp != 0) {
@@ -85,16 +82,6 @@ public int compare(OrderByRowResult<T> r1, OrderByRowResult<T> r2) {
8582
}
8683
}
8784

88-
private void checkOrderByItemType(List<QueryItem> orderByItems) {
89-
for (int i = 0; i < this.itemTypes.size(); ++i) {
90-
ItemType type = ItemTypeHelper.getOrderByItemType(orderByItems.get(i).getItem());
91-
if (type != this.itemTypes.get(i)) {
92-
throw new UnsupportedOperationException(
93-
String.format("Expected %s, but got %s.", this.itemTypes.get(i).toString(), type.toString()));
94-
}
95-
}
96-
}
97-
9885
public List<SortOrder> getSortOrders() {
9986
return this.sortOrders;
10087
}

‎sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/FeedResponseListValidator.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import com.azure.cosmos.models.ModelBridgeInternal;
1717
import com.fasterxml.jackson.databind.JsonNode;
1818
import com.fasterxml.jackson.databind.node.ArrayNode;
19+
import com.fasterxml.jackson.databind.node.ObjectNode;
1920

2021
import java.time.Duration;
2122
import java.util.ArrayList;
@@ -352,6 +353,9 @@ private <T> Resource getResource(T response) {
352353
|| response instanceof CosmosUserProperties) {
353354
return ModelBridgeInternal.getResource(response);
354355
}
356+
if (response instanceof ObjectNode) {
357+
return new Document((ObjectNode)response);
358+
}
355359
return null;
356360
}
357361
}

‎sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/rx/MultiOrderByQueryTests.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ public void queryDocumentsWithMultiOrder() throws InterruptedException {
261261
}
262262

263263
// CREATE document with numberField not set.
264-
// This query would then be invalid.
264+
// This query should be valid too as we now support mixed null values
265265
InternalObjectNode documentWithEmptyField = generateMultiOrderByDocument();
266266
BridgeInternal.remove(documentWithEmptyField, NUMBER_FIELD);
267267
documentCollection.createItem(documentWithEmptyField, new CosmosItemRequestOptions()).block();
@@ -272,7 +272,10 @@ public void queryDocumentsWithMultiOrder() throws InterruptedException {
272272
.instanceOf(UnsupportedOperationException.class)
273273
.build();
274274

275-
validateQueryFailure(queryObservable.byPage(), validator);
275+
FeedResponseListValidator<InternalObjectNode> feedResponseValidator =
276+
new FeedResponseListValidator.Builder<InternalObjectNode>().totalSize(this.documents.size() + 1).build();
277+
278+
validateQuerySuccess(queryObservable.byPage(), feedResponseValidator);
276279
}
277280

278281
private List<InternalObjectNode> top(List<InternalObjectNode> arrayList, boolean hasTop, int topCount) {

‎sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/rx/OrderbyDocumentQueryTest.java‎

Lines changed: 176 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,12 @@
99
import com.azure.cosmos.CosmosBridgeInternal;
1010
import com.azure.cosmos.CosmosClientBuilder;
1111
import com.azure.cosmos.CosmosException;
12-
import com.azure.cosmos.models.ModelBridgeInternal;
13-
import com.azure.cosmos.util.CosmosPagedFlux;
14-
import com.azure.cosmos.implementation.InternalObjectNode;
15-
import com.azure.cosmos.models.CosmosItemRequestOptions;
16-
import com.azure.cosmos.models.CosmosQueryRequestOptions;
17-
import com.azure.cosmos.models.FeedResponse;
18-
import com.azure.cosmos.models.PartitionKey;
19-
import com.azure.cosmos.implementation.Resource;
12+
import com.azure.cosmos.implementation.AsyncDocumentClient;
2013
import com.azure.cosmos.implementation.FeedResponseListValidator;
2114
import com.azure.cosmos.implementation.FeedResponseValidator;
15+
import com.azure.cosmos.implementation.InternalObjectNode;
16+
import com.azure.cosmos.implementation.PartitionKeyRange;
17+
import com.azure.cosmos.implementation.Resource;
2218
import com.azure.cosmos.implementation.ResourceValidator;
2319
import com.azure.cosmos.implementation.RetryAnalyzer;
2420
import com.azure.cosmos.implementation.Utils;
@@ -27,6 +23,15 @@
2723
import com.azure.cosmos.implementation.query.OrderByContinuationToken;
2824
import com.azure.cosmos.implementation.query.QueryItem;
2925
import com.azure.cosmos.implementation.routing.Range;
26+
import com.azure.cosmos.models.CosmosContainerProperties;
27+
import com.azure.cosmos.models.CosmosItemRequestOptions;
28+
import com.azure.cosmos.models.CosmosQueryRequestOptions;
29+
import com.azure.cosmos.models.FeedResponse;
30+
import com.azure.cosmos.models.IncludedPath;
31+
import com.azure.cosmos.models.IndexingPolicy;
32+
import com.azure.cosmos.models.ModelBridgeInternal;
33+
import com.azure.cosmos.models.PartitionKey;
34+
import com.azure.cosmos.util.CosmosPagedFlux;
3035
import com.fasterxml.jackson.core.JsonProcessingException;
3136
import io.reactivex.subscribers.TestSubscriber;
3237
import org.apache.commons.lang3.StringUtils;
@@ -39,6 +44,7 @@
3944
import reactor.core.publisher.Flux;
4045

4146
import java.util.ArrayList;
47+
import java.util.Arrays;
4248
import java.util.Collections;
4349
import java.util.Comparator;
4450
import java.util.HashMap;
@@ -52,6 +58,8 @@
5258
import static org.assertj.core.api.Assertions.assertThat;
5359

5460
public class OrderbyDocumentQueryTest extends TestSuiteBase {
61+
public static final String PROPTYPE = "proptype";
62+
public static final String PROP_MIXED = "propMixed";
5563
private final double minQueryRequestChargePerPartition = 2.0;
5664

5765
private CosmosAsyncClient client;
@@ -230,6 +238,117 @@ public void queryOrderByString() throws Exception {
230238
validateQuerySuccess(queryObservable.byPage(pageSize), validator);
231239
}
232240

241+
@Test(groups = {"simple"}, timeOut = TIMEOUT, dataProvider = "sortOrder")
242+
public void queryOrderByMixedTypes(String sortOrder) throws Exception {
243+
List<PartitionKeyRange> partitionKeyRanges = getPartitionKeyRanges(createdCollection.getId(),
244+
BridgeInternal
245+
.getContextClient(this.client));
246+
// Ensure its a cross partition query
247+
assertThat(partitionKeyRanges.size()).isGreaterThan(1);
248+
// We are inserting documents with int, float, string, array, object and missing propMixed.
249+
String query = String.format("SELECT * FROM r ORDER BY r.propMixed ", sortOrder);
250+
CosmosQueryRequestOptions options = new CosmosQueryRequestOptions();
251+
List<String> sourceIds = createdDocuments.stream()
252+
.map(Resource::getId)
253+
.collect(Collectors.toList());
254+
255+
int pageSize = 20;
256+
CosmosPagedFlux<InternalObjectNode> queryFlux = createdCollection
257+
.queryItems(query, options, InternalObjectNode.class);
258+
TestSubscriber<FeedResponse<InternalObjectNode>> subscriber = new TestSubscriber<>();
259+
queryFlux.byPage(pageSize).subscribe(subscriber);
260+
subscriber.awaitTerminalEvent();
261+
subscriber.assertComplete();
262+
subscriber.assertNoErrors();
263+
List<InternalObjectNode> results = new ArrayList<>();
264+
subscriber.values().forEach(feedResponse -> results.addAll(feedResponse.getResults()));
265+
// Make sure all elements inserted are returned
266+
assertThat(results.size()).isEqualTo(createdDocuments.size());
267+
268+
// Make sure all ids are present
269+
List<String> resultIds = results.stream().map(Resource::getId).collect(Collectors.toList());
270+
assertThat(resultIds).containsExactlyInAnyOrderElementsOf(sourceIds);
271+
272+
// Make sure the below defined group order for mixed types match with the result grouping
273+
final List<String> typeList = Arrays.asList("undefined", "null", "boolean", "number", "string", "array",
274+
"object");
275+
List<String> observedTypes = new ArrayList<>();
276+
277+
results.forEach(item -> {
278+
String propType = "undefined";
279+
if (item.has(PROPTYPE)) {
280+
propType = item.getString(PROPTYPE);
281+
}
282+
System.out.println("item.get(PROPTYPE) = " + item.get(PROPTYPE));
283+
System.out.println("propType = " + propType);
284+
if (!observedTypes.contains(propType)) {
285+
observedTypes.add(propType);
286+
} else {
287+
boolean equals = observedTypes.get(observedTypes.size() - 1).equals(propType);
288+
assertThat(equals).isTrue().as("Items of same type should be contiguous");
289+
}
290+
});
291+
292+
// Esuring that the returned results are grouped in the expected order
293+
assertThat(observedTypes).containsExactlyElementsOf(typeList);
294+
295+
// Now check ordering inside each type group
296+
for (String type : typeList) {
297+
List<InternalObjectNode> items = results.stream().filter(r -> {
298+
if ("undefined".equals(type)) {
299+
return !r.has(PROPTYPE);
300+
}
301+
return type.equals(r.getString(PROPTYPE));
302+
}).collect(Collectors.toList());
303+
304+
// Skip comparing undefined types
305+
// Skip comparing null types
306+
// Skip comparing Array and object types
307+
// Compare booleans, number and String types among their own groups to check if they are sorted
308+
if ("boolean".equals(type)) {
309+
List<Boolean> sourceList =
310+
items.stream().map(n -> n.getBoolean(PROP_MIXED)).collect(Collectors.toList());
311+
List<Boolean> toBeSortedList = new ArrayList<>(sourceList);
312+
toBeSortedList.sort(Comparator.comparing(Boolean::booleanValue));
313+
// making sure the list before and after sorting should be the same as we get already should have got
314+
// properly sorted results from the query
315+
assertThat(toBeSortedList).containsExactlyElementsOf(sourceList);
316+
}
317+
if ("number".equals(type)) {
318+
List<Number> numberList =
319+
items.stream().map(n -> (Number) n.get(PROP_MIXED)).collect(Collectors.toList());
320+
List<Number> toBeSortedList = new ArrayList<>(numberList);
321+
Collections.copy(toBeSortedList, numberList);
322+
toBeSortedList.sort(Comparator.comparingDouble(Number::doubleValue));
323+
// making sure the list before and after sorting should be the same as we get already should have got
324+
// properly sorted results from the query
325+
assertThat(toBeSortedList).containsExactlyElementsOf(numberList);
326+
}
327+
if ("string".equals(type)) {
328+
List<String> sourceList =
329+
items.stream().map(n -> n.getString(PROP_MIXED)).collect(Collectors.toList());
330+
List<String> toBeSortedList = new ArrayList<>(sourceList);
331+
Collections.copy(toBeSortedList, sourceList);
332+
toBeSortedList.sort(Comparator.comparing(String::valueOf));
333+
// making sure the list before and after sorting should be the same as we get already should have got
334+
// properly sorted results from the query
335+
assertThat(toBeSortedList).containsExactlyElementsOf(sourceList);
336+
}
337+
}
338+
}
339+
340+
private List<PartitionKeyRange> getPartitionKeyRanges(
341+
String containerId, AsyncDocumentClient asyncDocumentClient) {
342+
List<PartitionKeyRange> partitionKeyRanges = new ArrayList<>();
343+
List<FeedResponse<PartitionKeyRange>> partitionFeedResponseList = asyncDocumentClient
344+
.readPartitionKeyRanges("/dbs/" + createdDatabase.getId()
345+
+ "/colls/" + containerId,
346+
new CosmosQueryRequestOptions())
347+
.collectList().block();
348+
partitionFeedResponseList.forEach(f -> partitionKeyRanges.addAll(f.getResults()));
349+
return partitionKeyRanges;
350+
}
351+
233352
@DataProvider(name = "topValue")
234353
public Object[][] topValueParameter() {
235354
return new Object[][] { { 0 }, { 1 }, { 5 }, { createdDocuments.size() - 1 }, { createdDocuments.size() },
@@ -472,6 +591,7 @@ public void before_OrderbyDocumentQueryTest() throws Exception {
472591

473592
List<Map<String, Object>> keyValuePropsList = new ArrayList<>();
474593
Map<String, Object> props;
594+
boolean flag = false;
475595

476596
for(int i = 0; i < 30; i++) {
477597
props = new HashMap<>();
@@ -487,6 +607,40 @@ public void before_OrderbyDocumentQueryTest() throws Exception {
487607
}
488608
props.put("propArray", orderByArray);
489609
props.put("propObject", orderByObject);
610+
switch (i % 8) {
611+
case 0:
612+
props.put(PROP_MIXED, i);
613+
props.put(PROPTYPE, "number");
614+
break;
615+
case 1:
616+
props.put(PROP_MIXED, String.valueOf(i));
617+
props.put(PROPTYPE, "string");
618+
break;
619+
case 2:
620+
props.put(PROP_MIXED, orderByArray);
621+
props.put(PROPTYPE, "array");
622+
break;
623+
case 3:
624+
props.put(PROP_MIXED, orderByObject);
625+
props.put(PROPTYPE, "object");
626+
break;
627+
case 4:
628+
props.put(PROP_MIXED, (float)i*3.17);
629+
props.put(PROPTYPE, "number");
630+
break;
631+
case 5:
632+
props.put(PROP_MIXED, null);
633+
props.put(PROPTYPE, "null");
634+
break;
635+
case 6:
636+
flag = !flag;
637+
props.put(PROP_MIXED, flag);
638+
props.put(PROPTYPE, "boolean");
639+
break;
640+
default:
641+
// skips the propMixed
642+
break;
643+
}
490644
keyValuePropsList.add(props);
491645
}
492646

@@ -510,6 +664,20 @@ public void before_OrderbyDocumentQueryTest() throws Exception {
510664
.flatMap(p -> Flux.fromIterable(p.getResults())).collectList().single().block().size();
511665

512666
waitIfNeededForReplicasToCatchUp(getClientBuilder());
667+
updateCollectionIndex();
668+
}
669+
670+
private void updateCollectionIndex() {
671+
CosmosContainerProperties containerProperties = createdCollection.read().block().getProperties();
672+
IndexingPolicy indexingPolicy = containerProperties.getIndexingPolicy();
673+
List<IncludedPath> includedPaths = indexingPolicy.getIncludedPaths();
674+
IncludedPath includedPath = new IncludedPath("/propMixed/?");
675+
if (!includedPaths.contains(includedPath)) {
676+
includedPaths.add(includedPath);
677+
indexingPolicy.setIncludedPaths(includedPaths);
678+
containerProperties.setIndexingPolicy(indexingPolicy);
679+
createdCollection.replace(containerProperties).block();
680+
}
513681
}
514682

515683
@AfterClass(groups = { "simple" }, timeOut = SHUTDOWN_TIMEOUT, alwaysRun = true)

0 commit comments

Comments
 (0)