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

[Compatibility] Added LCS command #843

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
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
55 changes: 55 additions & 0 deletions libs/resources/RespCommandsDocs.json
Original file line number Diff line number Diff line change
Expand Up @@ -3188,6 +3188,61 @@
}
]
},
{
"Command": "LCS",
"Name": "LCS",
"Summary": "Finds the longest common substring.",
"Group": "String",
"Complexity": "O(N*M) where N and M are the lengths of s1 and s2, respectively",
"Arguments": [
{
"TypeDiscriminator": "RespCommandKeyArgument",
"Name": "KEY1",
"DisplayText": "key1",
"Type": "Key",
"KeySpecIndex": 0
},
{
"TypeDiscriminator": "RespCommandKeyArgument",
"Name": "KEY2",
"DisplayText": "key2",
"Type": "Key",
"KeySpecIndex": 0
},
{
"TypeDiscriminator": "RespCommandBasicArgument",
"Name": "LEN",
"DisplayText": "len",
"Type": "PureToken",
"Token": "LEN",
"ArgumentFlags": "Optional"
},
{
"TypeDiscriminator": "RespCommandBasicArgument",
"Name": "IDX",
"DisplayText": "idx",
"Type": "PureToken",
"Token": "IDX",
"ArgumentFlags": "Optional"
},
{
"TypeDiscriminator": "RespCommandBasicArgument",
"Name": "MIN-MATCH-LEN",
"DisplayText": "min-match-len",
"Type": "Integer",
"Token": "MINMATCHLEN",
"ArgumentFlags": "Optional"
},
{
"TypeDiscriminator": "RespCommandBasicArgument",
"Name": "WITHMATCHLEN",
"DisplayText": "withmatchlen",
"Type": "PureToken",
"Token": "WITHMATCHLEN",
"ArgumentFlags": "Optional"
}
]
},
{
"Command": "LINDEX",
"Name": "LINDEX",
Expand Down
25 changes: 25 additions & 0 deletions libs/resources/RespCommandsInfo.json
Original file line number Diff line number Diff line change
Expand Up @@ -2043,6 +2043,31 @@
}
]
},
{
"Command": "LCS",
"Name": "LCS",
"Arity": -3,
"Flags": "ReadOnly",
"FirstKey": 1,
"LastKey": 2,
"Step": 1,
"AclCategories": "Read, Slow, String",
"KeySpecifications": [
{
"BeginSearch": {
"TypeDiscriminator": "BeginSearchIndex",
"Index": 1
},
"FindKeys": {
"TypeDiscriminator": "FindKeysRange",
"LastKey": 1,
"KeyStep": 1,
"Limit": 0
},
"Flags": "RO, Access"
}
]
},
{
"Command": "LINDEX",
"Name": "LINDEX",
Expand Down
4 changes: 4 additions & 0 deletions libs/server/API/GarnetApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ public unsafe GarnetStatus GET(ArgSlice key, out ArgSlice value)
/// <inheritdoc />
public GarnetStatus GET(byte[] key, out GarnetObjectStoreOutput value)
=> storageSession.GET(key, out value, ref objectContext);

/// <inheritdoc />
public GarnetStatus LCS(ArgSlice key1, ArgSlice key2, ref SpanByteAndMemory output, bool lenOnly = false, bool withIndices = false, bool withMatchLen = false, int minMatchLen = 0)
=> storageSession.LCS(key1, key2, ref output, lenOnly, withIndices, withMatchLen, minMatchLen);
#endregion

#region GETEX
Expand Down
8 changes: 8 additions & 0 deletions libs/server/API/GarnetWatchApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,14 @@ public GarnetStatus GET(byte[] key, out GarnetObjectStoreOutput value)
garnetApi.WATCH(key, StoreType.Object);
return garnetApi.GET(key, out value);
}

