Skip to content

Commit d1ae3df

Browse files
genezhanggenezhang
committed
docs: Add AGENTS.md module guides for fragile code modules
Create detailed module-level documentation for the two most complex and bug-prone modules: - src/render_plan/AGENTS.md: plan_builder invariants, CTE naming, VLP+WITH JOIN patterns, common bug patterns - src/clickhouse_query_generator/AGENTS.md: VLP CTE generation paths, 10 code paths, multi-type browser expand, alias rewriting rules
1 parent b9b26cc commit d1ae3df

2 files changed

Lines changed: 306 additions & 0 deletions

File tree

‎src/clickhouse_query_generator/AGENTS.md‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
# clickhouse_query_generator Module — Agent Guide
2+
3+
> **Purpose**: Converts `RenderPlan` → ClickHouse SQL string.
4+
> Contains VLP (variable-length path) CTE generation — the most schema-sensitive code.
5+
6+
## Module Architecture
7+
8+
```
9+
RenderPlan (from render_plan)
10+
│
11+
▼
12+
to_sql_query.rs (3.2K) ← Main SQL renderer: SELECT/FROM/JOIN/WHERE/GROUP BY/ORDER BY
13+
│ Also: VLP alias rewriting, denormalized ORDER BY resolution
14+
│
15+
├─ variable_length_cte.rs (3.4K) ← Recursive CTE generator for *1..N path patterns
16+
│ 4 base-case generators × 5 schema variations = complexity
17+
│
18+
├─ multi_type_vlp_joins.rs (1.3K) ← UNION ALL of explicit JOINs for multi-type traversals
19+
│ Used when path crosses node types (User→Post via LIKES)
20+
│
21+
├─ function_translator.rs (952) ← Cypher→ClickHouse function mapping
22+
├─ function_registry.rs (1.2K) ← Function signatures & type info
23+
├─ json_builder.rs (331) ← formatRowNoNewline JSON blob generation
24+
├─ pagerank.rs (387) ← PageRank SQL generation
25+
└─ mod.rs (50) ← Entry point: generate_sql()
26+
```
27+
28+
## variable_length_cte.rs — The Core
29+
30+
### What It Does
31+
Generates `WITH RECURSIVE` CTEs for Cypher patterns like:
32+
```cypher
33+
MATCH (a:User)-[:FOLLOWS*1..3]->(b:User)
34+
MATCH path = (a)--(o) -- browser expand (1-hop, all types)
35+
```
36+
37+
### The Generator Struct
38+
```rust
39+
VariableLengthCteGenerator {
40+
schema: &GraphSchema,
41+
start_node_alias, end_node_alias, // Cypher aliases
42+
rel_type, start_label, end_label, // Type info
43+
min_hops, max_hops, // Range bounds
44+
is_fk_edge: bool, // FK column = edge
45+
start_is_denormalized: bool, // Start node in edge table
46+
end_is_denormalized: bool, // End node in edge table
47+
type_column: Option<String>, // Polymorphic discriminator
48+
shortest_path_mode: Option<...>, // shortestPath optimization
49+
// ... more fields
50+
}
51+
```
52+
53+
### 5 Schema Variations × 2 Cases = 10 Code Paths
54+
55+
| Variation | Base Case | Recursive Case |
56+
|-----------|-----------|----------------|
57+
| **Standard** | 3-way JOIN (start→edge→end) | Recursive JOIN on prev end_id |
58+
| **FK-edge** | 2-way JOIN (node→FK target) | Recursive on FK column |
59+
| **Denormalized** | Single-table scan | Recursive single-table |
60+
| **Mixed denorm** | Hybrid JOIN | Hybrid recursive |
61+
| **Polymorphic** | Standard + WHERE type_column = 'X' | Recursive + type filter |
62+
63+
### Key Functions
64+
65+
```
66+
generate_cte()
67+
└─ generate_recursive_sql()
68+
├─ generate_heterogeneous_polymorphic_sql() // 2-CTE approach
69+
└─ standard path:
70+
├─ generate_base_case() // First hop
71+
└─ generate_recursive_case_with_cte_name() // Subsequent hops
72+
```
73+
74+
### Critical Branching Points
75+
76+
```rust
77+
// These booleans control EVERYTHING:
78+
if self.is_fk_edge {
79+
// No separate edge table — FK column on node table
80+
// JOIN: start_table.fk_col = end_table.id
81+
}
82+
if self.start_is_denormalized {
83+
// Start node properties come from edge table, not node table
84+
// SELECT: edge.start_col AS start_prop (not node.col)
85+
}
86+
if self.type_column.is_some() {
87+
// Polymorphic: add WHERE type_column = 'REL_TYPE'
88+
// Critical: must appear in BOTH base AND recursive case
89+
}
90+
if self.is_heterogeneous_polymorphic_path() {
91+
// Intermediate hops use different type than final hop
92+
// Generates TWO CTEs instead of one recursive CTE
93+
}
94+
```
95+
96+
## multi_type_vlp_joins.rs — Browser Expand
97+
98+
### What It Does
99+
When browser sends `MATCH path = (a)--(o)` (undirected, all types), generates:
100+
```sql
101+
SELECT ... FROM users a JOIN user_follows r ON ... JOIN users u2 ON ...
102+
UNION ALL
103+
SELECT ... FROM users a JOIN post_likes r ON ... JOIN posts p2 ON ...
104+
UNION ALL
105+
SELECT ... FROM users a JOIN posts p2 ON a.user_id = p2.user_id -- FK-edge
106+
```
107+
108+
### Key Function
109+
```
110+
generate_cte_sql(cte_name)
111+
└─ for each path in enumerate_vlp_paths():
112+
generate_path_branch_sql(path, idx)
113+
└─ generate_select_items(node_type, mode)
114+
```
115+
116+
### PropertySelectionMode
117+
```rust
118+
enum PropertySelectionMode {
119+
IdOnly, // Just start_id, end_id
120+
Individual, // Named columns per type
121+
WholeNode, // JSON blob (formatRowNoNewline)
122+
}
123+
```
124+
Browser expand uses `WholeNode` for heterogeneous end nodes (User vs Post).
125+
126+
## to_sql_query.rs — VLP Rewriting
127+
128+
### VLP Alias Rewriting
129+
After CTEs are generated, SELECT items reference Cypher aliases (`a.name`, `o.name`).
130+
These must be rewritten to CTE columns (`t.start_name`, `t.end_properties`).
131+
132+
**Critical detection**:
133+
```rust
134+
// Standard VLP: FROM is the VLP CTE
135+
if from_ref.name.starts_with("vlp_") { ... }
136+
137+
// OPTIONAL VLP: FROM is anchor table, VLP is LEFT JOINed
138+
// Must NOT rewrite anchor node properties!
139+
140+
// WITH+VLP: FROM is VLP CTE, WITH CTE is JOINed
141+
// Must rewrite JOIN column to WITH CTE's actual ID column
142+
```
143+
144+
## Common Bug Patterns
145+
146+
| Pattern | Symptom | Where |
147+
|---------|---------|-------|
148+
| Type filter missing in recursive case | Traverses wrong relationship types | `generate_recursive_case`: polymorphic WHERE |
149+
| FK-edge self-JOIN | Redundant JOIN on same table | `generate_base_case`: `is_fk_edge` + same table |
150+
| Wrong property source | Column not found | `start_is_denormalized` vs node table |
151+
| Heterogeneous path filter loss | Wrong intermediate nodes | `generate_heterogeneous_polymorphic_sql` |
152+
| JSON vs individual columns | Mismatched SELECT in UNION ALL | `PropertySelectionMode` mismatch across branches |
153+
| VLP rewriting on WITH CTE | Overwrites WITH CTE columns | `rewrite_vlp_select_aliases` not checking FROM type |
154+
155+
## Testing After Changes
156+
157+
```bash
158+
# VLP-specific tests:
159+
cargo test variable_length # VLP unit tests
160+
cargo test multi_type_vlp # Multi-type VLP tests
161+
cargo test test_vlp_with_cte # VLP+WITH regression
162+
163+
# Manual: test the browser expand query
164+
curl -X POST localhost:8080/query -H "Content-Type: application/json" \
165+
-d '{"query": "MATCH (a:User) WHERE a.user_id = 1 WITH a, size([(a)--() | 1]) AS c MATCH path = (a)--(o) RETURN path, c LIMIT 10", "sql_only": true}'
166+
167+
# Check: no "a_start_id", must have "a_user_id" in JOIN condition
168+
```
169+
170+
## Schema Variation Checklist
171+
172+
When modifying VLP generation, verify SQL output for:
173+
- [ ] Standard: `MATCH (a:User)-[:FOLLOWS*1..3]->(b:User)`
174+
- [ ] FK-edge: `MATCH (o:Order)-[:PLACED_BY]->(c:Customer)`
175+
- [ ] Denormalized: `MATCH (a:Airport)-[:FLIGHT*1..2]->(b:Airport)`
176+
- [ ] Polymorphic: `MATCH (u:User)-[:FOLLOWS]->(f:User)` on `social_polymorphic` schema
177+
- [ ] Multi-type expand: `MATCH (a:User)--(o)` (browser pattern)
178+
- [ ] Undirected: `MATCH (a:User)-[r]-(b:User)` (UNION ALL both directions)

