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.

Sign up for GitHub

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

Jump to bottom

Add support for SORT_RO #2111

Merged
merged 9 commits into from
Apr 20, 2022
Merged

Add support for SORT_RO #2111

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d565e9c
SORT_RO
slorello89 Apr 19, 2022
a45b636
Apply suggestions from code review
slorello89 Apr 19, 2022
4244cf1
using correct feature-flags
slorello89 Apr 19, 2022
465bb85
DRYing feature selection out.
slorello89 Apr 19, 2022
f395ff8
Optimize GetSortMessage
NickCraver Apr 19, 2022
acad0a8
Simplify GetSortMessage
NickCraver Apr 19, 2022
00acce9
Fix tests up
NickCraver Apr 20, 2022
cf1e515
Merge branch 'main' into feature/sortRO
NickCraver Apr 20, 2022
8489a19
Add release notes
NickCraver Apr 20, 2022
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
4 changes: 3 additions & 1 deletion src/StackExchange.Redis/Enums/RedisCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ internal enum RedisCommand
SMISMEMBER,
SMOVE,
SORT,
SORT_RO,
SPOP,
SRANDMEMBER,
SREM,
Expand Down Expand Up @@ -320,6 +321,7 @@ internal static bool IsPrimaryOnly(this RedisCommand command)
case RedisCommand.SETRANGE:
case RedisCommand.SINTERSTORE:
case RedisCommand.SMOVE:
case RedisCommand.SORT:
case RedisCommand.SPOP:
case RedisCommand.SREM:
case RedisCommand.SUNIONSTORE:
Expand Down Expand Up @@ -428,7 +430,7 @@ internal static bool IsPrimaryOnly(this RedisCommand command)
case RedisCommand.SLOWLOG:
case RedisCommand.SMEMBERS:
case RedisCommand.SMISMEMBER:
case RedisCommand.SORT:
case RedisCommand.SORT_RO:
case RedisCommand.SRANDMEMBER:
case RedisCommand.STRLEN:
case RedisCommand.SUBSCRIBE:
Expand Down
2 changes: 2 additions & 0 deletions src/StackExchange.Redis/Interfaces/IDatabase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1540,6 +1540,7 @@ public interface IDatabase : IRedis, IDatabaseAsync
/// the <c>get</c> parameter (note that <c>#</c> specifies the element itself, when used in <c>get</c>).
/// Referring to the <a href="https://redis.io/commands/sort">redis SORT documentation </a> for examples is recommended.
/// When used in hashes, <c>by</c> and <c>get</c> can be used to specify fields using <c>-&gt;</c> notation (again, refer to redis documentation).
/// Uses <a href="https://redis.io/commands/sort_ro">SORT_RO</a> when possible.
/// </summary>
/// <param name="key">The key of the list, set, or sorted set.</param>
/// <param name="skip">How many entries to skip on the return.</param>
Expand All @@ -1551,6 +1552,7 @@ public interface IDatabase : IRedis, IDatabaseAsync
/// <param name="flags">The flags to use for this operation.</param>
/// <returns>The sorted elements, or the external values if <c>get</c> is specified.</returns>
/// <remarks><seealso href="https://redis.io/commands/sort"/></remarks>
/// <remarks><seealso href="https://redis.io/commands/sort_ro"/></remarks>
RedisValue[] Sort(RedisKey key, long skip = 0, long take = -1, Order order = Order.Ascending, SortType sortType = SortType.Numeric, RedisValue by = default, RedisValue[]? get = null, CommandFlags flags = CommandFlags.None);

/// <summary>
Expand Down
1 change: 1 addition & 0 deletions src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1505,6 +1505,7 @@ public interface IDatabaseAsync : IRedisAsync
/// the <c>get</c> parameter (note that <c>#</c> specifies the element itself, when used in <c>get</c>).
/// Referring to the <a href="https://redis.io/commands/sort">redis SORT documentation </a> for examples is recommended.
/// When used in hashes, <c>by</c> and <c>get</c> can be used to specify fields using <c>-&gt;</c> notation (again, refer to redis documentation).
/// Uses <a href="https://redis.io/commands/sort_ro">SORT_RO</a> when possible.
/// </summary>
/// <param name="key">The key of the list, set, or sorted set.</param>
/// <param name="skip">How many entries to skip on the return.</param>
Expand Down
22 changes: 12 additions & 10 deletions src/StackExchange.Redis/RedisDatabase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1762,25 +1762,25 @@ private CursorEnumerable<RedisValue> SetScanAsync(RedisKey key, RedisValue patte

public RedisValue[] Sort(RedisKey key, long skip = 0, long take = -1, Order order = Order.Ascending, SortType sortType = SortType.Numeric, RedisValue by = default, RedisValue[]? get = null, CommandFlags flags = CommandFlags.None)
{
var msg = GetSortedSetAddMessage(default(RedisKey), key, skip, take, order, sortType, by, get, flags);
var msg = GetSortMessage(RedisKey.Null, key, skip, take, order, sortType, by, get, flags);
return ExecuteSync(msg, ResultProcessor.RedisValueArray, defaultValue: Array.Empty<RedisValue>());
}

public long SortAndStore(RedisKey destination, RedisKey key, long skip = 0, long take = -1, Order order = Order.Ascending, SortType sortType = SortType.Numeric, RedisValue by = default, RedisValue[]? get = null, CommandFlags flags = CommandFlags.None)
{
var msg = GetSortedSetAddMessage(destination, key, skip, take, order, sortType, by, get, flags);
var msg = GetSortMessage(destination, key, skip, take, order, sortType, by, get, flags);
return ExecuteSync(msg, ResultProcessor.Int64);
}

public Task<long> SortAndStoreAsync(RedisKey destination, RedisKey key, long skip = 0, long take = -1, Order order = Order.Ascending, SortType sortType = SortType.Numeric, RedisValue by = default, RedisValue[]? get = null, CommandFlags flags = CommandFlags.None)
{
var msg = GetSortedSetAddMessage(destination, key, skip, take, order, sortType, by, get, flags);
var msg = GetSortMessage(destination, key, skip, take, order, sortType, by, get, flags);
return ExecuteAsync(msg, ResultProcessor.Int64);
}

public Task<RedisValue[]> SortAsync(RedisKey key, long skip = 0, long take = -1, Order order = Order.Ascending, SortType sortType = SortType.Numeric, RedisValue by = default, RedisValue[]? get = null, CommandFlags flags = CommandFlags.None)
{
var msg = GetSortedSetAddMessage(default(RedisKey), key, skip, take, order, sortType, by, get, flags);
var msg = GetSortMessage(RedisKey.Null, key, skip, take, order, sortType, by, get, flags);
return ExecuteAsync(msg, ResultProcessor.RedisValueArray, defaultValue: Array.Empty<RedisValue>());
}

Expand Down Expand Up @@ -3500,8 +3500,10 @@ private Message GetSortedSetAddMessage(RedisKey key, RedisValue member, double s
}
}

