Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
57 changes: 56 additions & 1 deletion src/plugins/sql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,21 @@ impl SqlPlugin {
.unwrap_or_else(|| qualifier.to_string())
}

/// Names introduced by a `WITH` clause. These act as table sources within
/// the query but are not part of the schema, so they must not be flagged as
/// "table not found".
fn extract_cte_names(statement: &Statement) -> Vec<String> {
let mut names = Vec::new();
if let Statement::Query(query) = statement
&& let Some(with) = &query.with
{
for cte in &with.cte_tables {
names.push(cte.alias.name.value.to_lowercase());
}
}
names
}

/// Extract all column references from a statement.
fn extract_column_refs(statement: &Statement) -> Vec<(Option<String>, String)> {
let mut cols = Vec::new();
Expand Down Expand Up @@ -386,8 +401,11 @@ impl QueryLanguagePlugin for SqlPlugin {
// Check table references
let table_refs = Self::extract_table_refs(stmt);
let aliases = Self::extract_table_aliases(stmt);
let cte_names = Self::extract_cte_names(stmt);
for table_name in &table_refs {
if !schema.tables.iter().any(|t| t.name == *table_name) {
if !cte_names.contains(table_name)
&& !schema.tables.iter().any(|t| t.name == *table_name)
{
issues.push(SchemaIssue {
message: format!("Table '{}' not found in schema", table_name),
});
Expand Down Expand Up @@ -518,6 +536,43 @@ impl QueryLanguagePlugin for SqlPlugin {
});
}
}
// Unqualified `*`: expand to every nullable column of
// each table in scope (so `SELECT * FROM users` is
// null-checked, not silently skipped).
SelectItem::Wildcard(_) => {
for table_name in &table_refs {
if let Some(table) =
schema.tables.iter().find(|t| t.name == *table_name)
{
for col in table.columns.iter().filter(|c| c.nullable) {
issues.push(NullIssue {
message: format!(
"Nullable column '{}' selected via wildcard without COALESCE or null handling",
col.name
),
column: col.name.clone(),
});
}
}
}
}
// Alias-qualified `u.*`: expand the resolved table only.
SelectItem::QualifiedWildcard(obj, _) => {
let table_name =
Self::resolve_qualifier(&aliases, &obj.to_string().to_lowercase());
if let Some(table) = schema.tables.iter().find(|t| t.name == table_name)
{
for col in table.columns.iter().filter(|c| c.nullable) {
issues.push(NullIssue {
message: format!(
"Nullable column '{}' selected via wildcard without COALESCE or null handling",
col.name
),
column: col.name.clone(),
});
}
}
}
_ => {}
}
}
Expand Down
45 changes: 38 additions & 7 deletions tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,17 +608,48 @@ fn l4_nullable_comment_author() {
}

#[test]
fn l4_select_star_not_flagged() {
// SELECT * doesn't produce individual Identifier expressions for each column,
// so the null checker won't flag individual columns.
fn l4_select_star_flags_nullable() {
// `SELECT *` is expanded to the table's columns, so its nullable columns
// (users.email, users.age) are flagged like an explicit selection would be.
let plugin = get_plugin("sql").unwrap();
let schema = test_schema();
let issues = plugin.null_check("SELECT * FROM users", &schema).unwrap();
// The current implementation only checks UnnamedExpr(Identifier), not Wildcard.
// This test documents current behavior.
assert!(
issues.is_empty(),
"SELECT * is not individually checked for null (current behavior)"
issues.iter().any(|i| i.column == "email"),
"SELECT * must flag nullable 'email'. Got: {:?}",
issues
);
assert!(
issues.iter().any(|i| i.column == "age"),
"SELECT * must flag nullable 'age'. Got: {:?}",
issues
);
// Non-nullable columns (id, name) must NOT be flagged.
assert!(
!issues
.iter()
.any(|i| i.column == "id" || i.column == "name"),
"SELECT * must not flag non-nullable columns. Got: {:?}",
issues
);
}

#[test]
fn l2_cte_name_not_flagged_as_missing_table() {
// A CTE name is a valid in-query table source, not a schema table, so it
// must not be reported as "not found in schema".
let plugin = get_plugin("sql").unwrap();
let schema = test_schema();
let issues = plugin
.schema_check(
"WITH recent AS (SELECT id FROM users) SELECT id FROM recent",
&schema,
)
.unwrap();
assert!(
!issues.iter().any(|i| i.message.contains("recent")),
"CTE name 'recent' must not be flagged as a missing table. Got: {:?}",
issues
);
}

Expand Down
Loading