From a494dba2b279c2c0078bd0bfd1e8a4d77f7315e0 Mon Sep 17 00:00:00 2001 From: dsecurity49 Date: Wed, 26 Aug 2026 10:17:07 +0530 Subject: [PATCH 1/3] feat: upgrade Squawk parser stack to 2.63.0 --- CHANGELOG.md | 12 + Cargo.lock | 18 +- Cargo.toml | 10 +- src/ast/visitor.rs | 579 +++++++++++++++++++++------------------ src/ast/visitor_tests.rs | 238 ++++++++++++++++ 5 files changed, 578 insertions(+), 279 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c6e48b..5117eda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,18 @@ commits and pull requests. Published binaries, checksums, and generated release notes are available on the [GitHub Releases page](https://github.com/dsecurity49/safe-migrate/releases). +## v0.6.1 — 2026-08-25 + +- Upgraded the exactly pinned Squawk parser stack from 2.62.0 to 2.63.0 and + migrated statement extraction to its typed AST children for transaction, + schema, table, view, sequence, routine, replication, privilege, and session + statements. +- Adopted Squawk's stricter validation for malformed `IN`/`NOT IN`, empty + tuples, and `OVERLAPS` expressions, plus its corrected compound-select + precedence and trailing-clause parsing. +- Added focused regressions for every affected extraction family and the new + parser-validation behavior. + ## v0.6.0 — 2026-08-22 - Expanded `sync` and Cache V6 to record effective migration timeouts, every diff --git a/Cargo.lock b/Cargo.lock index 073c657..7b49897 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1020,7 +1020,7 @@ checksum = "cf54715a573b99ac80df0bc206da022bcd442c974952c7b9720069370852e21f" [[package]] name = "safe-migrate" -version = "0.6.0" +version = "0.6.1" dependencies = [ "anyhow", "assert_cmd", @@ -1200,9 +1200,9 @@ dependencies = [ [[package]] name = "squawk-lexer" -version = "2.62.0" +version = "2.63.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "030c945c0ddb933143616d0ca9b1640aee616e22adcf038e574483694e67b125" +checksum = "efaba81a4ba786561a9636eacab44bdd90b3ed35ab547fddab9c912ea52afbb1" [[package]] name = "squawk-line-index" @@ -1217,9 +1217,9 @@ dependencies = [ [[package]] name = "squawk-linter" -version = "2.62.0" +version = "2.63.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7e40a8df25a128ff6a331e096fcb6ecb9e67ede10eeb078d238b59c36ea8d66" +checksum = "d2f01fc98f8bbe117e591f23a276af0242dec79585a3060ca5df393e922fa6f6" dependencies = [ "annotate-snippets", "enum-iterator", @@ -1233,9 +1233,9 @@ dependencies = [ [[package]] name = "squawk-parser" -version = "2.62.0" +version = "2.63.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a18956011c1eb5913d361d53cd703a60b11c42fa290d280ae2b553df548e1c3e" +checksum = "58327f2c81cd96cfc80c4af6f6d533ca0fe38c38f3c323be0eeb1fcc3677bf28" dependencies = [ "drop_bomb", "squawk-lexer", @@ -1243,9 +1243,9 @@ dependencies = [ [[package]] name = "squawk-syntax" -version = "2.62.0" +version = "2.63.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d7531d70f31cb65a880f00bdc29e020dc3917b90ba2042f33867ee9afa99dc52" +checksum = "27a59435a2597c355ff6bd6e0e2955043674ea5113d60e590dffecffda21540a" dependencies = [ "either", "rowan", diff --git a/Cargo.toml b/Cargo.toml index 5c0cbde..6293c03 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "safe-migrate" -version = "0.6.0" +version = "0.6.1" edition = "2024" rust-version = "1.94" description = "Sync PostgreSQL metadata, then lint migrations offline" @@ -14,10 +14,10 @@ keywords = ["postgres", "migration", "linter", "ast", "database"] categories = ["command-line-utilities", "database"] [dependencies] -squawk-syntax = "=2.62.0" -squawk-lexer = "=2.62.0" -squawk-parser = "=2.62.0" -squawk-linter = "=2.62.0" +squawk-syntax = "=2.63.0" +squawk-lexer = "=2.63.0" +squawk-parser = "=2.63.0" +squawk-linter = "=2.63.0" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" toml = "0.8" diff --git a/src/ast/visitor.rs b/src/ast/visitor.rs index 1149dcd..97f3d98 100644 --- a/src/ast/visitor.rs +++ b/src/ast/visitor.rs @@ -70,6 +70,18 @@ impl AstVisitor { Self::resolve_identifier_token(node.syntax().text().to_string().trim()) } + fn is_and_chain(clause: Option) -> bool { + matches!(clause, Some(ast::ChainClause::AndChain(_))) + } + + fn is_cascade(behavior: Option) -> bool { + matches!(behavior, Some(ast::DropBehavior::Cascade(_))) + } + + fn is_local(scope: Option) -> bool { + matches!(scope, Some(ast::SetScope::LocalScope(_))) + } + pub fn extract(stmt: &Stmt) -> Option { let syntax = stmt.syntax(); match stmt { @@ -93,13 +105,11 @@ impl AstVisitor { Stmt::CreateUser(node) => return Self::extract_create_user(node), Stmt::Begin(_) => return Some(StatementFact::BeginTransaction), Stmt::Commit(node) => { - return Some( - if node.chain_token().is_some() && node.no_token().is_none() { - StatementFact::CommitAndChain - } else { - StatementFact::CommitTransaction - }, - ); + return Some(if Self::is_and_chain(node.chain_clause()) { + StatementFact::CommitAndChain + } else { + StatementFact::CommitTransaction + }); } Stmt::Rollback(node) => return Self::extract_rollback(node), Stmt::SavepointCreate(node) => return Some(Self::extract_savepoint(node)), @@ -342,7 +352,7 @@ impl AstVisitor { Some(StatementFact::DropSchema { names, if_exists: node.if_exists().is_some(), - cascade: node.cascade_token().is_some(), + cascade: Self::is_cascade(node.drop_behavior()), }) } @@ -427,7 +437,7 @@ impl AstVisitor { Some(StatementFact::DropTable { name: path, if_exists: node.if_exists().is_some(), - cascade: node.cascade_token().is_some(), + cascade: Self::is_cascade(node.drop_behavior()), }) } @@ -520,7 +530,10 @@ impl AstVisitor { .map(crate::analysis::expr_visitor::ExprVisitor::convert) } Constraint::GeneratedConstraint(generated) - if generated.generated_identity().is_some() => + if matches!( + generated.generated_as(), + Some(ast::GeneratedAs::GeneratedIdentity(_)) + ) => { generation = crate::analysis::facts::ColumnGeneration::Identity; not_null = true; @@ -682,43 +695,23 @@ impl AstVisitor { } } AlterTableAction::DisableTrigger(dt) => { - let trigger_name = dt - .trigger_ref() - .and_then(|tr| tr.ident_token()) - .map(|n| Self::resolve_identifier_token(n.text())) - .or_else(|| { - if dt.all_token().is_some() { - Some("ALL".to_string()) - } else { - None - } - }) - .or_else(|| { - dt.syntax() - .descendants() - .find_map(NameRef::cast) - .map(|nr| Self::resolve_name_ref(&nr)) - }); + let trigger_name = match dt.trigger_target() { + Some(ast::TriggerTarget::TriggerRef(trigger)) => trigger + .ident_token() + .map(|name| Self::resolve_identifier_token(name.text())), + Some(ast::TriggerTarget::All(_)) => Some("ALL".to_string()), + Some(ast::TriggerTarget::User(_)) | None => None, + }; actions.push(AlterTableActionFact::DisableTrigger { trigger_name }); } AlterTableAction::EnableTrigger(et) => { - let trigger_name = et - .trigger_ref() - .and_then(|tr| tr.ident_token()) - .map(|n| Self::resolve_identifier_token(n.text())) - .or_else(|| { - if et.all_token().is_some() { - Some("ALL".to_string()) - } else { - None - } - }) - .or_else(|| { - et.syntax() - .descendants() - .find_map(NameRef::cast) - .map(|nr| Self::resolve_name_ref(&nr)) - }); + let trigger_name = match et.trigger_target() { + Some(ast::TriggerTarget::TriggerRef(trigger)) => trigger + .ident_token() + .map(|name| Self::resolve_identifier_token(name.text())), + Some(ast::TriggerTarget::All(_)) => Some("ALL".to_string()), + Some(ast::TriggerTarget::User(_)) | None => None, + }; actions.push(AlterTableActionFact::EnableTrigger { trigger_name }); } AlterTableAction::SetSchema(ss) => { @@ -746,26 +739,26 @@ impl AstVisitor { actions.push(AlterTableActionFact::SetUnlogged); } AlterTableAction::ReplicaIdentity(ri) => { - let option = ri - .index_ref() - .and_then(|ir| ir.path_ref()) - .and_then(|pr| Self::path_ref_to_qualified_name(&pr)) - .map(|qn| qn.name.resolve()) - .or_else(|| { - if ri.default_token().is_some() { - Some("DEFAULT".to_string()) - } else if ri.full_token().is_some() { - Some("FULL".to_string()) - } else if ri.syntax().descendants().find_map(NameRef::cast).is_some() { - ri.syntax() - .descendants() - .find_map(NameRef::cast) - .map(|nr| Self::resolve_name_ref(&nr)) - } else { - Some("NOTHING".to_string()) - } - }) - .unwrap_or_default(); + let option = match ri.replica_identity_option() { + Some(ast::ReplicaIdentityOption::ReplicaIdentityDefault(_)) => { + "DEFAULT".to_string() + } + Some(ast::ReplicaIdentityOption::ReplicaIdentityFull(_)) => { + "FULL".to_string() + } + Some(ast::ReplicaIdentityOption::ReplicaIdentityNothing(_)) => { + "NOTHING".to_string() + } + Some(ast::ReplicaIdentityOption::UsingIndexName(using_index)) => { + using_index + .index_ref() + .and_then(|index| index.path_ref()) + .and_then(|path| Self::path_ref_to_qualified_name(&path)) + .map(|name| name.name.resolve()) + .unwrap_or_default() + } + None => String::new(), + }; actions.push(AlterTableActionFact::ReplicaIdentity { option }); } AlterTableAction::ClusterOn(co) => { @@ -956,7 +949,7 @@ impl AstVisitor { crate::analysis::facts::ColumnGeneration::Serial } else if col.constraints().any(|constraint| { matches!(constraint, ColumnConstraint::GeneratedConstraint(generated) - if generated.generated_identity().is_some()) + if matches!(generated.generated_as(), Some(ast::GeneratedAs::GeneratedIdentity(_)))) }) { crate::analysis::facts::ColumnGeneration::Identity } else { @@ -1076,13 +1069,14 @@ impl AstVisitor { fn extract_add_constraint_fact( ac: &squawk_syntax::ast::AddConstraint, ) -> Option { - let not_valid = ac.not_valid().is_some(); - if let Some(fkc) = ac .syntax() .descendants() .find_map(ast::ForeignKeyConstraint::cast) { + let not_valid = fkc + .constraint_options() + .any(|option| matches!(option, ast::ConstraintOption::NotValid(_))); let constraint_name = fkc .constraint_name_clause() .and_then(|cn| cn.constraint_name()) @@ -1112,6 +1106,9 @@ impl AstVisitor { .descendants() .find_map(ast::CheckConstraint::cast) { + let not_valid = cc + .constraint_options() + .any(|option| matches!(option, ast::ConstraintOption::NotValid(_))); let constraint_name = cc .constraint_name_clause() .and_then(|cn| cn.constraint_name()) @@ -1436,26 +1433,25 @@ impl AstVisitor { let col_token = avc.name()?.ident_token()?; let col_name = Self::resolve_identifier_token(col_token.text()); - if avc.drop_default().is_some() { - Some(StatementFact::AlterView { + match avc.alter_view_column_action()? { + ast::AlterViewColumnAction::DropDefault(_) => Some(StatementFact::AlterView { name, action: crate::analysis::facts::AlterViewAction::DropDefault { column: col_name, }, - }) - } else if let Some(sd) = avc.set_default() { - let expr = sd.expr()?; - Some(StatementFact::AlterView { - name, - action: crate::analysis::facts::AlterViewAction::SetDefault { - column: col_name, - default: Some(crate::analysis::expr_visitor::ExprVisitor::convert( - expr, - )), - }, - }) - } else { - None + }), + ast::AlterViewColumnAction::SetDefault(set_default) => { + let expr = set_default.expr()?; + Some(StatementFact::AlterView { + name, + action: crate::analysis::facts::AlterViewAction::SetDefault { + column: col_name, + default: Some(crate::analysis::expr_visitor::ExprVisitor::convert( + expr, + )), + }, + }) + } } } ast::AlterViewAction::RenameColumn(rc) => { @@ -1529,7 +1525,7 @@ impl AstVisitor { Some(StatementFact::DropView { name: path, if_exists: node.if_exists().is_some(), - cascade: node.cascade_token().is_some(), + cascade: Self::is_cascade(node.drop_behavior()), }) } @@ -1545,7 +1541,7 @@ impl AstVisitor { Some(StatementFact::DropMaterializedView { names, if_exists: node.if_exists().is_some(), - cascade: node.cascade_token().is_some(), + cascade: Self::is_cascade(node.drop_behavior()), }) } @@ -1672,7 +1668,12 @@ impl AstVisitor { .descendants() .find_map(ast::OptionOwnedBy::cast); match owned_option { - Some(option) if option.none_token().is_some() => { + Some(option) + if matches!( + option.owned_by_target(), + Some(ast::OwnedByTarget::OwnedByNone(_)) + ) => + { crate::analysis::facts::AlterSequenceActionFact::OwnedBy(None) } Some(_) => crate::analysis::facts::AlterSequenceActionFact::OwnedBy( @@ -1700,13 +1701,16 @@ impl AstVisitor { Some(StatementFact::DropSequence { names, if_exists: node.if_exists().is_some(), - cascade: node.cascade_token().is_some(), + cascade: Self::is_cascade(node.drop_behavior()), }) } fn extract_owned_by(node: &squawk_syntax::SyntaxNode) -> Option<(QualifiedName, String)> { for opt in node.descendants().filter_map(ast::OptionOwnedBy::cast) { - let path_ref = opt.name()?.path_ref()?; + let ast::OwnedByTarget::QualifiedColumnNameRef(name) = opt.owned_by_target()? else { + continue; + }; + let path_ref = name.path_ref()?; let mut segments = Vec::new(); let mut current_ref = Some(path_ref); @@ -1805,7 +1809,7 @@ impl AstVisitor { Some(StatementFact::DropType { names, if_exists: node.if_exists().is_some(), - cascade: node.cascade_token().is_some(), + cascade: Self::is_cascade(node.drop_behavior()), }) } fn extract_drop_domain(node: &DropDomain) -> Option { @@ -1818,7 +1822,7 @@ impl AstVisitor { Some(StatementFact::DropDomain { names, if_exists: node.if_exists().is_some(), - cascade: node.cascade_token().is_some(), + cascade: Self::is_cascade(node.drop_behavior()), }) } @@ -1917,18 +1921,22 @@ impl AstVisitor { true }; - let command = if node.all_token().is_some() { - crate::analysis::facts::PolicyCommand::All - } else if node.select_token().is_some() { - crate::analysis::facts::PolicyCommand::Select - } else if node.insert_token().is_some() { - crate::analysis::facts::PolicyCommand::Insert - } else if node.update_token().is_some() { - crate::analysis::facts::PolicyCommand::Update - } else if node.delete_token().is_some() { - crate::analysis::facts::PolicyCommand::Delete - } else { - crate::analysis::facts::PolicyCommand::All + let command = match node.policy_command().and_then(|command| command.command()) { + Some(ast::PolicyCommandKind::PolicyCommandSelect(_)) => { + crate::analysis::facts::PolicyCommand::Select + } + Some(ast::PolicyCommandKind::PolicyCommandInsert(_)) => { + crate::analysis::facts::PolicyCommand::Insert + } + Some(ast::PolicyCommandKind::PolicyCommandUpdate(_)) => { + crate::analysis::facts::PolicyCommand::Update + } + Some(ast::PolicyCommandKind::PolicyCommandDelete(_)) => { + crate::analysis::facts::PolicyCommand::Delete + } + Some(ast::PolicyCommandKind::PolicyCommandAll(_)) | None => { + crate::analysis::facts::PolicyCommand::All + } }; Some(StatementFact::CreatePolicy { @@ -2059,22 +2067,28 @@ impl AstVisitor { ) } ast::FuncOption::VolatilityFuncOption(f) => { - let vol = if f.immutable_token().is_some() { - crate::analysis::facts::VolatilityKind::Immutable - } else if f.stable_token().is_some() { - crate::analysis::facts::VolatilityKind::Stable - } else { - crate::analysis::facts::VolatilityKind::Volatile + let vol = match f { + ast::VolatilityFuncOption::Immutable(_) => { + crate::analysis::facts::VolatilityKind::Immutable + } + ast::VolatilityFuncOption::Stable(_) => { + crate::analysis::facts::VolatilityKind::Stable + } + ast::VolatilityFuncOption::Volatile(_) => { + crate::analysis::facts::VolatilityKind::Volatile + } }; crate::analysis::facts::FuncOptionFact::Volatility(vol) } - ast::FuncOption::SecurityFuncOption(f) => { - let sec = if f.invoker_token().is_some() { - crate::analysis::facts::SecurityKind::Invoker - } else { - crate::analysis::facts::SecurityKind::Definer - }; - crate::analysis::facts::FuncOptionFact::Security(sec) + ast::FuncOption::SecurityInvokerFuncOption(_) => { + crate::analysis::facts::FuncOptionFact::Security( + crate::analysis::facts::SecurityKind::Invoker, + ) + } + ast::FuncOption::SecurityDefinerFuncOption(_) => { + crate::analysis::facts::FuncOptionFact::Security( + crate::analysis::facts::SecurityKind::Definer, + ) } ast::FuncOption::StrictFuncOption(_) => crate::analysis::facts::FuncOptionFact::Strict( crate::analysis::facts::StrictKind::Strict, @@ -2245,7 +2259,7 @@ impl AstVisitor { crate::analysis::facts::DropFunctionFact { signatures: sigs, if_exists: node.if_exists().is_some(), - cascade: node.cascade_token().is_some(), + cascade: Self::is_cascade(node.drop_behavior()), }, )) } @@ -2339,7 +2353,7 @@ impl AstVisitor { crate::analysis::facts::DropProcedureFact { signatures: sigs, if_exists: node.if_exists().is_some(), - cascade: node.cascade_token().is_some(), + cascade: Self::is_cascade(node.drop_behavior()), }, )) } @@ -2446,7 +2460,7 @@ impl AstVisitor { crate::analysis::facts::DropAggregateFact { signatures, if_exists: node.if_exists().is_some(), - cascade: node.cascade_token().is_some(), + cascade: Self::is_cascade(node.drop_behavior()), }, )) } @@ -2530,27 +2544,33 @@ impl AstVisitor { .publication() .map(|publication| Self::resolve_ast_identifier(&publication)) .unwrap_or_default(); - let scope = if let Some(fapo) = node.for_all_publication_objects() { - crate::analysis::facts::PublicationScope::AllTables { - except: fapo - .except_table_clause() - .map(|etc| { - etc.except_table_names() - .filter_map(|etn| etn.table_relation_name()) - .filter_map(|trn| trn.table_name_ref()) - .filter_map(|tnr| tnr.path_ref()) - .filter_map(|p| Self::path_ref_to_qualified_name(&p)) - .map(|qn| qn.name.resolve()) - .collect() - }) - .unwrap_or_default(), + let scope = match node.publication_for_clause() { + Some(ast::PublicationForClause::ForAllPublicationObjects(all_objects)) => { + crate::analysis::facts::PublicationScope::AllTables { + except: all_objects + .except_table_clause() + .map(|clause| { + clause + .except_table_names() + .filter_map(|name| name.table_relation_name()) + .filter_map(|name| name.table_name_ref()) + .filter_map(|name| name.path_ref()) + .filter_map(|path| Self::path_ref_to_qualified_name(&path)) + .map(|name| name.name.resolve()) + .collect() + }) + .unwrap_or_default(), + } } - } else { - let objects = node - .publication_objects() - .filter_map(Self::extract_publication_object) - .collect(); - crate::analysis::facts::PublicationScope::Explicit(objects) + Some(ast::PublicationForClause::ForPublicationObjects(objects)) => { + crate::analysis::facts::PublicationScope::Explicit( + objects + .publication_objects() + .filter_map(Self::extract_publication_object) + .collect(), + ) + } + None => crate::analysis::facts::PublicationScope::Explicit(Vec::new()), }; let params = Self::extract_attribute_list(node.with_params().and_then(|with| with.attribute_list())); @@ -2595,7 +2615,7 @@ impl AstVisitor { ), ) } - ast::AlterPublicationAction::SetAllPublicationObjects(action) => { + ast::AlterPublicationAction::SetAllPublicationObjectList(action) => { let except = action .except_table_clause() .map(|clause| { @@ -2645,7 +2665,7 @@ impl AstVisitor { crate::analysis::facts::DropPublicationFact { names, if_exists: node.if_exists().is_some(), - cascade: node.cascade_token().is_some(), + cascade: Self::is_cascade(node.drop_behavior()), }, )) } @@ -2656,16 +2676,22 @@ impl AstVisitor { let name = node .subscription() .map(|subscription| Self::resolve_ast_identifier(&subscription)); - let connection = if node.server_token().is_some() { - crate::analysis::facts::ConnectionTarget::Server( - node.server_ref() - .map(|server| Self::resolve_ast_identifier(&server)), - ) - } else { - crate::analysis::facts::ConnectionTarget::Literal( - node.literal() - .and_then(|literal| Self::resolve_string_literal(&literal)), - ) + let connection = match node.source() { + Some(ast::SubscriptionSource::ServerClause(server)) => { + crate::analysis::facts::ConnectionTarget::Server( + server + .server_ref() + .map(|server| Self::resolve_ast_identifier(&server)), + ) + } + Some(ast::SubscriptionSource::ConnectionClause(connection)) => { + crate::analysis::facts::ConnectionTarget::Literal( + connection + .literal() + .and_then(|literal| Self::resolve_string_literal(&literal)), + ) + } + None => crate::analysis::facts::ConnectionTarget::Literal(None), }; let publications = node .publication_refs() @@ -2690,7 +2716,7 @@ impl AstVisitor { ) -> Option { let name = Self::resolve_ast_identifier(&node.subscription_ref()?); let action = match node.action()? { - ast::AlterSubscriptionAction::SetConnection(action) => { + ast::AlterSubscriptionAction::ConnectionClause(action) => { crate::analysis::facts::AlterSubscriptionActionFact::SetConnection( crate::analysis::facts::ConnectionTarget::Literal( action @@ -2699,7 +2725,7 @@ impl AstVisitor { ), ) } - ast::AlterSubscriptionAction::SetServer(action) => { + ast::AlterSubscriptionAction::ServerClause(action) => { crate::analysis::facts::AlterSubscriptionActionFact::SetServer( action .server_ref() @@ -2713,7 +2739,11 @@ impl AstVisitor { .publication_refs() .map(|publication| Self::resolve_ast_identifier(&publication)) .collect(), - params: Self::extract_attribute_list(action.attribute_list()), + params: Self::extract_attribute_list( + action + .with_params() + .and_then(|params| params.attribute_list()), + ), } } ast::AlterSubscriptionAction::AddPublication(action) => { @@ -2723,7 +2753,11 @@ impl AstVisitor { .publication_refs() .map(|publication| Self::resolve_ast_identifier(&publication)) .collect(), - params: Self::extract_attribute_list(action.attribute_list()), + params: Self::extract_attribute_list( + action + .with_params() + .and_then(|params| params.attribute_list()), + ), } } ast::AlterSubscriptionAction::DropSubscriptionPublication(action) => { @@ -2733,12 +2767,20 @@ impl AstVisitor { .publication_refs() .map(|publication| Self::resolve_ast_identifier(&publication)) .collect(), - params: Self::extract_attribute_list(action.attribute_list()), + params: Self::extract_attribute_list( + action + .with_params() + .and_then(|params| params.attribute_list()), + ), } } ast::AlterSubscriptionAction::RefreshPublication(action) => { crate::analysis::facts::AlterSubscriptionActionFact::RefreshPublication( - Self::extract_attribute_list(action.attribute_list()), + Self::extract_attribute_list( + action + .with_params() + .and_then(|params| params.attribute_list()), + ), ) } ast::AlterSubscriptionAction::EnableSubscription(_) => { @@ -3028,22 +3070,22 @@ impl AstVisitor { } fn extract_grant(node: &Grant) -> Option { - let privileges = if node.all_privileges().is_some() { - crate::analysis::facts::PrivilegeSpec::All - } else { - crate::analysis::facts::PrivilegeSpec::List( - node.revoke_command_list() - .map(|rcl| { - rcl.revoke_commands() - .map(|rc| Self::extract_privilege_from_revoke_command(&rc)) - .collect() - }) - .unwrap_or_default(), - ) + let privileges = match node.privileges() { + Some(ast::Privileges::AllPrivileges(_)) => crate::analysis::facts::PrivilegeSpec::All, + Some(ast::Privileges::RevokeCommandList(commands)) => { + crate::analysis::facts::PrivilegeSpec::List( + commands + .revoke_commands() + .map(|command| Self::extract_privilege_from_revoke_command(&command)) + .collect(), + ) + } + None => crate::analysis::facts::PrivilegeSpec::List(Vec::new()), }; let target = Self::extract_grant_target_from_privilege_objects( - node.privilege_objects(), + node.on_privilege_objects_clause() + .and_then(|clause| clause.privilege_objects()), node.syntax(), )?; @@ -3052,7 +3094,10 @@ impl AstVisitor { .map(|rrl| rrl.role_refs().map(|r| Self::extract_role(&r)).collect()) .unwrap_or_default(); let with_grant_option = node.grant_with_clause().is_some(); - let granted_by = node.role_ref().map(|r| Self::extract_role(&r)); + let granted_by = node + .granted_by_clause() + .and_then(|clause| clause.role_ref()) + .map(|role| Self::extract_role(&role)); Some(StatementFact::Grant(crate::analysis::facts::GrantFact { privileges, @@ -3064,30 +3109,27 @@ impl AstVisitor { } fn extract_revoke(node: &Revoke) -> Option { - let grant_option_only = node.for_token().is_some() - && node.grant_token().is_some() - && node.option_token().is_some(); - - let privileges = if let Some(p) = node.privileges() { - if p.all_token().is_some() { - crate::analysis::facts::PrivilegeSpec::All - } else { + let grant_option_only = matches!( + node.revoke_option_for(), + Some(ast::RevokeOptionFor::GrantOptionFor(_)) + ); + + let privileges = match node.privileges() { + Some(ast::Privileges::AllPrivileges(_)) => crate::analysis::facts::PrivilegeSpec::All, + Some(ast::Privileges::RevokeCommandList(commands)) => { crate::analysis::facts::PrivilegeSpec::List( - p.revoke_command_list() - .map(|rcl| { - rcl.revoke_commands() - .map(|rc| Self::extract_privilege_from_revoke_command(&rc)) - .collect() - }) - .unwrap_or_default(), + commands + .revoke_commands() + .map(|command| Self::extract_privilege_from_revoke_command(&command)) + .collect(), ) } - } else { - crate::analysis::facts::PrivilegeSpec::List(vec![]) + None => crate::analysis::facts::PrivilegeSpec::List(Vec::new()), }; let target = Self::extract_grant_target_from_privilege_objects( - node.privilege_objects(), + node.on_privilege_objects_clause() + .and_then(|clause| clause.privilege_objects()), node.syntax(), )?; @@ -3095,8 +3137,11 @@ impl AstVisitor { .role_ref_list() .map(|rrl| rrl.role_refs().map(|r| Self::extract_role(&r)).collect()) .unwrap_or_default(); - let granted_by = node.role_ref().map(|r| Self::extract_role(&r)); - let cascade = node.cascade_token().is_some(); + let granted_by = node + .granted_by_clause() + .and_then(|clause| clause.role_ref()) + .map(|role| Self::extract_role(&role)); + let cascade = Self::is_cascade(node.drop_behavior()); Some(StatementFact::Revoke(crate::analysis::facts::RevokeFact { grant_option_only, @@ -3276,31 +3321,37 @@ impl AstVisitor { .name .resolve() .to_lowercase(); - let local = node.local_token().is_some(); + let local = Self::is_local(node.set_scope()); if setting_name == "search_path" { - if sc.default_token().is_some() { - return Some(StatementFact::SetSearchPath { - target: SearchPathTarget::Default, - local, - }); - } - - let schemas: Vec = sc - .config_values() - .filter_map(|cv| match cv { - ast::ConfigValue::ConfigValueName(cvn) => cvn - .ident_token() - .map(|t| Self::resolve_identifier_token(t.text())), - ast::ConfigValue::Literal(literal) => { - Self::resolve_string_literal(&literal) - } - }) - .collect(); - return (!schemas.is_empty()).then_some(StatementFact::SetSearchPath { - target: SearchPathTarget::Schemas(schemas), - local, - }); + return match sc.config_assignment() { + Some(ast::ConfigAssignment::ToConfigValue(assignment)) + if assignment.default_token().is_some() => + { + Some(StatementFact::SetSearchPath { + target: SearchPathTarget::Default, + local, + }) + } + Some(ast::ConfigAssignment::ToConfigValue(assignment)) => { + let schemas: Vec = assignment + .config_values() + .filter_map(|value| match value { + ast::ConfigValue::ConfigValueName(name) => name + .ident_token() + .map(|token| Self::resolve_identifier_token(token.text())), + ast::ConfigValue::Literal(literal) => { + Self::resolve_string_literal(&literal) + } + }) + .collect(); + (!schemas.is_empty()).then_some(StatementFact::SetSearchPath { + target: SearchPathTarget::Schemas(schemas), + local, + }) + } + Some(ast::ConfigAssignment::FromCurrent(_)) | None => None, + }; } if setting_name == "application_name" { @@ -3312,31 +3363,36 @@ impl AstVisitor { "statement_timeout" => TimeoutSetting::Statement, _ => return None, }; - let value = if sc.default_token().is_some() { - TimeoutSettingValue::Default - } else if sc.current_token().is_some() { - TimeoutSettingValue::Current - } else { - let values: Vec = sc - .config_values() - .filter_map(|value| match value { - ast::ConfigValue::ConfigValueName(name) => { - name.ident_token().map(|token| token.text().to_string()) - } - ast::ConfigValue::Literal(literal) => { - Self::resolve_string_literal(&literal) - .or_else(|| Some(literal.syntax().text().to_string())) + let value = match sc.config_assignment() { + Some(ast::ConfigAssignment::FromCurrent(_)) => TimeoutSettingValue::Current, + Some(ast::ConfigAssignment::ToConfigValue(assignment)) + if assignment.default_token().is_some() => + { + TimeoutSettingValue::Default + } + Some(ast::ConfigAssignment::ToConfigValue(assignment)) => { + let values: Vec = assignment + .config_values() + .filter_map(|value| match value { + ast::ConfigValue::ConfigValueName(name) => { + name.ident_token().map(|token| token.text().to_string()) + } + ast::ConfigValue::Literal(literal) => { + Self::resolve_string_literal(&literal) + .or_else(|| Some(literal.syntax().text().to_string())) + } + }) + .collect(); + if values.len() != 1 { + TimeoutSettingValue::Invalid(sc.syntax().text().to_string()) + } else { + match crate::analysis::settings::parse_timeout_ms(&values[0]) { + Ok(milliseconds) => TimeoutSettingValue::Milliseconds(milliseconds), + Err(error) => TimeoutSettingValue::Invalid(error), } - }) - .collect(); - if values.len() != 1 { - TimeoutSettingValue::Invalid(sc.syntax().text().to_string()) - } else { - match crate::analysis::settings::parse_timeout_ms(&values[0]) { - Ok(milliseconds) => TimeoutSettingValue::Milliseconds(milliseconds), - Err(error) => TimeoutSettingValue::Invalid(error), } } + None => TimeoutSettingValue::Invalid(sc.syntax().text().to_string()), }; Some(StatementFact::SetTimeout { setting: timeout_setting, @@ -3377,16 +3433,17 @@ impl AstVisitor { /// Extract `SET [LOCAL] ROLE { rolename | NONE }`. fn extract_set_role(node: &squawk_syntax::ast::SetRole) -> Option { - let local = node.local_token().is_some(); - // ROLE NONE — restore the session default. - if node.none_token().is_some() { - return Some(StatementFact::SetRole { - role: None, - local, - is_session_auth: false, - }); - } - let role = node.role_ref().map(|r| Self::extract_role(&r)); + let local = Self::is_local(node.set_scope()); + let role = match node.set_role_target()? { + ast::SetRoleTarget::SetRoleNone(_) => None, + ast::SetRoleTarget::RoleRef(role) => Some(Self::extract_role(&role)), + ast::SetRoleTarget::Literal(literal) => Self::resolve_string_literal(&literal) + .filter(|name| !name.is_empty()) + .map(|name| crate::analysis::facts::RoleFact::Named { + name, + via_legacy_group_syntax: false, + }), + }; if matches!( role, Some( @@ -3412,25 +3469,17 @@ impl AstVisitor { fn extract_set_session_auth( node: &squawk_syntax::ast::SetSessionAuth, ) -> Option { - let local = node.local_token().is_some(); - // DEFAULT — restore the session default. - if node.default_token().is_some() { - return Some(StatementFact::SetRole { - role: None, - local, - is_session_auth: true, - }); - } - // A literal string is also valid: SET SESSION AUTHORIZATION 'rolename'. - let role_from_literal = node - .literal() - .and_then(|literal| Self::resolve_string_literal(&literal)) - .filter(|name| !name.is_empty()) - .map(|name| crate::analysis::facts::RoleFact::Named { - name, - via_legacy_group_syntax: false, - }); - let role = role_from_literal.or_else(|| node.role_ref().map(|r| Self::extract_role(&r))); + let local = Self::is_local(node.set_scope()); + let role = match node.set_session_auth_target()? { + ast::SetSessionAuthTarget::SetSessionAuthDefault(_) => None, + ast::SetSessionAuthTarget::RoleRef(role) => Some(Self::extract_role(&role)), + ast::SetSessionAuthTarget::Literal(literal) => Self::resolve_string_literal(&literal) + .filter(|name| !name.is_empty()) + .map(|name| crate::analysis::facts::RoleFact::Named { + name, + via_legacy_group_syntax: false, + }), + }; if matches!( role, Some( @@ -3459,7 +3508,7 @@ impl AstVisitor { .map(|t| Self::resolve_identifier_token(t.text())) { Some(name) => Some(StatementFact::RollbackToSavepoint { name }), - None if node.chain_token().is_some() && node.no_token().is_none() => { + None if Self::is_and_chain(node.chain_clause()) => { Some(StatementFact::RollbackAndChain) } None => Some(StatementFact::RollbackTransaction), diff --git a/src/ast/visitor_tests.rs b/src/ast/visitor_tests.rs index 174db8f..2f473c6 100644 --- a/src/ast/visitor_tests.rs +++ b/src/ast/visitor_tests.rs @@ -1792,4 +1792,242 @@ mod tests { }); assert_eq!(owner, Some(crate::analysis::facts::RoleFact::SessionUser)); } + + #[test] + fn squawk_263_typed_alter_table_children_preserve_facts() { + let facts = parse_and_extract( + "ALTER TABLE events ADD COLUMN generated_id bigint GENERATED ALWAYS AS IDENTITY; + ALTER TABLE events ADD CONSTRAINT events_parent_fk + FOREIGN KEY (parent_id) REFERENCES parents(id) NOT VALID; + ALTER TABLE events ADD CONSTRAINT events_id_positive + CHECK (generated_id > 0) NOT VALID; + ALTER TABLE events DISABLE TRIGGER ALL; + ALTER TABLE events ENABLE TRIGGER audit_log; + ALTER TABLE events REPLICA IDENTITY USING INDEX events_identity_idx;", + ); + assert_eq!(facts.len(), 6); + + assert!(matches!( + &facts[0], + StatementFact::AlterTable { actions, .. } + if matches!( + actions.as_slice(), + [AlterTableActionFact::AddColumn { + name, + generation: crate::analysis::facts::ColumnGeneration::Identity, + not_null: true, + .. + }] if name == "generated_id" + ) + )); + assert!(matches!( + &facts[1], + StatementFact::AlterTable { actions, .. } + if matches!( + actions.as_slice(), + [AlterTableActionFact::AddForeignKey { + constraint_name: Some(name), + not_valid: true, + .. + }] if name == "events_parent_fk" + ) + )); + assert!(matches!( + &facts[2], + StatementFact::AlterTable { actions, .. } + if matches!( + actions.as_slice(), + [AlterTableActionFact::AddCheckConstraint { + constraint_name: Some(name), + not_valid: true, + }] if name == "events_id_positive" + ) + )); + assert!(matches!( + &facts[3], + StatementFact::AlterTable { actions, .. } + if matches!( + actions.as_slice(), + [AlterTableActionFact::DisableTrigger { trigger_name: Some(name) }] + if name == "ALL" + ) + )); + assert!(matches!( + &facts[4], + StatementFact::AlterTable { actions, .. } + if matches!( + actions.as_slice(), + [AlterTableActionFact::EnableTrigger { trigger_name: Some(name) }] + if name == "audit_log" + ) + )); + assert!(matches!( + &facts[5], + StatementFact::AlterTable { actions, .. } + if matches!( + actions.as_slice(), + [AlterTableActionFact::ReplicaIdentity { option }] + if option == "events_identity_idx" + ) + )); + } + + #[test] + fn squawk_263_typed_view_sequence_policy_and_function_children_preserve_facts() { + let facts = parse_and_extract( + "ALTER VIEW report ALTER COLUMN total SET DEFAULT 0; + ALTER VIEW report ALTER COLUMN total DROP DEFAULT; + CREATE SEQUENCE event_ids OWNED BY public.events.id; + ALTER SEQUENCE event_ids OWNED BY NONE; + CREATE POLICY readers ON events FOR SELECT TO PUBLIC USING (true); + CREATE FUNCTION stable_owner() RETURNS integer + LANGUAGE sql IMMUTABLE SECURITY DEFINER AS 'SELECT 1';", + ); + assert_eq!(facts.len(), 6); + + assert!(matches!( + &facts[0], + StatementFact::AlterView { + action: crate::analysis::facts::AlterViewAction::SetDefault { column, .. }, + .. + } if column == "total" + )); + assert!(matches!( + &facts[1], + StatementFact::AlterView { + action: crate::analysis::facts::AlterViewAction::DropDefault { column }, + .. + } if column == "total" + )); + assert!(matches!( + &facts[2], + StatementFact::CreateSequence { + owned_by: Some((table, column)), + .. + } if table.schema.as_ref().map(Ident::resolve).as_deref() == Some("public") + && table.name.resolve() == "events" + && column == "id" + )); + assert!(matches!( + &facts[3], + StatementFact::AlterSequence { + action: crate::analysis::facts::AlterSequenceActionFact::OwnedBy(None), + .. + } + )); + assert!(matches!( + &facts[4], + StatementFact::CreatePolicy { + command: crate::analysis::facts::PolicyCommand::Select, + .. + } + )); + let StatementFact::CreateFunction(function) = &facts[5] else { + panic!("expected create function fact"); + }; + assert!(function.options.iter().any(|option| matches!( + option, + crate::analysis::facts::FuncOptionFact::Volatility( + crate::analysis::facts::VolatilityKind::Immutable + ) + ))); + assert!(function.options.iter().any(|option| matches!( + option, + crate::analysis::facts::FuncOptionFact::Security( + crate::analysis::facts::SecurityKind::Definer + ) + ))); + } + + #[test] + fn squawk_263_typed_replication_and_privilege_children_preserve_facts() { + let facts = parse_and_extract( + "CREATE PUBLICATION all_events FOR ALL TABLES EXCEPT TABLE audit_events; + CREATE PUBLICATION selected_events FOR TABLE public.events; + CREATE SUBSCRIPTION event_sub SERVER event_server + PUBLICATION selected_events WITH (connect = false); + ALTER SUBSCRIPTION event_sub SERVER replacement_server; + GRANT ALL PRIVILEGES ON TABLE public.events TO app_user GRANTED BY admin; + REVOKE GRANT OPTION FOR SELECT ON TABLE public.events + FROM app_user GRANTED BY admin CASCADE;", + ); + assert_eq!(facts.len(), 6); + + assert!(matches!( + &facts[0], + StatementFact::CreatePublication(publication) + if publication.name == "all_events" + && publication.scope + == PublicationScope::AllTables { except: vec!["audit_events".into()] } + )); + assert!(matches!( + &facts[1], + StatementFact::CreatePublication(publication) + if matches!(&publication.scope, PublicationScope::Explicit(objects) if objects.len() == 1) + )); + assert!(matches!( + &facts[2], + StatementFact::CreateSubscription(subscription) + if subscription.connection + == crate::analysis::facts::ConnectionTarget::Server( + Some("event_server".into()) + ) + )); + assert!(matches!( + &facts[3], + StatementFact::AlterSubscription(subscription) + if subscription.action + == AlterSubscriptionActionFact::SetServer(Some("replacement_server".into())) + )); + assert!(matches!( + &facts[4], + StatementFact::Grant(grant) + if grant.privileges == crate::analysis::facts::PrivilegeSpec::All + && grant.granted_by + == Some(crate::analysis::facts::RoleFact::Named { + name: "admin".into(), + via_legacy_group_syntax: false, + }) + )); + assert!(matches!( + &facts[5], + StatementFact::Revoke(revoke) + if revoke.grant_option_only + && revoke.cascade + && revoke.privileges + == crate::analysis::facts::PrivilegeSpec::List(vec![ + crate::analysis::facts::PrivilegeFact::Select, + ]) + && revoke.granted_by + == Some(crate::analysis::facts::RoleFact::Named { + name: "admin".into(), + via_legacy_group_syntax: false, + }) + )); + } + + #[test] + fn squawk_263_validation_matrix_rejects_invalid_expression_shapes() { + for sql in [ + "SELECT * FROM t WHERE a NOT IN ();", + "SELECT * FROM t WHERE a NOT IN ARRAY[1, 2];", + "SELECT 1 OVERLAPS 2;", + "CREATE TABLE t (a int, FOREIGN KEY (a WITHOUT OVERLAPS) REFERENCES u (c));", + ] { + let parsed = SourceFile::parse(sql); + assert!( + !parsed.errors().is_empty(), + "expected validation error: {sql}" + ); + } + + for sql in [ + "SELECT * FROM t WHERE a NOT IN (1, 2);", + "SELECT (1, 2) OVERLAPS (3, 4);", + "SELECT 1 UNION SELECT 2 INTERSECT SELECT 3 ORDER BY 1 LIMIT 1;", + ] { + let parsed = SourceFile::parse(sql); + assert!(parsed.errors().is_empty(), "expected valid SQL: {sql}"); + } + } } From 5fcc6f06fce622e243640ebacf79363ee95edc5b Mon Sep 17 00:00:00 2001 From: dsecurity49 Date: Wed, 26 Aug 2026 10:38:45 +0530 Subject: [PATCH 2/3] docs: prepare 0.6.1 release references --- .github/ISSUE_TEMPLATE/database-feedback.yml | 2 +- CHANGELOG.md | 2 +- CONTRIBUTING.md | 2 +- README.md | 4 ++-- docs/CONTRACT.md | 2 +- docs/GITHUB_ACTIONS.md | 8 ++++---- scripts/test-action-contract | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/database-feedback.yml b/.github/ISSUE_TEMPLATE/database-feedback.yml index 262abd5..f52b6c7 100644 --- a/.github/ISSUE_TEMPLATE/database-feedback.yml +++ b/.github/ISSUE_TEMPLATE/database-feedback.yml @@ -13,7 +13,7 @@ body: attributes: label: safe-migrate version description: Paste the output of `safe-migrate --version`. - placeholder: safe-migrate 0.6.0 + placeholder: safe-migrate 0.6.1 validations: required: true diff --git a/CHANGELOG.md b/CHANGELOG.md index 5117eda..98d7cf2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ commits and pull requests. Published binaries, checksums, and generated release notes are available on the [GitHub Releases page](https://github.com/dsecurity49/safe-migrate/releases). -## v0.6.1 — 2026-08-25 +## v0.6.1 — 2026-08-26 - Upgraded the exactly pinned Squawk parser stack from 2.62.0 to 2.63.0 and migrated statement extraction to its typed AST children for transaction, diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 28c58e4..0119921 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -129,7 +129,7 @@ Use the pinned Squawk source and grammar when changing AST extraction: 5. test resolver, state, and rule effects when behavior crosses layers; 6. represent unsupported parser behavior explicitly. -A Squawk version upgrade is an AST migration. Update all three pinned Squawk +A Squawk version upgrade is an AST migration. Update all four pinned Squawk crates together and validate the full extraction surface through compilation and tests. diff --git a/README.md b/README.md index d28a135..f3f3c6f 100644 --- a/README.md +++ b/README.md @@ -310,7 +310,7 @@ and GitHub cache contents are not signed. Store a 64-character hexadecimal key as `SAFE_MIGRATE_CACHE_KEY` and pass it to both workflows. ```yaml -- uses: dsecurity49/safe-migrate@v0.6.0 +- uses: dsecurity49/safe-migrate@v0.6.1 env: DATABASE_URL: ${{ secrets.SAFE_MIGRATE_DATABASE_URL }} SAFE_MIGRATE_CACHE_KEY: ${{ secrets.SAFE_MIGRATE_CACHE_KEY }} @@ -329,7 +329,7 @@ Replace `public` with the schemas that contain your migrations, or omit Add this after checkout in the pull-request workflow: ```yaml -- uses: dsecurity49/safe-migrate@v0.6.0 +- uses: dsecurity49/safe-migrate@v0.6.1 env: SAFE_MIGRATE_CACHE_KEY: ${{ secrets.SAFE_MIGRATE_CACHE_KEY }} with: diff --git a/docs/CONTRACT.md b/docs/CONTRACT.md index c33c597..c29c90b 100644 --- a/docs/CONTRACT.md +++ b/docs/CONTRACT.md @@ -1,6 +1,6 @@ # CLI and Report Contract -This document defines safe-migrate v0.6.0's CLI, report, cache, and GitHub +This document defines safe-migrate v0.6.1's CLI, report, cache, and GitHub Action behavior. If you are learning safe-migrate, start with the [README](../README.md). This diff --git a/docs/GITHUB_ACTIONS.md b/docs/GITHUB_ACTIONS.md index fee1dc3..3243793 100644 --- a/docs/GITHUB_ACTIONS.md +++ b/docs/GITHUB_ACTIONS.md @@ -54,7 +54,7 @@ jobs: with: persist-credentials: false - - uses: dsecurity49/safe-migrate@v0.6.0 + - uses: dsecurity49/safe-migrate@v0.6.1 env: SAFE_MIGRATE_CACHE_KEY: ${{ secrets.SAFE_MIGRATE_CACHE_KEY }} with: @@ -83,7 +83,7 @@ explicitly. In a trusted branch job, pass the checked-out file directly: ```yaml - - uses: dsecurity49/safe-migrate@v0.6.0 + - uses: dsecurity49/safe-migrate@v0.6.1 with: path: migrations config: safe-migrate.toml @@ -107,7 +107,7 @@ separately: sparse-checkout-cone-mode: false persist-credentials: false - - uses: dsecurity49/safe-migrate@v0.6.0 + - uses: dsecurity49/safe-migrate@v0.6.1 with: path: migrations config: .safe-migrate-base/safe-migrate.toml @@ -153,7 +153,7 @@ jobs: with: persist-credentials: false - - uses: dsecurity49/safe-migrate@v0.6.0 + - uses: dsecurity49/safe-migrate@v0.6.1 env: DATABASE_URL: ${{ secrets.SAFE_MIGRATE_DATABASE_URL }} SAFE_MIGRATE_CACHE_KEY: ${{ secrets.SAFE_MIGRATE_CACHE_KEY }} diff --git a/scripts/test-action-contract b/scripts/test-action-contract index d15b39c..1696059 100755 --- a/scripts/test-action-contract +++ b/scripts/test-action-contract @@ -9,7 +9,7 @@ baseline="$repo_root/scripts/action-baseline" manifest="$repo_root/action.yml" workflow="$repo_root/.github/workflows/ci.yml" -test "$(/bin/sh "$resolver" v0.6.0 "$repo_root/Cargo.toml")" = v0.6.0 +test "$(/bin/sh "$resolver" v0.6.1 "$repo_root/Cargo.toml")" = v0.6.1 test "$(/bin/sh "$resolver" 0123456789abcdef0123456789abcdef01234567 "$repo_root/Cargo.toml")" = source if /bin/sh "$resolver" main "$repo_root/Cargo.toml" >/dev/null 2>&1; then From d502474058df8d5b8347731d5111df5decd0f719 Mon Sep 17 00:00:00 2001 From: dsecurity49 Date: Wed, 26 Aug 2026 11:58:11 +0530 Subject: [PATCH 3/3] fix: mark identity columns as non-null --- src/ast/visitor.rs | 16 +++++++++------- src/ast/visitor_tests.rs | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/ast/visitor.rs b/src/ast/visitor.rs index 97f3d98..5423f1a 100644 --- a/src/ast/visitor.rs +++ b/src/ast/visitor.rs @@ -905,9 +905,14 @@ impl AstVisitor { })?; let name = Self::resolve_identifier_token(name_token.text()); let ty = col.ty().map(|t| t.syntax().text().to_string()); - let not_null = col - .constraints() - .any(|c| matches!(c, ColumnConstraint::NotNullConstraint(_))); + let is_identity = col.constraints().any(|constraint| { + matches!(constraint, ColumnConstraint::GeneratedConstraint(generated) + if matches!(generated.generated_as(), Some(ast::GeneratedAs::GeneratedIdentity(_)))) + }); + let not_null = is_identity + || col + .constraints() + .any(|c| matches!(c, ColumnConstraint::NotNullConstraint(_))); let primary_key_constraint_name = col.constraints().find_map(|constraint| { let ColumnConstraint::PrimaryKeyConstraint(primary_key) = constraint else { return None; @@ -947,10 +952,7 @@ impl AstVisitor { }); let generation = if Self::is_serial_type(ty.as_deref()) { crate::analysis::facts::ColumnGeneration::Serial - } else if col.constraints().any(|constraint| { - matches!(constraint, ColumnConstraint::GeneratedConstraint(generated) - if matches!(generated.generated_as(), Some(ast::GeneratedAs::GeneratedIdentity(_)))) - }) { + } else if is_identity { crate::analysis::facts::ColumnGeneration::Identity } else { crate::analysis::facts::ColumnGeneration::Ordinary diff --git a/src/ast/visitor_tests.rs b/src/ast/visitor_tests.rs index 2f473c6..f444539 100644 --- a/src/ast/visitor_tests.rs +++ b/src/ast/visitor_tests.rs @@ -68,6 +68,24 @@ mod tests { } } + #[test] + fn create_table_identity_column_is_non_null() { + let fact = parse_and_extract_statement( + "CREATE TABLE events (id bigint GENERATED ALWAYS AS IDENTITY);", + ) + .expect("create table fact"); + + let StatementFact::CreateTable { columns, .. } = fact else { + panic!("expected create table fact"); + }; + assert_eq!(columns.len(), 1); + assert_eq!( + columns[0].generation, + crate::analysis::facts::ColumnGeneration::Identity + ); + assert!(columns[0].not_null); + } + #[test] fn test_create_table_preserves_inline_constraint_names() { let fact = parse_and_extract_statement(