Skip to content

Commit

Permalink
ORDER BY Undefined (#952)
Browse files Browse the repository at this point in the history
* reproduced issue

* got tests to pass

* removed mixed type order by check

* updated changelog
  • Loading branch information
bchong95 authored and ealsur committed Nov 19, 2019
1 parent 129e321 commit 29750ee
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 267 deletions.
2 changes: 1 addition & 1 deletion Microsoft.Azure.Cosmos/src/Query/Core/OrderByItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public CosmosElement Item
{
if (!this.cosmosObject.TryGetValue(ItemName, out CosmosElement cosmosElement))
{
throw new InvalidOperationException($"Underlying object does not have an 'item' field.");
cosmosElement = null;
}

return cosmosElement;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ namespace Microsoft.Azure.Cosmos.Query.ParallelQuery
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using Microsoft.Azure.Cosmos.CosmosElements;
using RMResources = Documents.RMResources;

/// <summary>
/// For cross partition order by queries we serve documents from the partition
Expand All @@ -17,12 +14,6 @@ namespace Microsoft.Azure.Cosmos.Query.ParallelQuery
/// </summary>
internal sealed class OrderByConsumeComparer : IComparer<ItemProducerTree>
{
/// <summary>
/// This flag used to determine whether we should support mixed type order by.
/// For testing purposes we might turn it on to test mixed type order by on index v2.
/// </summary>
public static bool AllowMixedTypeOrderByTestFlag = false;

/// <summary>
/// The sort orders for the query (1 for each order by in the query).
/// Until composite indexing is released this will just be an array of length 1.
Expand Down Expand Up @@ -132,11 +123,6 @@ public int CompareOrderByItems(IList<OrderByItem> items1, IList<OrderByItem> ite
this.sortOrders.Count == items1.Count,
"SortOrders must match size of order-by items.");

if (!AllowMixedTypeOrderByTestFlag)
{
this.CheckTypeMatching(items1, items2);
}

for (int i = 0; i < this.sortOrders.Count; ++i)
{
int cmp = ItemComparer.Instance.Compare(
Expand All @@ -151,39 +137,5 @@ public int CompareOrderByItems(IList<OrderByItem> items1, IList<OrderByItem> ite

return 0;
}

/// <summary>
/// With index V1 collections we have the check the types of the items since it is impossible to support mixed typed order by for V1 collections.
/// The reason for this is, since V1 does not order types.
/// The only constraint is that all the numbers will be sorted with respect to each other and same for the strings, but strings and numbers might get interleaved.
/// Take the following example:
/// Partition 1: "A", 1, "B", 2
/// Partition 2: 42, "Z", 0x5F3759DF
/// Step 1: Compare "A" to 42 and WLOG 42 comes first
/// Step 2: Compare "A" to "Z" and "A" comes first
/// Step 3: Compare "Z" to 1 and WLOG 1 comes first
/// Whoops: We have 42, "A", 1 and 1 should come before 42.
/// </summary>
/// <param name="items1">The items relevant to the sort for the first partition.</param>
/// <param name="items2">The items relevant to the sort for the second partition.</param>
private void CheckTypeMatching(IList<OrderByItem> items1, IList<OrderByItem> items2)
{
for (int i = 0; i < items1.Count; ++i)
{
CosmosElementType itemType1 = items1[i].Item.Type;
CosmosElementType itemType2 = items2[i].Item.Type;

if (itemType1 != itemType2)
{
throw new NotSupportedException(
string.Format(
CultureInfo.InvariantCulture,
RMResources.UnsupportedCrossPartitionOrderByQueryOnMixedTypes,
itemType1,
itemType2,
items1[i]));
}
}
}
}
}
Loading

0 comments on commit 29750ee

Please sign in to comment.