‎src/render_plan/AGENTS.md‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
# render_plan Module — Agent Guide
2+
3+
> **Purpose**: Converts `LogicalPlan` (Cypher AST) → `RenderPlan` (SQL-ready IR) → SQL string.
4+
> This is where most regressions happen. Read this before touching any file here.
5+
6+
## Module Architecture
7+
8+
```
9+
LogicalPlan (from query_planner)
10+
│
11+
▼
12+
plan_builder.rs ← trait RenderPlanBuilder: LogicalPlan → RenderPlan
13+
│ dispatches to build_chained_with_match_cte_plan for WITH+MATCH
14+
│
15+
├─ plan_builder_utils.rs (12K lines) ← the beast: CTE extraction, expression rewriting,
16+
│ WITH→CTE transformation, VLP+WITH JOIN generation
17+
│
18+
├─ plan_builder_helpers.rs (4.6K) ← schema lookups, property resolution, label fallbacks
19+
│
20+
├─ join_builder.rs (2.5K) ← JOIN clause extraction (standard, FK-edge, polymorphic)
21+
│
22+
├─ cte_extraction.rs (5.5K) ← VLP context building, relationship column extraction
23+
│
24+
├─ cte_generation.rs (815) ← CteGenerationContext: property→column mapping for VLPs
25+
│
26+
├─ cte_manager/mod.rs (3.5K) ← VLP CTE generation & management
27+
│
28+
├─ render_expr.rs (1.2K) ← RenderExpr enum (Column, Literal, Operator, etc.)
29+
│
30+
├─ select_builder.rs, from_builder.rs, filter_builder.rs, group_by_builder.rs
31+
│
32+
└─ mod.rs (531) ← RenderPlan, Join, Cte, SelectItem structs
33+
```
34+
35+
## Critical Invariants
36+
37+
### 1. CTE Column Naming Convention
38+
WITH CTE columns are named `{alias}_{property}`:
39+
- Node alias `a` with property `user_id` → CTE column `a_user_id`
40+
- Node alias `a` with property `name` → CTE column `a_name`
41+
- Scalar alias `allNeighboursCount` → CTE column `allNeighboursCount` (no prefix)
42+
43+
**NEVER** reference VLP internal columns (`start_id`, `end_id`) in WITH CTE schemas.
44+
The WITH CTE uses the node's actual ID column, not the VLP's.
45+
46+
### 2. find_id_column_for_alias() Priority
47+
In `plan_builder.rs`, this function traverses the plan to find a node's ID column.
48+
**Order matters**:
49+
1. First: check GraphNode branches (left/right) for actual node ID (e.g., `user_id`)
50+
2. Fallback: VLP endpoint columns (`start_id`/`end_id`) — only for denormalized schemas
51+
where no separate node table exists
52+
53+
If you reverse this order, WITH CTE JOINs break (the `a_start_id` regression).
54+
55+
### 3. build_chained_with_match_cte_plan Flow
56+
This is the most complex function (~3500 lines in plan_builder_utils.rs):
57+
```
58+
Input: LogicalPlan with WITH clause(s) + subsequent MATCH
59+
Output: RenderPlan with CTEs, JOINs, SELECT rewriting
60+
61+
Steps:
62+
1. Extract correlation predicates BEFORE any transformation
63+
2. Iteratively process WITH clauses (innermost first)
64+
3. For each WITH: render SQL, create CTE, store schema in cte_schemas
65+
4. Replace WITH clause in plan with CTE reference
66+
5. After all WITHs processed: render final plan
67+
6. Add CTE JOINs with correct column references
68+
7. Rewrite SELECT/WHERE/ORDER BY to use CTE column names
69+
```
70+
71+
**Key data structure**: `cte_schemas: HashMap<cte_name, (items, props, alias_to_id, property_mapping)>`
72+
- `alias_to_id`: maps node alias → CTE column name holding its ID
73+
- `property_mapping`: maps (alias, cypher_property) → CTE column name
74+
75+
### 4. VLP+WITH CTE JOIN
76+
When FROM is a VLP CTE and a WITH CTE needs to be JOINed:
77+
```sql
78+
-- VLP CTE has: start_id (= toString(user_id))
79+
-- WITH CTE has: a_user_id (the actual column)
80+
-- JOIN must be: t.start_id = toString(a_allNeighboursCount.a_user_id)
81+
-- NOT: t.start_id = toString(a_allNeighboursCount.a_start_id) ← BUG
82+
```
83+
84+
The `alias_to_id` map is populated by `compute_cte_id_column_for_alias()` which calls
85+
`find_id_column_for_alias()`. See invariant #2 above.
86+
87+
## Common Bug Patterns
88+
89+
| Pattern | Symptom | Root Cause |
90+
|---------|---------|------------|
91+
| `a_start_id` in JOIN | ClickHouse: "cannot be resolved" | `find_id_column_for_alias` VLP shortcut before GraphNode |
92+
| Wrong property mapping | Properties from wrong table | Denormalized vs standard property source confusion |
93+
| Missing CTE columns | Column not found in subquery | `cte_schemas` not populated for this CTE |
94+
| Duplicate CTEs | Same CTE name generated twice | CTE name collision, missing dedup check |
95+
| Passthrough WITH not collapsed | Extra unnecessary CTE layer | `is_passthrough` detection failure |
96+
| CTE name remapping missed | Old CTE name referenced | `cte_name_remaps` not applied to all expressions |
97+
98+
## Files You Should NOT Touch Casually
99+
100+
- **plan_builder_utils.rs** — 12K lines, hundreds of edge cases. Any change here can break
101+
any combination of WITH/VLP/UNION/CTE. Always run full test suite.
102+
- **plan_builder.rs** — The `find_id_column_for_alias` traversal order is critical.
103+
104+
## Testing After Changes
105+
106+
```bash
107+
# Must pass ALL of these:
108+
cargo test # 995 unit + 35 integration + 7 + 25 doc
109+
cargo test test_vlp_with_cte_join # VLP+WITH regression test
110+
cargo test test_with_clause_property_renaming # WITH alias propagation
111+
112+
# Manual browser test (if changing CTE/JOIN logic):
113+
# 1. Start server with schemas/dev/social_dev.yaml
114+
# 2. Connect Neo4j Browser to bolt://localhost:7687
115+
# 3. MATCH (u:User) RETURN u LIMIT 5
116+
# 4. Click a node to expand — must show edges
117+
```
118+
119+
## Schema Variation Awareness
120+
121+
Every function in this module may behave differently based on:
122+
1. **Standard** — separate node + edge tables, 3-way JOIN
123+
2. **FK-edge** — edge is FK column on node table (`is_fk_edge = true`)
124+
3. **Denormalized** — node properties stored in edge table (`is_denormalized = true`)
125+
4. **Polymorphic** — single edge table, `type_column` discriminator
126+
5. **Composite ID** — multi-column node identity
127+
128+
When fixing a bug, always check: does this fix work for ALL 5 variations?

0 commit comments

Comments
 (0)