/// <inheritdoc />
public GarnetStatus LCS(ArgSlice key1, ArgSlice key2, ref SpanByteAndMemory output, bool lenOnly = false, bool withIndices = false, bool withMatchLen = false, int minMatchLen = 0)
{
garnetApi.WATCH(key1, StoreType.Object);
garnetApi.WATCH(key2, StoreType.Object);
return garnetApi.LCS(key1, key2, ref output, lenOnly, withIndices, withMatchLen, minMatchLen);
}
#endregion

#region GETRANGE
Expand Down
14 changes: 14 additions & 0 deletions libs/server/API/IGarnetApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using Garnet.common;
using Tsavorite.core;
using static Garnet.server.StorageSession;
vazois marked this conversation as resolved.
Show resolved Hide resolved

namespace Garnet.server
{
Expand Down Expand Up @@ -1079,6 +1080,19 @@ public interface IGarnetReadApi
/// <param name="value"></param>
/// <returns></returns>
GarnetStatus GET(byte[] key, out GarnetObjectStoreOutput value);

/// <summary>
/// Finds the longest common subsequence (LCS) between two keys.
/// </summary>
/// <param name="key1">The first key to compare.</param>
/// <param name="key2">The second key to compare.</param>
/// <param name="output">The output containing the LCS result.</param>
/// <param name="lenOnly">If true, only the length of the LCS is returned.</param>
/// <param name="withIndices">If true, the indices of the LCS in both keys are returned.</param>
/// <param name="withMatchLen">If true, the length of each match is returned.</param>
/// <param name="minMatchLen">The minimum length of a match to be considered.</param>
/// <returns>The status of the operation.</returns>
GarnetStatus LCS(ArgSlice key1, ArgSlice key2, ref SpanByteAndMemory output, bool lenOnly = false, bool withIndices = false, bool withMatchLen = false, int minMatchLen = 0);
#endregion

#region GETRANGE
Expand Down
60 changes: 60 additions & 0 deletions libs/server/Resp/ArrayCommands.cs
Original file line number Diff line number Diff line change
Expand Up @@ -453,5 +453,65 @@ private bool NetworkArrayPING()
WriteDirectLargeRespString(message);
return true;
}

private bool NetworkLCS<TGarnetApi>(ref TGarnetApi storageApi)
where TGarnetApi : IGarnetApi
{
if (parseState.Count < 2)
return AbortWithWrongNumberOfArguments(nameof(RespCommand.LCS));

var key1 = parseState.GetArgSliceByRef(0);
var key2 = parseState.GetArgSliceByRef(1);

// Parse options
var lenOnly = false;
var withIndices = false;
var minMatchLen = 0;
var withMatchLen = false;
var tokenIdx = 2;
while (tokenIdx < parseState.Count)
{
var option = parseState.GetArgSliceByRef(tokenIdx++).ReadOnlySpan;

if (option.EqualsUpperCaseSpanIgnoringCase(CmdStrings.LEN))
{
lenOnly = true;
}
else if (option.EqualsUpperCaseSpanIgnoringCase(CmdStrings.IDX))
{
withIndices = true;
}
else if (option.EqualsUpperCaseSpanIgnoringCase(CmdStrings.MINMATCHLEN))
{
if (!parseState.TryGetInt(tokenIdx++, out var minLen) || minLen < 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't look like there's an issue with setting minLen to a non-positive number, it just doesn't do anything

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might be a miss from Redis side. Personally, I don't think we should inherit their flaws (Provided this is a flaw from Redis). Do you want to remove this check?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with you 100%, I'm just wondering if this might break any existing clients that rely on this logic for some reason...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

vazois marked this conversation as resolved.
Show resolved Hide resolved
{
while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_GENERIC_VALUE_IS_NOT_INTEGER, ref dcurr, dend))
SendAndReset();
return true;
}
minMatchLen = minLen;
}
else if (option.EqualsUpperCaseSpanIgnoringCase(CmdStrings.WITHMATCHLEN))
{
withMatchLen = true;
}
else
{
while (!RespWriteUtils.WriteError(CmdStrings.RESP_SYNTAX_ERROR, ref dcurr, dend))
SendAndReset();
return true;
}
}

