Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query : Fixes OFFSET, LIMIT, TOP data types to match those coming from query plan #4939

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions Microsoft.Azure.Cosmos.sln
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 16
VisualStudioVersion = 16.0.29123.88
# Visual Studio Version 17
adityasa marked this conversation as resolved.
Show resolved Hide resolved
VisualStudioVersion = 17.11.35431.28
MinimumVisualStudioVersion = 10.0.40219.1
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Azure.Cosmos", "Microsoft.Azure.Cosmos\src\Microsoft.Azure.Cosmos.csproj", "{36F6F6A8-CEC8-4261-9948-903495BC3C25}"
EndProject
Expand Down Expand Up @@ -180,6 +180,18 @@ Global
{021DDC27-02EF-42C4-9A9E-AA600833C2EE}.Release|Any CPU.Build.0 = Release|Any CPU
{021DDC27-02EF-42C4-9A9E-AA600833C2EE}.Release|x64.ActiveCfg = Release|Any CPU
{021DDC27-02EF-42C4-9A9E-AA600833C2EE}.Release|x64.Build.0 = Release|Any CPU
{D744906A-1091-403F-B0B6-794DE045169A}.Cover|Any CPU.ActiveCfg = Debug|Any CPU
{D744906A-1091-403F-B0B6-794DE045169A}.Cover|Any CPU.Build.0 = Debug|Any CPU
{D744906A-1091-403F-B0B6-794DE045169A}.Cover|x64.ActiveCfg = Debug|Any CPU
{D744906A-1091-403F-B0B6-794DE045169A}.Cover|x64.Build.0 = Debug|Any CPU
{D744906A-1091-403F-B0B6-794DE045169A}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{D744906A-1091-403F-B0B6-794DE045169A}.Debug|Any CPU.Build.0 = Debug|Any CPU
{D744906A-1091-403F-B0B6-794DE045169A}.Debug|x64.ActiveCfg = Debug|Any CPU
{D744906A-1091-403F-B0B6-794DE045169A}.Debug|x64.Build.0 = Debug|Any CPU
{D744906A-1091-403F-B0B6-794DE045169A}.Release|Any CPU.ActiveCfg = Release|Any CPU
{D744906A-1091-403F-B0B6-794DE045169A}.Release|Any CPU.Build.0 = Release|Any CPU
{D744906A-1091-403F-B0B6-794DE045169A}.Release|x64.ActiveCfg = Release|Any CPU
{D744906A-1091-403F-B0B6-794DE045169A}.Release|x64.Build.0 = Release|Any CPU
{CE4D6DA8-148D-4A98-943B-D8C2D532E1DC}.Cover|Any CPU.ActiveCfg = Debug|Any CPU
{CE4D6DA8-148D-4A98-943B-D8C2D532E1DC}.Cover|Any CPU.Build.0 = Debug|Any CPU
{CE4D6DA8-148D-4A98-943B-D8C2D532E1DC}.Cover|x64.ActiveCfg = Debug|Any CPU
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public static TryCatch<IQueryPipelineStage> MonadicCreate(
long optimalPageSize = maxItemCount;
if (queryInfo.HasOrderBy)
{
int top;
uint top;
if (queryInfo.HasTop && (queryInfo.Top.Value > 0))
{
top = queryInfo.Top.Value;
Expand All @@ -162,6 +162,11 @@ public static TryCatch<IQueryPipelineStage> MonadicCreate(
else
{
top = 0;
}

if (top > int.MaxValue)
{
throw new ArgumentOutOfRangeException(nameof(queryInfo.Top.Value));
}

if (top > 0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,15 @@ private ClientSkipQueryPipelineStage(
}

public static new TryCatch<IQueryPipelineStage> MonadicCreate(
int offsetCount,
uint offsetCount,
CosmosElement continuationToken,
MonadicCreatePipelineStage monadicCreatePipelineStage)
{
{
if (offsetCount > int.MaxValue)
{
throw new ArgumentOutOfRangeException(nameof(offsetCount));
}

if (monadicCreatePipelineStage == null)
{
throw new ArgumentNullException(nameof(monadicCreatePipelineStage));
Expand Down Expand Up @@ -126,7 +131,7 @@ public override async ValueTask<bool> MoveNextAsync(ITrace trace, CancellationTo
if ((sourcePage.State != null) && (sourcePage.DisallowContinuationTokenMessage == null))
{
string token = new OffsetContinuationToken(
offset: this.skipCount,
offset: (uint)this.skipCount,
sourceToken: sourcePage.State?.Value.ToString()).ToString();
state = new QueryState(CosmosElement.Parse(token));
}
Expand Down Expand Up @@ -160,7 +165,7 @@ private readonly struct OffsetContinuationToken
/// </summary>
/// <param name="offset">The number of items to skip in the query.</param>
/// <param name="sourceToken">The continuation token for the source component of the query.</param>
public OffsetContinuationToken(int offset, string sourceToken)
public OffsetContinuationToken(uint offset, string sourceToken)
{
if (offset < 0)
{
Expand All @@ -175,7 +180,7 @@ public OffsetContinuationToken(int offset, string sourceToken)
/// The number of items to skip in the query.
/// </summary>
[JsonProperty("offset")]
public int Offset { get; }
public uint Offset { get; }

/// <summary>
/// Gets the continuation token for the source component of the query.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ protected SkipQueryPipelineStage(
long skipCount)
: base(source)
{
if (skipCount > int.MaxValue)
if (skipCount > int.MaxValue || skipCount < 0)
{
throw new ArgumentOutOfRangeException(nameof(skipCount));
}
Expand All @@ -31,7 +31,7 @@ protected SkipQueryPipelineStage(
}

public static TryCatch<IQueryPipelineStage> MonadicCreate(
int offsetCount,
uint offsetCount,
CosmosElement continuationToken,
MonadicCreatePipelineStage monadicCreatePipelineStage)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,21 @@ private sealed class ClientTakeQueryPipelineStage : TakeQueryPipelineStage

private ClientTakeQueryPipelineStage(
IQueryPipelineStage source,
int takeCount,
uint takeCount,
TakeEnum takeEnum)
: base(source, takeCount)
{
this.takeEnum = takeEnum;
}

public static new TryCatch<IQueryPipelineStage> MonadicCreateLimitStage(
int limitCount,
uint limitCount,
CosmosElement requestContinuationToken,
MonadicCreatePipelineStage monadicCreatePipelineStage)
{
if (limitCount < 0)
if (limitCount > int.MaxValue)
{
throw new ArgumentException($"{nameof(limitCount)}: {limitCount} must be a non negative number.");
throw new ArgumentOutOfRangeException(nameof(limitCount));
}

if (monadicCreatePipelineStage == null)
Expand Down Expand Up @@ -102,7 +102,7 @@ private ClientTakeQueryPipelineStage(
}

public static new TryCatch<IQueryPipelineStage> MonadicCreateTopStage(
int topCount,
uint topCount,
CosmosElement requestContinuationToken,
MonadicCreatePipelineStage monadicCreatePipelineStage)
{
Expand Down Expand Up @@ -165,7 +165,7 @@ private ClientTakeQueryPipelineStage(

IQueryPipelineStage stage = new ClientTakeQueryPipelineStage(
tryCreateSource.Result,
topContinuationToken.Top,
(uint)topContinuationToken.Top,
TakeEnum.Top);

return TryCatch<IQueryPipelineStage>.FromResult(stage);
Expand Down Expand Up @@ -205,10 +205,10 @@ public override async ValueTask<bool> MoveNextAsync(ITrace trace, CancellationTo
string updatedContinuationToken = this.takeEnum switch
{
TakeEnum.Limit => new LimitContinuationToken(
limit: this.takeCount,
limit: (uint)this.takeCount,
sourceToken: sourcePage.State?.Value.ToString()).ToString(),
TakeEnum.Top => new TopContinuationToken(
top: this.takeCount,
top: (uint)this.takeCount,
sourceToken: sourcePage.State?.Value.ToString()).ToString(),
_ => throw new ArgumentOutOfRangeException($"Unknown {nameof(TakeEnum)}: {this.takeEnum}."),
};
Expand Down Expand Up @@ -255,11 +255,11 @@ private sealed class LimitContinuationToken : TakeContinuationToken
/// </summary>
/// <param name="limit">The limit to the number of document drained for the remainder of the query.</param>
/// <param name="sourceToken">The continuation token for the source component of the query.</param>
public LimitContinuationToken(int limit, string sourceToken)
public LimitContinuationToken(uint limit, string sourceToken)
{
if (limit < 0)
if (limit > int.MaxValue)
{
throw new ArgumentException($"{nameof(limit)} must be a non negative number.");
throw new ArgumentNullException(nameof(limit));
adityasa marked this conversation as resolved.
Show resolved Hide resolved
}

this.Limit = limit;
Expand All @@ -270,7 +270,7 @@ public LimitContinuationToken(int limit, string sourceToken)
/// Gets the limit to the number of document drained for the remainder of the query.
/// </summary>
[JsonProperty("limit")]
public int Limit
public uint Limit
{
get;
}
Expand Down Expand Up @@ -329,9 +329,14 @@ private sealed class TopContinuationToken : TakeContinuationToken
/// </summary>
/// <param name="top">The limit to the number of document drained for the remainder of the query.</param>
/// <param name="sourceToken">The continuation token for the source component of the query.</param>
public TopContinuationToken(int top, string sourceToken)
{
this.Top = top;
public TopContinuationToken(uint top, string sourceToken)
{
if (top > int.MaxValue)
{
throw new ArgumentOutOfRangeException(nameof(top));
}

this.Top = (int)top;
this.SourceToken = sourceToken;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,19 @@ internal abstract partial class TakeQueryPipelineStage : QueryPipelineStageBase

protected TakeQueryPipelineStage(
IQueryPipelineStage source,
int takeCount)
uint takeCount)
: base(source)
{
this.takeCount = takeCount;
{
if (takeCount > int.MaxValue)
{
throw new ArgumentOutOfRangeException(nameof(takeCount));
}

this.takeCount = (int)takeCount;
adityasa marked this conversation as resolved.
Show resolved Hide resolved
}

public static TryCatch<IQueryPipelineStage> MonadicCreateLimitStage(
int limitCount,
uint limitCount,
CosmosElement requestContinuationToken,
MonadicCreatePipelineStage monadicCreatePipelineStage)
{
Expand All @@ -35,7 +40,7 @@ public static TryCatch<IQueryPipelineStage> MonadicCreateLimitStage(
}

public static TryCatch<IQueryPipelineStage> MonadicCreateTopStage(
int limitCount,
uint limitCount,
CosmosElement requestContinuationToken,
MonadicCreatePipelineStage monadicCreatePipelineStage)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ public QueryInfo ProjectionQueryInfo
}

[JsonProperty("skip")]
public int? Skip
public uint? Skip
{
get;
set;
}

[JsonProperty("take")]
public int? Take
public uint? Take
{
get;
set;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,21 @@ public DistinctQueryType DistinctType
}

[JsonProperty("top")]
public int? Top
public uint? Top
{
get;
set;
}

[JsonProperty("offset")]
public int? Offset
public uint? Offset
{
get;
set;
}

[JsonProperty("limit")]
public int? Limit
public uint? Limit
{
get;
set;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,8 @@ private static async Task RunHybridSearchTest(HybridSearchTest testCase)
ranges: ranges,
requiresGlobalStatistics: testCase.RequiresGlobalStatistics,
pageSize: testCase.PageSize,
skip: testCase.Skip,
take: testCase.Take);
skip: (uint?)testCase.Skip,
take: (uint?)testCase.Take);

Assert.AreEqual(expectedIndices.Count(), results.Count);

Expand Down Expand Up @@ -447,8 +447,8 @@ private static async Task RunParityTests(
IReadOnlyList<FeedRangeEpk> ranges,
bool requiresGlobalStatistics,
int pageSize,
int? skip,
int? take)
uint? skip,
uint? take)
{
TryCatch<IQueryPipelineStage> tryCreatePipeline = PipelineFactory.MonadicCreate(
documentContainer,
Expand Down Expand Up @@ -1583,7 +1583,7 @@ private static void FischerYatesShuffle<T>(IList<T> list)
}
}

private static HybridSearchQueryInfo Create2ItemHybridSearchQueryInfo(bool requiresGlobalStatistics, int? skip, int? take)
private static HybridSearchQueryInfo Create2ItemHybridSearchQueryInfo(bool requiresGlobalStatistics, uint? skip, uint? take)
{
return new HybridSearchQueryInfo
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public async Task SanityTests()
{
List<CosmosElement> elements = await SkipQueryPipelineStageTests.CreateAndDrainAsync(
pages: pages,
offsetCount: offsetCount,
offsetCount: (uint)offsetCount,
continuationToken: null);

Assert.AreEqual(Math.Max(values.Length - offsetCount, 0), elements.Count);
Expand All @@ -41,7 +41,7 @@ public async Task SanityTests()

private static async Task<List<CosmosElement>> CreateAndDrainAsync(
IReadOnlyList<IReadOnlyList<CosmosElement>> pages,
int offsetCount,
uint offsetCount,
CosmosElement continuationToken)
{
IQueryPipelineStage source = new MockQueryPipelineStage(pages);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public async Task SanityTests()
{
(List<CosmosElement> elements, _) = await TakeQueryPipelineStageTests.CreateAndDrainAsync(
pages: pages,
takeCount: takeCount,
takeCount: (uint)takeCount,
continuationToken: null);

Assert.AreEqual(Math.Min(takeCount, values.Length), elements.Count);
Expand Down Expand Up @@ -70,7 +70,7 @@ public async Task BasicTests()
{
(List<CosmosElement> elements, long pageIndex) = await TakeQueryPipelineStageTests.CreateAndDrainAsync(
pages: pages,
takeCount: takeCount,
takeCount: (uint)takeCount,
continuationToken: null);

Assert.AreEqual(expectedPageIndex, pageIndex);
Expand All @@ -80,7 +80,7 @@ public async Task BasicTests()

private static async Task<(List<CosmosElement>, long)> CreateAndDrainAsync(
IReadOnlyList<IReadOnlyList<CosmosElement>> pages,
int takeCount,
uint takeCount,
CosmosElement continuationToken)
{
MockQueryPipelineStage source = new MockQueryPipelineStage(pages);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1144,6 +1144,11 @@ public void Top()
Hash(
@"TOP with GROUP BY",
@"SELECT TOP 5 VALUE c.name FROM c GROUP BY c.name",
@"/key"),

Hash(
@"TOP value beyond limit",
@"SELECT TOP 4294967296 c.name FROM c",
@"/key")
};

Expand Down
Loading