Skip to content

fix(lineage): hint column-level needs existing table; pin jsp node-key tags (#154) #163

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

Merged
c2j merged 2 commits into from
Sep 8, 2026
Merged

fix(lineage): hint column-level needs existing table; pin jsp node-key tags (#154) #163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
447685c
fix(lineage): hint column-level needs an existing table in missing-ta…
c2j Sep 8, 2026
827b7a3
test(graph): pin jsp/jspsql node-key tags under the jsp feature (#154)
c2j Sep 8, 2026
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
29 changes: 28 additions & 1 deletion src/graph/key.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ pub enum NodeKey {

/// Node-key type tags exactly as emitted by the [`fmt::Display`] implementation below.
/// Keep in sync with its match arms; `should_detect_every_display_tag_roundtrip` pins
/// the fixed tags (the custom and unresolved formats are intentionally excluded).
/// non-JSP tags always and JSP tags under `cfg(feature = "jsp")` (the custom and
/// unresolved formats are intentionally excluded).
const TYPE_TAG_PREFIXES: &[&str] = &[
"proc", "func", "mapper", "method", "class", "table", "view", "pkg", "trigger", "type", "seq",
"idx", "mview", "syn", "event", "builtin", "javasql", "jsp", "jspsql",
Expand Down Expand Up @@ -529,5 +530,31 @@ mod tests {
"tag not detected for Display key: {key}"
);
}

#[cfg(feature = "jsp")]
{
let jsp_cases = [
format!(
"{}",
NodeKey::JspPage {
path: "WEB-INF/a.jsp".into()
}
),
format!(
"{}",
NodeKey::JspSql {
file: "WEB-INF/a.jsp".into(),
line: 7,
sql_hash: "abc123".into()
}
),
];
for key in &jsp_cases {
assert!(
split_type_prefix(key).is_some(),

@c2j c2j Sep 8, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] The new JSP cases only assert split_type_prefix(key).is_some(). That does not pin the tags jsp / jspsql. JspSql Display (jspsql:file:line:hash) is one character away from JavaSql (javasql:file:line); if the JspSql arm were copy-pasted onto the javasql format, split_type_prefix would still succeed because javasql is already in TYPE_TAG_PREFIXES, and jspsql:… CLI keys would again last-dot-split. That is the drift this follow-up is meant to lock.

Suggestion: Assert the detected tag, e.g. assert_eq!(split_type_prefix(&jsp_page).map(|(tag, _)| tag), Some("jsp")) and Some("jspsql") for JspSql, rather than is_some().

"tag not detected for Display key: {key}"
);
}
}
}
}
2 changes: 1 addition & 1 deletion src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1594,7 +1594,7 @@ fn cmd_lineage(
}
graph::lineage::TableLookup::Missing => {
eprintln!(
"note: no table '{}' found — interpreting '{}' as a table reference",
"note: no table '{}' found — interpreting '{}' as a table reference (for column-level lineage, the table must exist)",

@c2j c2j Sep 8, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] The new parenthetical (for column-level lineage, the table must exist) is appended on every TableLookup::Missing fallback, not only when the table is actually missing. That branch is also the successful schema.table recovery: codeweb lineage "bigfund.mid_yjqs_detail" first looks up table bigfund (a schema, so Missing), prints this note, then resolves the whole target as a table and succeeds. Users who typed a valid schema-qualified table now see a note implying column-level lineage failed because the table does not exist, even though stdout is a correct table-level result. should_fall_back_to_table_level_for_bare_schema_qualified_target still passes because it only checks for interpreting plus the original target. The new test covers only the dead-end missing_table.some_col case.

Suggestion: Keep the original Missing note for the schema.table recovery. Add the column-level prerequisite only when the subsequent table-level resolve also fails (the No table found matching path), or thread a “fell back from a column spec” flag and emit the extra sentence there. Extend the existing schema.table fallback test to assert the column-level parenthetical is absent on success.

table_name, target
);
(target, None)
Expand Down
29 changes: 29 additions & 0 deletions tests/regress_issue_154_lineage_targets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,35 @@ fn should_report_clean_error_for_unknown_nodekey_target() {
);
}

#[test]
fn should_hint_column_level_requires_table_when_target_missing() {
let tmp = TempDir::new().unwrap();
let root = project_with_sql(&tmp, FIXTURE_SQL);
let out = run_codeweb_in(
&root,
&[
"lineage",
"missing_table.some_col",
"-p",
root.to_str().unwrap(),
],
);
let stderr = String::from_utf8_lossy(&out.stderr);
assert!(out.status.success());
assert!(
stderr.contains("interpreting"),
"existing fallback note must remain, stderr:\n{stderr}"
);
assert!(
stderr.contains("the table must exist"),
"missing-table fallback must explain the column-level prerequisite, stderr:\n{stderr}"
);
assert!(
stderr.contains("No table found matching"),
"final table-resolution error must still surface, stderr:\n{stderr}"
);
}

#[test]
fn should_say_ambiguous_when_table_half_is_ambiguous() {
let tmp = TempDir::new().unwrap();
Expand Down
Loading