diff --git a/.sisyphus/plans/2026-09-07-issue-159-inferred-sequence-nodes.md b/.sisyphus/plans/2026-09-07-issue-159-inferred-sequence-nodes.md new file mode 100644 index 0000000..a4d0ead --- /dev/null +++ b/.sisyphus/plans/2026-09-07-issue-159-inferred-sequence-nodes.md @@ -0,0 +1,208 @@ +# Issue #159: 无 CREATE SEQUENCE 时为 seq.nextval 建 inferred seq* + UsesSequence + +Issue: https://github.com/c2j/codeweb/issues/159 +Branch: feat-issue-159（当前 worktree） + +## 1. 目标 + +`seq.nextval` 被引用但分析范围内无 `CREATE SEQUENCE` DDL 时，当前 builder 静默丢弃 `UsesSequence` 边。改为对齐 `table*` 模式：创建 inferred `seq*` 节点并挂 `UsesSequence` 边，使 `detail` / `impact` 在缺 DDL 时仍能看到真实依赖。 + +**不做的事（issue 明确范围外）**： +- 不改 `sys_dummy`/`dual` 是否出现在 detail 默认 CALLEES +- 不把 sequence 编进 lineage 数据流（`UsesSequence` 已是 Reference） +- 不把 `seq.nextval` 建成 `TableAccess` +- 不动现有人类测试（`procedure_using_nextval_creates_uses_sequence_edge`、`procedure_using_dot_nextval_creates_uses_sequence_edge` 保持只读） + +## 2. 设计决策 + +### 2.1 推测标记：加 `explicit: bool` 字段（而非 `location.is_none()` 哨兵） + +镜像 `Node::Table`/`Node::View` 既有模式（mod.rs L505-532）： + +```rust +/// A database SEQUENCE. +Sequence { + schema: Option, + name: String, + /// true when sequence has a DDL definition (CREATE SEQUENCE), false when + /// only inferred from seq.nextval / currval / setval references. + #[serde(default)] + explicit: bool, + /// None when sequence node was created implicitly (referenced but not parsed from DDL). + #[serde(default)] + location: Option, +}, +``` + +理由： +1. `node_type_tag` 的 `"table*"`/`"view*"` 分支（mod.rs L670-677）可直接复制为 `"seq*"`；`location.is_none()` 方案语义不显式且无先例。 +2. 无论选哪种方案，`location` 都必须改成 `Option`（推测节点无 DDL 文件可指）。 +3. 与 Table/View 在 `is_inferred_node`（main.rs L2036）、export、merge 等处的处理方式保持一致。 + +### 2.2 Store 版本：`STORE_VERSION` 8 → 9 + +`Node::Sequence` 变体形状变化会破坏 bincode 位置式反序列化。仓库已有版本门禁机制（store.rs L1172 `stored_ver != STORE_VERSION` → 报错；L1225 `peek_version` → analyze fast path 强制重建，见 commit 0b636ac）。因此： + +- `STORE_VERSION: u32 = 8` → `9`（store.rs L22） +- 「旧 store 可加载」验收 = 旧版本 store 被检测为过期并触发重建，不 panic、不死循环（沿用 0b636ac 的既有路径，已有测试覆盖） +- store.rs L2325-2342 附近的版本测试使用 `STORE_VERSION` 常量，自动跟随 + +## 3. TDD 步骤 + +### 3.1 Red — 新建测试（全部先写，确认失败/编译失败） + +**单元测试**（`src/graph/builder.rs` `#[cfg(test)] mod tests`，复用现有 `build_from_sql` helper）： + +1. `procedure_using_nextval_without_ddl_creates_inferred_sequence_node` + - SQL: 仅 `CREATE PROCEDURE`（内含 `SELECT nextval('seq_batch_payment') INTO v FROM sys_dummy`），无 CREATE SEQUENCE + - 断言: 恰好 1 条 `UsesSequence` 边；目标节点是 `Node::Sequence { explicit: false, location: None, .. }` +2. `select_dot_nextval_into_from_sys_dummy_creates_edge_with_ddl` + - SQL: `CREATE SEQUENCE seq_batch_payment` + `SELECT seq_batch_payment.nextval INTO v FROM sys_dummy` + - 断言: 恰好 1 条 `UsesSequence` 边（**不重复**）；目标节点 `explicit: true` +3. `dot_nextval_assignment_without_ddl_creates_inferred_sequence_node` + - SQL: `v_id := my_seq.NEXTVAL` 赋值，无 DDL + - 断言: 1 条边 + inferred 节点 +4. `insert_values_nextval_without_ddl_creates_inferred_sequence_node` + - SQL: `INSERT INTO t(id) VALUES(my_seq.NEXTVAL)`，无 DDL + - 断言: 1 条边 + inferred 节点 +5. `inferred_sequence_schema_qualified_ref_resolves`（schema 回退） + - SQL: `SELECT s1.my_seq.nextval INTO v FROM sys_dummy`，无 DDL + - 断言: 1 条边 + inferred 节点名为 `my_seq` + +**tag/显示测试**（`src/graph/mod.rs` tests）： + +6. `node_type_tag_inferred_sequence_is_seq_star` + - `Node::Sequence { explicit: false, location: None, .. }` → `"seq*"`；`explicit: true` → `"seq"` + +**store 版本测试**（`src/graph/store.rs` tests）： + +7. `load_bincode_rejects_pre_issue_159_version`（完全跟随既有 `load_bincode_rejects_previous_layout_version`（约 L2348，version=7 场景）的模式） + - 构造字节：`STORE_MAGIC` + `8u32.to_le_bytes()`（本次改动淘汰的旧版本）+ 8 字节占位 + - 断言 1：`GraphStore::load_bincode(&path)` 返回 err，错误信息包含 `"unsupported cache version"` + - 断言 2：`GraphStore::file_is_current(&path) == false`（store.rs L1242 —— 这是 `Project::store_is_current()`（src/project/mod.rs L540）在 bincode 格式下调用的真实入口，即 analyze 增量快速路径判定"过期需重建"的依据） + +**集成回归测试**（`tests/regress_issue_159_sequence_inferred.rs`，跟随 regress_issue_140/144 先例）： + +8. 端到端：构建项目（仅 SELECT + sys_dummy，无 DDL）→ store 落盘 → `resolve`/detail 路径能看到 `seq_batch_payment` 节点与 `UsesSequence` 边；再跑一次 analyze（增量路径）不重复建边。 + +### 3.2 Green — 最小实现 + +**`src/graph/mod.rs`**： +- `Node::Sequence` 变体：加 `#[serde(default)] explicit: bool`、`location` → `#[serde(default)] Option` +- `node_type_tag`：`Sequence { explicit: false, .. } => "seq*"`（L681 拆成两臂） +- `Node::file()` L870：`&location.file` → 按 Table 模式（L859-866）`location.as_ref().map(...).unwrap_or(Path::new(""))` + +**`src/graph/builder.rs`**： +- L651 `CREATE SEQUENCE` 构造：`explicit: true, location: Some(...)` +- `create_object_ref_edges`（L1785-1800 proc、L1856-1871 func）与 `collect_package_object_ref_edges`（L1964）三处： + - lookup key 逻辑对齐表路径：`seq_ref.sequence_name` 含 `.`（schema 限定）→ 用全名 key 查，miss 再退短名；无前缀 → 短名查 + - miss 时：`graph.add_node(Node::Sequence { schema, name, explicit: false, location: None })` 并建 `UsesSequence` 边 + - 用函数内局部 `HashMap` 缓存本次调用已建的 inferred 节点（不修改 `sequence_index` 签名，避免 &mut 传染） + - 抽一个共享 helper（如 `fn resolve_or_infer_sequence(...) -> NodeIndex`）供三处调用，避免复制三遍 + +**`src/main.rs`**： +- `is_inferred_node`（L2036）：加 `Node::Sequence { explicit: false, .. }` → detail 自动打印 `⚠ inferred node`（L2290 既有路径，不改） + +**`src/export/json.rs`**（两处，精确形状）： +- `NodeKindJson::Sequence` 定义（L172-177）改为与 `NodeKindJson::Table` 完全一致的 Option 语义并补 `explicit`： + ```rust + Sequence { + name: String, + schema: Option, + explicit: bool, + file: Option, // None = inferred 节点（对齐 Table L434-458 的 JSON 形状） + line: Option, + }, + ``` +- `Node::Sequence` → `NodeJson` match 臂（L527-539）改为 Table 臂同款写法： + `file: location.as_ref().map(|l| l.file.to_string_lossy().to_string())`、`line: location.as_ref().map(|l| l.line)`、`explicit: *explicit` +- JSON 消费方无内部引用（server 静态资源、mcp、tui 均不解析 `NodeKindJson::Sequence`），Option 化仅影响对外 API 输出，与 Table/View 的既有输出惯例一致 + +**`src/import/parser.rs`** L420： +- CGEF sequence 节点：`explicit: true`（外部导入即有定义） + +**`src/graph/store.rs`**： +- `STORE_VERSION` 8 → 9 + +**构造点补字段**（编译器兜底，机械改动）： +- `src/graph/mod.rs` tests L1173、L1575：加 `explicit: true` + `location: Some(loc)`（测试代码，本任务可改） + +### 3.3 Refactor + +- 三处 miss 分支收敛到共享 helper 后，若 proc/func 两处外层循环结构仍重复，仅在当前改动路径内做小范围提取；不做超出路径的重构 +- 重构后立刻重跑同一组测试 + +## 4. 验收映射 + +| Issue 验收项 | 对应测试 | +|---|---| +| 无 DDL 时 detail CALLEES 出现 `seq_batch_payment [uses_seq]` | 单测 1 + 集成 8 | +| 有 DDL 时仍一条边、explicit、不重复 | 单测 2 | +| `SELECT seq.nextval INTO v FROM sys_dummy` 回归（抽取 + 有/无 DDL） | 单测 1、2 | +| `nextval('seq')` / 赋值 / `INSERT VALUES` 无 DDL 建 inferred 边 | 单测 3、4 | +| 旧 store 可加载 | store 版本门禁重建（决策 2.2）+ 测试 7 | + +## 5. 每任务 QA 场景（工具 + 步骤 + 预期结果） + +### QA-A 新增单元测试（Red 阶段） + +| 步骤 | 命令 | 预期 | +|---|---|---| +| A1 | `cargo test --features full procedure_using_nextval_without_ddl_creates_inferred_sequence_node 2>&1 \| tail -5` | **编译失败**（`Node::Sequence` 无 `explicit` 字段 / `location` 非 Option）—— 合法 Red | +| A2 | `cargo test --features full node_type_tag_inferred_sequence_is_seq_star` | 同上，编译失败 | +| A3 | `cargo test --features full load_bincode_rejects_pre_issue_159_version` | **断言失败**（当前 STORE_VERSION=8，version=8 的文件被接受）—— 合法 Red；先改 `STORE_VERSION=9` 后此测试即绿，作为 2.2 的验证 | +| A4 | `cargo test --features full --test regress_issue_159_sequence_inferred` | Red：编译失败或断言失败 | + +### QA-B 最小实现（Green 阶段） + +| 步骤 | 命令 | 预期 | +|---|---|---| +| B1 | `cargo build --features full` | 退出码 0；编译器逐个暴露所有 `Node::Sequence` 构造点 / exhaustive match 漏改处（mod.rs tests L1173/L1575、json.rs、import/parser.rs、builder.rs L651） | +| B2 | 重跑 QA-A 全部 4 条命令 | 全部 pass（0 failed） | +| B3 | `cargo test --features full procedure_using_nextval_creates_uses_sequence_edge` 与 `cargo test --features full procedure_using_dot_nextval_creates_uses_sequence_edge`（**两条独立命令**，`cargo test` 只接受一个 TESTNAME 过滤参数） | 既有 2 测试各自 pass（未改人类测试，DDL 存在路径行为不变） | + +### QA-C store 版本与增量回归 + +| 步骤 | 命令 | 预期 | +|---|---|---| +| C1 | `cargo test --features full -- --skip test_path_mapping_applied --skip test_serve_ store` | store 模块全部测试 pass，含既有 `load_bincode_rejects_previous_layout_version`（version=7 仍被拒） | +| C2 | 集成测试 8（`tests/regress_issue_159_sequence_inferred.rs` 内）：先以旧格式落盘（或手写 version=8 头文件），再调用 `GraphStore::file_is_current` → false；随后正常 `analyze` 全量重建 → `file_is_current` → true | 断言通过 = 「旧 store 可加载（触发重建、不 panic、不死循环）」 | + +### QA-D CLI 手工验收（issue 实测场景） + +在预授权临时目录 `/var/folders/xh/8xyzggmj4jg02gnjyxwwbnb00000gn/T/opencode/issue159` 建项目： + +```bash +TMP=/var/folders/xh/8xyzggmj4jg02gnjyxwwbnb00000gn/T/opencode/issue159 +mkdir -p $TMP/sql +printf 'CREATE PROCEDURE p_pay() AS $$\nBEGIN\n SELECT seq_batch_payment.nextval INTO v_seq FROM sys_dummy;\nEND;\n$$ LANGUAGE plpgsql;\n' > $TMP/sql/p.sql +cargo run -q --features cli -- init $TMP/demo -d $TMP/sql +cargo run -q --features cli -- detail p_pay -p $TMP/demo +cargo run -q --features cli -- export --format json -p $TMP/demo +``` + +（所有子命令显式 `-p $TMP/demo`：各子命令的 `--project` 默认是当前目录，`cargo run` 在仓库根执行时会找不到 `$TMP/demo` 的 codeweb.toml。） + +预期输出： +- `detail` 的 CALLEES 区出现 `seq:seq_batch_payment [seq*] [uses_seq]`（无 DDL 场景） +- 追加 `CREATE SEQUENCE seq_batch_payment;` 到 `$TMP/sql/p.sql` 后重新 `analyze -p $TMP/demo`，`detail` 仍只显示一条 uses_seq 边，tag 变为 `seq`（非 `seq*`），无重复行 +- `export --format json` 输出中该 sequence 节点含 `"explicit": false`（无 DDL）/ `true`（有 DDL） + +（QA-D 为人工抽查；CI 依赖 QA-A~C 的自动化断言。） + +## 6. 门禁（与 CI 一致） + +```bash +cargo fmt --all -- --check +cargo clippy --features full -- -D warnings +cargo test --features full -- --skip test_path_mapping_applied --skip test_serve_ +``` + +另跑 `cargo build --features full`（cross-feature 回归；Node 变体变化可能影响 server/mcp 匹配臂）。 + +## 7. 风险与边界 + +- `Node::file()` L870 若漏改会在 detail/文件列表对 inferred seq 节点时 panic —— 单测 6 覆盖 `file()` 行为 +- export/json.rs 的 None-location 渲染已在 3.2 固化为与 `NodeKindJson::Table` 完全一致的 Option 形状（`file: Option`、`line: Option` + `explicit`），无歧义空间 +- `--features full` 下 jsp/server/mcp 对 `Node::Sequence` 的 exhaustive match 由编译器强制检查 +- 既有环境性失败（`test_path_mapping_applied`、`test_serve_*`）按 AGENTS.md 跳过，不算本次回归 diff --git a/.sisyphus/plans/2026-09-08-pr160-inferred-sequence-cross-chunk-fix.md b/.sisyphus/plans/2026-09-08-pr160-inferred-sequence-cross-chunk-fix.md new file mode 100644 index 0000000..c71452a --- /dev/null +++ b/.sisyphus/plans/2026-09-08-pr160-inferred-sequence-cross-chunk-fix.md @@ -0,0 +1,116 @@ +# PR #160 Review 修复：inferred sequence 跨 chunk 重复与升级缺失 + +PR: https://github.com/c2j/codeweb/pull/160 +Review: c2j 的 [bug] 评论（inferred_sequence_index 不跨 chunk、CREATE SEQUENCE 不升级、无 dedup 兜底、测试盲区） +分支：feat/issue-159（追加 commit，不改写已推送历史之外的本次修复 commit） + +## 1. 根因（已逐条代码验证） + +1. `create_object_ref_edges`（builder.rs L1740）内 `inferred_sequence_index` 是函数局部 HashMap，每 chunk 重建；而 `Project::analyze` 按 ≤100 文件/chunk 循环 `build_sql_chunk(&mut ctx, ...)`（project/mod.rs L208-233，ctx 跨 chunk 共享） +2. `CREATE SEQUENCE` 处理（builder.rs L646-663）只查 `sequence_index.contains_key(&full_key)`；inferred 节点从不进入 `sequence_index` → 后续 chunk 的 DDL 走 `add_node` 造出兄弟 explicit 节点，不原位升级 +3. 兜底缺失：`finalize_graph`（L311）无 sequence 去重；`pick_richer_node`（store.rs L1775）无 Sequence 臂 +4. 触发条件：同一序列「引用 chunk 在前、DDL chunk 在后」（文件字母序使存过先于 DDL 是常态）；单 chunk 内 DDL pass 先于 inference pass 所以安全——正是测试盲区成因 + +## 2. 修复设计（4 处代码改动） + +### 2.1 `GraphBuildContext` 增加兄弟索引（builder.rs L127-160） + +```rust +pub inferred_sequence_index: HashMap, +``` + +- `new()` 初始化（唯一构造点；`build_graph_internal` 与 project/mod.rs 均走 `GraphBuildContext::new()`，无其他改动） +- 选择兄弟 map 而非直接写入 `sequence_index`：保持「DDL 纯索引」语义，消费方无需 `explicit` 判别；升级时从兄弟 map 移除并写入 `sequence_index` + +### 2.2 `create_object_ref_edges` 使用 ctx 级缓存（L1733-1740） + +- 签名追加 `inferred_sequence_index: &mut HashMap`（调用点 L300-306 传 `&mut ctx.inferred_sequence_index`；`collect_package_object_ref_edges` 的 `&mut` 透传保持不变） +- 删除 L1740 的局部 `HashMap::new()` + +### 2.3 `CREATE SEQUENCE` 原位升级（L646-663） + +镜像 `resolve_or_infer_sequence` 的**精确 key 纪律**（杜绝短名模糊匹配重新引入跨 schema 误绑定）： + +```text +若 sequence_index 含 full_key：跳过（现状不变） +否则： + promoted = schema.is_some() + ? inferred_sequence_index.remove(full_key) // 限定 DDL 只升级限定推测节点 + : inferred_sequence_index.remove(short_key) // 无前缀 DDL 只升级无前缀推测节点 + 命中 → 原位改写该节点：explicit = true, location = Some(DDL 位置) + 并 sequence_index.entry(short_key).or_insert(idx) + insert(full_key, idx) + （petgraph 权重原位改写不改 NodeIndex，既有 UsesSequence 边自动指向升级后节点） + 未命中 → 现状 add_node 路径不变 +``` + +- `create_sql_nodes` 签名追加 `inferred_sequence_index: &mut HashMap`（调用点同步） +- 升级分支加必要注释说明精确 key 纪律（非显而易见的不变量，防止未来重构回退） + +### 2.4 `pick_richer_node` 加 Sequence 臂（store.rs L1775，dedup/merge 兜底） + +```rust +(Node::Sequence { location: Some(_), .. }, Node::Sequence { location: None, .. }) => idx_a, +(Node::Sequence { location: None, .. }, Node::Sequence { location: Some(_), .. }) => idx_b, +``` + +镜像既有 Table 臂的 location 风格。既有 View 臂缺失属 pre-existing，不扩大范围。 + +### 2.5 明确不做 + +- 不新增 finalize 序列去重 pass（构建期已防重 + merge 期 pick_richer_node 兜底即可，避免过度工程） +- 不 bump `STORE_VERSION`（无序列化形状变更；旧 store 合法。含历史双节点的新构建产物由 `codeweb dedup` 清理——PR 回评说明） +- 残余歧义（chunk1 无前缀推测 `seq_id` + chunk2 限定 `CREATE SEQUENCE finance.seq_id` → key 不同不升级、双节点保留）记录于 PR 回评，不引入 finalize 重建 pass + +## 3. TDD 步骤 + +### 3.1 Red — 新测试（全部先写，确认失败） + +**builder.rs tests（两 chunk 风格，仿 L5464：共享 `GraphBuildContext` + 多次 `build_sql_chunk` + `finalize_graph`）**： + +1. `two_chunk_reference_then_ddl_promotes_inferred_sequence` + - chunk1：存过引用 `my_seq.NEXTVAL`（无 DDL）；chunk2：`CREATE SEQUENCE my_seq;` + - 断言：恰好 1 个 Sequence 节点；`explicit == true`；`location.is_some()`；UsesSequence 边指向该节点（升级不改 NodeIndex，边必须存活） +2. `two_chunk_duplicate_references_share_single_inferred_sequence` + - chunk1：存过 A 引用 `my_seq`；chunk2：存过 B 引用 `my_seq`（均无 DDL） + - 断言：1 个 Sequence 节点（explicit: false），2 条边指向同一 NodeIndex +3. `ddl_does_not_promote_other_schema_inferred_sequence` + - chunk1：引用 `hr.seq_id.NEXTVAL`；chunk2：`CREATE SEQUENCE finance.seq_id` + - 断言：2 个不同节点（hr.seq_id explicit:false；finance.seq_id explicit:true）；边仍指向 hr.seq_id + +**store.rs tests**： + +4. `pick_richer_node_prefers_located_sequence`（同模块直测私有 fn，仿既有 pick_richer_node 测试风格） + - `Node::Sequence { location: Some, .. }` vs `{ location: None, .. }` → 返回 Some 侧 idx + +### 3.2 Green — 按 2.1→2.2→2.3→2.4 顺序实施 + +### 3.3 Refactor + +无（改动本身即收敛）；重构后重跑同组测试。 + +## 4. QA 场景 + +| 步骤 | 命令 | 预期 | +|---|---|---| +| R1 | `cargo test --features full two_chunk_` | 3 个新两 chunk 测试 Red（当前断言失败：节点数 2） | +| R2 | `cargo test --features full pick_richer_node_prefers_located_sequence` | Red（无 Sequence 臂，返回 idx_a） | +| G1 | `cargo build --features full` | exit 0 | +| G2 | 重跑 R1、R2 | 全部 pass | +| G3 | `cargo test --features full procedure_using` | 既有测试 pass | +| G4 | `cargo test --features full --test regress_issue_159_sequence_inferred` | 既有集成回归 pass | +| G5 | 两 chunk 手工验证（可选）：临时项目 >100 文件或直接调 `sql_chunk_size` 配置构造跨 chunk 场景，analyze 后 `nodes -t seq` 仅 1 节点 | 与单测一致 | + +## 5. 门禁 + +```bash +cargo fmt --all -- --check +cargo clippy --features full -- -D warnings +cargo test --features full -- --skip test_path_mapping_applied --skip test_serve_ +``` + +## 6. 风险与边界 + +- 借用检查：`create_object_ref_edges` 调用点同时取 `&ctx.sequence_index` 与 `&mut ctx.inferred_sequence_index`——不相交字段借用，合法 +- 升级路径的 `graph[idx]` 原位改写：`CodeGraph = petgraph::Graph` 支持 `IndexMut`；不改 NodeIndex，既有边零迁移 +- 测试 1 的边存活断言是升级正确性的关键证据（若实现误删节点重建会在此失败） +- 既有全部测试（含 issue #159 的 10 个）必须保持通过——特别是 `ddl_does_not_promote_other_schema_inferred_sequence` 守护跨 schema 纪律 diff --git a/src/export/json.rs b/src/export/json.rs index 3cc4aa3..00cb0ff 100644 --- a/src/export/json.rs +++ b/src/export/json.rs @@ -172,8 +172,12 @@ enum NodeKindJson { Sequence { name: String, schema: Option, - file: String, - line: usize, + #[serde(skip_serializing_if = "is_false")] + explicit: bool, + #[serde(skip_serializing_if = "Option::is_none")] + file: Option, + #[serde(skip_serializing_if = "Option::is_none")] + line: Option, }, Index { name: Option, @@ -527,14 +531,18 @@ pub fn to_json(graph: &CodeGraph) -> Result { Node::Sequence { schema, name, + explicit, location, } => NodeJson { id: idx.index(), kind: NodeKindJson::Sequence { name: name.clone(), schema: schema.clone(), - file: location.file.to_string_lossy().to_string(), - line: location.line, + explicit: *explicit, + file: location + .as_ref() + .map(|l| l.file.to_string_lossy().to_string()), + line: location.as_ref().map(|l| l.line), }, }, Node::Index { diff --git a/src/graph/builder.rs b/src/graph/builder.rs index 44daeb3..7feb32c 100644 --- a/src/graph/builder.rs +++ b/src/graph/builder.rs @@ -129,6 +129,7 @@ pub struct GraphBuildContext { pub table_index: HashMap, pub type_index: HashMap, pub sequence_index: HashMap, + pub inferred_sequence_index: HashMap, /// Shared dedup index for BuiltinFunction nodes (keyed by lowercased name). /// Threaded through SQL-proc / XML-mapper / Java / JSP paths so the same /// builtin called from multiple paths is a single graph node. @@ -149,6 +150,7 @@ impl GraphBuildContext { table_index: HashMap::new(), type_index: HashMap::new(), sequence_index: HashMap::new(), + inferred_sequence_index: HashMap::new(), builtin_index: HashMap::new(), deferred_column_comments: Vec::new(), } @@ -287,6 +289,7 @@ impl GraphBuilder { &mut ctx.table_index, &mut ctx.type_index, &mut ctx.sequence_index, + &mut ctx.inferred_sequence_index, &mut ctx.deferred_column_comments, ); Self::create_sql_edges( @@ -303,6 +306,7 @@ impl GraphBuilder { &ctx.proc_index, &ctx.type_index, &ctx.sequence_index, + &mut ctx.inferred_sequence_index, ); } @@ -328,6 +332,7 @@ impl GraphBuilder { table_index: &mut HashMap, type_index: &mut HashMap, sequence_index: &mut HashMap, + inferred_sequence_index: &mut HashMap, deferred_column_comments: &mut Vec, ) { for file in files { @@ -648,15 +653,36 @@ impl GraphBuilder { let short_key = normalize_object_key(None, &name); let full_key = normalize_object_key(schema.as_deref(), &name); if !sequence_index.contains_key(&full_key) { - let seq_node = Node::Sequence { - schema: schema.as_ref().map(|s| s.to_lowercase()), - name: name.to_lowercase(), - location: SourceLocation { - file: file_arc.clone(), - line: info.start_line, - }, + // Promote only an exact inferred key; short-name fuzzing here + // could incorrectly bind sequences from different schemas. + let promoted = if schema.is_some() { + inferred_sequence_index.remove(&full_key) + } else { + inferred_sequence_index.remove(&short_key) + }; + let idx = if let Some(idx) = promoted { + if let Node::Sequence { + explicit, location, .. + } = &mut graph[idx] + { + *explicit = true; + *location = Some(SourceLocation { + file: file_arc.clone(), + line: info.start_line, + }); + } + idx + } else { + graph.add_node(Node::Sequence { + schema: schema.as_ref().map(|s| s.to_lowercase()), + name: name.to_lowercase(), + explicit: true, + location: Some(SourceLocation { + file: file_arc.clone(), + line: info.start_line, + }), + }) }; - let idx = graph.add_node(seq_node); sequence_index.entry(short_key).or_insert(idx); sequence_index.insert(full_key, idx); } @@ -1735,6 +1761,7 @@ impl GraphBuilder { proc_index: &HashMap, type_index: &HashMap, sequence_index: &HashMap, + inferred_sequence_index: &mut HashMap, ) { for file in files { let file_arc: Arc = Arc::new(file.path.clone()); @@ -1783,20 +1810,22 @@ impl GraphBuilder { } } for seq_ref in &extractor.sequence_refs { - if let Some(&seq_idx) = - sequence_index.get(&seq_ref.sequence_name.to_lowercase()) - { - graph.add_edge( - proc_idx, - seq_idx, - Edge::UsesSequence { - location: SourceLocation { - file: file_arc.clone(), - line: info.start_line, - }, + let seq_idx = Self::resolve_or_infer_sequence( + &seq_ref.sequence_name, + sequence_index, + inferred_sequence_index, + graph, + ); + graph.add_edge( + proc_idx, + seq_idx, + Edge::UsesSequence { + location: SourceLocation { + file: file_arc.clone(), + line: info.start_line, }, - ); - } + }, + ); } } } @@ -1854,20 +1883,22 @@ impl GraphBuilder { } } for seq_ref in &extractor.sequence_refs { - if let Some(&seq_idx) = - sequence_index.get(&seq_ref.sequence_name.to_lowercase()) - { - graph.add_edge( - proc_idx, - seq_idx, - Edge::UsesSequence { - location: SourceLocation { - file: file_arc.clone(), - line: info.start_line, - }, + let seq_idx = Self::resolve_or_infer_sequence( + &seq_ref.sequence_name, + sequence_index, + inferred_sequence_index, + graph, + ); + graph.add_edge( + proc_idx, + seq_idx, + Edge::UsesSequence { + location: SourceLocation { + file: file_arc.clone(), + line: info.start_line, }, - ); - } + }, + ); } } } @@ -1880,6 +1911,7 @@ impl GraphBuilder { proc_index, type_index, sequence_index, + inferred_sequence_index, graph, ); } @@ -1892,6 +1924,7 @@ impl GraphBuilder { proc_index, type_index, sequence_index, + inferred_sequence_index, graph, ); } @@ -1910,6 +1943,7 @@ impl GraphBuilder { proc_index: &HashMap, type_index: &HashMap, sequence_index: &HashMap, + inferred_sequence_index: &mut HashMap, graph: &mut CodeGraph, ) { let pkg_name_part = pkg_name.last().cloned().unwrap_or_default().to_string(); @@ -1961,22 +1995,68 @@ impl GraphBuilder { } } for seq_ref in &extractor.sequence_refs { - if let Some(&seq_idx) = sequence_index.get(&seq_ref.sequence_name.to_lowercase()) { - graph.add_edge( - proc_idx, - seq_idx, - Edge::UsesSequence { - location: SourceLocation { - file: file_path.clone(), - line: info.start_line, - }, + let seq_idx = Self::resolve_or_infer_sequence( + &seq_ref.sequence_name, + sequence_index, + inferred_sequence_index, + graph, + ); + graph.add_edge( + proc_idx, + seq_idx, + Edge::UsesSequence { + location: SourceLocation { + file: file_path.clone(), + line: info.start_line, }, - ); - } + }, + ); } } } + fn resolve_or_infer_sequence( + sequence_name: &str, + sequence_index: &HashMap, + inferred_sequence_index: &mut HashMap, + graph: &mut CodeGraph, + ) -> petgraph::graph::NodeIndex { + let normalized = sequence_name.to_lowercase(); + let (schema, name) = normalized + .rsplit_once('.') + .map_or((None, normalized.as_str()), |(schema, name)| { + (Some(schema), name) + }); + let full_key = normalize_object_key(schema, name); + let short_key = normalize_object_key(None, name); + + // Schema-qualified references must not fall back to the short-name + // alias: `hr.seq_id.nextval` is a different object from + // `finance.seq_id` even though both share the short name `seq_id`. + // Only unqualified references resolve through the short-name key + // (which equals `full_key` when `schema` is `None`). + let (lookup_key, insert_key) = if schema.is_some() { + (full_key.clone(), full_key) + } else { + (short_key.clone(), short_key) + }; + if let Some(&idx) = sequence_index + .get(&lookup_key) + .or_else(|| inferred_sequence_index.get(&lookup_key)) + { + return idx; + } + + let idx = graph.add_node(Node::Sequence { + schema: schema.map(str::to_string), + name: name.to_string(), + explicit: false, + location: None, + }); + inferred_sequence_index.insert(insert_key, idx); + idx + } + fn collect_package_call_edges( pkg_name: &ogsql_parser::ast::ObjectName, pkg_items: &[PackageItem], @@ -4900,6 +4980,372 @@ mod tests { ); } + fn assert_single_inferred_sequence(graph: &crate::graph::CodeGraph, expected_name: &str) { + let seq_edges: Vec<_> = graph + .edge_indices() + .filter(|e| matches!(&graph[*e], Edge::UsesSequence { .. })) + .collect(); + assert_eq!(seq_edges.len(), 1, "Expected exactly 1 UsesSequence edge"); + let (_, target) = graph.edge_endpoints(seq_edges[0]).unwrap(); + assert!( + matches!( + &graph[target], + Node::Sequence { + name, + explicit: false, + location: None, + .. + } if name == expected_name + ), + "UsesSequence should target inferred sequence {expected_name}" + ); + } + + #[test] + fn procedure_using_nextval_without_ddl_creates_inferred_sequence_node() { + let graph = build_from_sql( + r#" + CREATE PROCEDURE test_proc() AS $$ + DECLARE v BIGINT; + BEGIN + SELECT nextval('seq_batch_payment') INTO v FROM sys_dummy; + END; + $$ LANGUAGE plpgsql; + "#, + ); + + assert_single_inferred_sequence(&graph, "seq_batch_payment"); + } + + #[test] + fn select_dot_nextval_into_from_sys_dummy_creates_edge_with_ddl() { + let graph = build_from_sql( + r#" + CREATE SEQUENCE seq_batch_payment; + CREATE PROCEDURE test_proc() AS $$ + DECLARE v BIGINT; + BEGIN + SELECT seq_batch_payment.nextval INTO v FROM sys_dummy; + END; + $$ LANGUAGE plpgsql; + "#, + ); + + let seq_edges: Vec<_> = graph + .edge_indices() + .filter(|e| matches!(&graph[*e], Edge::UsesSequence { .. })) + .collect(); + assert_eq!(seq_edges.len(), 1, "Expected exactly 1 UsesSequence edge"); + let (_, target) = graph.edge_endpoints(seq_edges[0]).unwrap(); + assert!(matches!( + &graph[target], + Node::Sequence { explicit: true, .. } + )); + } + + #[test] + fn qualified_sequence_ref_does_not_collapse_to_other_schemas_sequence() { + // `finance.seq_id` has DDL, the procedure references `hr.seq_id.nextval`. + // The hr-qualified reference must NOT bind through the short-name alias + // to finance's sequence: same short name, different schema, different node. + let graph = build_from_sql( + r#" + CREATE SEQUENCE finance.seq_id START 1; + CREATE PROCEDURE test_proc() AS $$ + DECLARE v_id BIGINT; + BEGIN + v_id := hr.seq_id.NEXTVAL; + END; + $$ LANGUAGE plpgsql; + "#, + ); + + let seq_edges: Vec<_> = graph + .edge_indices() + .filter(|e| matches!(&graph[*e], Edge::UsesSequence { .. })) + .collect(); + assert_eq!(seq_edges.len(), 1, "Expected exactly 1 UsesSequence edge"); + let (_, target) = graph.edge_endpoints(seq_edges[0]).unwrap(); + assert!( + matches!( + &graph[target], + Node::Sequence { + schema: Some(schema), + name, + explicit: false, + .. + } if schema == "hr" && name == "seq_id" + ), + "qualified ref must target inferred hr.seq_id, got: {:?}", + &graph[target] + ); + } + + #[test] + fn two_schema_qualified_inferred_sequence_refs_create_distinct_nodes() { + let graph = build_from_sql( + r#" + CREATE PROCEDURE test_proc() AS $$ + DECLARE v_a BIGINT; v_b BIGINT; + BEGIN + v_a := finance.seq_id.NEXTVAL; + v_b := hr.seq_id.NEXTVAL; + END; + $$ LANGUAGE plpgsql; + "#, + ); + + let targets: Vec<_> = graph + .edge_indices() + .filter(|e| matches!(&graph[*e], Edge::UsesSequence { .. })) + .map(|e| graph.edge_endpoints(e).unwrap().1) + .collect(); + assert_eq!(targets.len(), 2, "Expected 2 UsesSequence edges"); + let mut schemas: Vec<&str> = targets + .iter() + .map(|&t| match &graph[t] { + Node::Sequence { + schema: Some(s), .. + } => s.as_str(), + other => panic!("unexpected target node: {other:?}"), + }) + .collect(); + schemas.sort_unstable(); + assert_eq!( + schemas, + vec!["finance", "hr"], + "distinct inferred nodes per schema" + ); + } + + #[test] + fn two_chunk_reference_then_ddl_promotes_inferred_sequence() { + use crate::graph::builder::GraphBuildContext; + + let mut ctx = GraphBuildContext::new(); + let file1 = ParsedFile { + path: PathBuf::from("chunk1.sql"), + statements: parse_sql( + r#" + CREATE PROCEDURE proc_a() AS $$ + DECLARE v_id BIGINT; + BEGIN + v_id := my_seq.NEXTVAL; + END; + $$ LANGUAGE plpgsql; + "#, + ), + content_hash: String::new(), + }; + GraphBuilder::build_sql_chunk(&mut ctx, &[file1]); + + let file2 = ParsedFile { + path: PathBuf::from("chunk2.sql"), + statements: parse_sql("CREATE SEQUENCE my_seq;"), + content_hash: String::new(), + }; + GraphBuilder::build_sql_chunk(&mut ctx, &[file2]); + GraphBuilder::finalize_graph(&mut ctx); + + let sequences: Vec<_> = ctx + .graph + .node_indices() + .filter( + |&idx| matches!(&ctx.graph[idx], Node::Sequence { name, .. } if name == "my_seq"), + ) + .collect(); + assert_eq!(sequences.len(), 1, "expected one promoted sequence node"); + let sequence_idx = sequences[0]; + assert!(matches!( + &ctx.graph[sequence_idx], + Node::Sequence { + explicit: true, + location: Some(_), + .. + } + )); + let uses_sequence_targets: Vec<_> = ctx + .graph + .edge_indices() + .filter(|&idx| matches!(&ctx.graph[idx], Edge::UsesSequence { .. })) + .map(|idx| ctx.graph.edge_endpoints(idx).unwrap().1) + .collect(); + assert_eq!(uses_sequence_targets, vec![sequence_idx]); + } + + #[test] + fn two_chunk_duplicate_references_share_single_inferred_sequence() { + use crate::graph::builder::GraphBuildContext; + + let mut ctx = GraphBuildContext::new(); + for (path, procedure) in [("chunk1.sql", "proc_a"), ("chunk2.sql", "proc_b")] { + let file = ParsedFile { + path: PathBuf::from(path), + statements: parse_sql(&format!( + r#" + CREATE PROCEDURE {procedure}() AS $$ + DECLARE v_id BIGINT; + BEGIN + v_id := my_seq.NEXTVAL; + END; + $$ LANGUAGE plpgsql; + "# + )), + content_hash: String::new(), + }; + GraphBuilder::build_sql_chunk(&mut ctx, &[file]); + } + GraphBuilder::finalize_graph(&mut ctx); + + let sequences: Vec<_> = ctx + .graph + .node_indices() + .filter( + |&idx| matches!(&ctx.graph[idx], Node::Sequence { name, .. } if name == "my_seq"), + ) + .collect(); + assert_eq!(sequences.len(), 1, "expected one shared inferred sequence"); + let sequence_idx = sequences[0]; + assert!(matches!( + &ctx.graph[sequence_idx], + Node::Sequence { + explicit: false, + location: None, + .. + } + )); + let uses_sequence_targets: Vec<_> = ctx + .graph + .edge_indices() + .filter(|&idx| matches!(&ctx.graph[idx], Edge::UsesSequence { .. })) + .map(|idx| ctx.graph.edge_endpoints(idx).unwrap().1) + .collect(); + assert_eq!(uses_sequence_targets, vec![sequence_idx, sequence_idx]); + } + + #[test] + fn ddl_does_not_promote_other_schema_inferred_sequence() { + use crate::graph::builder::GraphBuildContext; + + let mut ctx = GraphBuildContext::new(); + let file1 = ParsedFile { + path: PathBuf::from("chunk1.sql"), + statements: parse_sql( + r#" + CREATE PROCEDURE proc_a() AS $$ + DECLARE v_id BIGINT; + BEGIN + v_id := hr.seq_id.NEXTVAL; + END; + $$ LANGUAGE plpgsql; + "#, + ), + content_hash: String::new(), + }; + GraphBuilder::build_sql_chunk(&mut ctx, &[file1]); + + let file2 = ParsedFile { + path: PathBuf::from("chunk2.sql"), + statements: parse_sql("CREATE SEQUENCE finance.seq_id;"), + content_hash: String::new(), + }; + GraphBuilder::build_sql_chunk(&mut ctx, &[file2]); + GraphBuilder::finalize_graph(&mut ctx); + + let hr_sequence = ctx + .graph + .node_indices() + .find(|&idx| { + matches!( + &ctx.graph[idx], + Node::Sequence { + schema: Some(schema), + name, + explicit: false, + .. + } if schema == "hr" && name == "seq_id" + ) + }) + .expect("expected inferred hr.seq_id"); + let finance_sequences: Vec<_> = ctx + .graph + .node_indices() + .filter(|&idx| { + matches!( + &ctx.graph[idx], + Node::Sequence { + schema: Some(schema), + name, + explicit: true, + location: Some(_), + } if schema == "finance" && name == "seq_id" + ) + }) + .collect(); + assert_eq!(finance_sequences.len(), 1); + assert_eq!( + ctx.graph + .node_indices() + .filter(|&idx| matches!(&ctx.graph[idx], Node::Sequence { name, .. } if name == "seq_id")) + .count(), + 2 + ); + let uses_sequence_targets: Vec<_> = ctx + .graph + .edge_indices() + .filter(|&idx| matches!(&ctx.graph[idx], Edge::UsesSequence { .. })) + .map(|idx| ctx.graph.edge_endpoints(idx).unwrap().1) + .collect(); + assert_eq!(uses_sequence_targets, vec![hr_sequence]); + } + + #[test] + fn dot_nextval_assignment_without_ddl_creates_inferred_sequence_node() { + let graph = build_from_sql( + r#" + CREATE PROCEDURE test_proc() AS $$ + DECLARE v_id BIGINT; + BEGIN + v_id := my_seq.NEXTVAL; + END; + $$ LANGUAGE plpgsql; + "#, + ); + + assert_single_inferred_sequence(&graph, "my_seq"); + } + + #[test] + fn insert_values_nextval_without_ddl_creates_inferred_sequence_node() { + let graph = build_from_sql( + r#" + CREATE PROCEDURE test_proc() AS $$ + BEGIN + INSERT INTO t(id) VALUES(my_seq.NEXTVAL); + END; + $$ LANGUAGE plpgsql; + "#, + ); + + assert_single_inferred_sequence(&graph, "my_seq"); + } + + #[test] + fn inferred_sequence_schema_qualified_ref_resolves() { + let graph = build_from_sql( + r#" + CREATE PROCEDURE test_proc() AS $$ + DECLARE v BIGINT; + BEGIN + SELECT s1.my_seq.nextval INTO v FROM sys_dummy; + END; + $$ LANGUAGE plpgsql; + "#, + ); + + assert_single_inferred_sequence(&graph, "my_seq"); + } + #[test] fn gap_detection_creates_partial_nodes_for_missing_body_items() { use ogsql_parser::ast::{ diff --git a/src/graph/mod.rs b/src/graph/mod.rs index 0538fd6..3452f5f 100644 --- a/src/graph/mod.rs +++ b/src/graph/mod.rs @@ -569,7 +569,13 @@ pub enum Node { Sequence { schema: Option, name: String, - location: SourceLocation, + /// true when sequence has a DDL definition (CREATE SEQUENCE), false when + /// only inferred from seq.nextval / currval / setval references. + #[serde(default)] + explicit: bool, + /// None when sequence node was created implicitly (referenced but not parsed from DDL). + #[serde(default)] + location: Option, }, /// A database INDEX. Index { @@ -678,6 +684,9 @@ pub fn node_type_tag(node: &Node) -> &'static str { Node::Package { .. } => "pkg", Node::Trigger { .. } => "trigger", Node::Type { .. } => "type", + Node::Sequence { + explicit: false, .. + } => "seq*", Node::Sequence { .. } => "seq", Node::Index { .. } => "index", Node::MaterializedView { .. } => "mview", @@ -867,7 +876,10 @@ impl Node { Node::Package { location, .. } => &location.file, Node::Trigger { location, .. } => &location.file, Node::Type { location, .. } => &location.file, - Node::Sequence { location, .. } => &location.file, + Node::Sequence { location, .. } => location + .as_ref() + .map(|l| l.file.as_path()) + .unwrap_or(Path::new("")), Node::Index { location, .. } => &location.file, Node::MaterializedView { location, .. } => &location.file, Node::Synonym { location, .. } => &location.file, @@ -1173,7 +1185,8 @@ mod tests { let seq_node = Node::Sequence { schema: Some("public".to_string()), name: "my_seq".to_string(), - location: loc.clone(), + explicit: true, + location: Some(loc.clone()), }; assert_eq!(seq_node.file(), Path::new("test.sql")); @@ -1217,6 +1230,29 @@ mod tests { assert_eq!(event_node.file(), Path::new("test.sql")); } + #[test] + fn node_type_tag_inferred_sequence_is_seq_star() { + let inferred = Node::Sequence { + schema: None, + name: "inferred_seq".to_string(), + explicit: false, + location: None, + }; + let explicit = Node::Sequence { + schema: None, + name: "explicit_seq".to_string(), + explicit: true, + location: Some(SourceLocation { + file: Arc::new(PathBuf::from("sequence.sql")), + line: 1, + }), + }; + + assert_eq!(node_type_tag(&inferred), "seq*"); + assert_eq!(inferred.file(), Path::new("")); + assert_eq!(node_type_tag(&explicit), "seq"); + } + #[test] fn new_edge_variants_construct() { let file = Arc::new(PathBuf::from("test.sql")); @@ -1575,7 +1611,8 @@ mod tests { Node::Sequence { name: "seq".to_string(), schema: Some("public".to_string()), - location: loc.clone(), + explicit: true, + location: Some(loc.clone()), }, Node::Index { name: Some("idx".to_string()), diff --git a/src/graph/store.rs b/src/graph/store.rs index 9c80b3c..a337399 100644 --- a/src/graph/store.rs +++ b/src/graph/store.rs @@ -19,7 +19,7 @@ const STORE_MAGIC: [u8; 9] = *b"CWEBSTORE"; /// GraphStore on-disk format version. Bump when the serialized struct layout /// changes. Validated in the file header (post-header era files) and again in /// `GraphStore.version` after deserialize (legacy files + belt-and-suspenders). -const STORE_VERSION: u32 = 8; +const STORE_VERSION: u32 = 9; /// Pre-computed lightweight summary of a graph node for fast listing/filtering. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -1736,7 +1736,7 @@ pub fn node_source_file(node: &Node) -> Option { Node::Package { location, .. } => Some(location.file.to_path_buf()), Node::Trigger { location, .. } => Some(location.file.to_path_buf()), Node::Type { location, .. } => Some(location.file.to_path_buf()), - Node::Sequence { location, .. } => Some(location.file.to_path_buf()), + Node::Sequence { location, .. } => location.as_ref().map(|l| l.file.to_path_buf()), Node::Index { location, .. } => Some(location.file.to_path_buf()), Node::MaterializedView { location, .. } => Some(location.file.to_path_buf()), Node::Synonym { location, .. } => Some(location.file.to_path_buf()), @@ -1809,6 +1809,18 @@ fn pick_richer_node(a: &Node, idx_a: NodeIndex, b: &Node, idx_b: NodeIndex) -> N location: Some(_), .. }, ) => idx_b, + ( + Node::Sequence { + location: Some(_), .. + }, + Node::Sequence { location: None, .. }, + ) => idx_a, + ( + Node::Sequence { location: None, .. }, + Node::Sequence { + location: Some(_), .. + }, + ) => idx_b, _ => idx_a, } } @@ -2087,6 +2099,29 @@ mod tests { prepared.matches(sql_text) } + #[test] + fn pick_richer_node_prefers_located_sequence() { + let inferred = Node::Sequence { + schema: None, + name: "my_seq".to_string(), + explicit: false, + location: None, + }; + let located = Node::Sequence { + schema: None, + name: "my_seq".to_string(), + explicit: true, + location: Some(crate::graph::SourceLocation { + file: Arc::new(PathBuf::from("sequence.sql")), + line: 1, + }), + }; + let idx_a = NodeIndex::new(0); + let idx_b = NodeIndex::new(1); + + assert_eq!(pick_richer_node(&inferred, idx_a, &located, idx_b), idx_b); + } + #[test] fn test_bincode_roundtrip_edge_only() { let mut graph = CodeGraph::new(); @@ -2386,6 +2421,30 @@ mod tests { ); } + #[test] + fn load_bincode_rejects_pre_issue_159_version() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("v8.bincode"); + let mut bytes: Vec = Vec::new(); + bytes.extend_from_slice(&STORE_MAGIC); + bytes.extend_from_slice(&8u32.to_le_bytes()); + bytes.extend_from_slice(&[0u8; 8]); + std::fs::write(&path, &bytes).unwrap(); + + let result = GraphStore::load_bincode(&path); + assert!(result.is_err(), "pre-issue-159 cache must be rejected"); + let err_msg = result.unwrap_err().to_string(); + assert!( + err_msg.contains("unsupported cache version"), + "error should mention the version gate: {}", + err_msg + ); + assert!( + !GraphStore::file_is_current(&path), + "pre-issue-159 cache must be treated as stale" + ); + } + #[test] fn load_bincode_falls_back_for_legacy_headerless_file() { let dir = TempDir::new().unwrap(); diff --git a/src/import/parser.rs b/src/import/parser.rs index 5c1c439..93fac01 100644 --- a/src/import/parser.rs +++ b/src/import/parser.rs @@ -420,7 +420,8 @@ impl CgefParser { Ok(Node::Sequence { schema: key_get_str(key, "schema").map(String::from), name: name.to_string(), - location: loc, + explicit: true, + location: Some(loc), }) } "index" => { diff --git a/src/main.rs b/src/main.rs index fdc1fcc..e563c8a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1903,6 +1903,9 @@ fn node_type_tag(node: &Node) -> std::borrow::Cow<'static, str> { Node::Package { .. } => std::borrow::Cow::Borrowed("pkg"), Node::Trigger { .. } => std::borrow::Cow::Borrowed("trigger"), Node::Type { .. } => std::borrow::Cow::Borrowed("type"), + Node::Sequence { + explicit: false, .. + } => std::borrow::Cow::Borrowed("seq*"), Node::Sequence { .. } => std::borrow::Cow::Borrowed("seq"), Node::Index { .. } => std::borrow::Cow::Borrowed("index"), Node::MaterializedView { .. } => std::borrow::Cow::Borrowed("mview"), @@ -2074,6 +2077,9 @@ fn is_inferred_node(node: &Node) -> bool { } | Node::View { explicit: false, .. + } | Node::Sequence { + explicit: false, + .. } ) } diff --git a/tests/regress_issue_159_sequence_inferred.rs b/tests/regress_issue_159_sequence_inferred.rs new file mode 100644 index 0000000..d85d19e --- /dev/null +++ b/tests/regress_issue_159_sequence_inferred.rs @@ -0,0 +1,156 @@ +//! Regression for #159: sequence references without CREATE SEQUENCE must remain +//! visible through persisted project analysis and incremental re-analysis. + +use std::fs; +use std::path::{Path, PathBuf}; +use tempfile::TempDir; + +fn codeweb_bin() -> PathBuf { + let base = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("target"); + let bin_name = if cfg!(windows) { + "codeweb.exe" + } else { + "codeweb" + }; + let entries = fs::read_dir(&base).unwrap_or_else(|_| panic!("no target dir")); + for entry in entries.flatten() { + let path = entry.path().join("debug").join(bin_name); + if path.exists() { + return path; + } + } + base.join("debug").join(bin_name) +} + +fn run_codeweb_in(cwd: &Path, args: &[&str]) -> std::process::Output { + std::process::Command::new(codeweb_bin()) + .args(args) + .current_dir(cwd) + .output() + .expect("failed to run codeweb") +} + +fn export_json(project: &Path) -> serde_json::Value { + let output = run_codeweb_in( + project, + &[ + "export", + "--format", + "json", + "-p", + project.to_str().unwrap(), + ], + ); + assert!( + output.status.success(), + "export failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + serde_json::from_slice(&output.stdout).expect("export should produce JSON") +} + +fn assert_one_inferred_sequence_edge(json: &serde_json::Value) { + let nodes = json["nodes"].as_array().unwrap(); + let sequences: Vec<_> = nodes + .iter() + .filter(|node| { + node["type"].as_str() == Some("sequence") + && node["name"].as_str() == Some("seq_batch_payment") + }) + .collect(); + assert_eq!(sequences.len(), 1, "sequence must not be duplicated"); + assert_eq!( + sequences[0].get("explicit"), + None, + "inferred sequence omits explicit (false is skipped, matching table/view JSON)" + ); + assert_eq!( + sequences[0].get("file"), + None, + "inferred sequence omits file, matching table/view JSON" + ); + assert_eq!(sequences[0].get("line"), None); + + let sequence_id = sequences[0]["id"].as_u64().unwrap(); + let uses_sequence_edges: Vec<_> = json["edges"] + .as_array() + .unwrap() + .iter() + .filter(|edge| { + edge["type"].as_str() == Some("uses_sequence") + && edge["target"].as_u64() == Some(sequence_id) + }) + .collect(); + assert_eq!( + uses_sequence_edges.len(), + 1, + "UsesSequence edge must not be duplicated" + ); +} + +#[test] +fn inferred_sequence_survives_store_and_incremental_analyze_without_duplicates() { + let temp = TempDir::new().unwrap(); + let sql_dir = temp.path().join("sql"); + fs::create_dir_all(&sql_dir).unwrap(); + fs::write( + sql_dir.join("p.sql"), + r#"CREATE PROCEDURE p_pay() AS $$ +DECLARE v_seq BIGINT; +BEGIN + SELECT seq_batch_payment.nextval INTO v_seq FROM sys_dummy; +END; +$$ LANGUAGE plpgsql; +"#, + ) + .unwrap(); + + let sql_dir = fs::canonicalize(sql_dir).unwrap(); + let init = run_codeweb_in( + temp.path(), + &["init", "issue159", "-d", sql_dir.to_str().unwrap()], + ); + assert!( + init.status.success(), + "init failed: {}", + String::from_utf8_lossy(&init.stderr) + ); + + let store_path = temp.path().join(".codeweb/store.bincode"); + assert!(store_path.exists(), "init should persist the graph store"); + assert_one_inferred_sequence_edge(&export_json(temp.path())); + + let detail = run_codeweb_in( + temp.path(), + &["detail", "p_pay", "-p", temp.path().to_str().unwrap()], + ); + assert!( + detail.status.success(), + "detail failed: {}", + String::from_utf8_lossy(&detail.stderr) + ); + let detail_stdout = String::from_utf8_lossy(&detail.stdout); + assert!(detail_stdout.contains("seq:seq_batch_payment")); + assert!(detail_stdout.contains("[uses_seq]")); + + let mut old_store = fs::read(&store_path).unwrap(); + old_store[9..13].copy_from_slice(&8u32.to_le_bytes()); + fs::write(&store_path, old_store).unwrap(); + + let analyze = run_codeweb_in( + temp.path(), + &["analyze", "-p", temp.path().to_str().unwrap()], + ); + assert!( + analyze.status.success(), + "incremental analyze failed: {}", + String::from_utf8_lossy(&analyze.stderr) + ); + let rebuilt_store = fs::read(&store_path).unwrap(); + assert_eq!(&rebuilt_store[..9], b"CWEBSTORE"); + assert_eq!( + u32::from_le_bytes(rebuilt_store[9..13].try_into().unwrap()), + 9 + ); + assert_one_inferred_sequence_edge(&export_json(temp.path())); +}