private Message GetSortedSetAddMessage(RedisKey destination, RedisKey key, long skip, long take, Order order, SortType sortType, RedisValue by, RedisValue[]? get, CommandFlags flags)
private Message GetSortMessage(RedisKey destination, RedisKey key, long skip, long take, Order order, SortType sortType, RedisValue by, RedisValue[]? get, CommandFlags flags)
{
var command = destination.IsNull && multiplexer.GetServer(multiplexer.GetEndPoints()[0]).Version >= RedisFeatures.v7_0_0_rc1 ? RedisCommand.SORT_RO : RedisCommand.SORT;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we'd want to add a new internal member to RedisFeatures, e.g. internal bool SortedSetPop => Version >= v7_0_0_rc1; and use var features = GetFeatures(key, flags, out ServerEndPoint? server); like other places - note this means we need to pass server outward and use it upstream (to ensure we went to the server we said had the feature).

These types of switches are the only things we want to be added to RedisFeatures overall but let's keep internal for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @NickCraver, that's much cleaner, I'll clean up my other PR that does that hocky junk as well.


// most common cases; no "get", no "by", no "destination", no "skip", no "take"
if (destination.IsNull && skip == 0 && take == -1 && by.IsNull && (get == null || get.Length == 0))
{
Expand All @@ -3510,15 +3512,15 @@ private Message GetSortedSetAddMessage(RedisKey destination, RedisKey key, long
case Order.Ascending:
switch (sortType)
{
case SortType.Numeric: return Message.Create(Database, flags, RedisCommand.SORT, key);
case SortType.Alphabetic: return Message.Create(Database, flags, RedisCommand.SORT, key, RedisLiterals.ALPHA);
case SortType.Numeric: return Message.Create(Database, flags, command, key);
case SortType.Alphabetic: return Message.Create(Database, flags, command, key, RedisLiterals.ALPHA);
}
break;
case Order.Descending:
switch (sortType)
{
case SortType.Numeric: return Message.Create(Database, flags, RedisCommand.SORT, key, RedisLiterals.DESC);
case SortType.Alphabetic: return Message.Create(Database, flags, RedisCommand.SORT, key, RedisLiterals.DESC, RedisLiterals.ALPHA);
case SortType.Numeric: return Message.Create(Database, flags, command, key, RedisLiterals.DESC);
case SortType.Alphabetic: return Message.Create(Database, flags, command, key, RedisLiterals.DESC, RedisLiterals.ALPHA);
}
break;
}
Expand Down Expand Up @@ -3565,7 +3567,7 @@ private Message GetSortedSetAddMessage(RedisKey destination, RedisKey key, long
values.Add(item);
}
}
if (destination.IsNull) return Message.Create(Database, flags, RedisCommand.SORT, key, values.ToArray());
if (destination.IsNull) return Message.Create(Database, flags, command, key, values.ToArray());

// Because we are using STORE, we need to push this to a primary
if (Message.GetPrimaryReplicaFlags(flags) == CommandFlags.DemandReplica)
Expand Down
38 changes: 38 additions & 0 deletions tests/StackExchange.Redis.Tests/Sets.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,4 +343,42 @@ public void SetPopMulti_Nil()
var arr = db.SetPop(key, 1);
Assert.Empty(arr);
}

[Fact]
public async Task TestSort()
{
using var conn = Create();

var db = conn.GetDatabase();
var key = Me();
await db.KeyDeleteAsync(key);

var random = new Random();
var items = Enumerable.Repeat(0, 200).Select(x => random.Next()).ToList();
await db.SetAddAsync(key, items.Select(x=>(RedisValue)x).ToArray());
items.Sort();

var result = (await db.SortAsync(key)).Select(x=>(int)x);
Assert.Equal(items.Count,result.Count() );
Assert.Equivalent(items, result);
slorello89 marked this conversation as resolved.
Show resolved Hide resolved
}

[Fact]
public async Task TestSortRo()
{
using var conn = Create(require: RedisFeatures.v7_0_0_rc1);

var db = conn.GetDatabase();
var key = Me();
await db.KeyDeleteAsync(key);

var random = new Random();
var items = Enumerable.Repeat(0, 200).Select(x => random.Next()).ToList();
await db.SetAddAsync(key, items.Select(x=>(RedisValue)x).ToArray());
items.Sort();

var result = (await db.SortAsync(key)).Select(x=>(int)x);
Assert.Equal(items.Count,result.Count() );
Assert.Equivalent(items, result);
slorello89 marked this conversation as resolved.
Show resolved Hide resolved
}
}