RavenDB-22703 Added handling in JintCoraxDocumentConverter and fixed …

…handling in CoraxDocumentConverter
Lwiel · Aug 28, 2024 · 232a0b2 · 232a0b2
1 parent ebffa8b
commit 232a0b2
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 14 deletions.
diff --git a/src/Raven.Server/Documents/Indexes/Persistence/Corax/AnonymousCoraxDocumentConverter.cs b/src/Raven.Server/Documents/Indexes/Persistence/Corax/AnonymousCoraxDocumentConverter.cs
@@ -26,12 +26,10 @@ public abstract class AnonymousCoraxDocumentConverterBase : CoraxDocumentConvert
 {
     private readonly bool _isMultiMap;
     private IPropertyAccessor _propertyAccessor;
-    private Dictionary<string, bool> _nonExistingFieldsOfDocument;
 
     public AnonymousCoraxDocumentConverterBase(Index index, int numberOfBaseFields = 1, string keyFieldName = null, bool storeValue = false, bool canContainSourceDocumentId = false) : base(index, storeValue, indexImplicitNull: index.Configuration.IndexMissingFieldsAsNull, index.Configuration.IndexEmptyEntries, 1, keyFieldName, Constants.Documents.Indexing.Fields.ReduceKeyValueFieldName, canContainSourceDocumentId)
     {
         _isMultiMap = index.IsMultiMap;
-        _nonExistingFieldsOfDocument = new Dictionary<string, bool>();
     }
 
     protected override bool SetDocumentFields<TBuilder>(LazyStringValue key, LazyStringValue sourceDocumentId, object doc, JsonOperationContext indexContext, TBuilder builder,
@@ -70,9 +68,9 @@ protected override bool SetDocumentFields<TBuilder>(LazyStringValue key, LazyStr
                 throw new InvalidOperationException($"Field '{property.Key}' is not defined. Available fields: {string.Join(", ", _fields.Keys)}.");
 
             InsertRegularField(field, value, indexContext, builder, sourceDocument, out var innerShouldSkip);
-            
+
             if (innerShouldSkip)
-                _nonExistingFieldsOfDocument[field.Name] = true;
+                _nonExistingFieldsOfDocument.Add(field.Name);
 
             hasFields |= innerShouldSkip == false;
 
@@ -101,12 +99,14 @@ protected override bool SetDocumentFields<TBuilder>(LazyStringValue key, LazyStr
 
         builder.Write(0, string.Empty, id.AsSpan());
 
-        foreach (var kvp in _nonExistingFieldsOfDocument)
+        if (_index.Definition.Version >= IndexDefinitionBaseServerSide.IndexVersion.UseNonExistingPostingList)
         {
-            if (kvp.Value)
-                InsertNonExistingField(_fields[kvp.Key], builder);
+            foreach (var fieldName in _nonExistingFieldsOfDocument)
+                InsertNonExistingField(_fields[fieldName], builder);
         }
 
+        _nonExistingFieldsOfDocument.Clear();
+
         return true;
 
         void HandleCompoundFields()

diff --git a/src/Raven.Server/Documents/Indexes/Persistence/Corax/CoraxDocumentConverter.cs b/src/Raven.Server/Documents/Indexes/Persistence/Corax/CoraxDocumentConverter.cs
@@ -59,20 +59,26 @@ protected override bool SetDocumentFields<TBuilder>(LazyStringValue key, LazyStr
 
                 InsertRegularField(indexField, value, indexContext, builder, sourceDocument, out innerShouldSkip);
             }
-            else if (BlittableJsonTraverserHelper.TryRead(_blittableTraverser, document, indexField.OriginalName ?? indexField.Name, out value))
-            {
+
+            var successfulRead = BlittableJsonTraverserHelper.TryRead(_blittableTraverser, document, indexField.OriginalName ?? indexField.Name, out value);
+
+            if (successfulRead)
                 InsertRegularField(indexField, value, indexContext, builder, sourceDocument, out innerShouldSkip);
-            }
-            else if (_index.Definition.Version >= IndexDefinitionBaseServerSide.IndexVersion.UseNonExistingPostingList)
-            {
-                InsertNonExistingField(indexField, builder);
-            }
+
+            if (successfulRead == false || innerShouldSkip)
+                _nonExistingFieldsOfDocument.Add(indexField.Name);
 
             hasFields |= innerShouldSkip == false;
         }
 
         if (hasFields is false && _indexEmptyEntries is false)
             return false;
+
+        if (_index.Definition.Version >= IndexDefinitionBaseServerSide.IndexVersion.UseNonExistingPostingList)
+        {
+            foreach (var fieldName in _nonExistingFieldsOfDocument)
+                InsertNonExistingField(_fields[fieldName], builder);
+        }
 
         if (key != null)
         {

diff --git a/src/Raven.Server/Documents/Indexes/Persistence/Corax/CoraxDocumentConverterBase.cs b/src/Raven.Server/Documents/Indexes/Persistence/Corax/CoraxDocumentConverterBase.cs
@@ -53,6 +53,7 @@ public abstract class CoraxDocumentConverterBase : ConverterBase
     private HashSet<IndexField> _complexFields;
     public bool IgnoreComplexObjectsDuringIndex;
     public List<string[]> CompoundFields;
+    protected HashSet<string> _nonExistingFieldsOfDocument;
 
     protected abstract bool SetDocumentFields<TBuilder>(
         LazyStringValue key, LazyStringValue sourceDocumentId,
@@ -105,6 +106,8 @@ protected CoraxDocumentConverterBase(Index index, bool storeValue, bool indexImp
                 }
             }
         }
+
+        _nonExistingFieldsOfDocument = new HashSet<string>();
     }
 
     public IndexFieldsMapping GetKnownFieldsForQuerying() => _knownFieldsForReaders.Value;

diff --git a/src/Raven.Server/Documents/Indexes/Persistence/Corax/CoraxQueryBuilder.cs b/src/Raven.Server/Documents/Indexes/Persistence/Corax/CoraxQueryBuilder.cs
@@ -253,6 +253,7 @@ internal static IQueryMatch BuildQuery(Parameters builderParameters, out OrderMe
                 coraxQuery = MaterializeWhenNeeded(builderParameters, coraxQuery, ref streamingOptimization);
             }
             // We sort on known field types, we'll optimize based on the first one to get the rest
+            // Non-existing posting list isn't aware of dynamic fields, so we can't use this optimization for them
             else if (sortMetadata is [{ FieldType: MatchCompareFieldType.Floating or MatchCompareFieldType.Integer or MatchCompareFieldType.Sequence, Field.FieldId: not CoraxConstants.IndexWriter.DynamicField } sortBy, ..])
             {
                 var maxTermToScan = builderParameters.Take switch

diff --git a/src/Raven.Server/Documents/Indexes/Persistence/Corax/JintCoraxDocumentConverter.cs b/src/Raven.Server/Documents/Indexes/Persistence/Corax/JintCoraxDocumentConverter.cs
@@ -104,6 +104,9 @@ protected override bool SetDocumentFields<TBuilder>(LazyStringValue key, LazyStr
                 var disposable = value as IDisposable;
                 disposable?.Dispose();
             }
+
+            if (innerShouldSkip)
+                _nonExistingFieldsOfDocument.Add(field.Name);
         }
 
         if (hasFields is false && _indexEmptyEntries is false)
@@ -115,6 +118,14 @@ protected override bool SetDocumentFields<TBuilder>(LazyStringValue key, LazyStr
         if (sourceDocumentId != null && fieldMapping.TryGetByFieldName(Constants.Documents.Indexing.Fields.SourceDocumentIdFieldName, out var keyBinding))
             builder.Write(keyBinding.FieldId, sourceDocumentId.AsSpan());
 
+        if (_index.Definition.Version >= IndexDefinitionBaseServerSide.IndexVersion.UseNonExistingPostingList)
+        {
+            foreach (var fieldName in _nonExistingFieldsOfDocument)
+                InsertNonExistingField(_fields[fieldName], builder);
+        }
+
+        _nonExistingFieldsOfDocument.Clear();
+
         if (_storeValue)
         {
             //Write __stored_fields at the end of entry...

diff --git a/test/SlowTests/Issues/RavenDB-22703.cs b/test/SlowTests/Issues/RavenDB-22703.cs
@@ -333,4 +333,78 @@ public Products_ByAttributeKey()
                 };
         }
     }
+
+    [RavenTheory(RavenTestCategory.Corax | RavenTestCategory.Indexes)]
+    [RavenData(SearchEngineMode = RavenSearchEngineMode.Corax, DatabaseMode = RavenDatabaseMode.All)]
+    public void TestJavaScriptIndex(Options options)
+    {
+        using (var store = GetDocumentStore(options))
+        {
+            using (var session = store.OpenSession())
+            {
+                var bar1 = new Bar() { Foo = new Foo() { BarBool = false, BarShort = 9 } };
+                var bar2 = new Bar() { Foo = new Foo() { BarBool = null } };
+                var bar3 = new Bar() { Foo = null };
+                var bar4 = new Bar() { Foo = new Foo() { BarBool = true, BarShort = 21 } };
+                var bar5 = new Bar() { Foo = null };
+
+                session.Store(bar1);
+                session.Store(bar2);
+                session.Store(bar3);
+                session.Store(bar4);
+                session.Store(bar5);
+
+                session.SaveChanges();
+
+                var requestExecutor = store.GetRequestExecutor();
+                using (requestExecutor.ContextPool.AllocateOperationContext(out var context))
+                {
+                    var reader = context.ReadObject(new DynamicJsonValue
+                    {
+                        ["@metadata"] = new DynamicJsonValue{
+                            ["@collection"] = "Bars",
+                            ["Raven-Clr-Type"] = "SlowTests.Issues.RavenDB_22703+Bar, SlowTests"
+                        }
+                    }, "bars/6");
+                    requestExecutor.Execute(new PutDocumentCommand(store.Conventions, "bars/6", null, reader), context);
+                }
+
+                var index = new JavaScriptIndex();
+
+                index.Execute(store);
+
+                Indexes.WaitForIndexing(store);
+
+                var res = session.Query<JavaScriptIndex.IndexEntry, JavaScriptIndex>()
+                    .OrderByDescending(x => x.BarBool)
+                    .ThenByDescending(x => x.BarShort)
+                    .ProjectInto<Bar>()
+                    .ToList();
+
+                // We don't always distinguish null and non-existing value in Jint converter correctly
+                Assert.Equal(4, res.Count);
+            }
+        }
+    }
+
+    private class JavaScriptIndex : AbstractJavaScriptIndexCreationTask
+    {
+        public class IndexEntry
+        {
+            public short BarShort { get; set; }
+            public bool BarBool { get; set; }
+        }
+        public JavaScriptIndex()
+        {
+            Maps = new HashSet<string>
+            {
+                @"map('Bars', function (bar){ 
+                    return { 
+                        BarShort : bar.Foo.BarShort, 
+                        BarBool : bar.Foo.BarBool
+                    };
+                })",
+            };
+        }
+    }
 }