Skip to content

refactor(compiler): prepare graph normalized and required-prefix views in one traversal #205

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.

Sign up for GitHub

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

Draft
matthewpeterkort wants to merge 1 commit into
base: feature/postgres-backend
Choose a base branch
from
Draft

refactor(compiler): prepare graph normalized and required-prefix views in one traversal #205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 41 additions & 64 deletions internal/dataframe/compiler/lower/graph_plan.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ func BuildGraphPhysicalPlan(output semantic.OutputPlan, context semantic.Executi
if strings.TrimSpace(context.Project) == "" {
return ir.PhysicalPlan{}, fmt.Errorf("semantic plan project is required")
}
output.Root = normalizeGraphMatchModes(output.Root)
normalizedRoot, requiredPrefixView, traversalCount, hasRequiredPrefix := prepareGraphPlanViews(output.Root)
output.Root = normalizedRoot
if err := semantic.ValidateSemanticGraph(output.Root); err != nil {
return ir.PhysicalPlan{}, err
}
if countGraphTraversals(output.Root) > maxGraphTraversalDeclarations {
if traversalCount > maxGraphTraversalDeclarations {
return ir.PhysicalPlan{}, fmt.Errorf("graph traversal declaration count exceeds %d", maxGraphTraversalDeclarations)
}
if err := validateGraphNode(output.Root, true); err != nil {
Expand Down Expand Up @@ -57,7 +58,7 @@ func BuildGraphPhysicalPlan(output semantic.OutputPlan, context semantic.Executi
if err := appendRootPhysicalFilters(&physical, output.Root); err != nil {
return ir.PhysicalPlan{}, err
}
if err := appendRequiredTraversalMatchFilters(&physical, graphRequiredSemiJoinRoot(output.Root)); err != nil {
if err := appendRequiredTraversalMatchFilters(&physical, requiredPrefixView); err != nil {
return ir.PhysicalPlan{}, err
}

Expand All @@ -70,8 +71,8 @@ func BuildGraphPhysicalPlan(output semantic.OutputPlan, context semantic.Executi
}},
})

pathSets := make([]string, 0, countGraphTraversals(output.Root)+1)
if !graphHasRequiredTraversal(output.Root) {
pathSets := make([]string, 0, traversalCount+1)
if !hasRequiredPrefix {
pathSets = append(pathSets, "path_root")
}
routeOrder := 1
Expand All @@ -97,10 +98,6 @@ func BuildGraphPhysicalPlan(output semantic.OutputPlan, context semantic.Executi
traversal := traversalResult.Traversal
traversal.SourceVariable = sourceVariable
setVariable := fmt.Sprintf("path_%d", index)
matchMode := child.MatchMode
if strings.TrimSpace(string(matchMode)) == "" {
matchMode = spec.TraversalMatchRequired
}
pathSets = append(pathSets, setVariable)
scope := make([]ir.PhysicalOperation, 0, 8+len(child.Filters))
scope = appendProjectScope(scope, []string{edgeVariable, targetVariable}, child.EdgeLabel, child)
Expand All @@ -116,7 +113,7 @@ func BuildGraphPhysicalPlan(output semantic.OutputPlan, context semantic.Executi
Variable: setVariable, SourceVariable: sourcePathSet, SourcePath: nil, Traversal: traversal,
Node: ir.PhysicalPathNode{Alias: graphAlias(child, child.Alias), ResourceType: child.ResourceType, Value: ir.PhysicalValue{Variable: targetVariable}},
Relationship: ir.PhysicalPathRelationship{Alias: child.Alias, LabelBindKey: traversal.EdgeLabelBindKey, FromResourceType: parent.ResourceType, ToResourceType: child.ResourceType},
MatchMode: string(matchMode), RouteOrder: index, Scope: scope,
MatchMode: string(child.MatchMode), RouteOrder: index, Scope: scope,
},
})
if err := walk(child, setVariable); err != nil {
Expand All @@ -135,39 +132,48 @@ func BuildGraphPhysicalPlan(output semantic.OutputPlan, context semantic.Executi
return physical, nil
}

func normalizeGraphMatchModes(node semantic.SemanticNode) semantic.SemanticNode {
copy := node
copy.Children = make([]semantic.SemanticNode, len(node.Children))
for index, child := range node.Children {
childCopy := normalizeGraphMatchModes(child)
if strings.TrimSpace(string(childCopy.MatchMode)) == "" {
childCopy.MatchMode = spec.TraversalMatchRequired
}
copy.Children[index] = childCopy
func prepareGraphPlanViews(root semantic.SemanticNode) (semantic.SemanticNode, semantic.SemanticNode, int, bool) {
normalized := root
requiredPrefixView := root
normalized.Children = make([]semantic.SemanticNode, len(root.Children))
requiredPrefixView.Children = make([]semantic.SemanticNode, len(root.Children))
traversalCount := len(root.Children)
hasRequiredPrefix := false
for index, child := range root.Children {
childNormalized, childRequiredPrefixView, childCount, childHasRequiredPrefix := prepareGraphView(child, true)
normalized.Children[index] = childNormalized
requiredPrefixView.Children[index] = childRequiredPrefixView
traversalCount += childCount
hasRequiredPrefix = hasRequiredPrefix || childHasRequiredPrefix
}
return copy
return normalized, requiredPrefixView, traversalCount, hasRequiredPrefix
}

func graphRequiredSemiJoinRoot(node semantic.SemanticNode) semantic.SemanticNode {
copy := node
copy.Children = make([]semantic.SemanticNode, len(node.Children))
for index, child := range node.Children {
copy.Children[index] = graphRequiredSemiJoinNode(child, true)
func prepareGraphView(node semantic.SemanticNode, ancestorsRequired bool) (semantic.SemanticNode, semantic.SemanticNode, int, bool) {
matchMode := node.MatchMode
if strings.TrimSpace(string(matchMode)) == "" {
matchMode = spec.TraversalMatchRequired
}
return copy
}

func graphRequiredSemiJoinNode(node semantic.SemanticNode, ancestorsRequired bool) semantic.SemanticNode {
copy := node
childAncestorsRequired := ancestorsRequired && matchMode == spec.TraversalMatchRequired
normalized := node
normalized.MatchMode = matchMode
requiredPrefixView := node
requiredPrefixView.MatchMode = matchMode
if !ancestorsRequired {
copy.MatchMode = spec.TraversalMatchOptional
requiredPrefixView.MatchMode = spec.TraversalMatchOptional
}
childAncestorsRequired := ancestorsRequired && node.MatchMode.Required()
copy.Children = make([]semantic.SemanticNode, len(node.Children))
normalized.Children = make([]semantic.SemanticNode, len(node.Children))
requiredPrefixView.Children = make([]semantic.SemanticNode, len(node.Children))
traversalCount := len(node.Children)
hasRequiredPrefix := childAncestorsRequired
for index, child := range node.Children {
copy.Children[index] = graphRequiredSemiJoinNode(child, childAncestorsRequired)
childNormalized, childRequiredPrefixView, childCount, childHasRequiredPrefix := prepareGraphView(child, childAncestorsRequired)
normalized.Children[index] = childNormalized
requiredPrefixView.Children[index] = childRequiredPrefixView
traversalCount += childCount
hasRequiredPrefix = hasRequiredPrefix || childHasRequiredPrefix
}
return copy
return normalized, requiredPrefixView, traversalCount, hasRequiredPrefix
}

func CompileResolvedGraphPlan(resolved semantic.ResolvedRecipePlan, limit int, policy ir.PhysicalOptimizationPolicy) (ir.PhysicalPlan, error) {
Expand All @@ -191,35 +197,6 @@ func graphAlias(node semantic.SemanticNode, fallback string) string {
return fallback
}

func countGraphTraversals(node semantic.SemanticNode) int {
count := len(node.Children)
for _, child := range node.Children {
count += countGraphTraversals(child)
}
return count
}

func graphHasRequiredTraversal(node semantic.SemanticNode) bool {
var walk func(semantic.SemanticNode, bool) bool
walk = func(parent semantic.SemanticNode, requiredPrefix bool) bool {
for _, child := range parent.Children {
mode := child.MatchMode
if strings.TrimSpace(string(mode)) == "" {
mode = spec.TraversalMatchRequired
}
childRequired := mode == spec.TraversalMatchRequired
if requiredPrefix && childRequired {
return true
}
if walk(child, requiredPrefix && childRequired) {
return true
}
}
return false
}
return walk(node, true)
}

func validateGraphNode(node semantic.SemanticNode, root bool) error {
if !root && node.MatchMode != spec.TraversalMatchRequired && node.MatchMode != spec.TraversalMatchOptional && node.MatchMode != "" {
return fmt.Errorf("graph traversal %q has invalid match mode %q", node.Alias, node.MatchMode)
Expand Down
220 changes: 220 additions & 0 deletions internal/dataframe/compiler/lower/graph_plan_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package lower

import (
"fmt"
"reflect"
"sort"
"strings"
"testing"

Expand Down Expand Up @@ -58,3 +61,220 @@ func TestBuildGraphPhysicalPlanRootFallbackOnlyForOptionalBranches(t *testing.T)
}
}
}

func graphPlanTestRoot(children ...semantic.SemanticNode) semantic.SemanticNode {
return semantic.SemanticNode{Alias: "root", ResourceType: "Patient", Children: children}
}

func graphPlanTestCondition(alias string, mode spec.TraversalMatchMode, children ...semantic.SemanticNode) semantic.SemanticNode {
return semantic.SemanticNode{Alias: alias, ResourceType: "Condition", EdgeLabel: "subject_Patient", MatchMode: mode, Children: children}
}

func graphPlanTestObservation(alias string, mode spec.TraversalMatchMode) semantic.SemanticNode {
return semantic.SemanticNode{Alias: alias, ResourceType: "Observation", EdgeLabel: "subject_Patient", MatchMode: mode}
}

func graphPlanTestFocusedObservation(alias string, mode spec.TraversalMatchMode) semantic.SemanticNode {
return semantic.SemanticNode{Alias: alias, ResourceType: "Observation", EdgeLabel: "focus_Condition", MatchMode: mode}
}

func graphPlanTestObservationSiblings(count int, mode spec.TraversalMatchMode) []semantic.SemanticNode {
children := make([]semantic.SemanticNode, 0, count)
for index := 0; index < count; index++ {
children = append(children, graphPlanTestObservation(fmt.Sprintf("observation_%d", index), mode))
}
return children
}

func graphPlanTestBuild(t *testing.T, root semantic.SemanticNode) ir.PhysicalPlan {
t.Helper()
plan, err := BuildGraphPhysicalPlan(semantic.OutputPlan{Root: root}, semantic.ExecutionContext{Project: "p"}, 100, ir.DefaultPhysicalOptimizationPolicy())
if err != nil {
t.Fatalf("BuildGraphPhysicalPlan() error = %v", err)
}
return plan
}

func graphPlanTestPathSets(t *testing.T, plan ir.PhysicalPlan) []string {
t.Helper()
last := plan.Operations[len(plan.Operations)-1]
if last.GraphReturn == nil {
t.Fatalf("final operation = %#v, want graph return", last)
}
return last.GraphReturn.PathSets
}

func graphPlanTestRequiredMatchKeys(plan ir.PhysicalPlan) []string {
keys := []string{}
for key := range plan.BindVars {
if strings.HasPrefix(key, "required_") && strings.HasSuffix(key, "_result") {
keys = append(keys, key)
}
}
sort.Strings(keys)
return keys
}

func graphPlanTestRequiredFiltersPrecedePathSeed(t *testing.T, plan ir.PhysicalPlan) {
t.Helper()
seed := -1
for index, operation := range plan.Operations {
if operation.Kind == ir.PhysicalPathSeedOp {
seed = index
break
}
}
if seed < 0 {
t.Fatal("plan has no path seed operation")
}
for index := seed; index < len(plan.Operations); index++ {
operation := plan.Operations[index]
if operation.Kind == ir.PhysicalFilterOp && operation.Filter != nil && operation.Filter.Expression != nil && operation.Filter.Expression.Kind == ir.PhysicalExistsPredicate {
t.Fatalf("required semi-join filter at %d must precede the path seed at %d", index, seed)
}
}
}

func TestBuildGraphPhysicalPlanOmittedMatchModeDefaultsToRequired(t *testing.T) {
explicit := graphPlanTestBuild(t, graphPlanTestRoot(graphPlanTestCondition("condition", spec.TraversalMatchRequired)))
for _, omitted := range []spec.TraversalMatchMode{"", " "} {
plan := graphPlanTestBuild(t, graphPlanTestRoot(graphPlanTestCondition("condition", omitted)))
if !reflect.DeepEqual(plan, explicit) {
t.Fatalf("omitted match mode %q compiled differently from REQUIRED", omitted)
}
}
if got := graphPlanTestPathSets(t, explicit); !reflect.DeepEqual(got, []string{"path_1"}) {
t.Fatalf("required path sets = %v, want [path_1] without the root fallback", got)
}
for _, operation := range explicit.Operations {
if operation.PathExtend != nil && operation.PathExtend.MatchMode != string(spec.TraversalMatchRequired) {
t.Fatalf("path extend match mode = %q, want REQUIRED", operation.PathExtend.MatchMode)
}
}
}

func TestBuildGraphPhysicalPlanRequiredPrefixPathSensitivity(t *testing.T) {
cases := []struct {
name string
root semantic.SemanticNode
wantPathSets []string
wantRequiredKeys []string
}{
{
name: "required then required",
root: graphPlanTestRoot(graphPlanTestCondition("condition", spec.TraversalMatchRequired, graphPlanTestFocusedObservation("observation", spec.TraversalMatchRequired))),
wantPathSets: []string{"path_1", "path_2"},
wantRequiredKeys: []string{"required_0_result", "required_1_result"},
},
{
name: "required under optional",
root: graphPlanTestRoot(graphPlanTestCondition("condition", spec.TraversalMatchOptional, graphPlanTestFocusedObservation("observation", spec.TraversalMatchRequired))),
wantPathSets: []string{"path_root", "path_1", "path_2"},
wantRequiredKeys: []string{},
},
{
name: "required then optional",
root: graphPlanTestRoot(graphPlanTestCondition("condition", spec.TraversalMatchRequired, graphPlanTestFocusedObservation("observation", spec.TraversalMatchOptional))),
wantPathSets: []string{"path_1", "path_2"},
wantRequiredKeys: []string{"required_0_result"},
},
{
name: "branching required prefix",
root: graphPlanTestRoot(
graphPlanTestCondition("condition", spec.TraversalMatchRequired, graphPlanTestFocusedObservation("observation", spec.TraversalMatchRequired)),
graphPlanTestObservation("screening", spec.TraversalMatchOptional),
),
wantPathSets: []string{"path_1", "path_2", "path_3"},
wantRequiredKeys: []string{"required_0_result", "required_1_result"},
},
}
for _, test := range cases {
t.Run(test.name, func(t *testing.T) {
plan := graphPlanTestBuild(t, test.root)
if got := graphPlanTestPathSets(t, plan); !reflect.DeepEqual(got, test.wantPathSets) {
t.Fatalf("path sets = %v, want %v", got, test.wantPathSets)
}
if got := graphPlanTestRequiredMatchKeys(plan); !reflect.DeepEqual(got, test.wantRequiredKeys) {
t.Fatalf("required semi-join bindings = %v, want %v", got, test.wantRequiredKeys)
}
graphPlanTestRequiredFiltersPrecedePathSeed(t, plan)
})
}
}

func TestBuildGraphPhysicalPlanTraversalDeclarationLimitBoundary(t *testing.T) {
atLimit := graphPlanTestBuild(t, graphPlanTestRoot(graphPlanTestObservationSiblings(32, spec.TraversalMatchOptional)...))
wantAtLimit := []string{"path_root"}
for index := 1; index <= 32; index++ {
wantAtLimit = append(wantAtLimit, fmt.Sprintf("path_%d", index))
}
if got := graphPlanTestPathSets(t, atLimit); !reflect.DeepEqual(got, wantAtLimit) {
t.Fatalf("path sets at the declaration limit = %v, want %v", got, wantAtLimit)
}
_, err := BuildGraphPhysicalPlan(semantic.OutputPlan{Root: graphPlanTestRoot(graphPlanTestObservationSiblings(33, spec.TraversalMatchOptional)...)}, semantic.ExecutionContext{Project: "p"}, 100, ir.DefaultPhysicalOptimizationPolicy())
if err == nil || err.Error() != "graph traversal declaration count exceeds 32" {
t.Fatalf("over-limit error = %v, want the declaration count error", err)
}
shaping := graphPlanTestObservationSiblings(33, spec.TraversalMatchOptional)
shaping[0].Fields = []semantic.SemanticField{{Name: "status"}}
_, err = BuildGraphPhysicalPlan(semantic.OutputPlan{Root: graphPlanTestRoot(shaping...)}, semantic.ExecutionContext{Project: "p"}, 100, ir.DefaultPhysicalOptimizationPolicy())
if err == nil || err.Error() != "graph traversal declaration count exceeds 32" {
t.Fatalf("count check must precede shaping validation, got %v", err)
}
invalid := graphPlanTestObservationSiblings(33, spec.TraversalMatchOptional)
invalid[0].MatchMode = "SMALL"
_, err = BuildGraphPhysicalPlan(semantic.OutputPlan{Root: graphPlanTestRoot(invalid...)}, semantic.ExecutionContext{Project: "p"}, 100, ir.DefaultPhysicalOptimizationPolicy())
if err == nil || !strings.Contains(err.Error(), `unsupported traversal match mode "SMALL"`) {
t.Fatalf("semantic validation must precede the count check, got %v", err)
}
}

func TestBuildGraphPhysicalPlanMatchModeAndShapingValidation(t *testing.T) {
_, err := BuildGraphPhysicalPlan(semantic.OutputPlan{Root: graphPlanTestRoot(graphPlanTestObservation("observation", "SMALL"))}, semantic.ExecutionContext{Project: "p"}, 100, ir.DefaultPhysicalOptimizationPolicy())
if err == nil || err.Error() != `semantic traversal Patient -> Observation (subject_Patient) at Patient.children[0]: unsupported traversal match mode "SMALL"` {
t.Fatalf("invalid match mode error = %v", err)
}
shapingChild := graphPlanTestObservation("observation", spec.TraversalMatchOptional)
shapingChild.Fields = []semantic.SemanticField{{Name: "status"}}
_, err = BuildGraphPhysicalPlan(semantic.OutputPlan{Root: graphPlanTestRoot(shapingChild)}, semantic.ExecutionContext{Project: "p"}, 100, ir.DefaultPhysicalOptimizationPolicy())
if err == nil || err.Error() != `graph traversal "observation" cannot declare dataframe shaping fields` {
t.Fatalf("shaping field error = %v", err)
}
rootMode := graphPlanTestRoot(graphPlanTestObservation("observation", spec.TraversalMatchOptional))
rootMode.MatchMode = spec.TraversalMatchRequired
_, err = BuildGraphPhysicalPlan(semantic.OutputPlan{Root: rootMode}, semantic.ExecutionContext{Project: "p"}, 100, ir.DefaultPhysicalOptimizationPolicy())
if err == nil || err.Error() != `semantic graph root must not declare traversal match mode "REQUIRED"` {
t.Fatalf("root match mode error = %v", err)
}
bothChild := graphPlanTestObservation("observation", "SMALL")
bothChild.Fields = []semantic.SemanticField{{Name: "status"}}
_, err = BuildGraphPhysicalPlan(semantic.OutputPlan{Root: graphPlanTestRoot(bothChild)}, semantic.ExecutionContext{Project: "p"}, 100, ir.DefaultPhysicalOptimizationPolicy())
if err == nil || !strings.Contains(err.Error(), `unsupported traversal match mode "SMALL"`) {
t.Fatalf("semantic validation must precede shaping validation, got %v", err)
}
}

func TestBuildGraphPhysicalPlanLeavesCallerGraphUnmodified(t *testing.T) {
input := graphPlanTestRoot(
graphPlanTestCondition("condition", "", graphPlanTestFocusedObservation("observation", spec.TraversalMatchRequired)),
graphPlanTestObservation("screening", spec.TraversalMatchOptional),
)
template := graphPlanTestRoot(
graphPlanTestCondition("condition", "", graphPlanTestFocusedObservation("observation", spec.TraversalMatchRequired)),
graphPlanTestObservation("screening", spec.TraversalMatchOptional),
)
first := graphPlanTestBuild(t, input)
if !reflect.DeepEqual(input, template) {
t.Fatal("BuildGraphPhysicalPlan mutated the caller-provided graph")
}
second := graphPlanTestBuild(t, input)
if !reflect.DeepEqual(input, template) {
t.Fatal("second BuildGraphPhysicalPlan mutated the caller-provided graph")
}
if !reflect.DeepEqual(first, second) {
t.Fatal("plan output is not deterministic for an identical caller graph")
}
if input.Children[0].MatchMode != "" {
t.Fatalf("omitted match mode was mutated to %q", input.Children[0].MatchMode)
}
}
Loading