fix: kql implicit 'AND' and 'OR' follows the ms html spec instead of …

…the pdf spec
owncloud · Sep 11, 2023 · 7dfe798 · 7dfe798
1 parent e9d9757
commit 7dfe798
Show file tree

Hide file tree

Showing 4 changed files with 133 additions and 32 deletions.
diff --git a/services/search/pkg/query/kql/connect.go b/services/search/pkg/query/kql/connect.go
@@ -74,47 +74,34 @@ func (c DefaultConnector) Connect(head ast.Node, neighbor ast.Node, connections
 	}
 
 	// if the current node and the neighbor node have the same key
-	// the connection is of type OR, same applies if no keys are in place
-	//
-	//		"" == ""
+	// the connection is of type OR
 	//
 	// spec: same
 	//		author:"John Smith" author:"Jane Smith"
 	//		author:"John Smith" OR author:"Jane Smith"
 	//
+	// if the nodes have NO key, the edge is a AND connection
+	//
+	// spec: same
+	//		cat dog
+	//		cat AND dog
+	// from the spec:
+	// 		To construct complex queries, you can combine multiple
+	// 		free-text expressions with KQL query operators.
+	// 		If there are multiple free-text expressions without any
+	// 		operators in between them, the query behavior is the same
+	// 		as using the AND operator.
+	//
 	// nodes inside of group node are handled differently,
-	// if no explicit operator given, it uses OR
+	// if no explicit operator given, it uses AND
 	//
 	// spec: same
 	// 		author:"John Smith" AND author:"Jane Smith"
 	// 		author:("John Smith" "Jane Smith")
-	if headKey == neighborKey {
+	if headKey == neighborKey && headKey != "" && neighborKey != "" {
 		connection.Value = c.sameKeyOPValue
 	}
 
-	// decisions based on nearest neighbor node
-	switch neighbor.(type) {
-	// nearest neighbor node type can change the default case
-	// docs says, if the next value node:
-	//
-	//		is a group and has no key
-	//
-	//		and the head node has no key
-	//
-	// 		it should be an AND edge
-	//
-	// spec: same
-	// 		cat (dog OR fox)
-	// 		cat AND (dog OR fox)
-	//
-	// note:
-	// 		sounds contradictory to me
-	case *ast.GroupNode:
-		if headKey == "" && neighborKey == "" {
-			connection.Value = BoolAND
-		}
-	}
-
 	// decisions based on nearest neighbor operators
 	for i, node := range connections {
 		// consider direct neighbor operator only

diff --git a/services/search/pkg/query/kql/dictionary_test.go b/services/search/pkg/query/kql/dictionary_test.go
@@ -65,6 +65,7 @@ func TestParse(t *testing.T) {
 		//
 		// https://msopenspecs.azureedge.net/files/MS-KQL/%5bMS-KQL%5d.pdf
 		// https://learn.microsoft.com/en-us/openspecs/sharepoint_protocols/ms-kql/3bbf06cd-8fc1-4277-bd92-8661ccd3c9b0
+		// https://learn.microsoft.com/en-us/sharepoint/dev/general-development/keyword-query-language-kql-syntax-reference
 		//
 		// ++
 		// 2.1.2 AND Operator
@@ -146,7 +147,7 @@ func TestParse(t *testing.T) {
 			expectedAst: &ast.Ast{
 				Nodes: []ast.Node{
 					&ast.StringNode{Value: "cat"},
-					&ast.OperatorNode{Value: kql.BoolOR},
+					&ast.OperatorNode{Value: kql.BoolAND},
 					&ast.StringNode{Value: "dog"},
 				},
 			},
@@ -333,7 +334,7 @@ func TestParse(t *testing.T) {
 			expectedAst: &ast.Ast{
 				Nodes: []ast.Node{
 					&ast.StringNode{Value: "cat"},
-					&ast.OperatorNode{Value: kql.BoolOR},
+					&ast.OperatorNode{Value: kql.BoolAND},
 					&ast.StringNode{Value: "dog"},
 					&ast.OperatorNode{Value: kql.BoolAND},
 					&ast.StringNode{Value: "fox"},
@@ -689,7 +690,7 @@ func TestParse(t *testing.T) {
 					&ast.StringNode{
 						Value: "😂",
 					},
-					&ast.OperatorNode{Value: kql.BoolOR},
+					&ast.OperatorNode{Value: kql.BoolAND},
 					&ast.StringNode{
 						Value: "*😀 😁*",
 					},
@@ -903,6 +904,118 @@ func TestParse(t *testing.T) {
 				Node: &ast.OperatorNode{Value: kql.BoolOR},
 			},
 		},
+		{
+			name: `cat dog`,
+			expectedAst: &ast.Ast{
+				Nodes: []ast.Node{
+					&ast.StringNode{Value: "cat"},
+					&ast.OperatorNode{Value: kql.BoolAND},
+					&ast.StringNode{Value: "dog"},
+				},
+			},
+		},
+		{
+			name: `cat dog fox`,
+			expectedAst: &ast.Ast{
+				Nodes: []ast.Node{
+					&ast.StringNode{Value: "cat"},
+					&ast.OperatorNode{Value: kql.BoolAND},
+					&ast.StringNode{Value: "dog"},
+					&ast.OperatorNode{Value: kql.BoolAND},
+					&ast.StringNode{Value: "fox"},
+				},
+			},
+		},
+		{
+			name: `(cat dog) fox`,
+			expectedAst: &ast.Ast{
+				Nodes: []ast.Node{
+					&ast.GroupNode{
+						Nodes: []ast.Node{
+							&ast.StringNode{Value: "cat"},
+							&ast.OperatorNode{Value: kql.BoolAND},
+							&ast.StringNode{Value: "dog"},
+						},
+					},
+					&ast.OperatorNode{Value: kql.BoolAND},
+					&ast.StringNode{Value: "fox"},
+				},
+			},
+		},
+		{
+			name: `(mammal:cat mammal:dog) fox`,
+			expectedAst: &ast.Ast{
+				Nodes: []ast.Node{
+					&ast.GroupNode{
+						Nodes: []ast.Node{
+							&ast.StringNode{Key: "mammal", Value: "cat"},
+							&ast.OperatorNode{Value: kql.BoolOR},
+							&ast.StringNode{Key: "mammal", Value: "dog"},
+						},
+					},
+					&ast.OperatorNode{Value: kql.BoolAND},
+					&ast.StringNode{Value: "fox"},
+				},
+			},
+		},
+		{
+			name: `mammal:(cat dog) fox`,
+			expectedAst: &ast.Ast{
+				Nodes: []ast.Node{
+					&ast.GroupNode{
+						Key: "mammal",
+						Nodes: []ast.Node{
+							&ast.StringNode{Value: "cat"},
+							&ast.OperatorNode{Value: kql.BoolAND},
+							&ast.StringNode{Value: "dog"},
+						},
+					},
+					&ast.OperatorNode{Value: kql.BoolAND},
+					&ast.StringNode{Value: "fox"},
+				},
+			},
+		},
+		{
+			name: `mammal:(cat dog) mammal:fox`,
+			expectedAst: &ast.Ast{
+				Nodes: []ast.Node{
+					&ast.GroupNode{
+						Key: "mammal",
+						Nodes: []ast.Node{
+							&ast.StringNode{Value: "cat"},
+							&ast.OperatorNode{Value: kql.BoolAND},
+							&ast.StringNode{Value: "dog"},
+						},
+					},
+					&ast.OperatorNode{Value: kql.BoolOR},
+					&ast.StringNode{Key: "mammal", Value: "fox"},
+				},
+			},
+		},
+		{
+			name: `title:((Advanced OR Search OR Query) -"Advanced Search Query")`,
+			expectedAst: &ast.Ast{
+				Nodes: []ast.Node{
+					&ast.GroupNode{
+						Key: "title",
+						Nodes: []ast.Node{
+							&ast.GroupNode{
+								Nodes: []ast.Node{
+									&ast.StringNode{Value: "Advanced"},
+									&ast.OperatorNode{Value: kql.BoolOR},
+									&ast.StringNode{Value: "Search"},
+									&ast.OperatorNode{Value: kql.BoolOR},
+									&ast.StringNode{Value: "Query"},
+								},
+							},
+							&ast.OperatorNode{Value: kql.BoolAND},
+							&ast.OperatorNode{Value: kql.BoolNOT},
+							&ast.StringNode{Value: "Advanced Search Query"},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	assert := tAssert.New(t)

diff --git a/services/search/pkg/query/kql/doc.go b/services/search/pkg/query/kql/doc.go
@@ -21,6 +21,7 @@ The following spec parts are supported and tested:
   - 3.3.5 Date Tokens
 
 References:
+  - https://learn.microsoft.com/en-us/sharepoint/dev/general-development/keyword-query-language-kql-syntax-reference
   - https://learn.microsoft.com/en-us/openspecs/sharepoint_protocols/ms-kql/3bbf06cd-8fc1-4277-bd92-8661ccd3c9b0
   - https://msopenspecs.azureedge.net/files/MS-KQL/%5bMS-KQL%5d.pdf
 */

diff --git a/services/search/pkg/query/kql/factory.go b/services/search/pkg/query/kql/factory.go
@@ -165,7 +165,7 @@ func buildGroupNode(k, n interface{}, text []byte, pos position) (*ast.GroupNode
 	gn := &ast.GroupNode{
 		Base:  b,
 		Key:   key,
-		Nodes: connectNodes(DefaultConnector{sameKeyOPValue: BoolAND}, nodes...),
+		Nodes: connectNodes(DefaultConnector{sameKeyOPValue: BoolOR}, nodes...),
 	}
 
 	if err := validateGroupNode(gn); err != nil {