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

Bug in FanoutSearch with net6 #359

Open
alenas opened this issue Aug 16, 2022 · 11 comments
Open

Bug in FanoutSearch with net6 #359

alenas opened this issue Aug 16, 2022 · 11 comments

Comments

@alenas
Copy link

alenas commented Aug 16, 2022

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) });

@TaviTruman
Copy link
Contributor

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

@alenas
Copy link
Author

alenas commented Aug 16, 2022

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...

@TaviTruman
Copy link
Contributor

image

@TaviTruman
Copy link
Contributor

TaviTruman commented Aug 16, 2022

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

@alenas
Copy link
Author

alenas commented Aug 16, 2022

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.

@TaviTruman
Copy link
Contributor

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
Copy link
Contributor

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.

@alenas
Copy link
Author

alenas commented Aug 16, 2022

I actually changed few other files as well:
src\Modules\LIKQ\FanoutSearch\Descriptors\FanoutSearchDescriptor.cs
Line 141 was:
JObject jobj = JsonConvert.DeserializeObject(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}""}}}}";);

@TaviTruman
Copy link
Contributor

TaviTruman commented Aug 16, 2022

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
Copy link
Contributor

@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
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants