fix(cypher): frontend should always emit exclusive kind matchers BED-7495 (#37)

* fix(cypher): frontend should always emit exclusive kind matchers

* fix(pgutil): move nextKindID onto InMemoryKindMapper for determinism

* fix(pgutil): make InMemoryKindMapper.Put idempotent

* chore(query): add test for query.Kind(In) generating inclusive matcher

Author: seanjSO

cypher/frontend/expression.go

@@ -378,6 +378,8 @@ type NonArithmeticOperatorExpressionVisitor struct {
 func (s *NonArithmeticOperatorExpressionVisitor) EnterOC_NodeLabels(ctx *parser.OC_NodeLabelsContext) {
 	s.Expression = &cypher.KindMatcher{
 		Reference: s.Expression,
+		// Cypher-generated `KindMatcher`s should _always_ be exclusive to jive with the spec
+		IsExclusive: true,
 	}
 }
 

cypher/models/cypher/model.go

@@ -25,8 +25,10 @@ func (s AssignmentOperator) String() string {
 	return string(s)
 }
 
-type SyntaxNode any
-type Expression SyntaxNode
+type (
+	SyntaxNode any
+	Expression SyntaxNode
+)
 
 type ExpressionList interface {
 	Add(expression Expression)
@@ -744,12 +746,15 @@ func NewRangeQuantifier(value string) *RangeQuantifier {
 type KindMatcher struct {
 	Reference Expression
 	Kinds     graph.Kinds
+	// IsExclusive changes the kind matching operator from overlap (PG &&) to a stricter "left contains right" (PG @>)
+	IsExclusive bool
 }
 
-func NewKindMatcher(reference Expression, kinds graph.Kinds) *KindMatcher {
+func NewKindMatcher(reference Expression, kinds graph.Kinds, isExclusive bool) *KindMatcher {
 	return &KindMatcher{
-		Reference: reference,
-		Kinds:     kinds,
+		Reference:   reference,
+		Kinds:       kinds,
+		IsExclusive: isExclusive,
 	}
 }
 
@@ -759,8 +764,9 @@ func (s *KindMatcher) copy() *KindMatcher {
 	}
 
 	return &KindMatcher{
-		Reference: Copy(s.Reference),
-		Kinds:     Copy(s.Kinds),
+		Reference:   Copy(s.Reference),
+		Kinds:       Copy(s.Kinds),
+		IsExclusive: s.IsExclusive,
 	}
 }
 

cypher/models/pgsql/test/query_test.go

@@ -0,0 +1,47 @@
+package test
+
+import (
+	"context"
+	"slices"
+	"testing"
+
+	"github.com/specterops/dawgs/cypher/models/cypher"
+	"github.com/specterops/dawgs/cypher/models/pgsql"
+	"github.com/specterops/dawgs/cypher/models/pgsql/translate"
+	"github.com/specterops/dawgs/cypher/models/walk"
+	"github.com/specterops/dawgs/query"
+)
+
+func TestQuery_KindGeneratesInclusiveKindMatcher(t *testing.T) {
+	mapper := newKindMapper()
+
+	queries := []*cypher.Where{
+		query.Where(query.KindIn(query.Node(), NodeKind1)),
+		query.Where(query.Kind(query.Node(), NodeKind2)),
+	}
+
+	for _, nodeQuery := range queries {
+		builder := query.NewBuilderWithCriteria(nodeQuery)
+		builtQuery, err := builder.Build(false)
+		if err != nil {
+			t.Errorf("could not build query: %v", err)
+		}
+
+		translatedQuery, err := translate.Translate(context.Background(), builtQuery, mapper, nil)
+		if err != nil {
+			t.Errorf("could not translate query: %#v: %v", builtQuery, err)
+		}
+
+		walk.PgSQL(translatedQuery.Statement, walk.NewSimpleVisitor(func(node pgsql.SyntaxNode, visitorHandler walk.VisitorHandler) {
+			switch typedNode := node.(type) {
+			case *pgsql.BinaryExpression:
+				switch leftTyped := typedNode.LOperand.(type) {
+				case pgsql.CompoundIdentifier:
+					if slices.Equal(leftTyped, pgsql.AsCompoundIdentifier("n0", "kind_ids")) && typedNode.Operator != pgsql.OperatorPGArrayOverlap {
+						t.Errorf("query did not generate an array overlap operator (&&): %#v", nodeQuery)
+					}
+				}
+			}
+		}))
+	}
+}

cypher/models/pgsql/translate/kind.go

@@ -9,17 +9,30 @@ import (
 	"github.com/specterops/dawgs/cypher/models/pgsql/pgd"
 )
 
-func newPGKindIDMatcher(scope *Scope, treeTranslator *ExpressionTreeTranslator, binding *BoundIdentifier, kindIDs []int16) error {
+func newPGKindIDMatcher(scope *Scope, treeTranslator *ExpressionTreeTranslator, binding *BoundIdentifier, kindIDs []int16, isExclusive bool) error {
 	kindIDsLiteral := pgsql.NewLiteral(kindIDs, pgsql.Int2Array)
 
 	switch binding.DataType {
 	case pgsql.NodeComposite, pgsql.ExpansionRootNode, pgsql.ExpansionTerminalNode:
 		treeTranslator.PushOperand(pgd.Column(binding.Identifier, pgsql.ColumnKindIDs))
 		treeTranslator.PushOperand(kindIDsLiteral)
 
-		return treeTranslator.CompleteBinaryExpression(scope, pgsql.OperatorPGArrayLHSContainsRHS)
+		// In an exclusive kind match, if there are no kind IDs to be matched on,
+		// the behavior of the contains (`@>`) operator will select all nodes, which drastically differs from
+		// the overlap operator's behavior (matches nothing), so preserve the previous behavior.
+		//
+		// There shouldn't be a case in the Cypher frontend where a kind ID matcher is
+		// created without any kind IDs to match on, but `query.Kind`/`query.KindIn` can create those
+		// edge cases. We want any existing `query.Kind`/`query.KindIn` usages to match the previous behavior
+		// expectations by using overlap/`&&`, which protects from the empty RHS problem.
+		if isExclusive && len(kindIDs) > 0 {
+			return treeTranslator.CompleteBinaryExpression(scope, pgsql.OperatorPGArrayLHSContainsRHS)
+		} else {
+			return treeTranslator.CompleteBinaryExpression(scope, pgsql.OperatorPGArrayOverlap)
+		}
 
 	case pgsql.EdgeComposite, pgsql.ExpansionEdge:
+		// Edge kind checking is a strict equality, so the IsExclusive condition does not apply here.
 		treeTranslator.PushOperand(pgsql.CompoundIdentifier{binding.Identifier, pgsql.ColumnKindID})
 		treeTranslator.PushOperand(pgsql.NewAnyExpressionHinted(kindIDsLiteral))
 
@@ -39,6 +52,6 @@ func (s *Translator) translateKindMatcher(kindMatcher *cypher.KindMatcher) error
 	} else if kindIDs, err := s.kindMapper.MapKinds(kindMatcher.Kinds); err != nil {
 		return fmt.Errorf("failed to translate kinds: %w", err)
 	} else {
-		return newPGKindIDMatcher(s.scope, s.treeTranslator, binding, kindIDs)
+		return newPGKindIDMatcher(s.scope, s.treeTranslator, binding, kindIDs, kindMatcher.IsExclusive)
 	}
 }

drivers/pg/pgutil/kindmapper.go

@@ -7,17 +7,17 @@ import (
 	"github.com/specterops/dawgs/graph"
 )
 
-var nextKindID = int16(1)
-
 type InMemoryKindMapper struct {
-	KindToID map[graph.Kind]int16
-	IDToKind map[int16]graph.Kind
+	nextKindID int16
+	KindToID   map[graph.Kind]int16
+	IDToKind   map[int16]graph.Kind
 }
 
 func NewInMemoryKindMapper() *InMemoryKindMapper {
 	return &InMemoryKindMapper{
-		KindToID: map[graph.Kind]int16{},
-		IDToKind: map[int16]graph.Kind{},
+		nextKindID: int16(1),
+		KindToID:   map[graph.Kind]int16{},
+		IDToKind:   map[int16]graph.Kind{},
 	}
 }
 
@@ -67,6 +67,7 @@ func (s *InMemoryKindMapper) mapKinds(kinds graph.Kinds) ([]int16, graph.Kinds)
 
 	return ids, missing
 }
+
 func (s *InMemoryKindMapper) MapKinds(ctx context.Context, kinds graph.Kinds) ([]int16, error) {
 	if ids, missing := s.mapKinds(kinds); len(missing) > 0 {
 		return nil, fmt.Errorf("missing kinds: %v", missing)
@@ -86,8 +87,12 @@ func (s *InMemoryKindMapper) AssertKinds(ctx context.Context, kinds graph.Kinds)
 }
 
 func (s *InMemoryKindMapper) Put(kind graph.Kind) int16 {
-	kindID := nextKindID
-	nextKindID += 1
+	if kindID, ok := s.KindToID[kind]; ok {
+		return kindID
+	}
+
+	kindID := s.nextKindID
+	s.nextKindID += 1
 
 	s.KindToID[kind] = kindID
 	s.IDToKind[kindID] = kind

query/model.go

@@ -10,9 +10,7 @@ import (
 )
 
 func convertCriteria[T any](criteria ...graph.Criteria) []T {
-	var (
-		converted = make([]T, len(criteria))
-	)
+	converted := make([]T, len(criteria))
 
 	for idx, nextCriteria := range criteria {
 		converted[idx] = nextCriteria.(T)
@@ -67,8 +65,9 @@ func DeleteKind(reference graph.Criteria, kind graph.Kind) *cypherModel.Updating
 	return cypherModel.NewUpdatingClause(&cypherModel.Remove{
 		Items: []*cypherModel.RemoveItem{{
 			KindMatcher: &cypherModel.KindMatcher{
-				Reference: reference,
-				Kinds:     graph.Kinds{kind},
+				Reference:   reference,
+				Kinds:       graph.Kinds{kind},
+				IsExclusive: false,
 			},
 		}},
 	})
@@ -78,8 +77,9 @@ func DeleteKinds(reference graph.Criteria, kinds graph.Kinds) *cypherModel.Updat
 	return cypherModel.NewUpdatingClause(&cypherModel.Remove{
 		Items: []*cypherModel.RemoveItem{{
 			KindMatcher: &cypherModel.KindMatcher{
-				Reference: reference,
-				Kinds:     kinds,
+				Reference:   reference,
+				Kinds:       kinds,
+				IsExclusive: false,
 			},
 		}},
 	})
@@ -131,13 +131,14 @@ func DeleteProperties(reference graph.Criteria, propertyNames ...string) *cypher
 
 func Kind(reference graph.Criteria, kinds ...graph.Kind) *cypherModel.KindMatcher {
 	return &cypherModel.KindMatcher{
-		Reference: reference,
-		Kinds:     kinds,
+		Reference:   reference,
+		Kinds:       kinds,
+		IsExclusive: false,
 	}
 }
 
 func KindIn(reference graph.Criteria, kinds ...graph.Kind) *cypherModel.KindMatcher {
-	return cypherModel.NewKindMatcher(reference, kinds)
+	return cypherModel.NewKindMatcher(reference, kinds, false)
 }
 
 func NodeProperty(name string) *cypherModel.PropertyLookup {