Skip to content

fix: centralize denormalized property resolution with from/to awareness#225

Merged
genezhang merged 3 commits intomainfrom
fix/denormalized-from-to-property-resolution
Mar 13, 2026
Merged

fix: centralize denormalized property resolution with from/to awareness#225
genezhang merged 3 commits intomainfrom
fix/denormalized-from-to-property-resolution

Conversation

@genezhang
Copy link
Copy Markdown
Owner

Summary

  • Enhanced resolve_denormalized_property_in_expr() in plan_builder_utils.rs to handle both table alias rewriting AND property name mapping, replacing the alias-only approach from PR fix: denormalized node alias rewriting in WITH CTE body #224
  • From/to distinction: a.cityr.OriginCityName (from_node) vs b.cityr.DestCityName (to_node) now correctly resolved using get_properties_with_table_alias() which is aware of the node's position in the relationship
  • Cross-side correction: When rewrite_expression_with_property_mapping maps a property to the wrong side's DB column (e.g., b.cityOriginCityName), the function reverse-lookups through the node schema's from_properties/to_properties to find the original Cypher property name, then resolves it through the correct side's denormalized mapping
  • Also handles CASE expressions, coupled schema aliases, and recursive descent through all expression types
  • Removed 6 xfail markers from now-passing tests (3 denormalized + 3 coupled schema WITH aggregation tests)

Test plan

  • All 1,588 Rust unit tests pass
  • Pattern schema matrix: 0 xpassed (clean), 110 passed, 78 xfailed
  • Pattern matrix: 0 xpassed (clean), 141 passed, 54 xfailed
  • Schema variations comprehensive: 77 passed, 4 xfailed (unchanged)
  • Social benchmark WITH aggregation queries work correctly (no regression)
  • Total 9 tests promoted from xfail to passing (3 from PR fix: denormalized node alias rewriting in WITH CTE body #224 + 6 here)

🤖 Generated with Claude Code

genezhang and others added 3 commits March 12, 2026 23:11
Enhance resolve_denormalized_property_in_expr() to handle both table alias
rewriting AND property name mapping using get_properties_with_table_alias().

Key improvements:
- Property name resolution now correctly distinguishes from_node vs to_node
  (e.g., a.city → r.OriginCityName, b.city → r.DestCityName)
- Cross-side correction: when schema mapping maps to wrong side's column
  (e.g., b.city → OriginCityName), reverse-lookups through node schema's
  from_properties/to_properties to find the correct Cypher property name,
  then resolves through the correct side's denormalized mapping
- Also handles CASE expressions and covers coupled schema aliases

Fixes 6 more integration tests across denormalized and coupled schemas.
Total: 9 tests promoted from xfail to passing (3 from PR #224 + 6 here).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…riants

Address code review findings:
1. Cross-side correction now uses find_label_for_alias_in_plan() to scope
   the schema search to the specific node label, avoiding false matches
   when multiple denormalized node types share DB column names.
2. Added recursive descent for List, MapLiteral, ArraySubscript,
   ArraySlicing, InSubquery, and ReduceExpr variants.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@genezhang genezhang merged commit d0840a3 into main Mar 13, 2026
4 checks passed
@genezhang genezhang deleted the fix/denormalized-from-to-property-resolution branch March 13, 2026 14:55
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

Successfully merging this pull request may close these issues.

1 participant