GraphEngine icon indicating copy to clipboard operation
GraphEngine copied to clipboard

Bug in FanoutSearch with net6

Open alenas opened this issue 2 years ago • 11 comments

File: src\Modules\LIKQ\FanoutSearch\QueryLanguage\ExpressionBuilder.cs

line 22 should be like this (with a type), otherwise it errors out and some unit tests do not pass: private static readonly MethodInfo s_string_contains = typeof(String).GetMethod("Contains", new Type[] { typeof(String) });

alenas avatar Aug 16 '22 15:08 alenas

@alenas - I am looking at this right now; what Test Case failed for you?

TaviTruman avatar Aug 16 '22 17:08 TaviTruman

Most in FanoutSearch.UnitTest.JSONDSLTest (and 2). I ran from Visual Studio compiled with .net 6.0. And I see there are more JSON issues when using Newtonsoft version 13...

alenas avatar Aug 16 '22 17:08 alenas

image

TaviTruman avatar Aug 16 '22 17:08 TaviTruman

I found the actual bug! image

image

the and_pred has a value of null and the fails in the call: return Expression.Condition(GenerateBooleanPredicateExpression(pred_object, icell), action_const, noaction_const);

in method: GenerateConditionalPredicateExpression

TaviTruman avatar Aug 16 '22 18:08 TaviTruman

I changed language version to 10 on FanoutSearch (as I left only .net 6 compilation) and I that's why that line is broken for me. but seems like your and_pred is null for similar reasons, I will see how it works in my project.

alenas avatar Aug 16 '22 19:08 alenas

Here is the offending outer code block:

image

The call to FanoutSearchDescriptor fails: FanoutSearchDescriptor fanoutSearch_desc = new FanoutSearchDescriptor(queryPath, queryObject);

Here is the JsonQuery content that fails:

image

These LIKQ JsonQuery statements are slightly different:

image

The first one fails while the second one works. It looks like the results field is the first statement is expecting a Conditional while the second one is not.

TaviTruman avatar Aug 16 '22 19:08 TaviTruman

Something is not right with the JSON parsing logic. I don't use JsonDSL but do use LambdaDSL and KnowledgeGraph LIKQ dialects and have not had any problems with the .NET 6 version.

TaviTruman avatar Aug 16 '22 20:08 TaviTruman

I actually changed few other files as well: src\Modules\LIKQ\FanoutSearch\Descriptors\FanoutSearchDescriptor.cs Line 141 was: JObject jobj = JsonConvert.DeserializeObject<JObject>(m_origin_query); changed to: var jobj = JObject.Parse(m_origin_query);

file: src\Modules\LIKQ\FanoutSearch\QueryLanguage\Standard.cs line 31 was: string json_search_body = Newtonsoft.Json.JsonConvert.SerializeObject(search_body); changed to: string json_search_body; if (search_body is string) { json_search_body = search_body.ToString(); } else { json_search_body = Newtonsoft.Json.JsonConvert.SerializeObject(search_body); }

at the moment all my test pass. but there is one problem, as I think that JSON message that goes to the server is still not good. If I try to use json for start from like this, then I get server response error:

var search = KnowledgeGraph.StartFrom($@"{{""type"":""Person"",""match"":{{""text"":""{word}""}}}}";);

alenas avatar Aug 16 '22 21:08 alenas

Yeah - I totally don't use Json in any KnowledgeGraph productions:

  • Corrected to match actual working knowledge graph

This works just fine for me:

        var pathsx = KnowledgeGraph
            .StartFrom(123)
            .FollowEdge("isa_agent")
            .FollowEdge("isa_buyer_lead")
            .VisitNode(_ => _.return_if(_.GetField<string>("rdf_subject").Contains("business_process") == _.has("workflow")), select: new List<string> { "open_house","new_client" })
            .VisitNode(queryPart => queryPart.continue_if(queryPart.has_cell_id(0)))
                .ToList();

image

TaviTruman avatar Aug 16 '22 22:08 TaviTruman

@alenas not all LIKQ language dialects will process json statements. I am just looking at Lambda DSL, and Json DSL processing as there are some test case failing.

TaviTruman avatar Aug 17 '22 18:08 TaviTruman

@alenas I think I have the fix in place; will need to re-run the existing tests and add a few new tests and the changes to the JSON parser do require some changes to the code.

TaviTruman avatar Oct 18 '22 16:10 TaviTruman