var output = new SpanByteAndMemory(dcurr, (int)(dend - dcurr));
var status = storageApi.LCS(key1, key2, ref output, lenOnly, withIndices, withMatchLen, minMatchLen);

if (!output.IsSpanByte)
SendAndReset(output.Memory, output.Length);
else
dcurr += output.Length;

return true;
}
}
}
7 changes: 7 additions & 0 deletions libs/server/Resp/CmdStrings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ static partial class CmdStrings
public static ReadOnlySpan<byte> STOREDIST => "STOREDIST"u8;
public static ReadOnlySpan<byte> WITHDIST => "WITHDIST"u8;
public static ReadOnlySpan<byte> WITHHASH => "WITHHASH"u8;
public static ReadOnlySpan<byte> LEN => "LEN"u8;
public static ReadOnlySpan<byte> IDX => "IDX"u8;
public static ReadOnlySpan<byte> MINMATCHLEN => "MINMATCHLEN"u8;
public static ReadOnlySpan<byte> WITHMATCHLEN => "WITHMATCHLEN"u8;

/// <summary>
/// Response strings
Expand All @@ -126,6 +130,8 @@ static partial class CmdStrings
public static ReadOnlySpan<byte> RESP_PONG => "+PONG\r\n"u8;
public static ReadOnlySpan<byte> RESP_EMPTY => "$0\r\n\r\n"u8;
public static ReadOnlySpan<byte> RESP_QUEUED => "+QUEUED\r\n"u8;
public static ReadOnlySpan<byte> matches => "matches"u8;
public static ReadOnlySpan<byte> len => "len"u8;

/// <summary>
/// Simple error response strings, i.e. these are of the form "-errorString\r\n"
Expand Down Expand Up @@ -200,6 +206,7 @@ static partial class CmdStrings
public static ReadOnlySpan<byte> RESP_ERR_GT_LT_NX_NOT_COMPATIBLE => "ERR GT, LT, and/or NX options at the same time are not compatible"u8;
public static ReadOnlySpan<byte> RESP_ERR_INCR_SUPPORTS_ONLY_SINGLE_PAIR => "ERR INCR option supports a single increment-element pair"u8;
public static ReadOnlySpan<byte> RESP_ERR_INVALID_BITFIELD_TYPE => "ERR Invalid bitfield type. Use something like i16 u8. Note that u64 is not supported but i64 is"u8;
public static ReadOnlySpan<byte> RESP_ERR_LENGTH_AND_INDEXES => "If you want both the length and indexes, please just use IDX."u8;

/// <summary>
/// Response string templates
Expand Down
5 changes: 5 additions & 0 deletions libs/server/Resp/Parser/RespCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public enum RespCommand : ushort
HSTRLEN,
HVALS,
KEYS,
LCS,
LINDEX,
LLEN,
LPOS,
Expand Down Expand Up @@ -752,6 +753,10 @@ private RespCommand FastParseArrayCommand(ref int count, ref ReadOnlySpan<byte>
{
return RespCommand.DEL;
}
else if (*(ulong*)(ptr + 1) == MemoryMarshal.Read<ulong>("3\r\nLCS\r\n"u8))
{
return RespCommand.LCS;
}

break;

Expand Down
2 changes: 2 additions & 0 deletions libs/server/Resp/RespServerSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,8 @@ private bool ProcessOtherCommands<TGarnetApi>(RespCommand command, ref TGarnetAp
RespCommand.SCRIPT => TrySCRIPT(),
RespCommand.EVAL => TryEVAL(),
RespCommand.EVALSHA => TryEVALSHA(),
// Slow commands
RespCommand.LCS => NetworkLCS(ref storageApi),
_ => Process(command)
};

Expand Down
Loading