Skip to content

Conversation

@vroldanbet
Copy link
Contributor

Add support for both PreOrder and PostOrder traversal strategies to the schema walker, while maintaining backward compatibility with existing code.

Design Decision: Options Pattern with WithOptions Functions

Selected Approach: Create new *WithOptions variants for all Walk functions that accept a WalkOptions struct, following the pattern already established in pkg/schema/v2/flatten.go.

Rationale:

  • Backward Compatible: Existing callers don't need any changes
  • Follows Codebase Pattern: Matches FlattenSchema/FlattenSchemaWithOptions pattern in flatten.go
  • Explicit Control: New code can opt into different strategies using WithOptions variants
  • Extensible: Options struct can be extended with future traversal options without breaking APIs
  • Type Safety: No confusing variadic parameters with generics
  • Clean Separation: Strategy is configuration, not part of visitor logic

PreOrder (current behavior):

  1. Visit node first (call visitor method)
  2. If continue flag is true, visit children
  3. Return accumulated value

PostOrder (new behavior):

  1. Visit children first (unconditionally)
  2. Then visit node (call visitor method)
  3. Return accumulated value

Important Notes:

  • In PostOrder, children are visited unconditionally (the continue flag from the visitor is checked after children are already visited)
  • Terminal visitors (those returning (T, error)) work the same in both orders since they have no children
  • Options must be threaded through all recursive calls
  • For terminal nodes (Caveat, BaseRelation), the internal implementation can be simple passthrough since there's no difference between orders

Operation Tree Traversal

The walkOperationWithOptions function is the most complex due to its type-switch structure:

  • For compound operations (Union, Intersection, Exclusion): visit children based on strategy order
  • For terminal operations (RelationReference, ArrowReference, etc.): strategy doesn't affect behavior
  • Must handle both generic OperationVisitor[T] and specific visitors appropriately
  • Double-dispatch for arrow operations: call both ArrowOperationVisitor and specific arrow visitor

Continue Flag Semantics

Continue flag behavior differs between strategies:

  • PreOrder: cont=false means "skip visiting children"
  • PostOrder: Children are already visited before checking the flag, so cont=false has limited effect

Description

Testing

References

Add support for both PreOrder and PostOrder traversal strategies to the schema walker, while maintaining backward compatibility with existing code.

Design Decision: Options Pattern with WithOptions Functions

Selected Approach: Create new *WithOptions variants for all Walk functions that accept a WalkOptions struct, following the pattern already established in pkg/schema/v2/flatten.go.

Rationale:
 - Backward Compatible: Existing callers don't need any changes
 - Follows Codebase Pattern: Matches FlattenSchema/FlattenSchemaWithOptions pattern in flatten.go
 - Explicit Control: New code can opt into different strategies using WithOptions variants
 - Extensible: Options struct can be extended with future traversal options without breaking APIs
 - Type Safety: No confusing variadic parameters with generics
 - Clean Separation: Strategy is configuration, not part of visitor logic

PreOrder (current behavior):
1. Visit node first (call visitor method)
2. If continue flag is true, visit children
3. Return accumulated value

PostOrder (new behavior):
1. Visit children first (unconditionally)
2. Then visit node (call visitor method)
3. Return accumulated value

Important Notes:
- In PostOrder, children are visited unconditionally (the continue flag from the visitor is checked after children are already visited)
- Terminal visitors (those returning (T, error)) work the same in both orders since they have no children
- Options must be threaded through all recursive calls
- For terminal nodes (Caveat, BaseRelation), the internal implementation can be simple passthrough since there's no difference between orders

Operation Tree Traversal

The walkOperationWithOptions function is the most complex due to its type-switch structure:

- For compound operations (Union, Intersection, Exclusion): visit children based on strategy order
- For terminal operations (RelationReference, ArrowReference, etc.): strategy doesn't affect behavior
- Must handle both generic OperationVisitor[T] and specific visitors appropriately
- Double-dispatch for arrow operations: call both ArrowOperationVisitor and specific arrow visitor

Continue Flag Semantics

Continue flag behavior differs between strategies:
- PreOrder: cont=false means "skip visiting children"
- PostOrder: Children are already visited before checking the flag, so cont=false has limited effect
@github-actions github-actions bot added the area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) label Dec 11, 2025
@codecov
Copy link

codecov bot commented Dec 11, 2025

Codecov Report

❌ Patch coverage is 46.84015% with 143 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.50%. Comparing base (12293b4) to head (cd29692).
⚠️ Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
pkg/schema/v2/walk.go 46.85% 123 Missing and 20 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2761      +/-   ##
==========================================
- Coverage   77.62%   77.50%   -0.11%     
==========================================
  Files         471      472       +1     
  Lines       49565    49928     +363     
==========================================
+ Hits        38468    38691     +223     
- Misses       8255     8366     +111     
- Partials     2842     2871      +29     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

for a true post-order traversal, the resolved entity
of a ResolvedRelationReference should be visited first,
rather than calling ResolvedRelationReferenceVisitor,
which represents the node itself
@vroldanbet vroldanbet force-pushed the walker-post-order branch 3 times, most recently from c98c0e5 to 70a79e3 Compare December 24, 2025 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants