Add tests (and fix bugs) for commands WRONGTYPE'ing appropriately against Vector Sets #1935

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

Open

kevin-montrose wants to merge 59 commits into main from users/kmontrose/vectorSetWrongTypeTests

libs/server/API/GarnetApi.cs

-Original file line number
+Diff line change
@@ Expand Up @@
                 SessionParseState parseState = default;
                 var input = new StringInput(RespCommand.INCRBYFLOAT, ref parseState, BitConverter.DoubleToInt64Bits(val));
-                _ = Increment(key, ref input, ref output);
-                return GarnetStatus.OK;
+                return Increment(key, ref input, ref output);
             }
             /// <inheritdoc />
@@ Expand All @@
                 Span<byte> outputBuffer = stackalloc byte[NumUtils.MaximumFormatDoubleLength + 1];
                 var stringOutput = StringOutput.FromPinnedSpan(outputBuffer);
-                _ = IncrementByFloat(key, ref stringOutput, val);
+                var res = IncrementByFloat(key, ref stringOutput, val);
+                if (res == GarnetStatus.WRONGTYPE)
+                {
+                    output = 0;
+                    return res;
+                }
                 if (!stringOutput.HasError)
                 {
@@ Expand Down Expand Up / @@ -372,6 +377,21 @@ public void ResetScratchBuffer() @@
             #region VectorSet commands
+            /// <inheritdoc/>
+            public GarnetStatus VectorSetCardinality(PinnedSpanByte key, out long card)
+            => storageSession.VectorSetCardinality(key, out card);
+            /// <inheritdoc/>
+            public GarnetStatus VectorSetIsMember(PinnedSpanByte key, PinnedSpanByte element)
+            => storageSession.VectorSetIsMember(key, element);
+            /// <inheritdoc/>
+            public GarnetStatus VectorSetLinks(PinnedSpanByte key, PinnedSpanByte element, bool withScores, ref SpanByteAndMemory idResults, ref SpanByteAndMemory distanceResults)
+            => storageSession.VectorSetLinks(key, element, withScores, ref idResults, ref distanceResults);
+            /// <inheritdoc/>
+            public GarnetStatus VectorSetRandomMembers(PinnedSpanByte key, int count, ref SpanByteAndMemory idResults)
+            => storageSession.VectorSetRandomMembers(key, count, ref idResults);
             /// <inheritdoc />
             public unsafe GarnetStatus VectorSetAdd(PinnedSpanByte key, int reduceDims, VectorValueType valueType, PinnedSpanByte values, PinnedSpanByte element, VectorQuantType quantizer, int buildExplorationFactor, PinnedSpanByte attributes, int numLinks, VectorDistanceMetricType distanceMetric, out VectorManagerResult result, out ReadOnlySpan<byte> errorMsg)
             => storageSession.VectorSetAdd(key, reduceDims, valueType, values, element, quantizer, buildExplorationFactor, attributes, numLinks, distanceMetric, out result, out errorMsg);
@@ Expand All @@
             public unsafe GarnetStatus VectorSetRemove(PinnedSpanByte key, PinnedSpanByte element)
             => storageSession.VectorSetRemove(key, element);
+            /// <inheritdoc />
+            public GarnetStatus VectorSetSetAttribute(PinnedSpanByte key, PinnedSpanByte element, PinnedSpanByte attribute)
+            => storageSession.VectorSetSetAttribute(key, element, attribute);
             /// <inheritdoc />
             public unsafe GarnetStatus VectorSetValueSimilarity(PinnedSpanByte key, VectorValueType valueType, PinnedSpanByte values, int count, float delta, int searchExplorationFactor, PinnedSpanByte filter, int maxFilteringEffort, bool includeAttributes, ref SpanByteAndMemory outputIds, out VectorIdFormat outputIdFormat, out ReadOnlySpan<byte> errorMessage, ref SpanByteAndMemory outputDistances, ref SpanByteAndMemory outputAttributes, out VectorManagerResult result, ref SpanByteAndMemory filterBitmap)
             => storageSession.VectorSetValueSimilarity(key, valueType, values, count, delta, searchExplorationFactor, filter.ReadOnlySpan, maxFilteringEffort, includeAttributes, ref outputIds, out outputIdFormat, out errorMessage, ref outputDistances, ref outputAttributes, out result, ref filterBitmap);
@@ Expand Down @@

libs/server/API/GarnetWatchApi.cs

-Original file line number
+Diff line change
@@ Expand Up / @@ -622,6 +622,35 @@ public void ResetScratchBuffer() @@
             #endregion
             #region Vector Sets
+            /// <inheritdoc/>
+            public GarnetStatus VectorSetCardinality(PinnedSpanByte key, out long card)
+            {
+                garnetApi.WATCH(key, StoreType.Main);
+                return garnetApi.VectorSetCardinality(key, out card);
+            }
+            /// <inheritdoc/>
+            public GarnetStatus VectorSetIsMember(PinnedSpanByte key, PinnedSpanByte element)
+            {
+                garnetApi.WATCH(key, StoreType.Main);
+                return garnetApi.VectorSetIsMember(key, element);
+            }
+            /// <inheritdoc/>
+            public GarnetStatus VectorSetLinks(PinnedSpanByte key, PinnedSpanByte element, bool withScores, ref SpanByteAndMemory idResults, ref SpanByteAndMemory distanceResults)
+            {
+                garnetApi.WATCH(key, StoreType.Main);
+                return garnetApi.VectorSetLinks(key, element, withScores, ref idResults, ref distanceResults);
+            }
+            /// <inheritdoc/>
+            public GarnetStatus VectorSetRandomMembers(PinnedSpanByte key, int count, ref SpanByteAndMemory idResults)
+            {
+                garnetApi.WATCH(key, StoreType.Main);
+                return garnetApi.VectorSetRandomMembers(key, count, ref idResults);
+            }
             /// <inheritdoc/>
             public GarnetStatus VectorSetValueSimilarity(PinnedSpanByte key, VectorValueType valueType, PinnedSpanByte value, int count, float delta, int searchExplorationFactor, PinnedSpanByte filter, int maxFilteringEffort, bool includeAttributes, ref SpanByteAndMemory outputIds, out VectorIdFormat outputIdFormat, out ReadOnlySpan<byte> errorMessage, ref SpanByteAndMemory outputDistances, ref SpanByteAndMemory outputAttributes, out VectorManagerResult result, ref SpanByteAndMemory filterBitmap)
             {
@@ Expand Down @@

libs/server/API/IGarnetApi.cs

-Original file line number
+Diff line change
@@ Expand Up / @@ -1340,6 +1340,11 @@ GarnetStatus RangeIndexMetrics(PinnedSpanByte key, @@
             /// Remove a member from a vector set, if it is present and the key exists.
             /// </summary>
             GarnetStatus VectorSetRemove(PinnedSpanByte key, PinnedSpanByte element);
+            /// <summary>
+            /// Update attribute for element in a Vector Set.
+            /// </summary>
+            GarnetStatus VectorSetSetAttribute(PinnedSpanByte key, PinnedSpanByte element, PinnedSpanByte attribute);
             #endregion
         }
@@ Expand Down Expand Up @@
             #region Vector Sets
+            /// <summary>
+            /// Count the number of vectors in a Vector Set.
+            /// </summary>
+            GarnetStatus VectorSetCardinality(PinnedSpanByte key, out long card);
+            /// <summary>
+            /// Returns <see cref="GarnetStatus.OK"/> if the given element exists in the given Vector Set.
+            ///
+            /// Returns <see cref="GarnetStatus.NOTFOUND"/> if the given element is not in the Vector Set, or the Vector Set does not exist.
+            ///
+            /// Returns <see cref="GarnetStatus.WRONGTYPE"/> if the given key exists, but is not a Vector set.
+            /// </summary>
+            GarnetStatus VectorSetIsMember(PinnedSpanByte key, PinnedSpanByte element);
+            /// <summary>
+            /// For a given element, find all neighbors and (optionally) the distance to those neighbors.
+            ///
+            /// On success, <paramref name="idResults"/> has length prefixed element names, and <paramref name="distanceResults"/> (if <paramref name="withScores"/> is true) has a float for each of those elements.
+            /// </summary>
+            GarnetStatus VectorSetLinks(PinnedSpanByte key, PinnedSpanByte element, bool withScores, ref SpanByteAndMemory idResults, ref SpanByteAndMemory distanceResults);
+            /// <summary>
+            /// Fetch random elements from the given Vector Set.
+            ///
+            /// If <paramref name="count"/> is &lt; 0 we allow duplicates, if <paramref name="count"/> &gt; 0 we remove duplicates.
+            ///
+            /// It is OK to fetch fewer than the requested number of elements.
+            ///
+            /// On success, <paramref name="idResults"/> has length prefixed element names.
+            /// </summary>
+            GarnetStatus VectorSetRandomMembers(PinnedSpanByte key, int count, ref SpanByteAndMemory idResults);
             /// <summary>
             /// Perform a similarity search given a vector and these parameters.
             ///
@@ Expand Down @@

libs/server/Databases/DatabaseManagerBase.cs

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -445,7 +445,7 @@ private async ValueTask DoCompactionAsync(GarnetDatabase db, int mainStoreMaxSeg
  
                            break;

                        case LogCompactionType.Scan:

                            storeLog.Compact<PinnedSpanByte, Empty, Empty>(untilAddress, CompactionType.Scan);

                            storeLog.Compact<PinnedSpanByte, Empty, Empty, GarnetRecordTriggers>(db.Store.StoreFunctions.recordTriggers, untilAddress, CompactionType.Scan);

                            if (compactionForceDelete)

                            {

                                await CompactionCommitAofAsync(db).ConfigureAwait(false);

    @@ -454,7 +454,7 @@ private async ValueTask DoCompactionAsync(GarnetDatabase db, int mainStoreMaxSeg
  
                            break;

                        case LogCompactionType.Lookup:

                            storeLog.Compact<PinnedSpanByte, Empty, Empty>(untilAddress, CompactionType.Lookup);

                            storeLog.Compact<PinnedSpanByte, Empty, Empty, GarnetRecordTriggers>(db.Store.StoreFunctions.recordTriggers, untilAddress, CompactionType.Lookup);

                            if (compactionForceDelete)

                            {

                                await CompactionCommitAofAsync(db).ConfigureAwait(false);

libs/server/Objects/ItemBroker/CollectionItemBroker.cs

-Original file line number
+Diff line change
@@ Expand Up @@
                         return false;
                     // Check for type match between the observer and the source object type
-                    if ((GarnetObjectType)osObject.GarnetObject.Type != objectType)
+                    if (statusOp == GarnetStatus.WRONGTYPE || (GarnetObjectType)osObject.GarnetObject.Type != objectType)
                     {
                         // Return a type mismatch result if we should fail on source object type mismatch
                         if (failOnSrcTypeMismatch)
@@ Expand All @@
                             dstObj = osDstObject.GarnetObject;
                             // If there is a destination object type mismatch, we should always return a type mismatch result
-                            if ((GarnetObjectType)dstObj.Type != objectType)
+                            if (dstStatusOp == GarnetStatus.WRONGTYPE || (GarnetObjectType)dstObj.Type != objectType)
                             {
                                 result = CollectionItemResult.TypeMismatch;
                                 return true;
@@ Expand Down @@

libs/server/Resp/AdminCommands.cs

-Original file line number
+Diff line change
@@ Expand Up @@
                         while (!RespWriteUtils.TryWriteDirect(CmdStrings.RESP_OK, ref dcurr, dend))
                             SendAndReset();
                         break;
+                    case GarnetStatus.WRONGTYPE:
+                        WriteError(CmdStrings.RESP_ERR_WRONG_TYPE);
+                        break;
                     default:
                         while (!RespWriteUtils.TryWriteError(CmdStrings.RESP_ERR_HCOLLECT_ALREADY_IN_PROGRESS, ref dcurr, dend))
                             SendAndReset();
@@ Expand Down Expand Up @@
                         while (!RespWriteUtils.TryWriteDirect(CmdStrings.RESP_OK, ref dcurr, dend))
                             SendAndReset();
                         break;
+                    case GarnetStatus.WRONGTYPE:
+                        WriteError(CmdStrings.RESP_ERR_WRONG_TYPE);
+                        break;
                     default:
                         while (!RespWriteUtils.TryWriteError(CmdStrings.RESP_ERR_ZCOLLECT_ALREADY_IN_PROGRESS, ref dcurr, dend))
                             SendAndReset();
@@ Expand Down @@

libs/server/Resp/ArrayCommands.cs

-Original file line number
+Diff line change
@@ Expand Up @@
                     return AbortWithWrongNumberOfArguments(nameof(RespCommand.MSET));
                 }
-                for (int c = 0; c < parseState.Count; c += 2)
+                for (var c = 0; c < parseState.Count; c += 2)
                 {
                     var key = parseState.GetArgSliceByRef(c);
                     var val = parseState.GetArgSliceByRef(c + 1);
-                    _ = storageApi.SET(key, val);
+                    var res = storageApi.SET(key, val);
+                    // Type already stored in key which requires an explicit delete
+                    if (res == GarnetStatus.WRONGTYPE)
+                    {
+                        var createTransaction = false;
+                        if (txnManager.state != TxnState.Running)
+                        {
+                            createTransaction = true;
+                            txnManager.AddTransactionStoreTypes(TransactionStoreTypes.Main | TransactionStoreTypes.Object);
+                            txnManager.SaveKeyEntryToLock(key, LockType.Exclusive);
+                            _ = txnManager.Run(true);
+                        }
+                        try
+                        {
+                            _ = storageSession.DELETE(key, ref storageSession.unifiedTransactionalContext);
+                            _ = storageSession.SET(key, val, ref storageSession.stringTransactionalContext);
+                        }
+                        finally
+                        {
+                            if (createTransaction)
+                            {
+                                txnManager.Commit(true);
+                            }
+                        }
+                    }
                 }
                 while (!RespWriteUtils.TryWriteDirect(CmdStrings.RESP_OK, ref dcurr, dend))
                     SendAndReset();
                 return true;
             }
@@ Expand Down Expand Up @@
                 var output = GetStringOutput();
                 var status = storageApi.LCS(key1, key2, ref output, lenOnly, withIndices, withMatchLen, minMatchLen);
-                ProcessOutput(output.SpanByteAndMemory);
+                if (status == GarnetStatus.WRONGTYPE)
+                {
+                    WriteError(CmdStrings.RESP_ERR_WRONG_TYPE);
+                }
+                else
+                {
+                    ProcessOutput(output.SpanByteAndMemory);
+                }
                 return true;
             }
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add tests (and fix bugs) for commands WRONGTYPE'ing appropriately against Vector Sets #1935

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

Uh oh!

Add tests (and fix bugs) for commands WRONGTYPE'ing appropriately against Vector Sets #1935

Are you sure you want to change the base?

Uh oh!

Add tests (and fix bugs) for commands WRONGTYPE'ing appropriately against Vector Sets #1935

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!